Using of variable length arrays in captured statements and OpenMP constructs

[jcownie]

This looks very dubious
lib/CodeGen/CGStmt.cpp : 2076

assert(CD->hasBody() && "missing CapturedDecl body");

I believe the "&&" should be a comma.

[hfinkel]

• Original Message -----

From: "Jim Cownie" <[e-mail]>
To: "a bataev" <[e-mail]>, [e-mail], [e-mail], [e-mail],
[e-mail], [e-mail], [e-mail]
Cc: "james h cownie" <[e-mail]>, [e-mail]
Sent: Monday, June 9, 2014 6:13:09 AM
Subject: Re: [PATCH] Using of variable length arrays in captured statements and OpenMP constructs

This looks very dubious
lib/CodeGen/CGStmt.cpp : 2076

assert(CD->hasBody() && "missing CapturedDecl body");

I believe the "&&" should be a comma.

No, this looks fine (and follows the LLVM coding convention, see: http://llvm.org/docs/CodingStandards.html#assert-liberally).

-Hal

http://reviews.llvm.org/D4067

[jcownie]

Ah, sorry, I should keep my nose out of stuff that I don't understand.

To a C programmer it looks very odd indeed, and could never assert (since the pointer to the string would be cast to a truth value, and would always be true).

I'll crawl back under my stone and get back to what I'm supposed to be working on :-)

• Jim

James Cownie <[e-mail]>
SSG/DPD/TCAR (Technical Computing, Analyzers and Runtimes)
Tel: [phone]

[jcownie]

Man, I must be in a fuddle this morning. I blame it being Monday.
My logic is utterly wrong :-(

• Jim

James Cownie <[e-mail]>
SSG/DPD/TCAR (Technical Computing, Analyzers and Runtimes)
Tel: [phone]

[ABataev]

Jim, no problems. :)

Best regards,

Alexey Bataev

Software Engineer
Intel Compiler Team

9 Июнь 2014 г. 15:27:28, Jim Cownie писал:

Man, I must be in a fuddle this morning. I blame it being Monday.
My logic is utterly wrong :-(

• Jim

James Cownie <[e-mail]>
SSG/DPD/TCAR (Technical Computing, Analyzers and Runtimes)
Tel: [phone]

http://reviews.llvm.org/D4067

[ABataev]

Ping

[ABataev]

Ping

[rsmith]

LGTM

Seems like a nice approach. We might want to consider doing the same thing in lambdas at some point :)

[ABataev]

Richard,
Thanks for the review. I can try to prepare similar patch for lambdas
in a few days.

Best regards,

Alexey Bataev

Software Engineer
Intel Compiler Team

27 Июнь 2014 г. 19:52:40, Richard Smith писал:

LGTM

Seems like a nice approach. We might want to consider doing the same thing in lambdas at some point :)

http://reviews.llvm.org/D4067

[ABataev]

Closed by commit rL212010 (authored by @ABataev).

cfe/trunk/lib/CodeGen/CGStmt.cpp

for (CapturedStmt::capture_init_iterator I = S.capture_init_begin(),
        I != E; ++I, ++CurField) {
     LValue LV = CGF.EmitLValueForFieldInitialization(SlotLV, *CurField);
     CGF.EmitInitializerForField(*CurField, LV, *I, ArrayRef<VarDecl *>());
   }
 
   return SlotLV;
 }
 
+static void InitVLACaptures(CodeGenFunction &CGF, const CapturedStmt &S) {
+  for (CapturedStmt::const_capture_iterator I = S.capture_begin(),
+                                            E = S.capture_end();
+       I != E; ++I) {
+    if (I->capturesVariable()) {
+      QualType QTy = I->getCapturedVar()->getType();
+      if (QTy->isVariablyModifiedType()) {
+        CGF.EmitVariablyModifiedType(QTy);
+      }
+    }
+  }
+}
+
 /// Generate an outlined function for the body of a CapturedStmt, store any
 /// captured variables into the captured struct, and call the outlined function.
 llvm::Function *
 CodeGenFunction::EmitCapturedStmt(const CapturedStmt &S, CapturedRegionKind K) {
-  const CapturedDecl *CD = S.getCapturedDecl();
-  const RecordDecl *RD = S.getCapturedRecordDecl();
-  assert(CD->hasBody() && "missing CapturedDecl body");
-
   LValue CapStruct = InitCapturedStruct(*this, S);
 
   // Emit the CapturedDecl
   CodeGenFunction CGF(CGM, true);
   CGF.CapturedStmtInfo = new CGCapturedStmtInfo(S, K);
-  llvm::Function *F = CGF.GenerateCapturedStmtFunction(CD, RD, S.getLocStart());
+  llvm::Function *F = CGF.GenerateCapturedStmtFunction(S);
   delete CGF.CapturedStmtInfo;
 
   // Emit call to the helper function.
   EmitCallOrInvoke(F, CapStruct.getAddress());
 
   return F;
 }
 
 llvm::Value *
 CodeGenFunction::GenerateCapturedStmtArgument(const CapturedStmt &S) {
   LValue CapStruct = InitCapturedStruct(*this, S);
   return CapStruct.getAddress();
 }
 
 /// Creates the outlined function for a CapturedStmt.
 llvm::Function *
