[all-commits] [llvm/llvm-project] 5f5f56: AMDGPU: Don't use 16-bit FP inline constants in in...

Matt Arsenault via All-commits all-commits at lists.llvm.org
Wed Jun 17 16:14:28 PDT 2020


  Branch: refs/heads/master
  Home:   https://github.com/llvm/llvm-project
  Commit: 5f5f566b265db00f577ead268400d99f34ba9cdd
      https://github.com/llvm/llvm-project/commit/5f5f566b265db00f577ead268400d99f34ba9cdd
  Author: Matt Arsenault <Matthew.Arsenault at amd.com>
  Date:   2020-06-17 (Wed, 17 Jun 2020)

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

  Log Message:
  -----------
  AMDGPU: Don't use 16-bit FP inline constants in integer operands

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.




More information about the All-commits mailing list