Skip to content

Commit cdc6ef4

Browse files
sarnexLukacma
authored andcommitted
[SPIRV] Print split 64-bit OpSwitch operands as a single operand for text output (llvm#164886)
In binary form, 64-bit values are split into two 32-bit values as per the spec. Naturally this works fine with all tools. However, the text format does not have a formal specification but SPIR-V-Tools, which we already rely on in the SPIRV workflow (clang calls `spirv-as` for example), expects the full 64 bit value, but today we print the two 32-bit values. causing the tool to error and report that the format is invalid. The SPIR-V Translator also prints a single 64-bit value for text format. This case is already handled specifically for `OpConstant`, but `OpSwitch` was missed. The SPIR-V translator also has special code in `OpSwitch` handling for this case. Recombine the two 32-bit operands into a single 64-bit value to print in `AsmPrinter`. The actual ASM (aka binary form) emission is unchanged. --------- Signed-off-by: Sarnie, Nick <[email protected]>
1 parent 0a75149 commit cdc6ef4

File tree

6 files changed

+46
-3
lines changed

6 files changed

+46
-3
lines changed

llvm/lib/Target/SPIRV/MCTargetDesc/SPIRVBaseInfo.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,10 @@ struct ExtendedBuiltin {
245245

246246
enum InstFlags {
247247
// It is a half type
248-
INST_PRINTER_WIDTH16 = 1
248+
INST_PRINTER_WIDTH16 = 1,
249+
// It is a 64-bit type
250+
INST_PRINTER_WIDTH64 = INST_PRINTER_WIDTH16 << 1,
251+
249252
};
250253
} // namespace SPIRV
251254

llvm/lib/Target/SPIRV/MCTargetDesc/SPIRVInstPrinter.cpp

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,36 @@ void SPIRVInstPrinter::printInst(const MCInst *MI, uint64_t Address,
167167
MI, FirstVariableIndex, OS);
168168
printRemainingVariableOps(MI, FirstVariableIndex + 1, OS);
169169
break;
170+
case SPIRV::OpSwitch:
171+
if (MI->getFlags() & SPIRV::INST_PRINTER_WIDTH64) {
172+
// In binary format 64-bit types are split into two 32-bit operands,
173+
// but in text format combine these into a single 64-bit value as
174+
// this is what tools such as spirv-as require.
175+
const unsigned NumOps = MI->getNumOperands();
176+
for (unsigned OpIdx = NumFixedOps; OpIdx < NumOps;) {
177+
if (OpIdx + 1 >= NumOps || !MI->getOperand(OpIdx).isImm() ||
178+
!MI->getOperand(OpIdx + 1).isImm()) {
179+
llvm_unreachable("Unexpected OpSwitch operands");
180+
continue;
181+
}
182+
OS << ' ';
183+
uint64_t LowBits = MI->getOperand(OpIdx).getImm();
184+
uint64_t HighBits = MI->getOperand(OpIdx + 1).getImm();
185+
uint64_t CombinedValue = (HighBits << 32) | LowBits;
186+
OS << formatImm(CombinedValue);
187+
OpIdx += 2;
188+
189+
// Next should be the label
190+
if (OpIdx < NumOps) {
191+
OS << ' ';
192+
printOperand(MI, OpIdx, OS);
193+
OpIdx++;
194+
}
195+
}
196+
} else {
197+
printRemainingVariableOps(MI, NumFixedOps, OS);
198+
}
199+
break;
170200
case SPIRV::OpImageSampleImplicitLod:
171201
case SPIRV::OpImageSampleDrefImplicitLod:
172202
case SPIRV::OpImageSampleProjImplicitLod:

llvm/lib/Target/SPIRV/SPIRVInstrInfo.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,9 @@ class SPIRVInstrInfo : public SPIRVGenInstrInfo {
6262
namespace SPIRV {
6363
enum AsmComments {
6464
// It is a half type
65-
ASM_PRINTER_WIDTH16 = MachineInstr::TAsmComments
65+
ASM_PRINTER_WIDTH16 = MachineInstr::TAsmComments,
66+
// It is a 64 bit type
67+
ASM_PRINTER_WIDTH64 = ASM_PRINTER_WIDTH16 << 1,
6668
};
6769
} // namespace SPIRV
6870

llvm/lib/Target/SPIRV/SPIRVMCInstLower.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ void SPIRVMCInstLower::lower(const MachineInstr *MI, MCInst &OutMI,
2525
// Propagate previously set flags
2626
if (MI->getAsmPrinterFlags() & SPIRV::ASM_PRINTER_WIDTH16)
2727
OutMI.setFlags(SPIRV::INST_PRINTER_WIDTH16);
28+
if (MI->getAsmPrinterFlags() & SPIRV::ASM_PRINTER_WIDTH64)
29+
OutMI.setFlags(SPIRV::INST_PRINTER_WIDTH64);
2830
const MachineFunction *MF = MI->getMF();
2931
for (unsigned i = 0, e = MI->getNumOperands(); i != e; ++i) {
3032
const MachineOperand &MO = MI->getOperand(i);

llvm/lib/Target/SPIRV/SPIRVUtils.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,8 @@ void addNumImm(const APInt &Imm, MachineInstrBuilder &MIB) {
105105
uint32_t LowBits = FullImm & 0xffffffff;
106106
uint32_t HighBits = (FullImm >> 32) & 0xffffffff;
107107
MIB.addImm(LowBits).addImm(HighBits);
108+
// Asm Printer needs this info to print 64-bit operands correctly
109+
MIB.getInstr()->setAsmPrinterFlag(SPIRV::ASM_PRINTER_WIDTH64);
108110
return;
109111
}
110112
report_fatal_error("Unsupported constant bitwidth");

llvm/test/CodeGen/SPIRV/branching/OpSwitch64.ll

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,11 @@
1919

2020
; RUN: llc -O0 -mtriple=spirv64-unknown-unknown %s -o - | FileCheck %s --check-prefix=CHECK-SPIRV
2121

22-
; CHECK-SPIRV: OpSwitch %[[#]] %[[#]] 0 0 %[[#]] 1 0 %[[#]] 1 5 %[[#]]
22+
; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv64-unknown-unknown %s -o - -filetype=obj | spirv-val %}
23+
24+
; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv64-unknown-unknown %s -o - -filetype=asm | spirv-as - -o /dev/null %}
25+
26+
; CHECK-SPIRV: OpSwitch %[[#]] %[[#]] 0 %[[#]] 1 %[[#]] 21474836481 %[[#]]
2327

2428
define spir_kernel void @test_64(i32 addrspace(1)* %res) {
2529
entry:

0 commit comments

Comments
 (0)