This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Assembler: support SDWA for VOPC instructions
ClosedPublic

Authored by SamWot on Jun 15 2016, 5:57 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

SamWot updated this revision to Diff 60824.Jun 15 2016, 5:57 AM
SamWot retitled this revision from to [AMDGPU] Assembler: support SDWA for VOPC instructions.
SamWot updated this object.
SamWot added reviewers: artem.tamazov, vpykhtin.
artem.tamazov accepted this revision.Jun 16 2016, 5:07 AM
artem.tamazov edited edge metadata.
artem.tamazov added inline comments.
lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
2796–2797 ↗(On Diff #60824)

I am curious -- why 6? suggest adding a comment.

This revision is now accepted and ready to land.Jun 16 2016, 5:07 AM
SamWot added inline comments.Jun 16 2016, 5:10 AM
lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
2796–2797 ↗(On Diff #60824)

This is default value for SDWA src_sel operand. We don't add any comments in any other places so I don't think it is reasonable to add it here. Possibly we should refactor out all default values but it is out of scope of this change.

arsenm added inline comments.Jun 16 2016, 9:17 AM
lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
2800–2801 ↗(On Diff #60824)

So this will crash if you try to use this with an SALU instruction? This should probably be a report_fatal_error, or better if you can get to the diagnostic handler. There should also be a test for this

SamWot added inline comments.Jun 16 2016, 9:30 AM
lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
2800–2801 ↗(On Diff #60824)

This method is called only by LLVM generated AsmMatcher only for SDWA which are only VOP1/2/C. This llvm_unreachable is needed only as default case for switch.

This revision was automatically updated to reflect the committed changes.