[PATCH] D74562: [OpenMP][OMPIRBuilder] Introducing the `OMPBuilderCBHelpers` helper class

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

This is great, thanks a lot! I only have two comments where I am not sure I understand the code/change.



================
Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:3134
 
     // TODO: Replace with a generic helper function for emitting body
     auto BodyGenCB = [MasterRegionBodyStmt, this](InsertPointTy AllocaIP,
----------------
These TODOs are now obsolete ;)


================
Comment at: clang/lib/CodeGen/CodeGenFunction.h:354
+
+        FinilizationBlock = CGF.getJumpDestInCurrentScope(&FiniBB);
+      }
----------------
Don't we have to set/reset the `CGF.ReturnBlock` ? If not, why do we need `FinilizationBlock` here?


================
Comment at: clang/test/OpenMP/parallel_codegen.cpp:143
 // CHECK-DEBUG:       define internal void [[OMP_OUTLINED_DEBUG:@.+]](i32* noalias %.global_tid., i32* noalias %.bound_tid., i8*** dereferenceable({{4|8}}) %argc, i64 %{{.+}})
-// IRBUILDER-DEBUG:   define internal void [[OMP_OUTLINED_DEBUG:@.+]](i32* noalias %{{.*}}, i32* noalias %{{.*}}, i8*** [[ARGC_REF:%.*]], i64 %{{.+}})
+// IRBUILDER-DEBUG:   define internal void [[OMP_OUTLINED_DEBUG:@.+]](i32* noalias %{{.*}}, i32* noalias %{{.*}}, i8*** [[ARGC_REF:%.*]], double** [[VAR]], i64 %{{.+}})
 // CHECK-DEBUG:       store i8*** %argc, i8**** [[ARGC_PTR_ADDR:%.+]],
----------------
Do you know why this changed? Is this variable used in the parallel region?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74562