[PATCH] D69785: [OpenMP] Introduce the OpenMP-IR-Builder
Johannes Doerfert via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Nov 28 10:49:43 PST 2019
jdoerfert marked an inline comment as done.
jdoerfert added a comment.
In D69785#1762438 <https://reviews.llvm.org/D69785#1762438>, @JonChesterfield wrote:
> I'd very much like this to land soon. It's the prereq for a lot of other patches and the code looks good.
>
> It's tricky to test the infra before the users are landed so the unit test is particularly appreciated.
I'm confused. Was this a review? I'm waiting for a decision here so we can move on and improve on this instead of me modifying it inp-lace two comments at a time.
================
Comment at: llvm/include/llvm/Frontend/OpenMPKinds.def:165
+
+__OMP_RTL(__kmpc_barrier, false, Void, IdentPtr, Int32)
+__OMP_RTL(__kmpc_cancel_barrier, false, Int32, IdentPtr, Int32)
----------------
rogfer01 wrote:
> As we migrate, we will end with a significant number of interfaces here.
>
> @jdoerfert what do you think about adding a comment with their C prototype before each one like we do in `clang/lib/CodeGen/CGOpenMPRuntime.cpp`?
>
> Something like this
>
> ```lang=cpp
> // void __kmpc_barrier(ident_t *loc, kmp_int32 global_tid);
> __OMP_RTL(__kmpc_barrier, false, Void, IdentPtr, Int32)
> // kmp_int32 __kmpc_cancel_barrier(ident_t *loc, kmp_int32
> // global_tid)
> __OMP_RTL(__kmpc_cancel_barrier, false, Int32, IdentPtr, Int32)
> ...
> ```
I'm fine with this but I doubt it'll help much (compared to the lines we have that show name and types).
If you want this to happen you should create a patch do add comments for the ones we have here, and others can way in. If there is agreement to apply it, we will do so and continue that tradition from then on. Does that sound good?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69785/new/
https://reviews.llvm.org/D69785
More information about the cfe-commits
mailing list