On Mon, Jul 21, 2014 at 10:58 PM, Bataev, Alexey <a.bat...@hotmail.com> wrote:
> Richard, yes, I understood that. We have troubles in CodeGen in this case. > Yes, I can call getSizeExpr() for VariableArrayType, by it is impossible to > get llvm::Value* for the expression returned by getSizeExpr() in lambda > operator(). You can do this in exactly the same way you do it in your current patch; see attached patch for example. > This value is calculated in parent function for lambda expr and we have to > pass it somehow to lambda. Passing VariableArrayType* does not allow us to > get an access to this value. > We must not recalculate the value of expression from getSizeExpr(), we > must use the value, calculated in parent function. That's why it is not > enough just to pass VariableArrayType*, also we need to pass llvm::Value* > for getSizeExpr(), calculated in parent function. > > > Best regards, > Alexey Bataev > ============= > Software Engineer > Intel Compiler Team > > 22 Июль 2014 г. 9:41:43, Richard Smith писал: > >> On Mon, Jul 21, 2014 at 10:14 PM, Bataev, Alexey <a.bat...@hotmail.com >> <mailto:a.bat...@hotmail.com>> wrote: >> >> Richard, I don't think this is possible. If we storing >> VariableLengthArray* on FieldDecl, then in Lambda operator we can >> access VariableLengthArray->__getSizeExpr(). But! We won't be able >> >> to get the llvm::Value* for this >> VariableLengthArray->__getSizeExpr(), calculated in another >> >> function. We should pass this llvm::Value* as a member of >> RecordDecl for LambdaExpr! Here is a scheme: >> >> foo() { >> int vla[n+m]; >> .... >> some_lambda {vla....;} >> .... >> } >> >> What we have in CodeGen for this example: >> CodeGenFunction(foo), llvm::DenseMap<const Expr*, llvm::Value*> >> VLASizeMap[n+m]=value; VLASizeMap stores pre-calculated size of >> each VLA. >> { >> FD->field_for_m+n_expr = (int [n+m]*)0; >> CodeGenFunction CGF(some_lambda.operator(), FD); >> } >> >> CodeGenFunction(some_lambda.__operator()), llvm::DenseMap<const >> >> Expr*, llvm::Value*> VLASizeMap[n+m]=????? - llvm::Value* for >> 'n+m' is not captured here, we don't have an access to 'value', >> calculated in CodeGenFunction(foo) for expression 'n+m'. >> >> To solve this problem we can try to capture 2 fields instead - one >> for VariableLengthArray*, and another one for llvm::Value* >> calculated for VariableLengthArray->__getSizeExpr(). >> >> >> >> I'm sorry, I still think we're miscommunicating. What I'm suggesting >> is a mechanical transformation: store the VariableArrayType* on the >> FieldDecl, and then when you need the size expression that you're >> currently storing there, call getSizeExpr() on the type. >> >> Best regards, >> Alexey Bataev >> ============= >> Software Engineer >> Intel Compiler Team >> >> 22 Июль 2014 г. 8:57:54, Richard Smith писал: >> >> On Mon, Jul 21, 2014 at 5:43 AM, Alexey Bataev >> <a.bat...@hotmail.com <mailto:a.bat...@hotmail.com> >> <mailto:a.bat...@hotmail.com <mailto:a.bat...@hotmail.com>>> >> >> wrote: >> >> Updated version after last review. Unfortunately, I don't >> think it >> is possible to pass captured expression as a >> VariableLengthArray >> *. I have to capture expr of VariableLengthArray * and >> then cast >> it to SizeExpr->getType() type to make lambda capture this >> type, >> not VariableLengthArray *. I have to pass actual value of >> SizeExpr >> to the Lambda in this field, because it is defined only in >> calling >> function and in Lambda it can be received only in one of >> captured >> fields. >> >> >> I don't understand what you're saying. What I'm suggesting is >> storing >> the VariableLengthArray* in the InitializerOrBitWidth field on >> FieldDecl, instead of storing the array bound there. >> >> http://reviews.llvm.org/D4368 >> >> Files: >> include/clang/AST/Decl.h >> include/clang/AST/__LambdaCapture.h >> include/clang/Basic/__DiagnosticSemaKinds.td >> >> lib/AST/Decl.cpp >> lib/AST/Expr.cpp >> lib/AST/ExprCXX.cpp >> lib/AST/StmtPrinter.cpp >> lib/AST/StmtProfile.cpp >> lib/CodeGen/CGDebugInfo.cpp >> lib/CodeGen/CGExprCXX.cpp >> lib/CodeGen/CodeGenFunction.__cpp >> lib/Sema/SemaDecl.cpp >> lib/Sema/SemaExpr.cpp >> lib/Sema/TreeTransform.h >> test/CodeGenCXX/instantiate-__typeof-vla.cpp >> test/SemaTemplate/instantiate-__typeof.cpp >> tools/libclang/IndexBody.cpp >> >> >> >>
Index: test/SemaTemplate/instantiate-typeof.cpp =================================================================== --- test/SemaTemplate/instantiate-typeof.cpp (revision 213614) +++ test/SemaTemplate/instantiate-typeof.cpp (working copy) @@ -1,10 +1,11 @@ // RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s +// expected-no-diagnostics // Make sure we correctly treat __typeof as potentially-evaluated when appropriate template<typename T> void f(T n) { - int buffer[n]; // expected-note {{declared here}} - [] { __typeof(buffer) x; }(); // expected-error {{variable 'buffer' with variably modified type cannot be captured in a lambda expression}} + int buffer[n]; + [&buffer] { __typeof(buffer) x; }(); } int main() { - f<int>(1); // expected-note {{in instantiation}} + f<int>(1); } Index: include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- include/clang/Basic/DiagnosticSemaKinds.td (revision 213614) +++ include/clang/Basic/DiagnosticSemaKinds.td (working copy) @@ -5320,9 +5320,6 @@ "'this' cannot be %select{implicitly |}0captured in this context">; def err_lambda_capture_anonymous_var : Error< "unnamed variable cannot be implicitly captured in a lambda expression">; - def err_lambda_capture_vm_type : Error< - "variable %0 with variably modified type cannot be captured in " - "a lambda expression">; def err_lambda_capture_flexarray_type : Error< "variable %0 with flexible array member cannot be captured in " "a lambda expression">; Index: include/clang/Sema/ScopeInfo.h =================================================================== --- include/clang/Sema/ScopeInfo.h (revision 213614) +++ include/clang/Sema/ScopeInfo.h (working copy) @@ -481,6 +481,11 @@ void addThisCapture(bool isNested, SourceLocation Loc, QualType CaptureType, Expr *Cpy); + void addVLACapture(SourceLocation Loc, QualType SizeType) { + addCapture(nullptr, false, false, false, Loc, SourceLocation(), SizeType, + nullptr); + } + /// \brief Determine whether the C++ 'this' is captured. bool isCXXThisCaptured() const { return CXXThisCaptureIndex != 0; } Index: include/clang/AST/Decl.h =================================================================== --- include/clang/AST/Decl.h (revision 213614) +++ include/clang/AST/Decl.h (working copy) @@ -2155,8 +2155,9 @@ mutable unsigned CachedFieldIndex : 31; /// \brief An InClassInitStyle value, and either a bit width expression (if - /// the InClassInitStyle value is ICIS_NoInit), or a pointer to the in-class - /// initializer for this field (otherwise). + /// the InClassInitStyle value is ICIS_NoInit) in struct/class, or a captured + /// variable length array bound in a lambda expression, or a pointer to the + /// in-class initializer for this field (otherwise). /// /// We can safely combine these two because in-class initializers are not /// permitted for bit-fields. @@ -2192,11 +2193,8 @@ /// isMutable - Determines whether this field is mutable (C++ only). bool isMutable() const { return Mutable; } - /// isBitfield - Determines whether this field is a bitfield. - bool isBitField() const { - return getInClassInitStyle() == ICIS_NoInit && - InitializerOrBitWidth.getPointer(); - } + /// \brief Determines whether this field is a bitfield. + bool isBitField() const; /// @brief Determines whether this is an unnamed bitfield. bool isUnnamedBitfield() const { return isBitField() && !getDeclName(); } @@ -2252,6 +2250,18 @@ InitializerOrBitWidth.setInt(ICIS_NoInit); } + /// \brief Determine whether this field is an implicit lambda capture for the + /// bound in a variable length array type. + bool hasCapturedVLABound() const; + /// \brief Get the captured variable length array. + const VariableArrayType *getCapturedVLABound() const { + return hasCapturedVLABound() ? reinterpret_cast<const VariableArrayType *>( + InitializerOrBitWidth.getPointer()) + : nullptr; + } + /// \brief Set the captured variable length array for this field. + void setCapturedVLABound(const VariableArrayType *Captured); + /// getParent - Returns the parent of this field declaration, which /// is the struct in which this method is defined. const RecordDecl *getParent() const { @@ -3136,6 +3146,10 @@ /// \endcode bool isInjectedClassName() const; + /// \brief Determine whether this record is a class describing a lambda + /// function object. + bool isLambda() const; + /// getDefinition - Returns the RecordDecl that actually defines /// this struct/union/class. When determining whether or not a /// struct/union/class is completely defined, one should use this Index: include/clang/AST/LambdaCapture.h =================================================================== --- include/clang/AST/LambdaCapture.h (revision 213614) +++ include/clang/AST/LambdaCapture.h (working copy) @@ -68,13 +68,22 @@ /// \brief Determine whether this capture handles the C++ \c this /// pointer. - bool capturesThis() const { return DeclAndBits.getPointer() == nullptr; } + bool capturesThis() const { + return (DeclAndBits.getPointer() == nullptr) && + !(DeclAndBits.getInt() & Capture_ByCopy); + } /// \brief Determine whether this capture handles a variable. bool capturesVariable() const { return dyn_cast_or_null<VarDecl>(DeclAndBits.getPointer()); } + /// \brief Determine whether this captures a variable length array bound. + bool capturesVLABound() const { + return (DeclAndBits.getPointer() == nullptr) && + (DeclAndBits.getInt() & Capture_ByCopy); + } + /// \brief Determine whether this is an init-capture. bool isInitCapture() const { return capturesVariable() && getCapturedVar()->isInitCapture(); Index: tools/libclang/IndexBody.cpp =================================================================== --- tools/libclang/IndexBody.cpp (revision 213614) +++ tools/libclang/IndexBody.cpp (working copy) @@ -150,7 +150,7 @@ } bool TraverseLambdaCapture(LambdaExpr *LE, const LambdaCapture *C) { - if (C->capturesThis()) + if (C->capturesThis() || C->capturesVLABound()) return true; if (C->capturesVariable() && IndexCtx.shouldIndexFunctionLocalSymbols()) Index: lib/Sema/SemaExpr.cpp =================================================================== --- lib/Sema/SemaExpr.cpp (revision 213614) +++ lib/Sema/SemaExpr.cpp (working copy) @@ -11666,12 +11666,9 @@ } // Prohibit variably-modified types; they're difficult to deal with. - if (Var->getType()->isVariablyModifiedType() && (IsBlock || IsLambda)) { + if (Var->getType()->isVariablyModifiedType() && IsBlock) { 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(Loc, diag::err_ref_vm_type); S.Diag(Var->getLocation(), diag::note_previous_decl) << Var->getDeclName(); } @@ -12212,14 +12209,33 @@ break; case Type::VariableArray: { // Losing element qualification here is fine. - const VariableArrayType *Vat = cast<VariableArrayType>(Ty); + 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); + // Otherwise, record it. + if (auto Size = VAT->getSizeExpr()) { + if (auto LSI = dyn_cast<LambdaScopeInfo>(CSI)) { + // Build the non-static data member. + auto Field = FieldDecl::Create(Context, LSI->Lambda, + ExprLoc, ExprLoc, + /*Id*/ nullptr, + Context.getSizeType(), + /*TInfo*/ nullptr, + /*BW*/ nullptr, /*Mutable*/ false, + /*InitStyle*/ ICIS_NoInit); + Field->setImplicit(true); + Field->setAccess(AS_private); + Field->setCapturedVLABound(VAT); + LSI->Lambda->addDecl(Field); + + LSI->addVLACapture(ExprLoc, Field->getType()); + } else { + // Immediately mark all referenced vars for CapturedStatements, + // they all are captured by reference. + MarkDeclarationsReferencedInExpr(Size); + } } - QTy = Vat->getElementType(); + QTy = VAT->getElementType(); break; } case Type::FunctionProto: Index: lib/Sema/TreeTransform.h =================================================================== --- lib/Sema/TreeTransform.h (revision 213614) +++ lib/Sema/TreeTransform.h (working copy) @@ -8954,6 +8954,10 @@ getSema().CheckCXXThisCapture(C->getLocation(), C->isExplicit()); continue; } + // Captured expression will be recaptured during captured variables + // rebuilding. + if (C->capturesVLABound()) + continue; // Rebuild init-captures, including the implied field declaration. if (C->isInitCapture()) { Index: lib/Sema/SemaDecl.cpp =================================================================== --- lib/Sema/SemaDecl.cpp (revision 213614) +++ lib/Sema/SemaDecl.cpp (working copy) @@ -9833,6 +9833,7 @@ // Add the captures to the LSI so they can be noted as already // captured within tryCaptureVar. + auto I = LambdaClass->field_begin(); for (const auto &C : LambdaClass->captures()) { if (C.capturesVariable()) { VarDecl *VD = C.getCapturedVar(); @@ -9849,7 +9850,10 @@ } else if (C.capturesThis()) { LSI->addThisCapture(/*Nested*/ false, C.getLocation(), S.getCurrentThisType(), /*Expr*/ nullptr); + } else { + LSI->addVLACapture(C.getLocation(), I->getType()); } + ++I; } } Index: lib/AST/Decl.cpp =================================================================== --- lib/AST/Decl.cpp (revision 213614) +++ lib/AST/Decl.cpp (working copy) @@ -3259,6 +3259,15 @@ return false; } +bool FieldDecl::isBitField() const { + if (getInClassInitStyle() == ICIS_NoInit && + InitializerOrBitWidth.getPointer()) { + assert(getDeclContext() && "No parent context for FieldDecl"); + return !getDeclContext()->isRecord() || !getParent()->isLambda(); + } + return false; +} + unsigned FieldDecl::getBitWidthValue(const ASTContext &Ctx) const { assert(isBitField() && "not a bitfield"); Expr *BitWidth = InitializerOrBitWidth.getPointer(); @@ -3290,17 +3299,32 @@ } void FieldDecl::setBitWidth(Expr *Width) { + assert(isBitField() && "not a bitfield"); assert(!InitializerOrBitWidth.getPointer() && !hasInClassInitializer() && - "bit width or initializer already set"); + "bit width, initializer or captured expr already set"); InitializerOrBitWidth.setPointer(Width); } void FieldDecl::setInClassInitializer(Expr *Init) { assert(!InitializerOrBitWidth.getPointer() && hasInClassInitializer() && - "bit width or initializer already set"); + "bit width, initializer or captured expr already set"); InitializerOrBitWidth.setPointer(Init); } +bool FieldDecl::hasCapturedVLABound() const { + return getDeclContext()->isRecord() && getParent()->isLambda() && + getInClassInitStyle() == ICIS_NoInit && + InitializerOrBitWidth.getPointer(); +} + +void FieldDecl::setCapturedVLABound(const VariableArrayType *CapturedVLA) { + assert(getParent()->isLambda() && "capturing expression in non-lambda."); + assert(!InitializerOrBitWidth.getPointer() && !hasInClassInitializer() && + "bit width, initializer or captured expr already set"); + InitializerOrBitWidth.setPointer( + const_cast<Expr *>(reinterpret_cast<const Expr *>(CapturedVLA))); +} + //===----------------------------------------------------------------------===// // TagDecl Implementation //===----------------------------------------------------------------------===// @@ -3521,6 +3545,13 @@ cast<RecordDecl>(getDeclContext())->getDeclName() == getDeclName(); } +bool RecordDecl::isLambda() const { + if (auto RD = dyn_cast<CXXRecordDecl>(this)) { + return RD->isLambda(); + } + return false; +} + RecordDecl::field_iterator RecordDecl::field_begin() const { if (hasExternalLexicalStorage() && !LoadedFieldsFromExternalStorage) LoadFieldsFromExternalStorage(); Index: lib/AST/StmtProfile.cpp =================================================================== --- lib/AST/StmtProfile.cpp (revision 213614) +++ lib/AST/StmtProfile.cpp (working copy) @@ -1009,6 +1009,8 @@ for (LambdaExpr::capture_iterator C = S->explicit_capture_begin(), CEnd = S->explicit_capture_end(); C != CEnd; ++C) { + if (C->capturesVLABound()) + continue; ID.AddInteger(C->getCaptureKind()); switch (C->getCaptureKind()) { case LCK_This: Index: lib/AST/ExprCXX.cpp =================================================================== --- lib/AST/ExprCXX.cpp (revision 213614) +++ lib/AST/ExprCXX.cpp (working copy) @@ -905,7 +905,7 @@ case LCK_ByCopy: Bits |= Capture_ByCopy; - // Fall through + break; case LCK_ByRef: assert(Var && "capture must have a variable!"); break; @@ -915,7 +915,8 @@ LambdaCaptureKind LambdaCapture::getCaptureKind() const { Decl *D = DeclAndBits.getPointer(); - if (!D) + bool CapByCopy = DeclAndBits.getInt() & Capture_ByCopy; + if (!D && !CapByCopy) return LCK_This; return (DeclAndBits.getInt() & Capture_ByCopy) ? LCK_ByCopy : LCK_ByRef; Index: lib/AST/Expr.cpp =================================================================== --- lib/AST/Expr.cpp (revision 213614) +++ lib/AST/Expr.cpp (working copy) @@ -2992,7 +2992,7 @@ const LambdaExpr *LE = cast<LambdaExpr>(this); for (LambdaExpr::capture_iterator I = LE->capture_begin(), E = LE->capture_end(); I != E; ++I) - if (I->getCaptureKind() == LCK_ByCopy) + if (I->getCaptureKind() == LCK_ByCopy && I->capturesVariable()) // FIXME: Only has a side-effect if the variable is volatile or if // the copy would invoke a non-trivial copy constructor. return true; Index: lib/AST/StmtPrinter.cpp =================================================================== --- lib/AST/StmtPrinter.cpp (revision 213614) +++ lib/AST/StmtPrinter.cpp (working copy) @@ -1698,6 +1698,8 @@ CEnd = Node->explicit_capture_end(); C != CEnd; ++C) { + if (C->capturesVLABound()) + continue; if (NeedComma) OS << ", "; NeedComma = true; Index: lib/CodeGen/CGExprCXX.cpp =================================================================== --- lib/CodeGen/CGExprCXX.cpp (revision 213614) +++ lib/CodeGen/CGExprCXX.cpp (working copy) @@ -1800,19 +1800,23 @@ void CodeGenFunction::EmitLambdaExpr(const LambdaExpr *E, AggValueSlot Slot) { RunCleanupsScope Scope(*this); - LValue SlotLV = MakeAddrLValue(Slot.getAddr(), E->getType(), - Slot.getAlignment()); + LValue SlotLV = + MakeAddrLValue(Slot.getAddr(), E->getType(), Slot.getAlignment()); CXXRecordDecl::field_iterator CurField = E->getLambdaClass()->field_begin(); for (LambdaExpr::capture_init_iterator i = E->capture_init_begin(), e = E->capture_init_end(); i != e; ++i, ++CurField) { // Emit initialization - LValue LV = EmitLValueForFieldInitialization(SlotLV, *CurField); - ArrayRef<VarDecl *> ArrayIndexes; - if (CurField->getType()->isArrayType()) - ArrayIndexes = E->getCaptureInitIndexVars(i); - EmitInitializerForField(*CurField, LV, *i, ArrayIndexes); + if (CurField->hasCapturedVLABound()) { + auto *VAT = CurField->getCapturedVLABound(); + EmitStoreThroughLValue(RValue::get(VLASizeMap[VAT->getSizeExpr()]), LV); + } else { + ArrayRef<VarDecl *> ArrayIndexes; + if (CurField->getType()->isArrayType()) + ArrayIndexes = E->getCaptureInitIndexVars(i); + EmitInitializerForField(*CurField, LV, *i, ArrayIndexes); + } } } Index: lib/CodeGen/CodeGenFunction.cpp =================================================================== --- lib/CodeGen/CodeGenFunction.cpp (revision 213614) +++ lib/CodeGen/CodeGenFunction.cpp (working copy) @@ -663,6 +663,12 @@ CXXThisValue = EmitLoadOfLValue(ThisLValue, SourceLocation()).getScalarVal(); } + for (auto *FD : MD->getParent()->fields()) { + if (FD->hasCapturedVLABound()) + VLASizeMap[FD->getCapturedVLABound()->getSizeExpr()] = + EmitLoadOfLValue(EmitLValueForLambdaField(FD), SourceLocation()) + .getScalarVal(); + } } else { // Not in a lambda; just use 'this' from the method. // FIXME: Should we generate a new load for each use of 'this'? The Index: lib/CodeGen/CGDebugInfo.cpp =================================================================== --- lib/CodeGen/CGDebugInfo.cpp (revision 213614) +++ lib/CodeGen/CGDebugInfo.cpp (working copy) @@ -850,12 +850,11 @@ C.getLocation(), Field->getAccess(), layout.getFieldOffset(fieldno), VUnit, RecordTy); elements.push_back(fieldType); - } else { + } else if (C.capturesThis()) { // TODO: Need to handle 'this' in some way by probably renaming the // this of the lambda class and having a field member of 'this' or // by using AT_object_pointer for the function and having that be // used as 'this' for semantic references. - assert(C.capturesThis() && "Field that isn't captured and isn't this?"); FieldDecl *f = *Field; llvm::DIFile VUnit = getOrCreateFile(f->getLocation()); QualType type = f->getType();
_______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits