jdoerfert added a comment.
I have only minor remarks but I'd like you to check if my hunch is correct and
the proposed modifications will fix fix PR49777 *and* fix PR49779.
Also, the number of arguments need to be increased, let's go big and automatic
here.
Other than that I think this looks good.
================
Comment at: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp:2192
RCG(CGF);
}
}
----------------
Can we remove SeqGen while we are here please. We need to check in the runtime
anyway. That check is later folded, no need to make things more complicated
here.
================
Comment at: openmp/libomptarget/deviceRTLs/common/src/parallel.cu:294
+ // TODO: Add UNLIKELY to optimize?
+ if (!if_expr) {
+ __kmpc_serialized_parallel(ident, global_tid);
----------------
This should allow us to remove the `SeqGen` in the Clang CodeGen *and* fix
PR49777 *and* fix PR49779, a win-win-win situation.
================
Comment at: openmp/libomptarget/deviceRTLs/common/src/parallel.cu:368
+ // __kmpc_push_proc_bind(ident, global_tid, proc_bind);
+}
+
----------------
FWIW, The implementation here is a stopgap until we move to the new runtime.
The codegen and interface are the important parts.
================
Comment at: openmp/libomptarget/deviceRTLs/common/src/support.cu:370
+ printf("Too many arguments in kmp_invoke_microtask, aborting
execution.\n");
+ return;
+ }
----------------
Not a return but a `__builtin_trap()`, please.
We also need this for more than 16 unfortunately, I've seen 20 in miniqmc.
We might want to create a script to print the cases, and then generate 128 or
something like that in a file we include. The script can be in the utils folder
too.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95976/new/
https://reviews.llvm.org/D95976
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
