[PATCH] D69922: [OpenMP] Use the OpenMP-IR-Builder

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.


================
Comment at: clang/test/OpenMP/barrier_codegen.cpp:22
+// CLANGCG-NOT: readonly
+// IRBUILDER:      ; Function Attrs: nofree nosync nounwind readonly
+// IRBUILDER-NEXT: declare i32 @__kmpc_global_thread_num(%struct.ident_t*)
----------------
jdoerfert wrote:
> ABataev wrote:
> > Not sure about correct use of `nosync` and `readonly` attributes. OpenMP runtime uses lazy initialization of the runtime library and when any runtime function is called, the inner parts of the OpenMP runtime are initialized automatically. It may use some sync primitives and may modify memory, I assume. Same about `nofree`.
> There are two versions of these functions, host and device. I assume host functions are not inlined but device functions might be. This is basically all the modes we support right now.
> 
> If we do not inline the function (host) we don't necessarily care what they do but what effect the user can expect.
> The user can not expect to synchronize through `__kmpc_global_thread_num` calls in a defined way, thus `nosync`.
> Similarly, from the users perspective there is no way to determine if something was written or freed, no matter how many of these calls I issue and under which control conditions, all I see is the number as a result. Thus, `readonly` and `nofree`. I believe `readnone` is even fine here but it might not work for the device version (see below) so I removed it.
> 
> If we do inline the function (device) we need to make sure the attributes are compatible with the inlined code to not cause UB. The code of `__kmpc_global_thread_num` at least does not write anything and only reads stuff (as far as I can tell).
> 
> Please correct me if I overlooked something. 
But someone may try to inline the host-based runtime or try to use LTO with it.
The question is not about the user expectations but about the optimizations which can be triggered with these attributes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69922