Skip to content

Commit c616f24

Browse files
[SPIR-V] Do instruction selection for G_BITCAST on an earlier stage (#114216)
This PR implements instruction selection for G_BITCAST on an earlier stage to avoid MachineVerifier complains on subtle semantics difference between G_BITCAST and OpBitcast. We do instruction selections for OpBitcast after IR Translation instead of calling MIB.buildBitcast() generating the general op code G_BITCAST, because when MachineVerifier validates G_BITCAST we see a check of a kind: 'if Source Type is equal to Destination Type then report error "bitcast must change the type"'. This doesn't take into account the notion of a typed pointer that is important for SPIR-V where a user may and should use bitcast between pointers with different pointee types (https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#OpBitcast). It's important for correct lowering in SPIR-V, because interpretation of the data type is not left to instructions that utilize the pointer, but encoded by the pointer declaration, and the SPIRV target can and must handle the declaration and use of pointers that specify the type of data they point to. It's not feasible to improve validation of G_BITCAST using just information provided by low level types of source and destination. Therefore we don't produce G_BITCAST as the general op code with semantics different from OpBitcast, but rather lower to OpBitcast immediately. See discussion in #110270 for even more context.
1 parent d210964 commit c616f24

File tree

3 files changed

+54
-17
lines changed

3 files changed

+54
-17
lines changed

llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp

Lines changed: 52 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,57 @@ static MachineInstr *findAssignTypeInstr(Register Reg,
165165
return nullptr;
166166
}
167167

168+
static void buildOpBitcast(SPIRVGlobalRegistry *GR, MachineIRBuilder &MIB,
169+
Register ResVReg, Register OpReg) {
170+
SPIRVType *ResType = GR->getSPIRVTypeForVReg(ResVReg);
171+
SPIRVType *OpType = GR->getSPIRVTypeForVReg(OpReg);
172+
assert(ResType && OpType && "Operand types are expected");
173+
if (!GR->isBitcastCompatible(ResType, OpType))
174+
report_fatal_error("incompatible result and operand types in a bitcast");
175+
MachineRegisterInfo *MRI = MIB.getMRI();
176+
if (!MRI->getRegClassOrNull(ResVReg))
177+
MRI->setRegClass(ResVReg, GR->getRegClass(ResType));
178+
MIB.buildInstr(SPIRV::OpBitcast)
179+
.addDef(ResVReg)
180+
.addUse(GR->getSPIRVTypeID(ResType))
181+
.addUse(OpReg);
182+
}
183+
184+
// We do instruction selections early instead of calling MIB.buildBitcast()
185+
// generating the general op code G_BITCAST. When MachineVerifier validates
186+
// G_BITCAST we see a check of a kind: if Source Type is equal to Destination
187+
// Type then report error "bitcast must change the type". This doesn't take into
188+
// account the notion of a typed pointer that is important for SPIR-V where a
189+
// user may and should use bitcast between pointers with different pointee types
190+
// (https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#OpBitcast).
191+
// It's important for correct lowering in SPIR-V, because interpretation of the
192+
// data type is not left to instructions that utilize the pointer, but encoded
193+
// by the pointer declaration, and the SPIRV target can and must handle the
194+
// declaration and use of pointers that specify the type of data they point to.
195+
// It's not feasible to improve validation of G_BITCAST using just information
196+
// provided by low level types of source and destination. Therefore we don't
197+
// produce G_BITCAST as the general op code with semantics different from
198+
// OpBitcast, but rather lower to OpBitcast immediately. As for now, the only
199+
// difference would be that CombinerHelper couldn't transform known patterns
200+
// around G_BUILD_VECTOR. See discussion
201+
// in https://github.com/llvm/llvm-project/pull/110270 for even more context.
202+
static void selectOpBitcasts(MachineFunction &MF, SPIRVGlobalRegistry *GR,
203+
MachineIRBuilder MIB) {
204+
SmallVector<MachineInstr *, 16> ToErase;
205+
for (MachineBasicBlock &MBB : MF) {
206+
for (MachineInstr &MI : MBB) {
207+
if (MI.getOpcode() != TargetOpcode::G_BITCAST)
208+
continue;
209+
MIB.setInsertPt(*MI.getParent(), MI);
210+
buildOpBitcast(GR, MIB, MI.getOperand(0).getReg(),
211+
MI.getOperand(1).getReg());
212+
ToErase.push_back(&MI);
213+
}
214+
}
215+
for (MachineInstr *MI : ToErase)
216+
MI->eraseFromParent();
217+
}
218+
168219
static void insertBitcasts(MachineFunction &MF, SPIRVGlobalRegistry *GR,
169220
MachineIRBuilder MIB) {
170221
// Get access to information about available extensions
@@ -202,15 +253,6 @@ static void insertBitcasts(MachineFunction &MF, SPIRVGlobalRegistry *GR,
202253
} else {
203254
GR->assignSPIRVTypeToVReg(AssignedPtrType, Def, MF);
204255
MIB.buildBitcast(Def, Source);
205-
// MachineVerifier requires that bitcast must change the type.
206-
// Change AddressSpace if needed to hint that Def and Source points to
207-
// different types: this doesn't change actual code generation.
208-
LLT DefType = MRI->getType(Def);
209-
if (DefType == MRI->getType(Source))
210-
MRI->setType(Def,
211-
LLT::pointer((DefType.getAddressSpace() + 1) %
212-
SPIRVSubtarget::MaxLegalAddressSpace,
213-
GR->getPointerSize()));
214256
}
215257
}
216258
}
@@ -1007,6 +1049,7 @@ bool SPIRVPreLegalizer::runOnMachineFunction(MachineFunction &MF) {
10071049
removeImplicitFallthroughs(MF, MIB);
10081050
insertSpirvDecorations(MF, MIB);
10091051
insertInlineAsm(MF, GR, ST, MIB);
1052+
selectOpBitcasts(MF, GR, MIB);
10101053

10111054
return true;
10121055
}

llvm/test/CodeGen/SPIRV/pointers/phi-valid-operand-types-rev.ll

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,4 @@
1-
; The goal of the test case is to ensure that OpPhi is consistent with respect to operand types.
2-
; -verify-machineinstrs is not available due to mutually exclusive requirements for G_BITCAST and G_PHI.
3-
4-
; RUN: llc -O0 -mtriple=spirv64-unknown-unknown %s -o - | FileCheck %s
1+
; RUN: llc -verify-machineinstrs -O0 -mtriple=spirv64-unknown-unknown %s -o - | FileCheck %s
52
; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv64-unknown-unknown %s -o - -filetype=obj | spirv-val %}
63

74
; CHECK: %[[#Char:]] = OpTypeInt 8 0

llvm/test/CodeGen/SPIRV/pointers/phi-valid-operand-types.ll

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,4 @@
1-
; The goal of the test case is to ensure that OpPhi is consistent with respect to operand types.
2-
; -verify-machineinstrs is not available due to mutually exclusive requirements for G_BITCAST and G_PHI.
3-
4-
; RUN: llc -O0 -mtriple=spirv64-unknown-unknown %s -o - | FileCheck %s
1+
; RUN: llc -verify-machineinstrs -O0 -mtriple=spirv64-unknown-unknown %s -o - | FileCheck %s
52
; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv64-unknown-unknown %s -o - -filetype=obj | spirv-val %}
63

74
; CHECK: %[[#Char:]] = OpTypeInt 8 0

0 commit comments

Comments
 (0)