This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Rename $dst operand to $vdst for VOP instructions.
ClosedPublic

Authored by SamWot on Feb 5 2016, 2:09 AM.

Details

Summary

This change renames output operand for VOP instructions from dst to vdst. This is needed to enable decoding named operands for disassembler.

Diff Detail

Event Timeline

SamWot updated this revision to Diff 47004.Feb 5 2016, 2:09 AM
SamWot retitled this revision from to [AMDGPU] Rename $dst operand to $vdst for VOP instructions..
SamWot updated this object.
SamWot added subscribers: nhaustov, llvm-commits.
SamWot updated this object.Feb 5 2016, 2:10 AM
arsenm added inline comments.Feb 5 2016, 3:10 AM
lib/Target/AMDGPU/SIInstrInfo.td
1126–1129

This looks like an unrelated change

1353

This is a scalar output, so it should not be a vdst

1400

Is this ever used for instructions with scalar outputs?

1941

This is a scalar output, so it should not be a vdst

1952

Ditto

1965

Ditto

1979

Ditto

SamWot added inline comments.Feb 5 2016, 7:57 AM
lib/Target/AMDGPU/SIInstrInfo.td
1941

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.

arsenm added inline comments.Feb 5 2016, 10:22 AM
lib/Target/AMDGPU/SIInstrInfo.td
1941

Yes, this should be fixed. I'm surprised it doesn't matter

SamWot updated this revision to Diff 47447.Feb 10 2016, 5:43 AM

Renamed $vdst to $sdst for VOPC instructions. Added VOP3ce encoding for VOPC.

SamWot added a project: Restricted Project.Feb 12 2016, 6:27 AM

Can you rebase this on top of the latest llvm code?

SamWot updated this revision to Diff 47965.Feb 15 2016, 2:32 AM
SamWot edited edge metadata.
SamWot marked 7 inline comments as done.

Updated diff with latest LLVM master

SamWot updated this revision to Diff 47983.Feb 15 2016, 5:40 AM

Updated DPP encoding to use vdst name instead of dst

arsenm accepted this revision.Feb 15 2016, 11:16 AM
arsenm edited edge metadata.

LGTM with a few nits

lib/Target/AMDGPU/SIInstrInfo.td
1189–1190

This would be less intrusive if DstVT was the last argument defaulting to i32

1348

Extra space after =

lib/Target/AMDGPU/VIInstrFormats.td
135

Extra spaces before =

143

Ditto

This revision is now accepted and ready to land.Feb 15 2016, 11:16 AM
SamWot updated this revision to Diff 48061.Feb 16 2016, 3:28 AM
SamWot edited edge metadata.
SamWot marked 4 inline comments as done.

Fix issues from Matts review

This revision was automatically updated to reflect the committed changes.