[PATCH] D79677: [clang][OpenMP][OMPIRBuilder] Adding some Privatization clauses to OpenMP `parallel` Directive

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

Generally you copied the existing Clang logic, correct?



================
Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:803
-                                                                      OrigVD);
-        else
-          (void)CGM.getOpenMPRuntime().registerTargetFirstprivateCopy(*this,
----------------
Wasn't this part of D79675?

(btw I tried to comprehend why this is needed and it is on my list for things we replace eventually).


================
Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:887
                           *this, OrigVD);
+
                   llvm::Value *V = EmitLoadOfScalar(
----------------
Please undo as it doesn't seem to change anything.


================
Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:3736
       HintInst =
-          Builder.CreateIntCast(EmitScalarExpr(Hint), CGM.Int32Ty, false);
+          Builder.CreateIntCast(EmitScalarExpr(Hint), CGM.IntPtrTy, false);
 
----------------
These 4 lines can go in on their own, LGTM on them.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79677/new/

https://reviews.llvm.org/D79677