See Bug 33509: https://bugs.llvm.org//show_bug.cgi?id=33509
I've commented out a lot of asserts for packed constants. Should they be removed?
Paths
| Differential D34360
[AMDGPU][MC][GFX9] Corrected VOP3P relevant code to fix disassembler failures ClosedPublic Authored by dp on Jun 19 2017, 12:02 PM.
Details Summary See Bug 33509: https://bugs.llvm.org//show_bug.cgi?id=33509 I've commented out a lot of asserts for packed constants. Should they be removed?
Diff Detail Event TimelineHerald added subscribers: t-tye, tpr, dstuttard and 5 others. · View Herald TranscriptJun 19 2017, 12:02 PM Comment Actions
After quick glance -- You can run our standard set of AMDGPU backend tests (notably, CodeGen tests) *and* set of negative TestGen-produced tests to see if those are important. Comment Actions
Of course, this is acceptable as a short-term solution only. I believe those asserts were introduced intentionally. Comment Actions
I did run those tests and found no regressions. Comment Actions
Most asserts look obsoleted. They assumed that packed constants are duplicated in both low and high 16 bits, but looks like they are not. Comment Actions
Basically yes, but I am suspecting that changes in SIMCCodeEmitter.cpp may affect CodeGen (e.g. may allow for wrong code instead of early failure in debug builds). Shall we ask Matt for feedback. Comment Actions
Sure.
Comment Actions I agree, it's better to remove assert ,rather them commenting them out. This revision is now accepted and ready to land.Jun 20 2017, 2:58 AM Closed by commit rL305923: [AMDGPU][MC][GFX9] Corrected VOP3P relevant code to fix disassembler failures (authored by dpreobra). · Explain WhyJun 21 2017, 9:01 AM This revision was automatically updated to reflect the committed changes.
Revision Contents
Diff 103089 lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
lib/Target/AMDGPU/InstPrinter/AMDGPUInstPrinter.cpp
lib/Target/AMDGPU/MCTargetDesc/SIMCCodeEmitter.cpp
lib/Target/AMDGPU/VOPInstructions.td
test/MC/Disassembler/AMDGPU/gfx9_dasm_all.txt
|
The problem is these operand types don't really exist. The inline immediates are actually encoded as 16-bit inline immediates and then VOP3P instructions are responsible for encoding the proper swizzle. This is leftover from when I thought they were encoded as the splat vector, so these should be removed