[PATCH] D89746: [OpenMP] Emit calls to int64_t functions for amdgcn

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

Generally yes. Some comments. Can you add the functions to the test that adds attributes. It should "pick up" the signature and add attributes only for the right one. We might need to add the functions to some attribute group in the OMPKinds.def but that would be good anyway, even if we just do `nounwind`.



================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:433
   ///{
+  Type *LanemaskTy = nullptr;
 #define OMP_TYPE(VarName, InitValue) Type *VarName = nullptr;
----------------
I would suggest to put it in OMPKinds.def similar to size_t:
  `OMP_TYPE(SizeTy, M.getDataLayout().getIntPtrType(Ctx))`
I guess you can keep `getLanemaskType` and use it though.


================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:66
   Function *Fn = nullptr;
+  Type *LanemaskTy = getLanemaskType();
 
----------------
This shadows the other declaration, doesn't it? See also other comments.


================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:1190
   StructType *T;
+  LanemaskTy = getLanemaskType();
+
----------------
Not needed once in OMPKinds.def, probably the shadowing stuff neither.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:334
     Module &M = *((*ModuleSlice.begin())->getParent());
+    Type *LanemaskTy = OMPBuilder.getLanemaskType();
 
----------------
Looks again like shadowing, necessary?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89746