[PATCH] D123693: Transform illegal intrinsics to V_ILLEGAL

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIInstructions.td:3364
+
+def V_ILLEGAL_ALL_SET : Enc32, InstSI<(outs), (ins), "v_illegal_all_set"> {
+  let Inst{31-0} = 1;
----------------
Leonc wrote:
> bcahoon wrote:
> > Do we want different mnemonics, or should the same mnemonic be used for the cases when the encoding is all 0's or all 1's?
> > 
> > When you add the llvm-mc tests, it will also be a good idea to add a test that pipes the output of llvm-mc through llvm-objdump to the that the different encodings show up properly for the two cases.
> > Do we want different mnemonics, or should the same mnemonic be used for the cases when the encoding is all 0's or all 1's?
> 
> @arsenm asked for separate definitions:
> 
> > I would prefer to define a separate V_ILLEGAL that uses all 1s pre gfx10
> 
> 
The instruction is actually named v_illegal, so you shouldn't invent new names here. I would name these V_ILLEGAL for the gfx10 version and V_ILLEGAL_gfx6_gfx7_gf8_gfx9?

I also think V_ILLEGAL is available in a vop3 encoding (for gfx10) but probably should define that in a separate patch for the benefit of the disassembler


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123693/new/

https://reviews.llvm.org/D123693