[PATCH] D81841: AMDGPU: Don't use 16-bit FP inline constants in integer operands

[Matt Arsenault via Phabricator via llvm-commits]

arsenm created this revision.
arsenm added reviewers: rampitec, dp, hakzsam, b-sumner.
Herald added subscribers: kerbowa, hiraditya, t-tye, tpr, dstuttard, yaxunl, nhaehnle, wdng, jvesely, kzhuravl.
Herald added a project: LLVM.

It seems to be a hardware defect that the half inline constants do not
work as expected for the 16-bit integer operations (the inverse does
work correctly). Experimentation seems to show these are really
reading the 32-bit inline constants, which can be observed by writing
inline asm using op_sel to see what's in the high half of the
constant. Theoretically we could fold the high halves of the 32-bit
constants using op_sel.

      

The *_asm_all.s MC tests are broken, and I don't know where the script
to autogenerate these are. I started manually fixing it, but there's
just too many cases to fix. This also does break the
assembler/disassembler support for these values, and I'm not sure what
to do about it. These are still valid encodings, so it seems like you
should be able to use them in some way. If you wrote assembly using
them, you could have really meant it (perhaps to read the high bits
with op_sel?). The disassembler will print the invalid literal
constant which will fail to re-assemble. The behavior is also
different depending on the use context. Consider this example, which
was previously accepted and encoded using the inline constant:

      
  v_mad_i16 v5, v1, -4.0, v3
  ; encoding: [0x05,0x00,0xec,0xd1,0x01,0xef,0x0d,0x04]
      

In contexts where an inline immediate is required (such as on gfx8/9),
this will now be rejected. For gfx10, this will produce the literal
encoding and change the printed format:

  v_mad_i16 v5, v1, 0xc400, v3
  ; encoding: [0x05,0x00,0x5e,0xd7,0x01,0xff,0x0d,0x04,0x00,0xc4,0x00,0x00]
      

This is just another variation of the issue that we don't perfectly
handle round trip assembly/disassembly due to not tracking how
immediates were encoded. This doesn't matter much in practice, since
compilers don't emit the suboptimal encoding. I doubt any users are
relying on this behavior (although I did make use of the old behavior
to figure out what was wrong).

      

Fixes bug 46302.


https://reviews.llvm.org/D81841

Files:
  llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
  llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUInstPrinter.cpp
  llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUInstPrinter.h
  llvm/lib/Target/AMDGPU/MCTargetDesc/SIMCCodeEmitter.cpp
  llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
  llvm/lib/Target/AMDGPU/SIInstrInfo.td
  llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp
  llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h
  llvm/lib/Target/AMDGPU/VOP2Instructions.td
  llvm/test/CodeGen/AMDGPU/imm16.ll
  llvm/test/CodeGen/AMDGPU/immv216.ll
  llvm/test/CodeGen/AMDGPU/shrink-add-sub-constant.ll
  llvm/test/MC/AMDGPU/gfx10_asm_all.s
  llvm/test/MC/AMDGPU/gfx8_asm_all.s
  llvm/test/MC/AMDGPU/gfx9-asm-err.s
  llvm/test/MC/AMDGPU/gfx9_asm_all.s
  llvm/test/MC/AMDGPU/literalv216-err.s
  llvm/test/MC/AMDGPU/vop3-gfx10.s
  llvm/test/MC/AMDGPU/vop3-gfx9.s
  llvm/test/MC/AMDGPU/vop3.s
  llvm/test/MC/AMDGPU/vop_sdwa.s
  llvm/test/MC/Disassembler/AMDGPU/gfx10_dasm_all.txt
  llvm/test/MC/Disassembler/AMDGPU/gfx8_dasm_all.txt
  llvm/test/MC/Disassembler/AMDGPU/gfx9_dasm_all.txt
  llvm/test/MC/Disassembler/AMDGPU/literalv216_gfx10.txt
  llvm/test/MC/Disassembler/AMDGPU/vop3_gfx9.txt

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D81841.270726.patch
Type: text/x-patch
Size: 56569 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200615/9cec1010/attachment.bin>