[SPIRV] Fix failing assertion in SPIRVAsmPrinter#166909
Merged
jmmartinez merged 2 commits intomainfrom Nov 10, 2025
Merged
Conversation
Member
|
@llvm/pr-subscribers-backend-spir-v Author: Juan Manuel Martinez Caamaño (jmmartinez) ChangesWith This patch adds the missing condition. 2 Files Affected:
diff --git a/llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp b/llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp
index 0175f2fb3698b..b10846aeee6a4 100644
--- a/llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp
@@ -612,13 +612,10 @@ void SPIRVAsmPrinter::outputExecutionMode(const Module &M) {
// Collect the SPIRVTypes for fp16, fp32, and fp64 and the constant of
// type int32 with 0 value to represent the FP Fast Math Mode.
std::vector<const MachineInstr *> SPIRVFloatTypes;
- const MachineInstr *ConstZero = nullptr;
+ const MachineInstr *ConstZeroInt32 = nullptr;
for (const MachineInstr *MI :
MAI->getMSInstrs(SPIRV::MB_TypeConstVars)) {
- // Skip if the instruction is not OpTypeFloat or OpConstant.
unsigned OpCode = MI->getOpcode();
- if (OpCode != SPIRV::OpTypeFloat && OpCode != SPIRV::OpConstantNull)
- continue;
// Collect the SPIRV type if it's a float.
if (OpCode == SPIRV::OpTypeFloat) {
@@ -629,14 +626,19 @@ void SPIRVAsmPrinter::outputExecutionMode(const Module &M) {
continue;
}
SPIRVFloatTypes.push_back(MI);
- } else {
+ continue;
+ }
+
+ if (OpCode == SPIRV::OpConstantNull) {
// Check if the constant is int32, if not skip it.
const MachineRegisterInfo &MRI = MI->getMF()->getRegInfo();
MachineInstr *TypeMI = MRI.getVRegDef(MI->getOperand(1).getReg());
- if (!TypeMI || TypeMI->getOperand(1).getImm() != 32)
- continue;
-
- ConstZero = MI;
+ bool IsInt32Ty = TypeMI &&
+ TypeMI->getOpcode() == SPIRV::OpTypeInt &&
+ TypeMI->getOperand(1).getImm() == 32;
+ if (IsInt32Ty)
+ ConstZeroInt32 = MI;
+ continue;
}
}
@@ -657,9 +659,9 @@ void SPIRVAsmPrinter::outputExecutionMode(const Module &M) {
MCRegister TypeReg =
MAI->getRegisterAlias(MF, MI->getOperand(0).getReg());
Inst.addOperand(MCOperand::createReg(TypeReg));
- assert(ConstZero && "There should be a constant zero.");
+ assert(ConstZeroInt32 && "There should be a constant zero.");
MCRegister ConstReg = MAI->getRegisterAlias(
- ConstZero->getMF(), ConstZero->getOperand(0).getReg());
+ ConstZeroInt32->getMF(), ConstZeroInt32->getOperand(0).getReg());
Inst.addOperand(MCOperand::createReg(ConstReg));
outputMCInst(Inst);
}
diff --git a/llvm/test/CodeGen/SPIRV/non_int_constant_null.ll b/llvm/test/CodeGen/SPIRV/non_int_constant_null.ll
new file mode 100644
index 0000000000000..0ba016aaa30aa
--- /dev/null
+++ b/llvm/test/CodeGen/SPIRV/non_int_constant_null.ll
@@ -0,0 +1,25 @@
+; RUN: llc -mtriple spirv64-unknown-unknown %s --spirv-ext=+SPV_KHR_float_controls2 -o - | FileCheck %s
+; RUN: %if spirv-tools %{ llc -mtriple spirv64-unknown-unknown %s --spirv-ext=+SPV_KHR_float_controls2 -o - -filetype=obj | spirv-val %}
+
+@A = addrspace(1) constant [1 x i8] zeroinitializer
+
+; CHECK: OpName %[[#FOO:]] "foo"
+; CHECK: OpName %[[#A:]] "A"
+; CHECK: OpDecorate %[[#A]] Constant
+; CHECK: OpDecorate %[[#A]] LinkageAttributes "A" Export
+; CHECK: %[[#INT8:]] = OpTypeInt 8 0
+; CHECK: %[[#INT32:]] = OpTypeInt 32 0
+; CHECK: %[[#ONE:]] = OpConstant %[[#INT32]] 1
+; CHECK: %[[#ARR_INT8:]] = OpTypeArray %[[#INT8]] %7
+; CHECK: %[[#ARR_INT8_PTR:]] = OpTypePointer CrossWorkgroup %[[#ARR_INT8]]
+; CHECK: %[[#ARR_INT8_ZERO:]] = OpConstantNull %[[#ARR_INT8]]
+; CHECK: %13 = OpVariable %[[#ARR_INT8_PTR]] CrossWorkgroup %[[#ARR_INT8_ZERO]]
+; CHECK: %[[#FOO]] = OpFunction
+; CHECK: = OpLabel
+; CHECK: OpReturn
+; CHECK: OpFunctionEnd
+
+define spir_kernel void @foo() {
+entry:
+ ret void
+}
|
Contributor
Author
|
I've added a last commit where I refactor a redundant condition and rename ConstZero into ConstZeroInt32, but if it's not relevant I can remove it (it polutes the PR a bit and is an unrelated NFC). |
maarquitos14
approved these changes
Nov 7, 2025
Contributor
maarquitos14
left a comment
There was a problem hiding this comment.
Good catch! Thanks for the fix. Just a nit, otherwise LGTM.
db3552b to
a38d0d1
Compare
When +SPV_KHR_float_controls2 and there was a non-int OpConstantZero we would call MI.getOperand(1).getImm() when MI was not an OpTypeInt (the associated test has an OpTypeArray zeroinitialized). This patch adds the missing condition.
a38d0d1 to
1e86b00
Compare
ckoparkar
added a commit
to ckoparkar/llvm-project
that referenced
this pull request
Nov 10, 2025
* main: (1028 commits) [clang][DebugInfo] Attach `DISubprogram` to additional call variants (llvm#166202) [C2y] Claim nonconformance to WG14 N3348 (llvm#166966) [X86] 2012-01-10-UndefExceptionEdge.ll - regenerate test checks (llvm#167307) Remove unused standard headers: <string>, <optional>, <numeric>, <tuple> (llvm#167232) [DebugInfo] Add Verifier check for incorrectly-scoped retainedNodes (llvm#166855) [VPlan] Don't apply predication discount to non-originally-predicated blocks (llvm#160449) [libc++] Avoid overloaded `operator,` for (`T`, `Iter`) cases (llvm#161049) [tools][llc] Make save-stats.ll test target independent (llvm#167238) [AArch64] Fallback to PRFUM for PRFM with negative or unaligned offset (llvm#166756) [X86] ldexp-avx512.ll - add v8f16/v16f16/v32f16 test coverage for llvm#165694 (llvm#167294) [DropAssumes] Drop dereferenceable assumptions after vectorization. (llvm#166947) [VPlan] Simplify branch-cond with getVectorTripCount (llvm#155604) Remove unused <algorithm> inclusion (llvm#166942) [AArch64] Combine subtract with borrow to SBC. (llvm#165271) [AArch64][SVE] Avoid redundant extend of unsigned i8/i16 extracts. (llvm#165863) [SPIRV] Fix failing assertion in SPIRVAsmPrinter (llvm#166909) [libc++] Merge insert/emplace(const_iterator, Args...) implementations (llvm#166470) [libc++] Replace __libcpp_is_final with a variable template (llvm#167137) [gn build] Port 152bda7 [libc++] Replace the last uses of __tuple_types with __type_list (llvm#167214) ...
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
With
+SPV_KHR_float_controls2and when there is a non-intOpConstantNullwewould call
MI.getOperand(1).getImm()whenMIwas not anOpTypeInt(theassociated test has an
OpTypeArrayzeroinitialized).Under this conditions an assertion is triggered.
This patch adds the missing condition.