[PATCH] D147218: [OpenMP][Flang][MLIR] Lowering of OpenMP requires directive from parse tree to MLIR

[Sergio Afonso via Phabricator via cfe-commits]

skatrak added a comment.

Thank you for the review. After I address your last comment my plan is to land this and the other two accepted REQUIRES patches that depended on it.

Is there a preferred approach as to how to go about it? I could rebase them all and wait until the pre-merge builds finish without errors and then land them in quick succession or I could go one by one to make sure post-merge builds don't find any issues before landing the next, which will be over a couple of days most likely.



================
Comment at: flang/lib/Lower/Bridge.cpp:2366-2367
     mlir::OpBuilder::InsertPoint insertPt = builder->saveInsertionPoint();
+    analyzeOpenMPDeclarativeConstruct(*this, getEval(), ompDecl,
+                                      ompDeviceCodeFound);
     genOpenMPDeclarativeConstruct(*this, getEval(), ompDecl);
----------------
kiranchandramohan wrote:
> Can this be rewritten this way.
> 
> And rename `analyzeOpenMPDeclarativeConstruct` to `isTargetDeclare` or something like that?
My idea was to make it generic to allow the analysis of other declarative constructs in the future there as well, that's the reason for the function name and returning through an output argument. But I'll follow your suggestion, since we don't know for sure that we'll need this later on.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147218