Page MenuHomePhabricator

[AMDGPU][MC] Improved diagnostics for instructions with missing features
ClosedPublic

Authored by dp on Tue, Oct 6, 4:19 AM.

Details

Summary

Now that assembler can reliably identify unsupported instructions, we should handle the case of missing features. In this case the specified instruction is supported but operands do not match.

There are two typical cases:

  1. Instruction uses operands not valid for the specified GPU (but valid for some other GPU):
v_add_u32 v1, v2, v3
// GFX9: v_add_u32_e32 v1, v2, v3       ; encoding: [0x02,0x07,0x02,0x68]
// ERR-SICI: error: instruction not supported on this GPU
// ERR-VI: error: operands are not valid for this GPU or mode
  1. Instruction requires a different wavesize mode:
v_add_co_ci_u32_e32 v5, vcc, v1, v2, vcc
// W64: encoding: [0x01,0x05,0x0a,0x50]
// W32-ERR: error: operands are not valid for this GPU or mode

Any advice for a better error message is appreciated.

Diff Detail

Event Timeline

dp created this revision.Tue, Oct 6, 4:19 AM
Herald added a project: Restricted Project. · View Herald TranscriptTue, Oct 6, 4:19 AM
dp requested review of this revision.Tue, Oct 6, 4:19 AM
rampitec added inline comments.Tue, Oct 6, 9:05 AM
llvm/test/MC/AMDGPU/gfx1030_err.s
29

It does not seem right, this instruction is really unsupported.

dp marked an inline comment as done.Tue, Oct 6, 9:39 AM
dp added inline comments.
llvm/test/MC/AMDGPU/gfx1030_err.s
29

Exactly. I've already filed a bug 47741.

The flag HasMadMacF32Insts is missing in dpp variant table for this opcode.

rampitec added inline comments.Tue, Oct 6, 9:53 AM
llvm/test/MC/AMDGPU/gfx1030_err.s
29

But this is not a dpp variant?

dp marked 2 inline comments as done.Tue, Oct 6, 10:11 AM
dp added inline comments.
llvm/test/MC/AMDGPU/gfx1030_err.s
29

When an error occurs, parser first calls checkUnsupportedInstruction to check if the specified instruction is supported or not. And because the mnemonic is found in the 'dpp' table, the instruction is considered supported. And it is indeed supported, because

v_mac_f32 v5, v1, v2 dpp8:[7,6,5,4,3,2,1,0]

is assembled w/o errors for GFX1030.

This leads us to handling Match_MissingFeature case. We got a match with DEFAULT variant of this instruction but this variant do require HasMadMacF32Insts flag. So for parser it looks like instruction is valid but operands are not.

rampitec accepted this revision.Tue, Oct 6, 10:14 AM

LGTM

llvm/test/MC/AMDGPU/gfx1030_err.s
29

OK, thanks for the explanation. I hope it will be fixed soon anyway.

This revision is now accepted and ready to land.Tue, Oct 6, 10:14 AM
This revision was automatically updated to reflect the committed changes.
dp marked an inline comment as done.