-CodeGenFunction::GenerateCapturedStmtFunction(const CapturedDecl *CD,
-                                              const RecordDecl *RD,
-                                              SourceLocation Loc) {
+CodeGenFunction::GenerateCapturedStmtFunction(const CapturedStmt &S) {
   assert(CapturedStmtInfo &&
     "CapturedStmtInfo should be set when generating the captured function");
+  const CapturedDecl *CD = S.getCapturedDecl();
+  const RecordDecl *RD = S.getCapturedRecordDecl();
+  SourceLocation Loc = S.getLocStart();
+  assert(CD->hasBody() && "missing CapturedDecl body");
 
   // Build the argument list.
   ASTContext &Ctx = CGM.getContext();
   FunctionArgList Args;
   Args.append(CD->param_begin(), CD->param_end());
 
   // Create the function declaration.
   FunctionType::ExtInfo ExtInfo;
   const CGFunctionInfo &FuncInfo =
       CGM.getTypes().arrangeFreeFunctionDeclaration(Ctx.VoidTy, Args, ExtInfo,
                                                     /*IsVariadic=*/false);
   llvm::FunctionType *FuncLLVMTy = CGM.getTypes().GetFunctionType(FuncInfo);
 
   llvm::Function *F =
     llvm::Function::Create(FuncLLVMTy, llvm::GlobalValue::InternalLinkage,
                            CapturedStmtInfo->getHelperName(), &CGM.getModule());
   CGM.SetInternalFunctionAttributes(CD, F, FuncInfo);
 
   // Generate the function.
   StartFunction(CD, Ctx.VoidTy, F, FuncInfo, Args,
                 CD->getLocation(),
                 CD->getBody()->getLocStart());
-
   // Set the context parameter in CapturedStmtInfo.
   llvm::Value *DeclPtr = LocalDeclMap[CD->getContextParam()];
   assert(DeclPtr && "missing context parameter for CapturedStmt");
   CapturedStmtInfo->setContextValue(Builder.CreateLoad(DeclPtr));
 
+  // Initialize variable-length arrays.
+  InitVLACaptures(*this, S);
+
   // If 'this' is captured, load it into CXXThisValue.
   if (CapturedStmtInfo->isCXXThisExprCaptured()) {
     FieldDecl *FD = CapturedStmtInfo->getThisFieldDecl();
     LValue LV = MakeNaturalAlignAddrLValue(CapturedStmtInfo->getContextValue(),
                                            Ctx.getTagDeclType(RD));
     LValue ThisLValue = EmitLValueForField(LV, FD);
     CXXThisValue = EmitLoadOfLValue(ThisLValue, Loc).getScalarVal();
   }

cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp

void CodeGenFunction::EmitOMPParallelDirective(const OMPParallelDirective &S) {
   const CapturedStmt *CS = cast<CapturedStmt>(S.getAssociatedStmt());
   llvm::Value *CapturedStruct = GenerateCapturedStmtArgument(*CS);
 
   llvm::Value *OutlinedFn;
   {
     CodeGenFunction CGF(CGM, true);
     CGCapturedStmtInfo CGInfo(*CS, CS->getCapturedRegionKind());
     CGF.CapturedStmtInfo = &CGInfo;
-    OutlinedFn = CGF.GenerateCapturedStmtFunction(
-        CS->getCapturedDecl(), CS->getCapturedRecordDecl(), CS->getLocStart());
+    OutlinedFn = CGF.GenerateCapturedStmtFunction(*CS);
   }
 
   // Build call __kmpc_fork_call(loc, 1, microtask, captured_struct/*context*/)
   llvm::Value *Args[] = {
       CGM.getOpenMPRuntime().EmitOpenMPUpdateLocation(*this, S.getLocStart()),
       Builder.getInt32(1), // Number of arguments after 'microtask' argument
       // (there is only one additional argument - 'context')
       Builder.CreateBitCast(OutlinedFn,

cfe/trunk/lib/CodeGen/CodeGenFunction.h

#endif
   void ExitCXXTryStmt(const CXXTryStmt &S, bool IsFnTryBlock = false);
 
   void EmitCXXTryStmt(const CXXTryStmt &S);
   void EmitSEHTryStmt(const SEHTryStmt &S);
   void EmitCXXForRangeStmt(const CXXForRangeStmt &S,
                            const ArrayRef<const Attr *> &Attrs = None);
 
   llvm::Function *EmitCapturedStmt(const CapturedStmt &S, CapturedRegionKind K);
-  llvm::Function *GenerateCapturedStmtFunction(const CapturedDecl *CD,
-                                               const RecordDecl *RD,
-                                               SourceLocation Loc);
+  llvm::Function *GenerateCapturedStmtFunction(const CapturedStmt &S);
   llvm::Value *GenerateCapturedStmtArgument(const CapturedStmt &S);
 
   void EmitOMPParallelDirective(const OMPParallelDirective &S);
   void EmitOMPSimdDirective(const OMPSimdDirective &S);
   void EmitOMPForDirective(const OMPForDirective &S);
   void EmitOMPSectionsDirective(const OMPSectionsDirective &S);
   void EmitOMPSectionDirective(const OMPSectionDirective &S);
   void EmitOMPSingleDirective(const OMPSingleDirective &S);

cfe/trunk/lib/Sema/SemaExpr.cpp

if (IsLambda && !Var->getDeclName()) {
     if (Diagnose) {
       S.Diag(Loc, diag::err_lambda_capture_anonymous_var);
       S.Diag(Var->getLocation(), diag::note_declared_at);
     }
     return false;
   }
 
   // Prohibit variably-modified types; they're difficult to deal with.
-  if (Var->getType()->isVariablyModifiedType()) {
+  if (Var->getType()->isVariablyModifiedType() && (IsBlock || IsLambda)) {
     if (Diagnose) {
       if (IsBlock)
         S.Diag(Loc, diag::err_ref_vm_type);
       else
         S.Diag(Loc, diag::err_lambda_capture_vm_type) << Var->getDeclName();
       S.Diag(Var->getLocation(), diag::note_previous_decl) 
         << Var->getDeclName();
     }
if (isGenericLambdaCallOperatorSpecialization(DC)) {
           diagnoseUncapturableValueReference(*this, ExprLoc, Var, DC);
       }
       return true;
     }
     // Certain capturing entities (lambdas, blocks etc.) are not allowed to capture 
     // certain types of variables (unnamed, variably modified types etc.)
     // so check for eligibility.
     if (!isVariableCapturable(CSI, Var, ExprLoc, BuildAndDiagnose, *this))
-       return true;    
-    
+       return true;
+    // Try to capture variable-length arrays types.
+    if (Var->getType()->isVariablyModifiedType()) {
+      // We're going to walk down into the type and look for VLA
+      // expressions.
+      QualType QTy = Var->getType();
+      if (ParmVarDecl *PVD = dyn_cast_or_null<ParmVarDecl>(Var))
+        QTy = PVD->getOriginalType();
+      do {
+        const Type *Ty = QTy.getTypePtr();
+        switch (Ty->getTypeClass()) {
+#define TYPE(Class, Base)
+#define ABSTRACT_TYPE(Class, Base)
+#define NON_CANONICAL_TYPE(Class, Base)
+#define DEPENDENT_TYPE(Class, Base) case Type::Class:
+#define NON_CANONICAL_UNLESS_DEPENDENT_TYPE(Class, Base)
+#include "clang/AST/TypeNodes.def"
+          QTy = QualType();
+          break;
+        // These types are never variably-modified.
+        case Type::Builtin:
+        case Type::Complex:
+        case Type::Vector:
+        case Type::ExtVector:
+        case Type::Record:
+        case Type::Enum:
+        case Type::Elaborated:
+        case Type::TemplateSpecialization:
+        case Type::ObjCObject:
+        case Type::ObjCInterface:
+        case Type::ObjCObjectPointer:
+          llvm_unreachable("type class is never variably-modified!");
+        case Type::Adjusted:
+          QTy = cast<AdjustedType>(Ty)->getOriginalType();
+          break;
+        case Type::Decayed:
+          QTy = cast<DecayedType>(Ty)->getPointeeType();
+          break;
+        case Type::Pointer:
+          QTy = cast<PointerType>(Ty)->getPointeeType();
+          break;
+        case Type::BlockPointer:
+          QTy = cast<BlockPointerType>(Ty)->getPointeeType();
+          break;
+        case Type::LValueReference:
+        case Type::RValueReference:
+          QTy = cast<ReferenceType>(Ty)->getPointeeType();
+          break;
+        case Type::MemberPointer:
+          QTy = cast<MemberPointerType>(Ty)->getPointeeType();
+          break;
+        case Type::ConstantArray:
+        case Type::IncompleteArray:
+          // Losing element qualification here is fine.
+          QTy = cast<ArrayType>(Ty)->getElementType();
+          break;
+        case Type::VariableArray: {
+          // Losing element qualification here is fine.
+          const VariableArrayType *Vat = cast<VariableArrayType>(Ty);
+
+          // Unknown size indication requires no size computation.
+          // Otherwise, evaluate and record it.
+          if (Expr *Size = Vat->getSizeExpr()) {
+            MarkDeclarationsReferencedInExpr(Size);
+          }
+          QTy = Vat->getElementType();
+          break;
+        }
+        case Type::FunctionProto:
+        case Type::FunctionNoProto:
+          QTy = cast<FunctionType>(Ty)->getReturnType();
+          break;
+        case Type::Paren:
+        case Type::TypeOf:
+        case Type::UnaryTransform:
+        case Type::Attributed:
+        case Type::SubstTemplateTypeParm:
+        case Type::PackExpansion:
+          // Keep walking after single level desugaring.
+          QTy = QTy.getSingleStepDesugaredType(getASTContext());
+          break;
+        case Type::Typedef:
+          QTy = cast<TypedefType>(Ty)->desugar();
+          break;
+        case Type::Decltype:
+          QTy = cast<DecltypeType>(Ty)->desugar();
+          break;
+        case Type::Auto:
+          QTy = cast<AutoType>(Ty)->getDeducedType();
+          break;
+        case Type::TypeOfExpr:
+          QTy = cast<TypeOfExprType>(Ty)->getUnderlyingExpr()->getType();
+          break;
+        case Type::Atomic:
+          QTy = cast<AtomicType>(Ty)->getValueType();
+          break;
+        }
+      } while (!QTy.isNull() && QTy->isVariablyModifiedType());
+    }
+
     if (CSI->ImpCaptureStyle == CapturingScopeInfo::ImpCap_None && !Explicit) {
       // No capture-default, and this is not an explicit capture 
       // so cannot capture this variable.  
       if (BuildAndDiagnose) {
         Diag(ExprLoc, diag::err_lambda_impcap) << Var->getDeclName();
         Diag(Var->getLocation(), diag::note_previous_decl) 
           << Var->getDeclName();
         Diag(cast<LambdaScopeInfo>(CSI)->Lambda->getLocStart(),

cfe/trunk/test/CodeGen/captured-statements-nested.c

 // RUN: %clang_cc1 -fblocks -emit-llvm %s -o %t
 // RUN: FileCheck %s -input-file=%t -check-prefix=CHECK1
 // RUN: FileCheck %s -input-file=%t -check-prefix=CHECK2
 
 struct A {
   int a;
   float b;
   char c;
 };
 
-void test_nest_captured_stmt(int param) {
+void test_nest_captured_stmt(int param, int size, int param_arr[size]) {
   int w;
-  // CHECK1: %struct.anon{{.*}} = type { i32*, i32* }
-  // CHECK1: %struct.anon{{.*}} = type { i32*, i32*, i32**, i32* }
-  // CHECK1: [[T:%struct.anon.*]] = type { i32*, i32*, %struct.A*, i32**, i32* }
+  int arr[param][size];
+  // CHECK1: %struct.anon{{.*}} = type { i32*, i32*, i32*, i32**, i32* }
+  // CHECK1: %struct.anon{{.*}} = type { i32*, i32*, i32**, i32*, i32*, i32**, i32* }
+  // CHECK1: [[T:%struct.anon.*]] = type { i32*, i32*, %struct.A*, i32**, i32*, i32*, i32**, i32* }
   #pragma clang __debug captured
   {
     int x;
     int *y = &w;
     #pragma clang __debug captured
     {
       struct A z;
       #pragma clang __debug captured
       {
         w = x = z.a = 1;
         *y = param;
         z.b = 0.1f;
         z.c = 'c';
+        param_arr[size - 1] = 2;
+        arr[10][z.a] = 12;
 
         // CHECK1: define internal void @__captured_stmt{{.*}}([[T]]
         //
         // CHECK1: getelementptr inbounds [[T]]* {{.*}}, i32 0, i32 2
         // CHECK1-NEXT: load %struct.A**
         // CHECK1-NEXT: getelementptr inbounds %struct.A*
         // CHECK1-NEXT: store i32 1
         //
#pragma clang __debug captured
         // CHECK1-NEXT: load %struct.A**
         // CHECK1-NEXT: getelementptr inbounds %struct.A*
         // CHECK1-NEXT: store float
         //
         // CHECK1: getelementptr inbounds [[T]]* {{.*}}, i32 0, i32 2
         // CHECK1-NEXT: load %struct.A**
         // CHECK1-NEXT: getelementptr inbounds %struct.A*
         // CHECK1-NEXT: store i8 99
+        //
+        // CHECK1: [[SIZE_ADDR_REF:%.*]] = getelementptr inbounds [[T]]* {{.*}}, i32 0, i32 5
+        // CHECK1-NEXT: [[SIZE_ADDR:%.*]] = load i32** [[SIZE_ADDR_REF]]
+        // CHECK1-NEXT: [[SIZE:%.*]] = load i32* [[SIZE_ADDR]]
+        // CHECK1-NEXT: [[SIZE_MINUS_1:%.*]] = sub nsw i32 [[SIZE]], 1
+        // CHECK1-NEXT: [[PARAM_ARR_IDX:%.*]] = {{.*}} [[SIZE_MINUS_1]]
+        // CHECK1-NEXT: [[PARAM_ARR_ADDR_REF:%.*]] = getelementptr inbounds [[T]]* {{.*}}, i32 0, i32 6
+        // CHECK1-NEXT: [[PARAM_ARR_ADDR:%.*]] = load i32*** [[PARAM_ARR_ADDR_REF]]
+        // CHECK1-NEXT: [[PARAM_ARR:%.*]] = load i32** [[PARAM_ARR_ADDR]]
+        // CHECK1-NEXT: [[PARAM_ARR_SIZE_MINUS_1_ADDR:%.*]] = getelementptr inbounds i32* [[PARAM_ARR]], i{{.*}} [[PARAM_ARR_IDX]]
+        // CHECK1-NEXT: store i32 2, i32* [[PARAM_ARR_SIZE_MINUS_1_ADDR]]
+        //
+        // CHECK1: [[Z_ADDR_REF:%.*]] = getelementptr inbounds [[T]]* {{.*}}, i32 0, i32 2
+        // CHECK1-NEXT: [[Z_ADDR:%.*]] = load %struct.A** [[Z_ADDR_REF]]
+        // CHECK1-NEXT: [[Z_A_ADDR:%.*]] = getelementptr inbounds %struct.A* [[Z_ADDR]], i32 0, i32 0
+        // CHECK1-NEXT: [[Z_A:%.*]] = load i32* [[Z_A_ADDR]]
+        // CHECK1-NEXT: [[ARR_IDX_2:%.*]] = {{.*}} [[Z_A]]
+        // CHECK1-NEXT: [[ARR_ADDR_REF:%.*]] = getelementptr inbounds [[T]]* {{.*}}, i32 0, i32 7
+        // CHECK1-NEXT: [[ARR_ADDR:%.*]] = load i32** [[ARR_ADDR_REF]]
+        // CHECK1-NEXT: [[ARR_IDX_1:%.*]] = mul {{.*}} 10
+        // CHECK1-NEXT: [[ARR_10_ADDR:%.*]] = getelementptr inbounds i32* [[ARR_ADDR]], i{{.*}} [[ARR_IDX_1]]
+        // CHECK1-NEXT: [[ARR_10_Z_A_ADDR:%.*]] = getelementptr inbounds i32* [[ARR_10_ADDR]], i{{.*}} [[ARR_IDX_2]]
+        // CHECK1-NEXT: store i32 12, i32* [[ARR_10_Z_A_ADDR]]
       }
     }
   }
 }
 
 void test_nest_block() {
   __block int x;
   int y;

cfe/trunk/test/CodeGen/captured-statements.c

void test2(int x) {
   // CHECK-2: call void @[[HelperName:__captured_stmt[0-9]+]]
 }
 
 // CHECK-2: define internal void @[[HelperName]]
 // CHECK-2-NOT: }
 // CHECK-2:   %i = alloca i32
 
 // Capture array
-void test3() {
+void test3(int size) {
   int arr[] = {1, 2, 3, 4, 5};
+  int vla_arr[size];
   #pragma clang __debug captured
   {
-    arr[2] = arr[1];
+    arr[2] = vla_arr[size - 1];
   }
   // CHECK-3: test3
   // CHECK-3: alloca [5 x i32]
   // CHECK-3: call void @__captured_stmt
 }
 
 void dont_capture_global() {
   static int s;