This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] AsmParser: disable DPP for unsupported instructions. New dpp tests. Fix v_nop_dpp.
ClosedPublic

Authored by SamWot on Mar 29 2016, 7:28 AM.

Details

Summary
  1. Disable DPP encoding for instructions that do not support it:
    • VOP1:
      • v_readfirstlane_b32
      • v_clrexcp
      • v_movreld_b32
      • v_movrels_b32
      • v_movrelsd_b32
    • VOP2:
      • v_madmk_f16/32
      • v_madak_f16/32
    • VOPC, VINTRP, VOP3
  2. Fix DPP for v_nop
  3. New DPP tests for VOP1 and VOP2 instructions

Diff Detail

Event Timeline

SamWot updated this revision to Diff 51907.Mar 29 2016, 7:28 AM
SamWot retitled this revision from to [AMDGPU] AsmParser: disable DPP for unsupported instructions. New dpp tests. Fix v_nop_dpp..
SamWot updated this object.
SamWot added reviewers: nhaustov, vpykhtin.
SamWot added subscribers: arsenm, tstellarAMD.
SamWot edited edge metadata.Mar 29 2016, 7:29 AM
SamWot added a project: Restricted Project.
vpykhtin added inline comments.Mar 29 2016, 8:05 AM
lib/Target/AMDGPU/SIInstrInfo.td
1396

its unclear why destination operand name is selected based on DstVT.Size. May be it worth to add helper class with a name that designate what we're really checking here, something like isVOPCDest

1750

it looks like VOP1_DPP instruction that isn't really DPP. This is may be ok if there're just a few such exceptions but the question is: should any other fields in VOP1_DPP/VOP2_DPP be defined depending on p.HasDPP?

SamWot added inline comments.Mar 30 2016, 3:48 AM
lib/Target/AMDGPU/SIInstrInfo.td
1396

This is a convinient way to decide if this is VOPC that is used in several places in SIInstrInfo.td. I agree that it should be changed but it is not the target of this change and should be done in different change.

1750

Yes, you are right that most VOP1 and VOP2 instructions support DPP and those that don't support are exceptions.
If we try to disable other field of VOP1_DPP/VOP2_DPP (such as InOperandList, OutOperandList, AsmString and so on) then there arise problems with TableGen that assumes to have this field defined. For example V_MAC_F32 instruction set constraints on $vdst but TableGen fails to find this operand. Also it looks like there is no need for disabling this fields: disabling AssemblerPredicate disables support for instruction in AsmParser and assembler is the only place where we currently support DPP.

vpykhtin accepted this revision.Apr 5 2016, 9:29 AM
vpykhtin edited edge metadata.

LGTM.

This revision is now accepted and ready to land.Apr 5 2016, 9:29 AM
nhaustov accepted this revision.Apr 5 2016, 9:35 AM
nhaustov edited edge metadata.

LGTM.

This revision was automatically updated to reflect the committed changes.