[llvm] c616f24 - [SPIR-V] Do instruction selection for G_BITCAST on an earlier stage (#114216)
via llvm-commits
llvm-commits at lists.llvm.org
Wed Oct 30 12:49:25 PDT 2024
Author: Vyacheslav Levytskyy
Date: 2024-10-30T20:49:21+01:00
New Revision: c616f24bcb00150fedc999d47933603e099dd659
URL: https://github.com/llvm/llvm-project/commit/c616f24bcb00150fedc999d47933603e099dd659
DIFF: https://github.com/llvm/llvm-project/commit/c616f24bcb00150fedc999d47933603e099dd659.diff
LOG: [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 https://github.com/llvm/llvm-project/pull/110270 for
even more context.
Added:
Modified:
llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp
llvm/test/CodeGen/SPIRV/pointers/phi-valid-operand-types-rev.ll
llvm/test/CodeGen/SPIRV/pointers/phi-valid-operand-types.ll
Removed:
################################################################################
diff --git a/llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp b/llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp
index 3c2af34dd55239..cc34cf877dea97 100644
--- a/llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp
@@ -165,6 +165,57 @@ static MachineInstr *findAssignTypeInstr(Register Reg,
return nullptr;
}
+static void buildOpBitcast(SPIRVGlobalRegistry *GR, MachineIRBuilder &MIB,
+ Register ResVReg, Register OpReg) {
+ SPIRVType *ResType = GR->getSPIRVTypeForVReg(ResVReg);
+ SPIRVType *OpType = GR->getSPIRVTypeForVReg(OpReg);
+ assert(ResType && OpType && "Operand types are expected");
+ if (!GR->isBitcastCompatible(ResType, OpType))
+ report_fatal_error("incompatible result and operand types in a bitcast");
+ MachineRegisterInfo *MRI = MIB.getMRI();
+ if (!MRI->getRegClassOrNull(ResVReg))
+ MRI->setRegClass(ResVReg, GR->getRegClass(ResType));
+ MIB.buildInstr(SPIRV::OpBitcast)
+ .addDef(ResVReg)
+ .addUse(GR->getSPIRVTypeID(ResType))
+ .addUse(OpReg);
+}
+
+// We do instruction selections early instead of calling MIB.buildBitcast()
+// generating the general op code G_BITCAST. 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
diff erent 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
diff erent from
+// OpBitcast, but rather lower to OpBitcast immediately. As for now, the only
+//
diff erence would be that CombinerHelper couldn't transform known patterns
+// around G_BUILD_VECTOR. See discussion
+// in https://github.com/llvm/llvm-project/pull/110270 for even more context.
+static void selectOpBitcasts(MachineFunction &MF, SPIRVGlobalRegistry *GR,
+ MachineIRBuilder MIB) {
+ SmallVector<MachineInstr *, 16> ToErase;
+ for (MachineBasicBlock &MBB : MF) {
+ for (MachineInstr &MI : MBB) {
+ if (MI.getOpcode() != TargetOpcode::G_BITCAST)
+ continue;
+ MIB.setInsertPt(*MI.getParent(), MI);
+ buildOpBitcast(GR, MIB, MI.getOperand(0).getReg(),
+ MI.getOperand(1).getReg());
+ ToErase.push_back(&MI);
+ }
+ }
+ for (MachineInstr *MI : ToErase)
+ MI->eraseFromParent();
+}
+
static void insertBitcasts(MachineFunction &MF, SPIRVGlobalRegistry *GR,
MachineIRBuilder MIB) {
// Get access to information about available extensions
@@ -202,15 +253,6 @@ static void insertBitcasts(MachineFunction &MF, SPIRVGlobalRegistry *GR,
} else {
GR->assignSPIRVTypeToVReg(AssignedPtrType, Def, MF);
MIB.buildBitcast(Def, Source);
- // MachineVerifier requires that bitcast must change the type.
- // Change AddressSpace if needed to hint that Def and Source points to
- //
diff erent types: this doesn't change actual code generation.
- LLT DefType = MRI->getType(Def);
- if (DefType == MRI->getType(Source))
- MRI->setType(Def,
- LLT::pointer((DefType.getAddressSpace() + 1) %
- SPIRVSubtarget::MaxLegalAddressSpace,
- GR->getPointerSize()));
}
}
}
@@ -1007,6 +1049,7 @@ bool SPIRVPreLegalizer::runOnMachineFunction(MachineFunction &MF) {
removeImplicitFallthroughs(MF, MIB);
insertSpirvDecorations(MF, MIB);
insertInlineAsm(MF, GR, ST, MIB);
+ selectOpBitcasts(MF, GR, MIB);
return true;
}
diff --git a/llvm/test/CodeGen/SPIRV/pointers/phi-valid-operand-types-rev.ll b/llvm/test/CodeGen/SPIRV/pointers/phi-valid-operand-types-rev.ll
index 6fa3f4e53cc598..8d14c3a359963f 100644
--- a/llvm/test/CodeGen/SPIRV/pointers/phi-valid-operand-types-rev.ll
+++ b/llvm/test/CodeGen/SPIRV/pointers/phi-valid-operand-types-rev.ll
@@ -1,7 +1,4 @@
-; The goal of the test case is to ensure that OpPhi is consistent with respect to operand types.
-; -verify-machineinstrs is not available due to mutually exclusive requirements for G_BITCAST and G_PHI.
-
-; RUN: llc -O0 -mtriple=spirv64-unknown-unknown %s -o - | FileCheck %s
+; RUN: llc -verify-machineinstrs -O0 -mtriple=spirv64-unknown-unknown %s -o - | FileCheck %s
; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv64-unknown-unknown %s -o - -filetype=obj | spirv-val %}
; CHECK: %[[#Char:]] = OpTypeInt 8 0
diff --git a/llvm/test/CodeGen/SPIRV/pointers/phi-valid-operand-types.ll b/llvm/test/CodeGen/SPIRV/pointers/phi-valid-operand-types.ll
index 4fbaae25567300..07824d4ed6cd85 100644
--- a/llvm/test/CodeGen/SPIRV/pointers/phi-valid-operand-types.ll
+++ b/llvm/test/CodeGen/SPIRV/pointers/phi-valid-operand-types.ll
@@ -1,7 +1,4 @@
-; The goal of the test case is to ensure that OpPhi is consistent with respect to operand types.
-; -verify-machineinstrs is not available due to mutually exclusive requirements for G_BITCAST and G_PHI.
-
-; RUN: llc -O0 -mtriple=spirv64-unknown-unknown %s -o - | FileCheck %s
+; RUN: llc -verify-machineinstrs -O0 -mtriple=spirv64-unknown-unknown %s -o - | FileCheck %s
; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv64-unknown-unknown %s -o - -filetype=obj | spirv-val %}
; CHECK: %[[#Char:]] = OpTypeInt 8 0
More information about the llvm-commits
mailing list