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

[Fady Ghanim via Phabricator via cfe-commits]

fghanim added a comment.

Regarding the third comment (which was removed for some reason), This new update should fix that



================
Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:3134
 
     // TODO: Replace with a generic helper function for emitting body
     auto BodyGenCB = [MasterRegionBodyStmt, this](InsertPointTy AllocaIP,
----------------
jdoerfert wrote:
> These TODOs are now obsolete ;)
We need to split our patches to keep them small :p


================
Comment at: clang/lib/CodeGen/CodeGenFunction.h:354
+
+        FinilizationBlock = CGF.getJumpDestInCurrentScope(&FiniBB);
+      }
----------------
jdoerfert wrote:
> Don't we have to set/reset the `CGF.ReturnBlock` ? If not, why do we need `FinilizationBlock` here?
Yes, and we still do for outlined regions (check `OutlinedRegionBodyRAII` ctor above). 

This is for inlined regions, and those shouldn't change the `CGF.ReturnBlock`. It should re-use the `CGF.ReturnBlock` for the enclosing outlined region.
The only reason I kept it, is because `CGF.getJumpDestInCurrentScope()` was updating a counter that seemed to me to relate to `EHStack`, and I didn't want to remove it without investigating first.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74562