[PATCH] D17019: [OpenMP] Code generation for teams - kernel launching

[Samuel Antao via cfe-commits]

sfantao added inline comments.

================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:256-296
@@ -255,26 +255,43 @@
 /// \brief RAII for emitting code of OpenMP constructs.
 class InlinedOpenMPRegionRAII {
   CodeGenFunction &CGF;
+  /// \brief Saves the varaibles that were forced to be local in the current
+  /// inlined region.
+  SmallVector<const VarDecl *, 4> ForcedLocalVars;
 
 public:
-  /// \brief Constructs region for combined constructs.
+  /// \brief Constructs inlined region. Mostly used for combined constructs. If
+  /// a captured statement is provided it also ensures the captured variables
+  /// are all defined in the scope of the enclosing function. This is typical
+  /// used for regions that make local instances of global variables, e.g.
+  /// target regions.
   /// \param CodeGen Code generation sequence for combined directives. Includes
   /// a list of functions used for code generation of implicitly inlined
   /// regions.
   InlinedOpenMPRegionRAII(CodeGenFunction &CGF, const RegionCodeGenTy &CodeGen,
-                          OpenMPDirectiveKind Kind, bool HasCancel)
+                          OpenMPDirectiveKind Kind, bool HasCancel,
+                          const CapturedStmt *CS)
       : CGF(CGF) {
     // Start emission for the construct.
     CGF.CapturedStmtInfo = new CGOpenMPInlinedRegionInfo(
         CGF.CapturedStmtInfo, CodeGen, Kind, HasCancel);
+
+    // Ensures that all the captures are local in the current inlined region.
+    if (CS)
+      CGF.StartOpenMPInlinedCapturedRegion(*CS, ForcedLocalVars);
   }
 
   ~InlinedOpenMPRegionRAII() {
+    // Restore the local variable information if we have anything forced in this
+    // inlined region.
+    if (!ForcedLocalVars.empty())
+      CGF.CloseOpenMPInlinedCapturedRegion(ForcedLocalVars);
+
     // Restore original CapturedStmtInfo only if we're done with code emission.
     auto *OldCSI =
         cast<CGOpenMPInlinedRegionInfo>(CGF.CapturedStmtInfo)->getOldCSI();
     delete CGF.CapturedStmtInfo;
     CGF.CapturedStmtInfo = OldCSI;
   }
 };
 
----------------
ABataev wrote:
> Do not modify this one, add a new one like this:
> ```
> static void EmptyCodeGen(CodeGenFunction &) {
>   llvm_unreachable("No codegen for expressions");
> }
> /// \brief API for generation of expressions captured in OpenMP region in outer
> /// scope.
> class CGOpenMPOuterExprInfo : public CGOpenMPInlinedRegionInfo {
> public:
>   CGOpenMPOuterExprInfo(CodeGenFunction &CGF)
>       : CGOpenMPInlinedRegionInfo(CGF.CapturedStmtInfo, CodeGen, OMPD_unknown,
>                                   /*HasCancel=*/false),
>         CGF(CGF) {}
> 
>   /// \brief Lookup the captured field decl for a variable.
>   const FieldDecl *lookup(const VarDecl *VD) const override {
>     if (auto *FD = CGOpenMPInlinedRegionInfo::lookup(VD))
>       return FD;
>     if (!VD->isLocalVarDeclOrParm() && Mapped.count(VD) == 0) {
>       auto It =
>           PrivateGlobals.insert(new CodeGenFunction::OMPPrivateScope(CGF));
>       DeclRefExpr DRE(const_cast<VarDecl *>(VD),
>                       /*RefersToEnclosingVariableOrCapture=*/false,
>                       VD->getType().getNonReferenceType(), VK_LValue,
>                       SourceLocation());
>       It.first->addPrivate(
>           VD, [&]() -> Address { return CGF.EmitLValue(&DRE).getAddress(); });
>       (void)It.first->Privatize();
>       Mapped.insert(VD);
>     }
>     return nullptr;
>   }
> 
>   /// \brief Emit the captured statement body.
>   void EmitBody(CodeGenFunction &CGF, const Stmt *S) override {
>     llvm_unreachable("No body for expressions");
>   }
> 
>   /// \brief Get a variable or parameter for storing global thread id
>   /// inside OpenMP construct.
>   const VarDecl *getThreadIDVariable() const override {
>     llvm_unreachable("No thread id for expressions");
>   }
> 
>   /// \brief Get the name of the capture helper.
>   StringRef getHelperName() const override {
>     llvm_unreachable("No helper name for expressions");
>   }
> 
>   static bool classof(const CGCapturedStmtInfo *Info) {
>     llvm_unreachable("No helper name for expressions");
>   }
> 
>   virtual ~CGOpenMPOuterExprInfo() {
>     for (auto *Scope : PrivateGlobals)
>       delete Scope;
>     PrivateGlobals.clear();
>   }
> 
> private:
>   CodeGenFunction &CGF;
>   /// Private scopes for each captured global variables.
>   llvm::SmallPtrSet<CodeGenFunction::OMPPrivateScope *, 4> PrivateGlobals;
>   SmallSet<VarDecl *> Mapped;
> };
> ```
Ok, I adapted the code you pasted above and I am now creating a new inline region API. I am naming it `CGOpenMPInnerExprInfo` given that it relates to the emission of expression defined in the inner scope. Also, I am doing the privatization in the constructor given that the globals have to be local already by the time the expression is emitted. 


http://reviews.llvm.org/D17019