[PATCH] D127037: [OpenMP][IRBuilder][OpenACC] Move common code from OpenACCToLLVMIRTranslation to Utils/DirectiveToLLVMIRTranslation so that it can be used between OpenACC and OpenMP.

[Raghu via Phabricator via llvm-commits]

raghavendhra added inline comments.


================
Comment at: mlir/lib/Target/LLVMIR/Dialect/Utils/DirectiveToLLVMIRTranslation.cpp:288
+
+  if(isa<acc::DataOp>(op)){
+    acc::DataOp dataOp = dyn_cast<acc::DataOp>(op);
----------------
clementval wrote:
> shraiysh wrote:
> > clementval wrote:
> > > I'm not sure it makes sense to reuse the part where we process each operands. There is likely no 1 to 1 mapping between OpenMP and OpenACC so it's likely the part that will be specific for each operation. 
> > how about adding a common data interface to both data operations and use that here?
> Not sure that will work either. We need to be able to fine tune the flags that are passed for each operands. This will not be common between the two languages for at least some of them. 
I tried moving entire mlir::convertDataOp() function and its supporting utilities back to OpenACCToLLVMIRTranslation.cpp and tried to maintain convertStandaloneDataOp() and some of it's utilities in DirectiveToLLVMIRTranslation.cpp but that change is causing cmake cyclic dependency as each library is dependent on other library. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127037