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

Johannes Doerfert via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Feb 17 16:07:38 PST 2020


jdoerfert accepted this revision.
jdoerfert added a comment.
This revision is now accepted and ready to land.

LGTM assuming the comment below is addressed.



================
Comment at: clang/lib/CodeGen/CodeGenFunction.h:354
+
+        FinilizationBlock = CGF.getJumpDestInCurrentScope(&FiniBB);
+      }
----------------
fghanim wrote:
> 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.
I see. Maybe remove the `FinilizationBlock` member though. Call `getJumpDestInCurrentScope` and add a comment (TODO) that explains this, e.g., says it has to be investigated further.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74562





More information about the cfe-commits mailing list