This change renames output operand for VOP instructions from dst to vdst. This is needed to enable decoding named operands for disassembler.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
lib/Target/AMDGPU/SIInstrInfo.td | ||
---|---|---|
1092–1095 ↗ | (On Diff #47004) | This looks like an unrelated change |
1274 ↗ | (On Diff #47004) | This is a scalar output, so it should not be a vdst |
1320 ↗ | (On Diff #47004) | Is this ever used for instructions with scalar outputs? |
1830 ↗ | (On Diff #47004) | This is a scalar output, so it should not be a vdst |
1841 ↗ | (On Diff #47004) | Ditto |
1854 ↗ | (On Diff #47004) | Ditto |
1868 ↗ | (On Diff #47004) | Ditto |
lib/Target/AMDGPU/SIInstrInfo.td | ||
---|---|---|
1830 ↗ | (On Diff #47004) | Here is problem with output operand name. Purpose of this renaming is to match operands names and names of fields in instruction encoding. VOPC instructions with 64-bit encoding use instruction format VOP3e: class VOP3e <bits<9> op> : Enc64 { bits<8> vdst; bits<2> src0_modifiers; bits<9> src0; bits<2> src1_modifiers; bits<9> src1; bits<2> src2_modifiers; bits<9> src2; bits<1> clamp; bits<2> omod; let Inst{7-0} = vdst; let Inst{8} = src0_modifiers{1}; let Inst{9} = src1_modifiers{1}; let Inst{10} = src2_modifiers{1}; let Inst{11} = clamp; let Inst{25-17} = op; let Inst{31-26} = 0x34; //encoding let Inst{40-32} = src0; let Inst{49-41} = src1; let Inst{58-50} = src2; let Inst{60-59} = omod; let Inst{61} = src0_modifiers{0}; let Inst{62} = src1_modifiers{0}; let Inst{63} = src2_modifiers{0}; } This encoding correspond to VOP3a encoding from spec. It does not have fields for output operands other than vdst. Thats why I renamed operand to $vdst. Possibly we should change VOPC instructions format to VOP3be as it is stated in spec. |
lib/Target/AMDGPU/SIInstrInfo.td | ||
---|---|---|
1830 ↗ | (On Diff #47004) | Yes, this should be fixed. I'm surprised it doesn't matter |
LGTM with a few nits
lib/Target/AMDGPU/SIInstrInfo.td | ||
---|---|---|
1189 ↗ | (On Diff #47983) | This would be less intrusive if DstVT was the last argument defaulting to i32 |
1348 ↗ | (On Diff #47983) | Extra space after = |
lib/Target/AMDGPU/VIInstrFormats.td | ||
135 ↗ | (On Diff #47983) | Extra spaces before = |
143 ↗ | (On Diff #47983) | Ditto |