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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to