This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU][MC][GFX8][GFX9] Added XNACK_MASK support
ClosedPublic

Authored by dp on Dec 28 2017, 2:56 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

dp created this revision.Dec 28 2017, 2:56 AM
dp updated this revision to Diff 128282.Dec 28 2017, 3:17 AM

Removed incorrect assert in disassembler;
added a few tests for GFX9

artem.tamazov accepted this revision.Dec 28 2017, 5:13 AM
This revision is now accepted and ready to land.Dec 28 2017, 5:13 AM
arsenm added inline comments.Dec 28 2017, 8:41 AM
lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
2607 ↗(On Diff #128282)

Shouldn't this be xnack enabled (or at least >= VI)

dp added inline comments.Dec 28 2017, 8:55 AM
lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
2607 ↗(On Diff #128282)

Thanks, Matt! I'll correct that shortly.

artem.tamazov added inline comments.Dec 28 2017, 8:57 AM
lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
2607 ↗(On Diff #128282)

I am not sure. https://github.com/ROCm-Developer-Tools/ROCm-ComputeABI-Doc/blob/master/AMDGPU-ABI.md#instruction-set-architecture says:
AMD AMDGPU 8 0 1 GFX8, XNACK enabled A10-8700 series APU

arsenm added inline comments.Dec 28 2017, 9:07 AM
lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
2607 ↗(On Diff #128282)

I suppose for encoding/decoding purposes, I suppose it won't make a difference, it will just produce an unusable program for the target. Maybe a warning?

dp added inline comments.Dec 28 2017, 9:20 AM
lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
2607 ↗(On Diff #128282)

We have a FeatureXNACK, but it is enabled only for 8_0_1 and 8_1_0. And not for any of GFX9 targets. So I do not think we could use that. Is the feature list for GFX9 incomplete?

dp added inline comments.Dec 28 2017, 9:33 AM
lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
2607 ↗(On Diff #128282)

Do you know which GFX9 targets support this feature?

kzhuravl added inline comments.Dec 28 2017, 9:59 AM
lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
2607 ↗(On Diff #128282)

+ @t-tye

I think this should check the FeatureXNACK (and maybe >=VI, xnack started from VI). Users can pass -mxnack, -mno-xnack regardless the target from clang, which will set FeatureXNACK to true or false respectively. Runtime has to ensure it is valid.

https://llvm.org/docs/AMDGPUUsage.html#target-features

dp updated this revision to Diff 128334.Dec 29 2017, 7:16 AM

Corrected XNACK availability check as suggested by Matt and Konstantin; updated tests.

arsenm accepted this revision.Dec 29 2017, 8:11 AM

LGTM

artem.tamazov accepted this revision.Jan 9 2018, 8:01 AM
This revision was automatically updated to reflect the committed changes.