[PATCH] D81031: [OpenMP] Add Additional Function Attribute Information to OMPKinds.def

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert accepted this revision.
jdoerfert added a comment.
This revision is now accepted and ready to land.

Apologies for the wait. I think with the comments below this is good to go. If you have questions or concerns, let me know. Thanks for these changes!



================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def:587
+                    ? AttributeSet(EnumAttr(NoUnwind), EnumAttr(WillReturn))
                     : AttributeSet(EnumAttr(NoUnwind)))
 
----------------
I think (for now) we should even remove willreturn here. I'm not sure we have forward progress guarantees for C + OpenMP :(


================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def:621
+                    ? AttributeSet(EnumAttr(WriteOnly), EnumAttr(NoFree))
+                    : AttributeSet())
+
----------------
Here and above, `nocapture`.


================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def:632
+                                   EnumAttrInt(DereferenceableOrNull, 8))
+                    : AttributeSet())
+
----------------
I guess this means the attribute is returned, right? If we are sure it is, we can add the `returned` attribute. I checked `__kmpc_threadprivate_cached` and I wasn't sure the argument is actually returned, maybe I misunderstand what this means. We could add a comment here.

We should also not go for alignment or dereferenceability for now.


================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def:729
+__OMP_RTL_ATTRS(__kmpc_end_critical, BarrierAttrs, AttributeSet(),
+                ParamAttrs(ReadOnlyPtrAttrs, AttributeSet(), AttributeSet()))
+
----------------
I think this will mitigate PR46210, which is good! 


================
Comment at: llvm/test/Transforms/OpenMP/add_attributes.ll:1542
+; OPTIMISTIC: ; Function Attrs: inaccessiblemem_or_argmemonly nofree nosync nounwind willreturn
+; OPTIMISTIC-NEXT: declare void @__kmpc_end_single(%struct.ident_t* nofree readonly, i32)
+
----------------
the `single` ones should be barrier like


================
Comment at: llvm/test/Transforms/OpenMP/add_attributes.ll:1551
+; OPTIMISTIC: ; Function Attrs: nofree nosync nounwind willreturn
+; OPTIMISTIC-NEXT: declare void @__kmpc_end_taskgroup(%struct.ident_t* nofree readonly, i32)
+
----------------
barrier like


================
Comment at: llvm/test/Transforms/OpenMP/add_attributes.ll:1614
+; OPTIMISTIC: ; Function Attrs: nofree nosync nounwind willreturn
+; OPTIMISTIC-NEXT: declare void @__kmpc_omp_wait_deps(%struct.ident_t* nofree readonly, i32, i32, i8* nofree readonly, i32, i8*)
+
----------------
barrier like, probably all `wait` functions


================
Comment at: llvm/test/Transforms/OpenMP/add_attributes.ll:1653
+; OPTIMISTIC: ; Function Attrs: nofree nosync nounwind willreturn
+; OPTIMISTIC-NEXT: declare void @__kmpc_doacross_wait(%struct.ident_t* nofree readonly, i32, i64* nofree readonly)
+
----------------
this is a barrier like directive. Let's just mark all doacross as such for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81031