This is an archive of the discontinued LLVM Phabricator instance.

[OpenMP] Emit calls to int64_t functions for amdgcn
ClosedPublic

Authored by JonChesterfield on Oct 19 2020, 3:22 PM.

Details

Summary

[OpenMP] Emit calls to int64_t functions for amdgcn

Two functions, syncwarp and active_thread_mask, return lanemask_t. Currently
this is assumed to be int32, which is true for nvptx. Patch makes the type
target architecture dependent.

Diff Detail

Event Timeline

JonChesterfield requested review of this revision.Oct 19 2020, 3:22 PM
llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
221

This seems ugly - is there a better path from module to triple?

Generally yes. Some comments. Can you add the functions to the test that adds attributes. It should "pick up" the signature and add attributes only for the right one. We might need to add the functions to some attribute group in the OMPKinds.def but that would be good anyway, even if we just do nounwind.

llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
433

I would suggest to put it in OMPKinds.def similar to size_t:

`OMP_TYPE(SizeTy, M.getDataLayout().getIntPtrType(Ctx))`

I guess you can keep getLanemaskType and use it though.

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
66

This shadows the other declaration, doesn't it? See also other comments.

1189

Not needed once in OMPKinds.def, probably the shadowing stuff neither.

llvm/lib/Transforms/IPO/OpenMPOpt.cpp
334 ↗(On Diff #299192)

Looks again like shadowing, necessary?

  • address some review comments
JonChesterfield marked 3 inline comments as done.Oct 21 2020, 4:50 PM
JonChesterfield added a comment.EditedOct 21 2020, 4:52 PM

Good call on shadowing, those are all gone. Attributes are more difficult as the test is IR, so can't declare the function twice. Haven't managed to tag nounwind onto either yet but will try a few more things.

  • tests for i32 version of active_thread_mask, syncwarp
  • test attributes on syncwarp, thread_mask
This revision is now accepted and ready to land.Oct 21 2020, 7:38 PM
This revision was landed with ongoing or failed builds.Oct 22 2020, 7:03 AM
This revision was automatically updated to reflect the committed changes.