Page MenuHomePhabricator

[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

Repository
rL LLVM

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 ↗(On Diff #51907)

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 ↗(On Diff #51907)

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 ↗(On Diff #51907)

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 ↗(On Diff #51907)

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.