[PATCH] D80222: Replace Clang's createRuntimeFunction with the definitions in OMPKinds.def

Fri May 22 09:06:00 PDT 2020

fghanim added inline comments.

================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def:244

-// TODO: Replace this with the real size_t type
-#define __OMP_SIZE_TYPE(NAME) OMP_TYPE(NAME, Type::getInt64Ty(Ctx))
+#define __OMP_SIZE_TYPE(NAME) OMP_TYPE(NAME, M.getDataLayout().getIntPtrType(Ctx))
 __OMP_SIZE_TYPE(SizeTy)
----------------
jhuber6 wrote:
> fghanim wrote:
> > jdoerfert wrote:
> > > jhuber6 wrote:
> > > > I'm just directly getting the SizeTy from the Module, I'm not sure if this is a permanent solution.
> > > I actually think this is fine. Less burden on the frontends also. @fghanim looks good?
> > Agreed. But to avoid conflict with patch D79675, let this change go in a separate patch after that. This way, we can also do some clean ups related to this, and add `Int8PtrPtr`, `Int8PtrPtrPtr`, and any other typing issues, without delaying patch D79675 any further - I am done with that patch, and made my final changes a couple of days ago.
> > 
> > Nit: Also, I saw below that some formatting was done. Wouldn't it be better if formatting/comments related to changes in D79739, are handled as part of that patch?
> Doesn't this patch already support Int8PtrPtrPtr? It's the same thing as a VoidPtrPtrPtr which I have defined above here.
You are correct. But think of it in the same way we have voidptr and int8ptr, even though they are the same.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80222