[SPIR-V] Emit OpConstant instead of OpConstantNull to conform to NonSemantic.Shader.DebugInfo.100 DebugTypeBasic's flags definition #118333
Conversation
…hader.DebugInfo.100 DebugTypeBasic's flags definition
|
@llvm/pr-subscribers-backend-spir-v Author: Vyacheslav Levytskyy (VyacheslavLevytskyy) ChangesThis PR is to fix #118011 by emitting OpConstant instead of OpConstantNull to conform to NonSemantic.Shader.DebugInfo.100 DebugTypeBasic's flags definition. Full diff: https://github.com/llvm/llvm-project/pull/118333.diff 4 Files Affected:
diff --git a/llvm/lib/Target/SPIRV/SPIRVEmitNonSemanticDI.cpp b/llvm/lib/Target/SPIRV/SPIRVEmitNonSemanticDI.cpp
index d3e323efaee91b..70a7d37b243afc 100644
--- a/llvm/lib/Target/SPIRV/SPIRVEmitNonSemanticDI.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVEmitNonSemanticDI.cpp
@@ -268,7 +268,7 @@ bool SPIRVEmitNonSemanticDI::emitGlobalDI(MachineFunction &MF) {
// We aren't extracting any DebugInfoFlags now so we
// emitting zero to use as <id>Flags argument for DebugBasicType
const Register I32ZeroReg =
- GR->buildConstantInt(0, MIRBuilder, I32Ty, false);
+ GR->buildConstantInt(0, MIRBuilder, I32Ty, false, false);
// We need to store pairs because further instructions reference
// the DIBasicTypes and size will be always small so there isn't
diff --git a/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp b/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp
index 9ac659f6b4f111..91b9cbcf15128c 100644
--- a/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp
@@ -325,8 +325,8 @@ Register SPIRVGlobalRegistry::getOrCreateConstInt(uint64_t Val, MachineInstr &I,
Register SPIRVGlobalRegistry::buildConstantInt(uint64_t Val,
MachineIRBuilder &MIRBuilder,
- SPIRVType *SpvType,
- bool EmitIR) {
+ SPIRVType *SpvType, bool EmitIR,
+ bool ZeroAsNull) {
assert(SpvType);
auto &MF = MIRBuilder.getMF();
const IntegerType *LLVMIntTy =
@@ -348,7 +348,7 @@ Register SPIRVGlobalRegistry::buildConstantInt(uint64_t Val,
} else {
Register SpvTypeReg = getSPIRVTypeID(SpvType);
MachineInstrBuilder MIB;
- if (Val) {
+ if (Val || !ZeroAsNull) {
MIB = MIRBuilder.buildInstr(SPIRV::OpConstantI)
.addDef(Res)
.addUse(SpvTypeReg);
diff --git a/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.h b/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.h
index ff4b0ea8757fa4..df92325ed19802 100644
--- a/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.h
+++ b/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.h
@@ -509,7 +509,8 @@ class SPIRVGlobalRegistry {
public:
Register buildConstantInt(uint64_t Val, MachineIRBuilder &MIRBuilder,
- SPIRVType *SpvType, bool EmitIR = true);
+ SPIRVType *SpvType, bool EmitIR = true,
+ bool ZeroAsNull = true);
Register getOrCreateConstInt(uint64_t Val, MachineInstr &I,
SPIRVType *SpvType, const SPIRVInstrInfo &TII,
bool ZeroAsNull = true);
diff --git a/llvm/test/CodeGen/SPIRV/debug-info/debug-type-basic.ll b/llvm/test/CodeGen/SPIRV/debug-info/debug-type-basic.ll
index d12914d378542a..d29cdefe0d8c67 100644
--- a/llvm/test/CodeGen/SPIRV/debug-info/debug-type-basic.ll
+++ b/llvm/test/CodeGen/SPIRV/debug-info/debug-type-basic.ll
@@ -7,7 +7,7 @@
; CHECK-MIR-DAG: [[type_i32:%[0-9]+\:type]] = OpTypeInt 32, 0
; CHECK-MIR-DAG: [[encoding_signedchar:%[0-9]+\:iid\(s32\)]] = OpConstantI [[type_i32]], 5
; CHECK-MIR-DAG: [[encoding_float:%[0-9]+\:iid\(s32\)]] = OpConstantI [[type_i32]], 3
-; CHECK-MIR-DAG: [[flag_zero:%[0-9]+\:iid\(s32\)]] = OpConstantNull [[type_i32]]
+; CHECK-MIR-DAG: [[flag_zero:%[0-9]+\:iid\(s32\)]] = OpConstantI [[type_i32]], 0
; CHECK-MIR-DAG: [[str_bool:%[0-9]+\:id\(s32\)]] = OpString 1819242338, 0
; CHECK-MIR-DAG: [[size_8bits:%[0-9]+\:iid\(s32\)]] = OpConstantI [[type_i32]], 8
; CHECK-MIR-DAG: [[encoding_boolean:%[0-9]+\:iid\(s32\)]] = OpConstantI [[type_i32]], 2
@@ -60,7 +60,7 @@
; CHECK-SPIRV-DAG: [[type_int16:%[0-9]+]] = OpTypeInt 16 0
; CHECK-SPIRV-DAG: [[type_int32:%[0-9]+]] = OpTypeInt 32 0
; CHECK-SPIRV-DAG: [[encoding_signedchar:%[0-9]+]] = OpConstant [[type_int32]] 5
-; CHECK-SPIRV-DAG: [[flag_zero:%[0-9]+]] = OpConstantNull [[type_int32]]
+; CHECK-SPIRV-DAG: [[flag_zero:%[0-9]+]] = OpConstant [[type_int32]] 0
; CHECK-SPIRV-DAG: [[encoding_float:%[0-9]+]] = OpConstant [[type_int32]] 3
; CHECK-SPIRV-DAG: [[size_8bit:%[0-9]+]] = OpConstant [[type_int32]] 8
; CHECK-SPIRV-DAG: [[encoding_boolean:%[0-9]+]] = OpConstant [[type_int32]] 2
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Thank you, I also think that this should be fixed on spirv-val side. However, until it is fixed we should either proceed with this PR or add XFAIL to the test case. Also, as a side note, I guess we will anyway need to figure out OpConstant vs. OpConstantNull in debug-info to account for environments without null constants (HLSL). |
If it's just until we fix upstream, I'd be in favor of an XFAIL.
What do you mean by that? |
Please see https://llvm.org/docs/doxygen/SPIRVInstructionSelector_8cpp_source.html line 1772, the comment is "// OpenCL uses nulls for Zero. In HLSL we don't use null constants.", and the whole thing about |
|
@VyacheslavLevytskyy: It looks like this PR failed to remove the XFAIL added to the test when fixing the issue. We are hitting an unexpected pass causing testing to fail in our pipeline now due to this. |
Thank you, this should be fixed in the main branch already. |
This PR is to fix #118011 by emitting OpConstant instead of OpConstantNull to conform to NonSemantic.Shader.DebugInfo.100 DebugTypeBasic's flags definition.