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

Tue Jul 12 12:03:41 PDT 2022

clementval 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);
----------------
raghavendhra wrote:
> 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. 
Utils should probably not depend on OpenACC/OpenMP translation but the opposite is more likely. 

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127037