Skip to content

Commit 4923e88

Browse files
authored
Fix Optional none lowering for DifferentialPair (#9562)
Fixes #9526 fix Optional none lowering for DifferentialPair add regression test
1 parent f10d602 commit 4923e88

File tree

8 files changed

+60
-16
lines changed

8 files changed

+60
-16
lines changed

external/spirv-tools

Submodule spirv-tools updated 102 files

source/slang/slang-ir-insts.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3681,7 +3681,7 @@ struct IRBuilder
36813681
IRInst* emitOptionalHasValue(IRInst* optValue);
36823682
IRInst* emitGetOptionalValue(IRInst* optValue);
36833683
IRInst* emitMakeOptionalValue(IRInst* optType, IRInst* value);
3684-
IRInst* emitMakeOptionalNone(IRInst* optType, IRInst* defaultValue);
3684+
IRInst* emitMakeOptionalNone(IRInst* optType);
36853685

36863686
IRInst* emitDifferentialPairGetDifferential(IRType* diffType, IRInst* diffPair);
36873687
IRInst* emitDifferentialValuePairGetDifferential(IRType* diffType, IRInst* diffPair);

source/slang/slang-ir-insts.lua

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -939,7 +939,7 @@ local insts = {
939939
{ getOptionalValue = { operands = { { "optionalOperand" } } } },
940940
{ optionalHasValue = { operands = { { "optionalOperand" } } } },
941941
{ makeOptionalValue = { operands = { { "value" } } } },
942-
{ makeOptionalNone = { operands = { { "defaultValue" } } } },
942+
{ makeOptionalNone = {} },
943943
{ CombinedTextureSamplerGetTexture = { operands = { { "sampler" } } } },
944944
{ CombinedTextureSamplerGetSampler = { operands = { { "sampler" } } } },
945945
{ call = { operands = { { "callee" } } } },

source/slang/slang-ir-lower-optional-type.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,8 +169,12 @@ struct OptionalTypeLoweringContext
169169
auto info = getLoweredOptionalType(builder, inst->getDataType());
170170
if (info->loweredType != info->valueType)
171171
{
172+
// Synthesize a default-constructed placeholder for the payload.
173+
// The payload is semantically irrelevant when hasValue == false,
174+
// but we need a well-formed value to satisfy the struct layout.
175+
auto defaultVal = builder->emitDefaultConstruct(info->valueType);
172176
List<IRInst*> operands;
173-
operands.add(inst->getDefaultValue());
177+
operands.add(defaultVal);
174178
operands.add(builder->getBoolValue(false));
175179
auto makeStruct = builder->emitMakeStruct(info->loweredType, operands);
176180
inst->replaceUsesWith(makeStruct);

source/slang/slang-ir.cpp

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3721,11 +3721,9 @@ IRInst* IRBuilder::emitDefaultConstruct(IRType* type, bool fallback)
37213721
return getNullPtrValue(type);
37223722
case kIROp_OptionalType:
37233723
{
3724-
auto inner =
3725-
emitDefaultConstruct(as<IROptionalType>(actualType)->getValueType(), fallback);
3726-
if (!inner)
3727-
return nullptr;
3728-
return emitMakeOptionalNone(type, inner);
3724+
// The Optional-type lowering pass synthesizes the placeholder payload,
3725+
// so we don't need to construct `inner` here.
3726+
return emitMakeOptionalNone(type);
37293727
}
37303728
case kIROp_TupleType:
37313729
{
@@ -4247,9 +4245,9 @@ IRInst* IRBuilder::emitMakeOptionalValue(IRInst* optType, IRInst* value)
42474245
return emitIntrinsicInst((IRType*)optType, kIROp_MakeOptionalValue, 1, &value);
42484246
}
42494247

4250-
IRInst* IRBuilder::emitMakeOptionalNone(IRInst* optType, IRInst* defaultValue)
4248+
IRInst* IRBuilder::emitMakeOptionalNone(IRInst* optType)
42514249
{
4252-
return emitIntrinsicInst((IRType*)optType, kIROp_MakeOptionalNone, 1, &defaultValue);
4250+
return emitIntrinsicInst((IRType*)optType, kIROp_MakeOptionalNone, 0, nullptr);
42534251
}
42544252

42554253
IRInst* IRBuilder::emitMakeVectorFromScalar(IRType* type, IRInst* scalarValue)

source/slang/slang-lower-to-ir.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5802,8 +5802,9 @@ struct ExprLoweringVisitorBase : public ExprVisitor<Derived, LoweredValInfo>
58025802
else
58035803
{
58045804
auto optType = lowerType(context, expr->type);
5805-
auto defaultVal = getDefaultVal(as<OptionalType>(expr->type)->getValueType());
5806-
auto irVal = context->irBuilder->emitMakeOptionalNone(optType, defaultVal.val);
5805+
// `none` for Optional<T> no longer requires a payload value from the front-end.
5806+
// The Optional-type lowering pass will synthesize a default-constructed placeholder.
5807+
auto irVal = context->irBuilder->emitMakeOptionalNone(optType);
58075808
return LoweredValInfo::simple(irVal);
58085809
}
58095810
}
@@ -6080,8 +6081,8 @@ struct ExprLoweringVisitorBase : public ExprVisitor<Derived, LoweredValInfo>
60806081
builder->emitStore(var, optionalVal);
60816082
builder->emitBranch(afterBlock);
60826083
builder->setInsertInto(falseBlock);
6083-
auto defaultVal = getDefaultVal(as<OptionalType>(expr->type)->getValueType());
6084-
auto noneVal = builder->emitMakeOptionalNone(optType, defaultVal.val);
6084+
// See `visitMakeOptionalExpr`: no longer need to pass a defaultVal.
6085+
auto noneVal = builder->emitMakeOptionalNone(optType);
60856086
builder->emitStore(var, noneVal);
60866087
builder->emitBranch(afterBlock);
60876088
builder->setInsertInto(afterBlock);
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// Regression test for issue #9526:
2+
// Compiler crash during specialization when passing `none` to `Optional<DifferentialPair<T>>`.
3+
//
4+
// The intent here is compilation-only: the shader is trivial and just ensures the frontend +
5+
// specialization pipeline can handle passing `none` without materializing/specializing a dummy `T`.
6+
//
7+
// Compilation-only regression test: should not crash during specialization.
8+
// Note: specify `-o` so slang-test doesn't treat SPIR-V text on stdout as a failure.
9+
//TEST:SIMPLE: -target spirv -o gh-9526.spv
10+
11+
struct MyStruct : IDifferentiable
12+
{
13+
int a;
14+
int b;
15+
}
16+
17+
int processOptionalDiffPair(Optional<DifferentialPair<MyStruct>> s)
18+
{
19+
if (s.hasValue)
20+
{
21+
int x = s.value.p.a + s.value.p.b;
22+
return x;
23+
}
24+
return 0;
25+
}
26+
27+
RWStructuredBuffer<int> outBuffer;
28+
29+
[shader("compute")]
30+
void computeMain()
31+
{
32+
// "Has value" case (idiomatic): assign a `T` into `Optional<T>` to construct an optional-with-value.
33+
// Here `T` is `DifferentialPair<MyStruct>`.
34+
DifferentialPair<MyStruct> dp = diffPair(MyStruct(1, 2));
35+
Optional<DifferentialPair<MyStruct>> optWithValue = dp;
36+
outBuffer[0] = processOptionalDiffPair(optWithValue);
37+
38+
// This used to crash during specialization.
39+
outBuffer[1] = processOptionalDiffPair(none);
40+
}
41+

0 commit comments

Comments
 (0)