[PATCH] D142914: [MLIR][OpenMP] Added OMPIRBuilder support for Target Data directives.

Johannes Doerfert via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 7 08:53:14 PST 2023


jdoerfert added a comment.

Please avoid the MLIR style in any other subproject. I know some of the OpenMP Builder stuff already uses `llvm::` and MLIR style names, but that is in itself bad, not something we should extend.



================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:1092
+  /// Computes the size of type in bytes.
+  llvm::Value *getSizeInBytes(llvm::Value *basePtr);
+
----------------
Nit: Style, no llvm::


================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:1573
+                   llvm::Value *ifCond, BodyGenCallbackTy processMapOpCB,
+                   BodyGenCallbackTy BodyGenCB);
+
----------------
Style, see above.


================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:4093
+  return Builder.saveIP();
+}
+
----------------
Style, see above. 


================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:4152
+  return sizePtrToInt;
+}
+
----------------
Style, see above. 


================
Comment at: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp:4991
+  EXPECT_TRUE(targetDataCall->getOperand(8)->getType()->isPointerTy());
+}
+
----------------
Style, see above. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142914



More information about the llvm-commits mailing list