From 520addfa0a03bd0b0bd79c03260507618a2e9916 Mon Sep 17 00:00:00 2001 From: Yong He Date: Tue, 4 Mar 2025 13:59:13 -0800 Subject: [PATCH] Fix SPIRV val issue. --- external/spirv-tools | 2 +- source/slang/slang-emit-spirv.cpp | 42 ++++++------------- source/slang/slang-ir-inst-defs.h | 2 + source/slang/slang-ir-insts.h | 6 +++ .../slang-ir-lower-buffer-element-type.cpp | 8 +++- source/slang/slang-ir-spirv-legalize.cpp | 2 + 6 files changed, 31 insertions(+), 31 deletions(-) diff --git a/external/spirv-tools b/external/spirv-tools index 12abd77a6b..86bfa20768 160000 --- a/external/spirv-tools +++ b/external/spirv-tools @@ -1 +1 @@ -Subproject commit 12abd77a6b6d779360c25a0d956c08ccfb8f1a3e +Subproject commit 86bfa20768c73840b7a7b9bf330432652fee9643 diff --git a/source/slang/slang-emit-spirv.cpp b/source/slang/slang-emit-spirv.cpp index 89280acfdf..4fe26f3495 100644 --- a/source/slang/slang-emit-spirv.cpp +++ b/source/slang/slang-emit-spirv.cpp @@ -1439,8 +1439,6 @@ struct SPIRVEmitContext : public SourceEmitterBase, public SPIRVEmitSharedContex bool shouldEmitArrayStride(IRInst* elementType) { - if (isResourceType((IRType*)elementType)) - return false; for (auto decor : elementType->getDecorations()) { switch (decor->getOp()) @@ -1670,41 +1668,22 @@ struct SPIRVEmitContext : public SourceEmitterBase, public SPIRVEmitSharedContex case kIROp_ArrayType: case kIROp_UnsizedArrayType: { - const auto elementType = static_cast(inst)->getElementType(); + auto irArrayType = static_cast(inst); + const auto elementType = irArrayType->getElementType(); const auto arrayType = inst->getOp() == kIROp_ArrayType - ? emitOpTypeArray( - inst, - elementType, - static_cast(inst)->getElementCount()) + ? emitOpTypeArray(inst, elementType, irArrayType->getElementCount()) : emitOpTypeRuntimeArray(inst, elementType); - auto strideInst = as(inst)->getArrayStride(); - int stride = 0; - if (strideInst) - { - stride = (int)getIntVal(strideInst); - } - else - { - IRSizeAndAlignment sizeAndAlignment; - getNaturalSizeAndAlignment( - m_targetProgram->getOptionSet(), - elementType, - &sizeAndAlignment); - stride = (int)sizeAndAlignment.getStride(); - } - - // Avoid validation error: Arrays whose element type is a Block or BufferBlock, - // or an opaque resource must not be decorated with ArrayStride. - if (shouldEmitArrayStride(elementType)) + auto strideInst = irArrayType->getArrayStride(); + if (strideInst && shouldEmitArrayStride(irArrayType->getElementType())) { + int stride = (int)getIntVal(strideInst); emitOpDecorateArrayStride( getSection(SpvLogicalSectionID::Annotations), nullptr, arrayType, SpvLiteralInteger::from32(stride)); } - return arrayType; } case kIROp_AtomicType: @@ -4989,6 +4968,7 @@ struct SPIRVEmitContext : public SourceEmitterBase, public SPIRVEmitSharedContex layoutRuleName = layout->getLayoutName(); } int32_t id = 0; + bool isPhysicalType = structType->findDecoration(); for (auto field : structType->getFields()) { for (auto decor : field->getKey()->getDecorations()) @@ -5069,6 +5049,10 @@ struct SPIRVEmitContext : public SourceEmitterBase, public SPIRVEmitSharedContex } } + if (!isPhysicalType) + continue; + + // Emit explicit struct field layout decorations if the struct is physical. IRIntegerValue offset = 0; if (auto offsetDecor = field->getKey()->findDecoration()) { @@ -5099,8 +5083,8 @@ struct SPIRVEmitContext : public SourceEmitterBase, public SPIRVEmitSharedContex if (matrixType) { // SPIRV sepc on MatrixStride: - // Applies only to a member of a structure type.Only valid on a - // matrix or array whose most basic element is a matrix.Matrix + // Applies only to a member of a structure type. Only valid on a + // matrix or array whose most basic element is a matrix. Matrix // Stride is an unsigned 32 - bit integer specifying the stride // of the rows in a RowMajor - decorated matrix or columns in a // ColMajor - decorated matrix. diff --git a/source/slang/slang-ir-inst-defs.h b/source/slang/slang-ir-inst-defs.h index 714ba146db..d4e26ea18c 100644 --- a/source/slang/slang-ir-inst-defs.h +++ b/source/slang/slang-ir-inst-defs.h @@ -812,6 +812,8 @@ INST_RANGE(BindingQuery, GetRegisterIndex, GetRegisterSpace) INST(InterpolationModeDecoration, interpolationMode, 1, 0) INST(NameHintDecoration, nameHint, 1, 0) + INST(PhysicalTypeDecoration, PhysicalType, 1, 0) + // Marks a type as being used as binary interface (e.g. shader parameters). // This prevents the legalizeEmptyType() pass from eliminating it on C++/CUDA targets. INST(BinaryInterfaceTypeDecoration, BinaryInterfaceType, 0, 0) diff --git a/source/slang/slang-ir-insts.h b/source/slang/slang-ir-insts.h index 5231592ca5..057163db3e 100644 --- a/source/slang/slang-ir-insts.h +++ b/source/slang/slang-ir-insts.h @@ -879,6 +879,8 @@ IR_SIMPLE_DECORATION(ForceInlineDecoration) IR_SIMPLE_DECORATION(ForceUnrollDecoration) +IR_SIMPLE_DECORATION(PhysicalTypeDecoration) + struct IRSizeAndAlignmentDecoration : IRDecoration { IR_LEAF_ISA(SizeAndAlignmentDecoration) @@ -4725,6 +4727,10 @@ struct IRBuilder IRVarLayout* getVarLayout(List const& operands); IREntryPointLayout* getEntryPointLayout(IRVarLayout* paramsLayout, IRVarLayout* resultLayout); + void addPhysicalTypeDecoration(IRInst* value) + { + addDecoration(value, kIROp_PhysicalTypeDecoration); + } void addNameHintDecoration(IRInst* value, IRStringLit* name) { diff --git a/source/slang/slang-ir-lower-buffer-element-type.cpp b/source/slang/slang-ir-lower-buffer-element-type.cpp index 74e84f1eed..8f33ad1819 100644 --- a/source/slang/slang-ir-lower-buffer-element-type.cpp +++ b/source/slang/slang-ir-lower-buffer-element-type.cpp @@ -451,6 +451,8 @@ struct LoweredElementTypeContext } auto loweredType = builder.createStructType(); + builder.addPhysicalTypeDecoration(loweredType); + StringBuilder nameSB; bool isColMajor = getIntVal(matrixType->getLayout()) == SLANG_MATRIX_LAYOUT_COLUMN_MAJOR; @@ -551,7 +553,7 @@ struct LoweredElementTypeContext // For spirv backend, we always want to lower all array types for non-varying // parameters, even if the element type comes out the same. This is because different // layout rules may have different array stride requirements. - if (!target->shouldEmitSPIRVDirectly() || config.addressSpace == AddressSpace::Input) + if (!target->shouldEmitSPIRVDirectly()) { if (!loweredInnerTypeInfo.convertLoweredToOriginal) { @@ -561,6 +563,8 @@ struct LoweredElementTypeContext } auto loweredType = builder.createStructType(); + builder.addPhysicalTypeDecoration(loweredType); + info.loweredType = loweredType; StringBuilder nameSB; nameSB << "_Array_" << getLayoutName(config.layoutRule->ruleName) << "_"; @@ -625,6 +629,8 @@ struct LoweredElementTypeContext } } auto loweredType = builder.createStructType(); + builder.addPhysicalTypeDecoration(loweredType); + StringBuilder nameSB; getTypeNameHint(nameSB, type); nameSB << "_" << getLayoutName(config.layoutRule->ruleName); diff --git a/source/slang/slang-ir-spirv-legalize.cpp b/source/slang/slang-ir-spirv-legalize.cpp index b19af364e0..59b2e1712f 100644 --- a/source/slang/slang-ir-spirv-legalize.cpp +++ b/source/slang/slang-ir-spirv-legalize.cpp @@ -132,6 +132,7 @@ struct SPIRVLegalizationContext : public SourceEmitterBase inst->getElementType(), builder.getIntValue(builder.getIntType(), elementSize.getStride())); const auto structType = builder.createStructType(); + builder.addPhysicalTypeDecoration(structType); const auto arrayKey = builder.createStructKey(); builder.createStructField(structType, arrayKey, arrayType); IRSizeAndAlignment structSize; @@ -213,6 +214,7 @@ struct SPIRVLegalizationContext : public SourceEmitterBase IRBuilder builder(cbParamInst); builder.setInsertBefore(cbParamInst); auto structType = builder.createStructType(); + builder.addPhysicalTypeDecoration(structType); addToWorkList(structType); StringBuilder sb; sb << "cbuffer_";