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

[Johannes Doerfert via Phabricator via Openmp-commits]

jdoerfert added inline comments.


================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def:250
+__OMP_SIZE_TYPE(SizeTy)
+#undef __OMP_SIZE_TYPE
+
----------------
Why the indirection via `__OMP_SIZE_TYPE`? Wouldn't `OMP_TYPE(SizeTy, M.getDataLayout().getIntPtrType(Ctx))` suffice?


================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:124
+    assert(FnTy == Fn->getFunctionType() &&
+           "Found OpenMP runtime function has mismatched types");
   }
----------------
I now think we either put the stuff in `#ifndef NDEBUG` or move the `FnTy =` into the first switch that does the lookup.


================
Comment at: openmp/runtime/test/tasking/kmp_taskloop.c:100
         th_counter[i] = 0;
-    #pragma omp parallel num_threads(N)
+    #pragma omp parallel // num_threads(N)
     {
----------------
jhuber6 wrote:
> I am not entirely sure why, but commenting this out causes the problem to go away. I tried adding proper names to the forward-declared functions but since clang already knew I had something called ident_t, I couldn't declare a new struct with the same name.
This is not good. The difference should only be that the `kmpc_fork_call` has a different argument, right? Does the segfault happen at compile or runtime?

You can just use the ident_t clang created, right? Did you print the function names requested by clang as we discussed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80222