This is an archive of the discontinued LLVM Phabricator instance.

[HIP] compile option code-object-v3 propagate to llc
ClosedPublic

Authored by ashi1 on Feb 8 2019, 2:20 PM.

Diff Detail

Event Timeline

ashi1 created this revision.Feb 8 2019, 2:20 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 8 2019, 2:20 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
kzhuravl added a comment.EditedFeb 8 2019, 3:37 PM

Can you handle all features as follows instead of checking a particular option?

handleTargetFeaturesGroup(Args, Features, options::OPT_m_amdgpu_Features_Group)

There is code in AMDGPU tool chain you can use as a reference.

ashi1 updated this revision to Diff 186519.Feb 12 2019, 11:09 AM

Changed patch to us handleTargetFeatures, to consume all -m options passed into clang.

Can you add some tests? Preferably with -mxnack/-mno-xnack, -msram-ecc/-mno-sram-ecc, -mcode-object-v3/-mno-code-object-v3

ashi1 updated this revision to Diff 186528.Feb 12 2019, 12:22 PM

I've added a lit test to check for options such as -mxnack/-mno-xnack, -msram-ecc/-mno-sram-ecc, -mcode-object-v3/-mno-code-object-v3

ashi1 added subscribers: scchan, Restricted Project.Feb 12 2019, 12:33 PM
ashi1 removed a subscriber: Restricted Project.

Is it still code object v2 by default?

Yes, but it will need this PR: https://github.com/ROCm-Developer-Tools/HIP/pull/910
So that the default is set.

kzhuravl accepted this revision.Feb 12 2019, 12:41 PM

LGTM. Please make sure to update your commit message as your patch processes all m options now.

This revision is now accepted and ready to land.Feb 12 2019, 12:41 PM
This revision was automatically updated to reflect the committed changes.
arsenm added a subscriber: arsenm.Feb 12 2019, 2:46 PM

I think this breaks the attributes already listed in the functions on the IR since -mattr overrides those. I think we need to stop using subtarget features to communicate this

rnk added a subscriber: rnk.Feb 12 2019, 2:52 PM

I reverted this in rC353893 because it was still causing test failures.

This was submitted again with the fix. Thanks!