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

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 marked 3 inline comments as done.
jhuber6 added a comment.

Here's the tests it fails, there might be a few that are wrong for reasons beyond the size_t stuff but it's hard to tell until that issue is resolved. The cuda test is just because I have CUDA set up incorrectly on my machine, libomp used to pass until I changed it to us getOrCreateRuntimeFunction I think.

  Clang :: Driver/cuda-simple.cu
  Clang :: OpenMP/barrier_codegen.cpp
  Clang :: OpenMP/distribute_parallel_for_reduction_codegen.cpp
  Clang :: OpenMP/distribute_parallel_for_reduction_task_codegen.cpp
  Clang :: OpenMP/for_reduction_task_codegen.cpp
  Clang :: OpenMP/nvptx_target_parallel_reduction_codegen.cpp
  Clang :: OpenMP/nvptx_teams_reduction_codegen.cpp
  Clang :: OpenMP/openmp_win_codegen.cpp
  Clang :: OpenMP/parallel_firstprivate_codegen.cpp
  Clang :: OpenMP/parallel_for_reduction_task_codegen.cpp
  Clang :: OpenMP/parallel_master_reduction_task_codegen.cpp
  Clang :: OpenMP/parallel_reduction_task_codegen.cpp
  Clang :: OpenMP/parallel_sections_reduction_task_codegen.cpp
  Clang :: OpenMP/sections_reduction_task_codegen.cpp
  Clang :: OpenMP/target_depend_codegen.cpp
  Clang :: OpenMP/target_enter_data_depend_codegen.cpp
  Clang :: OpenMP/target_exit_data_depend_codegen.cpp
  Clang :: OpenMP/target_parallel_depend_codegen.cpp
  Clang :: OpenMP/target_parallel_for_depend_codegen.cpp
  Clang :: OpenMP/target_parallel_for_reduction_task_codegen.cpp
  Clang :: OpenMP/target_parallel_for_simd_depend_codegen.cpp
  Clang :: OpenMP/target_parallel_reduction_task_codegen.cpp
  Clang :: OpenMP/target_simd_depend_codegen.cpp
  Clang :: OpenMP/target_teams_depend_codegen.cpp
  Clang :: OpenMP/target_teams_distribute_depend_codegen.cpp
  Clang :: OpenMP/target_teams_distribute_parallel_for_depend_codegen.cpp
  Clang :: OpenMP/target_teams_distribute_parallel_for_reduction_codegen.cpp
  Clang :: OpenMP/target_teams_distribute_parallel_for_reduction_task_codegen.cpp
  Clang :: OpenMP/target_teams_distribute_parallel_for_simd_depend_codegen.cpp
  Clang :: OpenMP/target_teams_distribute_parallel_for_simd_reduction_codegen.cpp
  Clang :: OpenMP/target_teams_distribute_reduction_codegen.cpp
  Clang :: OpenMP/target_teams_distribute_simd_depend_codegen.cpp
  Clang :: OpenMP/target_teams_distribute_simd_reduction_codegen.cpp
  Clang :: OpenMP/target_teams_map_codegen.cpp
  Clang :: OpenMP/target_update_depend_codegen.cpp
  Clang :: OpenMP/teams_distribute_parallel_for_reduction_codegen.cpp
  Clang :: OpenMP/teams_distribute_parallel_for_reduction_task_codegen.cpp
  Clang :: OpenMP/teams_distribute_parallel_for_simd_reduction_codegen.cpp
  Clang :: OpenMP/teams_distribute_reduction_codegen.cpp
  Clang :: OpenMP/teams_distribute_simd_reduction_codegen.cpp
  libomp :: tasking/kmp_taskloop.c
  libomp :: worksharing/for/kmp_doacross_check.c



================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:203
+  static Function *getOrCreateRuntimeFunction(Module &Md,
+                                              omp::RuntimeFunction FnID);
 
----------------
jdoerfert wrote:
> Nit: M is the commonly used name for a module ;)
The OpenMPIRBuilder struct has a Module named M so I wasn't sure if I should be shadowing it.


================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def:370
 __OMP_RTL(__kmpc_critical_with_hint, false, Void, IdentPtr, Int32,
-          KmpCriticalNamePtrTy, Int32)
+          KmpCriticalNamePtrTy, /* int32? */ Int32)
 __OMP_RTL(__kmpc_end_critical, false, Void, IdentPtr, Int32,
----------------
jdoerfert wrote:
> What about the comments with a `?`?
That's just where I was unsure because I saw some conflicting information, like this one with Int32 in the runtime and uintptr_t in Clang.


================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:103
+      }
+    } else if (Fn->getName() == "__kmpc_fork_teams") {
+      if (!Fn->hasMetadata(LLVMContext::MD_callback)) {
----------------
jdoerfert wrote:
> Please use `FnID` for the comparison.
Don't know why I didn't think of doing that, will do.


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

https://reviews.llvm.org/D80222