This is an archive of the discontinued LLVM Phabricator instance.

Triple: Add AMDPAL operating system type
ClosedPublic

Authored by tpr on Sep 1 2017, 8:24 AM.

Details

Summary

This operating system type represents the AMDGPU PAL runtime, and will
be required by the AMDGPU backend in order to generate correct code for
this runtime.

Currently it generates the same code as not specifying an OS at all.
That will change in future commits.

Patch from Tim Corringham.

Diff Detail

Repository
rL LLVM

Event Timeline

tpr created this revision.Sep 1 2017, 8:24 AM
arsenm edited edge metadata.Sep 1 2017, 10:53 AM

These patches usually include tests in unittests/ADT/TripleTest

tpr added a comment.Sep 1 2017, 2:11 PM

I couldn't figure out a way of usefully testing it. It doesn't make any difference to code output with this commit, and specifying the new OS type in the triple does not fail if you do not have the commit.

nhaehnle edited edge metadata.Sep 4 2017, 12:45 PM

TripleTest.cpp has some minimally meaningful tests though. So a test of amdgcn-amd-amdpal should be added there. Apart from that the patch looks good.

tpr updated this revision to Diff 113859.Sep 5 2017, 7:45 AM

Added test, although it's pointless for now as it passes even without the code
change.

How about in unittests/ADT/TripleTest.cpp?

tpr updated this revision to Diff 114600.Sep 11 2017, 7:11 AM

Added real test (thanks for pointer Nicolai).

tpr added a comment.Sep 12 2017, 10:48 PM

Sorry Matt, just noticed that you also told me about TripleTest earlier on.

tpr added a comment.Sep 20 2017, 2:05 AM

Ping.

Thanks Matt for the approvals on those other two changes, but I need this one to go in first...

kzhuravl accepted this revision.Sep 27 2017, 12:08 PM
kzhuravl added a subscriber: kzhuravl.

LGTM

This revision is now accepted and ready to land.Sep 27 2017, 12:08 PM
This revision was automatically updated to reflect the committed changes.