From 924e570aca3ee17f3a05e8643d178fd0e32ab3b2 Mon Sep 17 00:00:00 2001 From: Mukund Date: Mon, 24 Feb 2025 14:04:24 +0530 Subject: [PATCH 1/5] Consolidate multiple inout/outs into struct Fixes #5121 VUID-StandaloneSpirv-IncomingRayPayloadKHR-04700 requires that there be only one IncomingRayPayloadKHR per entry point. This change does two things: 1. If an entry point has the one inout or out, or has only ins, then stay with current implementation. 2. If there are multiple outs/inouts, then create a new structure to consolidate these fields and emit this structure. These two code paths are split into two separate functions for clarity. This patch also adds a new test: multipleinout.slang to test this. --- source/slang/slang-ir-glsl-legalize.cpp | 173 +++++++++++++++++++++++- tests/vkray/multipleinout.slang | 69 ++++++++++ 2 files changed, 239 insertions(+), 3 deletions(-) create mode 100644 tests/vkray/multipleinout.slang diff --git a/source/slang/slang-ir-glsl-legalize.cpp b/source/slang/slang-ir-glsl-legalize.cpp index c11ccc66dd..6580ed82d4 100644 --- a/source/slang/slang-ir-glsl-legalize.cpp +++ b/source/slang/slang-ir-glsl-legalize.cpp @@ -329,6 +329,10 @@ struct GLSLLegalizationContext IRBuilder* builder; IRBuilder* getBuilder() { return builder; } + + // For ray tracing shaders, we need to consolidate all parameters into a single structure + Dictionary rayTracingConsolidatedVars; + Dictionary> rayTracingProcessedParams; }; // This examines the passed type and determines the GLSL mesh shader indices @@ -2302,7 +2306,7 @@ IRInst* materializeValue(IRBuilder* builder, ScalarizedVal const& val) } } -void legalizeRayTracingEntryPointParameterForGLSL( +static void handleSingleParam( GLSLLegalizationContext* context, IRFunc* func, IRParam* pp, @@ -2354,6 +2358,149 @@ void legalizeRayTracingEntryPointParameterForGLSL( builder->addDependsOnDecoration(func, globalParam); } +static void consolidateParameters( + GLSLLegalizationContext* context, + IRFunc* func, + List& params) +{ + auto builder = context->getBuilder(); + + // Create a struct type to hold all parameters + IRInst* consolidatedVar = nullptr; + auto structType = builder->createStructType(); + + // Inside the structure, add fields for each parameter + for (auto _param : params) + { + auto _paramType = _param->getDataType(); + IRType* valueType = _paramType; + + if (as(_paramType)) + valueType = as(_paramType)->getValueType(); + else if (auto inOutType = as(_paramType)) + valueType = inOutType->getValueType(); + + auto key = builder->createStructKey(); + builder->addNameHintDecoration(key, UnownedStringSlice("field")); + auto field = builder->createStructField(structType, key, valueType); + field->removeFromParent(); + field->insertAtEnd(structType); + } + + // Create a global variable to hold the consolidated struct + consolidatedVar = builder->createGlobalVar(structType); + auto ptrType = builder->getPtrType(kIROp_PtrType, structType, AddressSpace::IncomingRayPayload); + consolidatedVar->setFullType(ptrType); + consolidatedVar->moveToEnd(); + + // Add the ray payload decoration and assign location 0. + builder->addVulkanRayPayloadDecoration(consolidatedVar, 0); + + // Store the consolidated variable for this function + context->rayTracingConsolidatedVars[func] = consolidatedVar; + + // Replace each parameter with a field in the consolidated struct + for (Index i = 0; i < params.getCount(); ++i) + { + auto _param = params[i]; + + // Find the i-th field + IRStructField* targetField = nullptr; + Index fieldIndex = 0; + for (auto field : structType->getFields()) + { + if (fieldIndex == i) + { + targetField = field; + break; + } + fieldIndex++; + } + SLANG_ASSERT(targetField); + + // Create the field address with the correct type + auto _paramType = _param->getDataType(); + auto fieldType = targetField->getFieldType(); + + // If the parameter is an out/inout type, we need to create a pointer type + IRType* fieldPtrType = nullptr; + if (as(_paramType)) + { + fieldPtrType = builder->getPtrType(kIROp_OutType, fieldType); + } + else if (as(_paramType)) + { + fieldPtrType = builder->getPtrType(kIROp_InOutType, fieldType); + } + + auto fieldAddr = + builder->emitFieldAddress(fieldPtrType, consolidatedVar, targetField->getKey()); + + // Replace parameter uses with field address + _param->replaceUsesWith(fieldAddr); + } +} + +static void handleMultipleParams(GLSLLegalizationContext* context, IRFunc* func, IRParam* pp) +{ + auto firstBlock = func->getFirstBlock(); + + // Now we run the consolidation step, but if we've already + // processed this parameter, skip it. + List* processedParams = nullptr; + if (auto foundList = context->rayTracingProcessedParams.tryGetValue(func)) + { + processedParams = foundList; + if (processedParams->contains(pp)) + return; + } + else + { + context->rayTracingProcessedParams[func] = List(); + processedParams = &context->rayTracingProcessedParams[func]; + } + + // Collect all parameters that need to be consolidated + List params; + List paramLayouts; + + for (auto _param = firstBlock->getFirstParam(); _param; _param = _param->getNextParam()) + { + auto pLayoutDecoration = _param->findDecoration(); + SLANG_ASSERT(pLayoutDecoration); + auto pLayout = as(pLayoutDecoration->getLayout()); + SLANG_ASSERT(pLayout); + + // Only include parameters that haven't been processed yet + auto _paramType = _param->getDataType(); + bool needsConsolidation = (as(_paramType) || as(_paramType)); + if (!processedParams->contains(_param) && needsConsolidation) + { + params.add(_param); + paramLayouts.add(pLayout); + processedParams->add(_param); + } + } + + consolidateParameters(context, func, params); +} + +void legalizeRayTracingEntryPointParameterForGLSL( + GLSLLegalizationContext* context, + IRFunc* func, + IRParam* pp, + IRVarLayout* paramLayout, + bool hasSingleOutOrInOutParam) +{ + if (hasSingleOutOrInOutParam) + { + handleSingleParam(context, func, pp, paramLayout); + return; + } + + handleMultipleParams(context, func, pp); +} + static void legalizeMeshPayloadInputParam( GLSLLegalizationContext* context, CodeGenContext* codeGenContext, @@ -3041,7 +3188,6 @@ void legalizeEntryPointParameterForGLSL( } } - // We need to create a global variable that will replace the parameter. // It seems superficially obvious that the variable should have // the same type as the parameter. @@ -3198,7 +3344,28 @@ void legalizeEntryPointParameterForGLSL( case Stage::Intersection: case Stage::Miss: case Stage::RayGeneration: - legalizeRayTracingEntryPointParameterForGLSL(context, func, pp, paramLayout); + { + // Count the number of inout or out parameters + int inoutOrOutParamCount = 0; + auto firstBlock = func->getFirstBlock(); + for (auto _param = firstBlock->getFirstParam(); _param; _param = _param->getNextParam()) + { + auto _paramType = _param->getDataType(); + if (as(_paramType) || as(_paramType)) + { + inoutOrOutParamCount++; + } + } + + // If we have just one inout or out param, we don't need consolidation. + bool hasSingleOutOrInOutParam = inoutOrOutParamCount <= 1; + legalizeRayTracingEntryPointParameterForGLSL( + context, + func, + pp, + paramLayout, + hasSingleOutOrInOutParam); + } return; } diff --git a/tests/vkray/multipleinout.slang b/tests/vkray/multipleinout.slang new file mode 100644 index 0000000000..667f912263 --- /dev/null +++ b/tests/vkray/multipleinout.slang @@ -0,0 +1,69 @@ +//TEST:SIMPLE(filecheck=CHECK): -stage closesthit -entry main -target spirv -emit-spirv-directly + +// This test checks whether the spirv generated when there are multiple inout or out variables, they +// all get consolidated into one IncomingRayPayloadKHR. + +struct ReflectionRay +{ + float4 color; +}; + +StructuredBuffer colors; + +[shader("closesthit")] +void main( + BuiltInTriangleIntersectionAttributes attributes, + inout ReflectionRay ioPayload, + out float3 dummy) +{ + uint materialID = (InstanceIndex() << 1) + + InstanceID() + + PrimitiveIndex() + + HitKind(); + + ioPayload.color = colors[materialID]; + dummy = HitTriangleVertexPosition(0); +} + +// CHECK: OpCapability RayTracingKHR +// CHECK: OpCapability RayTracingPositionFetchKHR +// CHECK: OpCapability Shader +// CHECK: OpExtension "SPV_KHR_ray_tracing" +// CHECK: OpExtension "SPV_KHR_storage_buffer_storage_class" +// CHECK: OpExtension "SPV_KHR_ray_tracing_position_fetch" +// CHECK: OpMemoryModel Logical GLSL450 +// CHECK: OpEntryPoint ClosestHitKHR %main "main" %{{.*}} %{{.*}} %gl_PrimitiveID %{{.*}} %gl_InstanceID %colors %{{.*}} + +// CHECK: OpName %ReflectionRay "ReflectionRay" +// CHECK: OpMemberName %ReflectionRay 0 "color" +// CHECK: OpName %materialID "materialID" +// CHECK: OpName %StructuredBuffer "StructuredBuffer" +// CHECK: OpName %colors "colors" +// CHECK: OpName %main "main" + +// CHECK-DAG: OpDecorate %gl_InstanceID BuiltIn InstanceId +// CHECK-DAG: OpDecorate %{{.*}} BuiltIn InstanceCustomIndexKHR +// CHECK-DAG: OpDecorate %gl_PrimitiveID BuiltIn PrimitiveId +// CHECK-DAG: OpDecorate %{{.*}} BuiltIn HitKindKHR +// CHECK-DAG: OpDecorate %_runtimearr_v4float ArrayStride 16 +// CHECK-DAG: OpDecorate %StructuredBuffer Block +// CHECK-DAG: OpDecorate %colors Binding 0 +// CHECK-DAG: OpDecorate %colors DescriptorSet 0 +// CHECK-DAG: OpDecorate %colors NonWritable +// CHECK-DAG: OpDecorate %{{.*}} BuiltIn HitTriangleVertexPositionsKHR + +// CHECK: %ReflectionRay = OpTypeStruct %v4float +// CHECK: %_struct_{{.*}} = OpTypeStruct %ReflectionRay %v3float +// CHECK: %_ptr_IncomingRayPayloadKHR__struct_{{.*}} = OpTypePointer IncomingRayPayloadKHR %_struct_{{.*}} +// CHECK: %_ptr_IncomingRayPayloadKHR_ReflectionRay = OpTypePointer IncomingRayPayloadKHR %ReflectionRay +// CHECK: %_ptr_IncomingRayPayloadKHR_v3float = OpTypePointer IncomingRayPayloadKHR %v3float +// CHECK: %StructuredBuffer = OpTypeStruct %_runtimearr_v4float +// CHECK: %_ptr_StorageBuffer_StructuredBuffer = OpTypePointer StorageBuffer %StructuredBuffer +// CHECK: %_ptr_Input__arr_v3float_{{.*}} = OpTypePointer Input %_arr_v3float_{{.*}} + +// CHECK: %main = OpFunction %void None %{{.*}} +// CHECK: %materialID = OpIAdd %uint %{{.*}} %{{.*}} +// CHECK: %{{.*}} = OpAccessChain %_ptr_StorageBuffer_v4float %colors %int_0 %materialID +// CHECK: %{{.*}} = OpLoad %v4float %{{.*}} +// CHECK: %{{.*}} = OpAccessChain %_ptr_Input_v3float %{{.*}} %uint_0 +// CHECK: %{{.*}} = OpLoad %v3float %{{.*}} From 029ecc1a277bb162c199d040c1162f7659cee44d Mon Sep 17 00:00:00 2001 From: Mukund Date: Thu, 27 Feb 2025 10:30:06 +0530 Subject: [PATCH 2/5] Address review comments --- tests/vkray/multipleinout.slang | 33 --------------------------------- 1 file changed, 33 deletions(-) diff --git a/tests/vkray/multipleinout.slang b/tests/vkray/multipleinout.slang index 667f912263..52e1758b03 100644 --- a/tests/vkray/multipleinout.slang +++ b/tests/vkray/multipleinout.slang @@ -25,42 +25,9 @@ void main( dummy = HitTriangleVertexPosition(0); } -// CHECK: OpCapability RayTracingKHR -// CHECK: OpCapability RayTracingPositionFetchKHR -// CHECK: OpCapability Shader -// CHECK: OpExtension "SPV_KHR_ray_tracing" -// CHECK: OpExtension "SPV_KHR_storage_buffer_storage_class" -// CHECK: OpExtension "SPV_KHR_ray_tracing_position_fetch" -// CHECK: OpMemoryModel Logical GLSL450 // CHECK: OpEntryPoint ClosestHitKHR %main "main" %{{.*}} %{{.*}} %gl_PrimitiveID %{{.*}} %gl_InstanceID %colors %{{.*}} - -// CHECK: OpName %ReflectionRay "ReflectionRay" -// CHECK: OpMemberName %ReflectionRay 0 "color" -// CHECK: OpName %materialID "materialID" -// CHECK: OpName %StructuredBuffer "StructuredBuffer" -// CHECK: OpName %colors "colors" -// CHECK: OpName %main "main" - -// CHECK-DAG: OpDecorate %gl_InstanceID BuiltIn InstanceId -// CHECK-DAG: OpDecorate %{{.*}} BuiltIn InstanceCustomIndexKHR -// CHECK-DAG: OpDecorate %gl_PrimitiveID BuiltIn PrimitiveId -// CHECK-DAG: OpDecorate %{{.*}} BuiltIn HitKindKHR -// CHECK-DAG: OpDecorate %_runtimearr_v4float ArrayStride 16 -// CHECK-DAG: OpDecorate %StructuredBuffer Block -// CHECK-DAG: OpDecorate %colors Binding 0 -// CHECK-DAG: OpDecorate %colors DescriptorSet 0 -// CHECK-DAG: OpDecorate %colors NonWritable -// CHECK-DAG: OpDecorate %{{.*}} BuiltIn HitTriangleVertexPositionsKHR - -// CHECK: %ReflectionRay = OpTypeStruct %v4float // CHECK: %_struct_{{.*}} = OpTypeStruct %ReflectionRay %v3float // CHECK: %_ptr_IncomingRayPayloadKHR__struct_{{.*}} = OpTypePointer IncomingRayPayloadKHR %_struct_{{.*}} -// CHECK: %_ptr_IncomingRayPayloadKHR_ReflectionRay = OpTypePointer IncomingRayPayloadKHR %ReflectionRay -// CHECK: %_ptr_IncomingRayPayloadKHR_v3float = OpTypePointer IncomingRayPayloadKHR %v3float -// CHECK: %StructuredBuffer = OpTypeStruct %_runtimearr_v4float -// CHECK: %_ptr_StorageBuffer_StructuredBuffer = OpTypePointer StorageBuffer %StructuredBuffer -// CHECK: %_ptr_Input__arr_v3float_{{.*}} = OpTypePointer Input %_arr_v3float_{{.*}} - // CHECK: %main = OpFunction %void None %{{.*}} // CHECK: %materialID = OpIAdd %uint %{{.*}} %{{.*}} // CHECK: %{{.*}} = OpAccessChain %_ptr_StorageBuffer_v4float %colors %int_0 %materialID From ef9a937db57e89657cb6ec79a9f2bed00d016b0e Mon Sep 17 00:00:00 2001 From: Mukund Date: Fri, 28 Feb 2025 14:02:44 +0530 Subject: [PATCH 3/5] Refactor code as per review comments --- source/slang/slang-ir-glsl-legalize.cpp | 138 +++++++++++------------- 1 file changed, 60 insertions(+), 78 deletions(-) diff --git a/source/slang/slang-ir-glsl-legalize.cpp b/source/slang/slang-ir-glsl-legalize.cpp index 6580ed82d4..e5fc574d74 100644 --- a/source/slang/slang-ir-glsl-legalize.cpp +++ b/source/slang/slang-ir-glsl-legalize.cpp @@ -329,10 +329,6 @@ struct GLSLLegalizationContext IRBuilder* builder; IRBuilder* getBuilder() { return builder; } - - // For ray tracing shaders, we need to consolidate all parameters into a single structure - Dictionary rayTracingConsolidatedVars; - Dictionary> rayTracingProcessedParams; }; // This examines the passed type and determines the GLSL mesh shader indices @@ -2306,7 +2302,7 @@ IRInst* materializeValue(IRBuilder* builder, ScalarizedVal const& val) } } -static void handleSingleParam( +void handleSingleParam( GLSLLegalizationContext* context, IRFunc* func, IRParam* pp, @@ -2360,7 +2356,6 @@ static void handleSingleParam( static void consolidateParameters( GLSLLegalizationContext* context, - IRFunc* func, List& params) { auto builder = context->getBuilder(); @@ -2381,7 +2376,8 @@ static void consolidateParameters( valueType = inOutType->getValueType(); auto key = builder->createStructKey(); - builder->addNameHintDecoration(key, UnownedStringSlice("field")); + if (auto nameDecor = _param->findDecoration()) + builder->addNameHintDecoration(key, nameDecor->getName()); auto field = builder->createStructField(structType, key, valueType); field->removeFromParent(); field->insertAtEnd(structType); @@ -2396,9 +2392,6 @@ static void consolidateParameters( // Add the ray payload decoration and assign location 0. builder->addVulkanRayPayloadDecoration(consolidatedVar, 0); - // Store the consolidated variable for this function - context->rayTracingConsolidatedVars[func] = consolidatedVar; - // Replace each parameter with a field in the consolidated struct for (Index i = 0; i < params.getCount(); ++i) { @@ -2441,64 +2434,54 @@ static void consolidateParameters( } } -static void handleMultipleParams(GLSLLegalizationContext* context, IRFunc* func, IRParam* pp) +// Consolidate ray tracing parameters for an entry point function +void consolidateRayTracingParameters( + GLSLLegalizationContext* context, + IRFunc* func) { + auto builder = context->getBuilder(); auto firstBlock = func->getFirstBlock(); + if (!firstBlock) + return; - // Now we run the consolidation step, but if we've already - // processed this parameter, skip it. - List* processedParams = nullptr; - if (auto foundList = context->rayTracingProcessedParams.tryGetValue(func)) - { - processedParams = foundList; - if (processedParams->contains(pp)) - return; - } - else - { - context->rayTracingProcessedParams[func] = List(); - processedParams = &context->rayTracingProcessedParams[func]; - } - - // Collect all parameters that need to be consolidated - List params; - List paramLayouts; + // Collect all out/inout parameters that need to be consolidated + List outParams; + List otherParams; - for (auto _param = firstBlock->getFirstParam(); _param; _param = _param->getNextParam()) + for (auto param = firstBlock->getFirstParam(); param; param = param->getNextParam()) { - auto pLayoutDecoration = _param->findDecoration(); - SLANG_ASSERT(pLayoutDecoration); - auto pLayout = as(pLayoutDecoration->getLayout()); - SLANG_ASSERT(pLayout); - - // Only include parameters that haven't been processed yet - auto _paramType = _param->getDataType(); - bool needsConsolidation = (as(_paramType) || as(_paramType)); - if (!processedParams->contains(_param) && needsConsolidation) + builder->setInsertBefore(firstBlock->getFirstOrdinaryInst()); + if (as(param->getDataType())) + { + outParams.add(param); + } + else { - params.add(_param); - paramLayouts.add(pLayout); - processedParams->add(_param); + otherParams.add(param); } } - consolidateParameters(context, func, params); -} - -void legalizeRayTracingEntryPointParameterForGLSL( - GLSLLegalizationContext* context, - IRFunc* func, - IRParam* pp, - IRVarLayout* paramLayout, - bool hasSingleOutOrInOutParam) -{ - if (hasSingleOutOrInOutParam) + // We don't need consolidation here. + if (outParams.getCount() <= 1) { - handleSingleParam(context, func, pp, paramLayout); + // We have one out/inout param, so add it as part of otherParams so we can + // just do one pass of handleSingleParam(). + if (outParams.getCount() == 1) + { + otherParams.add(outParams[0]); + } + for (auto param : otherParams) + { + auto paramLayoutDecoration = param->findDecoration(); + SLANG_ASSERT(paramLayoutDecoration); + auto paramLayout = as(paramLayoutDecoration->getLayout()); + handleSingleParam(context, func, param, paramLayout); + } return; } - handleMultipleParams(context, func, pp); + // Consolidate the parameters + consolidateParameters(context, outParams); } static void legalizeMeshPayloadInputParam( @@ -3344,28 +3327,6 @@ void legalizeEntryPointParameterForGLSL( case Stage::Intersection: case Stage::Miss: case Stage::RayGeneration: - { - // Count the number of inout or out parameters - int inoutOrOutParamCount = 0; - auto firstBlock = func->getFirstBlock(); - for (auto _param = firstBlock->getFirstParam(); _param; _param = _param->getNextParam()) - { - auto _paramType = _param->getDataType(); - if (as(_paramType) || as(_paramType)) - { - inoutOrOutParamCount++; - } - } - - // If we have just one inout or out param, we don't need consolidation. - bool hasSingleOutOrInOutParam = inoutOrOutParamCount <= 1; - legalizeRayTracingEntryPointParameterForGLSL( - context, - func, - pp, - paramLayout, - hasSingleOutOrInOutParam); - } return; } @@ -3941,12 +3902,33 @@ void legalizeEntryPointForGLSL( invokePathConstantFuncInHullShader(&context, codeGenContext, scalarizedGlobalOutput); } + // Special handling for ray tracing shaders + bool isRayTracingShader = false; + switch (stage) + { + case Stage::AnyHit: + case Stage::Callable: + case Stage::ClosestHit: + case Stage::Intersection: + case Stage::Miss: + case Stage::RayGeneration: + isRayTracingShader = true; + consolidateRayTracingParameters(&context, func); + break; + default: + break; + } + // Next we will walk through any parameters of the entry-point function, // and turn them into global variables. if (auto firstBlock = func->getFirstBlock()) { for (auto pp = firstBlock->getFirstParam(); pp; pp = pp->getNextParam()) { + if (isRayTracingShader) + { + continue; + } // Any initialization code we insert for parameters needs // to be at the start of the "ordinary" instructions in the block: builder.setInsertBefore(firstBlock->getFirstOrdinaryInst()); @@ -3962,7 +3944,7 @@ void legalizeEntryPointForGLSL( legalizeEntryPointParameterForGLSL(&context, codeGenContext, func, pp, paramLayout); } - + // At this point we should have eliminated all uses of the // parameters of the entry block. Also, our control-flow // rules mean that the entry block cannot be the target From 35c35fda72c05ad88682f924b5c53c609738bb3d Mon Sep 17 00:00:00 2001 From: slangbot <186143334+slangbot@users.noreply.github.com> Date: Fri, 28 Feb 2025 08:39:57 +0000 Subject: [PATCH 4/5] format code --- source/slang/slang-ir-glsl-legalize.cpp | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/source/slang/slang-ir-glsl-legalize.cpp b/source/slang/slang-ir-glsl-legalize.cpp index e5fc574d74..95f084be25 100644 --- a/source/slang/slang-ir-glsl-legalize.cpp +++ b/source/slang/slang-ir-glsl-legalize.cpp @@ -2354,9 +2354,7 @@ void handleSingleParam( builder->addDependsOnDecoration(func, globalParam); } -static void consolidateParameters( - GLSLLegalizationContext* context, - List& params) +static void consolidateParameters(GLSLLegalizationContext* context, List& params) { auto builder = context->getBuilder(); @@ -2435,9 +2433,7 @@ static void consolidateParameters( } // Consolidate ray tracing parameters for an entry point function -void consolidateRayTracingParameters( - GLSLLegalizationContext* context, - IRFunc* func) +void consolidateRayTracingParameters(GLSLLegalizationContext* context, IRFunc* func) { auto builder = context->getBuilder(); auto firstBlock = func->getFirstBlock(); @@ -2466,9 +2462,9 @@ void consolidateRayTracingParameters( { // We have one out/inout param, so add it as part of otherParams so we can // just do one pass of handleSingleParam(). - if (outParams.getCount() == 1) - { - otherParams.add(outParams[0]); + if (outParams.getCount() == 1) + { + otherParams.add(outParams[0]); } for (auto param : otherParams) { @@ -3918,7 +3914,7 @@ void legalizeEntryPointForGLSL( default: break; } - + // Next we will walk through any parameters of the entry-point function, // and turn them into global variables. if (auto firstBlock = func->getFirstBlock()) @@ -3944,7 +3940,7 @@ void legalizeEntryPointForGLSL( legalizeEntryPointParameterForGLSL(&context, codeGenContext, func, pp, paramLayout); } - + // At this point we should have eliminated all uses of the // parameters of the entry block. Also, our control-flow // rules mean that the entry block cannot be the target From 5694fe594875bff42ad9c42b247ade1935771016 Mon Sep 17 00:00:00 2001 From: Mukund Date: Fri, 28 Feb 2025 15:29:13 +0530 Subject: [PATCH 5/5] fix failing tests --- source/slang/slang-ir-glsl-legalize.cpp | 42 ++++++++++++++----------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/source/slang/slang-ir-glsl-legalize.cpp b/source/slang/slang-ir-glsl-legalize.cpp index 95f084be25..c12bc6ee3b 100644 --- a/source/slang/slang-ir-glsl-legalize.cpp +++ b/source/slang/slang-ir-glsl-legalize.cpp @@ -2368,10 +2368,8 @@ static void consolidateParameters(GLSLLegalizationContext* context, ListgetDataType(); IRType* valueType = _paramType; - if (as(_paramType)) - valueType = as(_paramType)->getValueType(); - else if (auto inOutType = as(_paramType)) - valueType = inOutType->getValueType(); + if (as(_paramType)) + valueType = as(_paramType)->getValueType(); auto key = builder->createStructKey(); if (auto nameDecor = _param->findDecoration()) @@ -2442,31 +2440,22 @@ void consolidateRayTracingParameters(GLSLLegalizationContext* context, IRFunc* f // Collect all out/inout parameters that need to be consolidated List outParams; - List otherParams; + List params; for (auto param = firstBlock->getFirstParam(); param; param = param->getNextParam()) { builder->setInsertBefore(firstBlock->getFirstOrdinaryInst()); - if (as(param->getDataType())) + if (as(param->getDataType()) || as(param->getDataType())) { outParams.add(param); } - else - { - otherParams.add(param); - } + params.add(param); } // We don't need consolidation here. if (outParams.getCount() <= 1) { - // We have one out/inout param, so add it as part of otherParams so we can - // just do one pass of handleSingleParam(). - if (outParams.getCount() == 1) - { - otherParams.add(outParams[0]); - } - for (auto param : otherParams) + for (auto param : params) { auto paramLayoutDecoration = param->findDecoration(); SLANG_ASSERT(paramLayoutDecoration); @@ -2475,9 +2464,24 @@ void consolidateRayTracingParameters(GLSLLegalizationContext* context, IRFunc* f } return; } + else + { + // We need consolidation here, but before that, handle parameters other than inout/out. + for (auto param : params) + { + if (outParams.contains(param)) + { + continue; + } + auto paramLayoutDecoration = param->findDecoration(); + SLANG_ASSERT(paramLayoutDecoration); + auto paramLayout = as(paramLayoutDecoration->getLayout()); + handleSingleParam(context, func, param, paramLayout); + } - // Consolidate the parameters - consolidateParameters(context, outParams); + // Now, consolidate the inout/out parameters + consolidateParameters(context, outParams); + } } static void legalizeMeshPayloadInputParam(