Skip to content

Commit 7eaaf1a

Browse files
author
Tim Foley
authored
Initial work on validating "constexpr"-ness in IR (shader-slang#420)
* Initial work on validating "constexpr"-ness in IR The underlying issue here is that certain operations in the target shading languages constrain their operands to be compile-time constants. A notable example is the optional texel offset parameter to the `Texture2D.Sample` operation. When calling these operations in GLSL, the user is required to pass a "constant expression," and any variables in that expression must therefore be marked with the `const` qualifier (and themselves be initialized with constant expressions). Any GLSL output we generate must of course respect these rules. When calling these operations in HLSL, the user is not so constrained. Instead, they can pass an arbitrary expression, which may involve ordinary variables with no particular markup, and then the compiler is responsible for determining if the actual value after simplification works out to be a constant. In some cases, the requirement that a value be constant might actually trigger things like loop unrolling. Also, it is okay to use a function parameter to determine such a constant expression, as long as the argument turns out to be a constant at all call sites. The way we have decided to tackle these challenges in Slang is that we we propagate a notion of `constexpr`-ness through the IR. This is currently being tackled in `ir-constexpr.cpp` with a combination of forward and backward iterative dataflow: * When the operands to an instruction are all `constexpr`, and the opcode is one we believe can be constant-folded, then we infer that the instruction *can* be evaluated as `constexpr` * When instruction is required to be `constexpr`, then we infer that all of its operands are also required to be `constexpr`. If this process ever infers that a function parameter is required to be `constexpr`, then we might have to continue propagation at all the call sites to that function. If after all the propagation is done, there are any cases where an instruction is *required* to be `constexpr`, but it *can't* be `constexpr` (we weren't able to infer `constexpr`-ness for its operands), then we issue an error. This implementation encodes the idea of `constexpr`-ness in the IR as part of the type system, using a simplified notion of rates. This change adds a `RateQualifiedType` that can represent `@R T`, and then introduces a `ConstExprRate` that can be used for `R`. Many accessors for the type information on IR nodes were updated to distinguish when one wants the "full" type of an IR value (which might include rate information) vs. just the "data" type. A `constexpr` qualifier was added in the front-end, and is being used to decorate the texel offset parameter for `Texture2D.Sample`. Lowering from AST to IR looks for this qalifier and infers when a function parameter must be typed as `@ConstExpr T` instead of just `T`. There are lots of limitations and gotchas in the implementation so far: * The `@ConstExpr` rate is the only one added in this change, but it seems clear that the conceptual `ThreadGroup` rate that was added to represent `groupshared` should probably get folded into the representation. * I'm not 100% pleased with how many places in the IR I have to special-case for rate-qualified types. At the same type, pulling out rate as a distinct field on `IRValue` would probably require that we pay attention to rate everywhere. * I've added a test case to show that we can issue errors when users fail to provide a constant expression for the texel offset, but the actual error message isn't great because it doesn't indicate *why* a constant expression was required. Realistically the "initial IR" should contain a few more decorations we can use to relate error conditions back to the original code (even if this is in a side-band structure). * I've added a test case that is supposed to show that we can back-propagate `constexpr`-ness to local variables, and I've manually confirmed that it works for Vulkan/SPIR-V output, but the level of Vulkan support in `render_test` today means I can't enable the test for check-in. * While I'm attempting to propagate `@ConstExpr` information from callees to callers, I haven't implemented any logic to specialize callee functions based on values at call sites. * In a similar vein, there is no handling of control-flow dependence in the current code. If we infer that a phi (block parameter) needs to be `@ConstExpr`, then it isn't actually enough to require that the inputs to the phi (arguments from predecessor blocks) are all `@ConstExpr` because we also need any control-flow decisions that pick which incoming edge we take to be `@ConstExpr` as well. * As a practical matter, implicit propagation of `@ConstExpr` from a function body to a function parameter should only be allowed for functions that are "local" to a module. Any function that might be accessed from outside of a module should really have had its `@ConstExpr` parameter marked manually, and our pass should validate that they follow their own rules. Right now we have no kind of visibility (`public` vs `private`) system, so I'm kind of ignoring this issue. While that is a lot of gaps, this is also just enough code to get the Falcor MultiPassPostProcess example working, so I'm inclined to get it checked in. * Fixup: missing expected output for test * Fixup: disable test that relies on [unroll] for now
1 parent 01c4134 commit 7eaaf1a

28 files changed

+1181
-109
lines changed

slang.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -1237,12 +1237,13 @@ namespace slang
12371237
#include "source/slang/dxc-support.cpp"
12381238
#include "source/slang/emit.cpp"
12391239
#include "source/slang/ir.cpp"
1240-
#include "source/slang/memory_pool.cpp"
1240+
#include "source/slang/ir-constexpr.cpp"
12411241
#include "source/slang/ir-legalize-types.cpp"
12421242
#include "source/slang/ir-ssa.cpp"
12431243
#include "source/slang/legalize-types.cpp"
12441244
#include "source/slang/lexer.cpp"
12451245
#include "source/slang/mangle.cpp"
1246+
#include "source/slang/memory_pool.cpp"
12461247
#include "source/slang/name.cpp"
12471248
#include "source/slang/options.cpp"
12481249
#include "source/slang/parameter-binding.cpp"

source/slang/bytecode.cpp

+8-8
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,7 @@ void encodeOperand(
319319

320320
bool opHasResult(IRValue* inst)
321321
{
322-
auto type = inst->getType();
322+
auto type = inst->getDataType();
323323
if (!type) return false;
324324

325325
// As a bit of a hack right now, we need to check whether
@@ -352,7 +352,7 @@ void generateBytecodeForInst(
352352

353353
auto argCount = inst->getArgCount();
354354
encodeUInt(context, inst->op);
355-
encodeOperand(context, inst->getType());
355+
encodeOperand(context, inst->getDataType());
356356
encodeUInt(context, argCount);
357357
for( UInt aa = 0; aa < argCount; ++aa )
358358
{
@@ -381,7 +381,7 @@ void generateBytecodeForInst(
381381
{
382382
auto ii = (IRConstant*) inst;
383383
encodeUInt(context, ii->op);
384-
encodeOperand(context, ii->getType());
384+
encodeOperand(context, ii->getDataType());
385385

386386
// TODO: probably want distinct encodings
387387
// for signed vs. unsigned here.
@@ -396,7 +396,7 @@ void generateBytecodeForInst(
396396
{
397397
auto cInst = (IRConstant*) inst;
398398
encodeUInt(context, cInst->op);
399-
encodeOperand(context, cInst->getType());
399+
encodeOperand(context, cInst->getDataType());
400400

401401
static const UInt size = sizeof(IRFloatingPointValue);
402402
unsigned char buffer[size];
@@ -446,7 +446,7 @@ void generateBytecodeForInst(
446446

447447
// We need to encode the type being stored, to make
448448
// our lives easier.
449-
encodeOperand(context, inst->getArg(1)->getType());
449+
encodeOperand(context, inst->getArg(1)->getDataType());
450450
encodeOperand(context, inst->getArg(0));
451451
encodeOperand(context, inst->getArg(1));
452452
}
@@ -455,7 +455,7 @@ void generateBytecodeForInst(
455455
case kIROp_Load:
456456
{
457457
encodeUInt(context, inst->op);
458-
encodeOperand(context, inst->getType());
458+
encodeOperand(context, inst->getDataType());
459459
encodeOperand(context, inst->getArg(0));
460460
encodeOperand(context, inst);
461461
}
@@ -611,7 +611,7 @@ uint32_t getTypeIDForGlobalSymbol(
611611
BytecodeGenerationContext* context,
612612
IRValue* inst)
613613
{
614-
auto type = inst->getType();
614+
auto type = inst->getDataType();
615615
if(!type)
616616
return 0;
617617

@@ -836,7 +836,7 @@ BytecodeGenerationPtr<BCSymbol> generateBytecodeSymbolForInst(
836836
bcRegs[localID+1].op = ii->op;
837837
bcRegs[localID+1].previousVarIndexPlusOne = (uint32_t)localID+1;
838838
bcRegs[localID+1].typeID = getTypeID(context,
839-
(ii->getType()->As<PtrType>())->getValueType());
839+
(ii->getDataType()->As<PtrType>())->getValueType());
840840
}
841841
break;
842842
}

source/slang/compiler.h

+12
Original file line numberDiff line numberDiff line change
@@ -448,6 +448,7 @@ namespace Slang
448448
RefPtr<Type> errorType;
449449
RefPtr<Type> initializerListType;
450450
RefPtr<Type> overloadedType;
451+
RefPtr<Type> constExprRate;
451452
RefPtr<Type> irBasicBlockType;
452453

453454
Dictionary<int, RefPtr<Type>> builtinTypes;
@@ -467,6 +468,17 @@ namespace Slang
467468
Type* getOverloadedType();
468469
Type* getErrorType();
469470

471+
Type* getConstExprRate();
472+
RefPtr<RateQualifiedType> getRateQualifiedType(
473+
Type* rate,
474+
Type* valueType);
475+
476+
RefPtr<RateQualifiedType> getConstExprType(
477+
Type* valueType)
478+
{
479+
return getRateQualifiedType(getConstExprRate(), valueType);
480+
}
481+
470482
// Should not be used in front-end code
471483
Type* getIRBasicBlockType();
472484

source/slang/core.meta.slang

+16-12
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
// Slang `core` library
22

3+
// Modifier for variables that must resolve to compile-time constants
4+
// as part of translation.
5+
syntax constexpr : ConstExprModifier;
6+
37
// A type that can be used as an operand for builtins
48
interface __BuiltinType {}
59

@@ -623,7 +627,7 @@ for (int tt = 0; tt < kBaseTextureTypeCount; ++tt)
623627
{
624628
sb << ", int sampleIndex";
625629
}
626-
sb << ", int" << loadCoordCount << " offset";
630+
sb << ", constexpr int" << loadCoordCount << " offset";
627631
sb << ");\n";
628632

629633

@@ -633,7 +637,7 @@ for (int tt = 0; tt < kBaseTextureTypeCount; ++tt)
633637
{
634638
sb << ", int sampleIndex";
635639
}
636-
sb << ", int" << kBaseTextureTypes[tt].coordCount << " offset";
640+
sb << ", constexpr int" << kBaseTextureTypes[tt].coordCount << " offset";
637641
sb << ", out uint status";
638642
sb << ");\n";
639643
}
@@ -699,22 +703,22 @@ for (int tt = 0; tt < kBaseTextureTypeCount; ++tt)
699703
sb << "__target_intrinsic(glsl, \"textureOffset($$p, $2, $3)\")\n";
700704
sb << "T Sample(SamplerState s, ";
701705
sb << "float" << kBaseTextureTypes[tt].coordCount + isArray << " location, ";
702-
sb << "int" << kBaseTextureTypes[tt].coordCount << " offset);\n";
706+
sb << "constexpr int" << kBaseTextureTypes[tt].coordCount << " offset);\n";
703707
}
704708

705709
sb << "T Sample(SamplerState s, ";
706710
sb << "float" << kBaseTextureTypes[tt].coordCount + isArray << " location, ";
707711
if( baseShape != TextureType::ShapeCube )
708712
{
709-
sb << "int" << kBaseTextureTypes[tt].coordCount << " offset, ";
713+
sb << "constexpr int" << kBaseTextureTypes[tt].coordCount << " offset, ";
710714
}
711715
sb << "float clamp);\n";
712716

713717
sb << "T Sample(SamplerState s, ";
714718
sb << "float" << kBaseTextureTypes[tt].coordCount + isArray << " location, ";
715719
if( baseShape != TextureType::ShapeCube )
716720
{
717-
sb << "int" << kBaseTextureTypes[tt].coordCount << " offset, ";
721+
sb << "constexpr int" << kBaseTextureTypes[tt].coordCount << " offset, ";
718722
}
719723
sb << "float clamp, out uint status);\n";
720724

@@ -729,7 +733,7 @@ for (int tt = 0; tt < kBaseTextureTypeCount; ++tt)
729733
sb << "__target_intrinsic(glsl, \"textureOffset($$p, $2, $3, $4)\")\n";
730734
sb << "T SampleBias(SamplerState s, ";
731735
sb << "float" << kBaseTextureTypes[tt].coordCount + isArray << " location, float bias, ";
732-
sb << "int" << kBaseTextureTypes[tt].coordCount << " offset);\n";
736+
sb << "constexpr int" << kBaseTextureTypes[tt].coordCount << " offset);\n";
733737
}
734738

735739
// `SampleCmp()` and `SampleCmpLevelZero`
@@ -796,12 +800,12 @@ for (int tt = 0; tt < kBaseTextureTypeCount; ++tt)
796800
sb << "T SampleCmp(SamplerState s, ";
797801
sb << "float" << kBaseTextureTypes[tt].coordCount + isArray << " location, ";
798802
sb << "float compareValue, ";
799-
sb << "int" << kBaseTextureTypes[tt].coordCount << " offset);\n";
803+
sb << "constexpr int" << kBaseTextureTypes[tt].coordCount << " offset);\n";
800804

801805
sb << "T SampleCmpLevelZero(SamplerState s, ";
802806
sb << "float" << kBaseTextureTypes[tt].coordCount + isArray << " location, ";
803807
sb << "float compareValue, ";
804-
sb << "int" << kBaseTextureTypes[tt].coordCount << " offset);\n";
808+
sb << "constexpr int" << kBaseTextureTypes[tt].coordCount << " offset);\n";
805809
}
806810

807811

@@ -821,7 +825,7 @@ for (int tt = 0; tt < kBaseTextureTypeCount; ++tt)
821825
sb << "float" << kBaseTextureTypes[tt].coordCount + isArray << " location, ";
822826
sb << "float" << kBaseTextureTypes[tt].coordCount << " gradX, ";
823827
sb << "float" << kBaseTextureTypes[tt].coordCount << " gradY, ";
824-
sb << "int" << kBaseTextureTypes[tt].coordCount << " offset);\n";
828+
sb << "constexpr int" << kBaseTextureTypes[tt].coordCount << " offset);\n";
825829
}
826830

827831
// `SampleLevel`
@@ -837,7 +841,7 @@ for (int tt = 0; tt < kBaseTextureTypeCount; ++tt)
837841
sb << "T SampleLevel(SamplerState s, ";
838842
sb << "float" << kBaseTextureTypes[tt].coordCount + isArray << " location, ";
839843
sb << "float level, ";
840-
sb << "int" << kBaseTextureTypes[tt].coordCount << " offset);\n";
844+
sb << "constexpr int" << kBaseTextureTypes[tt].coordCount << " offset);\n";
841845
}
842846
}
843847

@@ -908,12 +912,12 @@ for (int tt = 0; tt < kBaseTextureTypeCount; ++tt)
908912
sb << "__target_intrinsic(glsl, \"textureGatherOffset($$p, $2, $3, " << componentIndex << ")\")\n";
909913
sb << "vector<T, 4> Gather" << componentName << "(SamplerState s, ";
910914
sb << "float" << kBaseTextureTypes[tt].coordCount << " location, ";
911-
sb << "int" << kBaseTextureTypes[tt].coordCount << " offset);\n";
915+
sb << "constexpr int" << kBaseTextureTypes[tt].coordCount << " offset);\n";
912916

913917
EMIT_LINE_DIRECTIVE();
914918
sb << "vector<T, 4> Gather" << componentName << "(SamplerState s, ";
915919
sb << "float" << kBaseTextureTypes[tt].coordCount << " location, ";
916-
sb << "int" << kBaseTextureTypes[tt].coordCount << " offset, ";
920+
sb << "constexpr int" << kBaseTextureTypes[tt].coordCount << " offset, ";
917921
sb << "out uint status);\n";
918922

919923
EMIT_LINE_DIRECTIVE();

source/slang/core.meta.slang.h

+16-12
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
sb << "// Slang `core` library\n";
22
sb << "\n";
3+
sb << "// Modifier for variables that must resolve to compile-time constants\n";
4+
sb << "// as part of translation.\n";
5+
sb << "syntax constexpr : ConstExprModifier;\n";
6+
sb << "\n";
37
sb << "// A type that can be used as an operand for builtins\n";
48
sb << "interface __BuiltinType {}\n";
59
sb << "\n";
@@ -626,7 +630,7 @@ for (int tt = 0; tt < kBaseTextureTypeCount; ++tt)
626630
{
627631
sb << ", int sampleIndex";
628632
}
629-
sb << ", int" << loadCoordCount << " offset";
633+
sb << ", constexpr int" << loadCoordCount << " offset";
630634
sb << ");\n";
631635

632636

@@ -636,7 +640,7 @@ for (int tt = 0; tt < kBaseTextureTypeCount; ++tt)
636640
{
637641
sb << ", int sampleIndex";
638642
}
639-
sb << ", int" << kBaseTextureTypes[tt].coordCount << " offset";
643+
sb << ", constexpr int" << kBaseTextureTypes[tt].coordCount << " offset";
640644
sb << ", out uint status";
641645
sb << ");\n";
642646
}
@@ -702,22 +706,22 @@ for (int tt = 0; tt < kBaseTextureTypeCount; ++tt)
702706
sb << "__target_intrinsic(glsl, \"textureOffset($p, $2, $3)\")\n";
703707
sb << "T Sample(SamplerState s, ";
704708
sb << "float" << kBaseTextureTypes[tt].coordCount + isArray << " location, ";
705-
sb << "int" << kBaseTextureTypes[tt].coordCount << " offset);\n";
709+
sb << "constexpr int" << kBaseTextureTypes[tt].coordCount << " offset);\n";
706710
}
707711

708712
sb << "T Sample(SamplerState s, ";
709713
sb << "float" << kBaseTextureTypes[tt].coordCount + isArray << " location, ";
710714
if( baseShape != TextureType::ShapeCube )
711715
{
712-
sb << "int" << kBaseTextureTypes[tt].coordCount << " offset, ";
716+
sb << "constexpr int" << kBaseTextureTypes[tt].coordCount << " offset, ";
713717
}
714718
sb << "float clamp);\n";
715719

716720
sb << "T Sample(SamplerState s, ";
717721
sb << "float" << kBaseTextureTypes[tt].coordCount + isArray << " location, ";
718722
if( baseShape != TextureType::ShapeCube )
719723
{
720-
sb << "int" << kBaseTextureTypes[tt].coordCount << " offset, ";
724+
sb << "constexpr int" << kBaseTextureTypes[tt].coordCount << " offset, ";
721725
}
722726
sb << "float clamp, out uint status);\n";
723727

@@ -732,7 +736,7 @@ for (int tt = 0; tt < kBaseTextureTypeCount; ++tt)
732736
sb << "__target_intrinsic(glsl, \"textureOffset($p, $2, $3, $4)\")\n";
733737
sb << "T SampleBias(SamplerState s, ";
734738
sb << "float" << kBaseTextureTypes[tt].coordCount + isArray << " location, float bias, ";
735-
sb << "int" << kBaseTextureTypes[tt].coordCount << " offset);\n";
739+
sb << "constexpr int" << kBaseTextureTypes[tt].coordCount << " offset);\n";
736740
}
737741

738742
// `SampleCmp()` and `SampleCmpLevelZero`
@@ -799,12 +803,12 @@ for (int tt = 0; tt < kBaseTextureTypeCount; ++tt)
799803
sb << "T SampleCmp(SamplerState s, ";
800804
sb << "float" << kBaseTextureTypes[tt].coordCount + isArray << " location, ";
801805
sb << "float compareValue, ";
802-
sb << "int" << kBaseTextureTypes[tt].coordCount << " offset);\n";
806+
sb << "constexpr int" << kBaseTextureTypes[tt].coordCount << " offset);\n";
803807

804808
sb << "T SampleCmpLevelZero(SamplerState s, ";
805809
sb << "float" << kBaseTextureTypes[tt].coordCount + isArray << " location, ";
806810
sb << "float compareValue, ";
807-
sb << "int" << kBaseTextureTypes[tt].coordCount << " offset);\n";
811+
sb << "constexpr int" << kBaseTextureTypes[tt].coordCount << " offset);\n";
808812
}
809813

810814

@@ -824,7 +828,7 @@ for (int tt = 0; tt < kBaseTextureTypeCount; ++tt)
824828
sb << "float" << kBaseTextureTypes[tt].coordCount + isArray << " location, ";
825829
sb << "float" << kBaseTextureTypes[tt].coordCount << " gradX, ";
826830
sb << "float" << kBaseTextureTypes[tt].coordCount << " gradY, ";
827-
sb << "int" << kBaseTextureTypes[tt].coordCount << " offset);\n";
831+
sb << "constexpr int" << kBaseTextureTypes[tt].coordCount << " offset);\n";
828832
}
829833

830834
// `SampleLevel`
@@ -840,7 +844,7 @@ for (int tt = 0; tt < kBaseTextureTypeCount; ++tt)
840844
sb << "T SampleLevel(SamplerState s, ";
841845
sb << "float" << kBaseTextureTypes[tt].coordCount + isArray << " location, ";
842846
sb << "float level, ";
843-
sb << "int" << kBaseTextureTypes[tt].coordCount << " offset);\n";
847+
sb << "constexpr int" << kBaseTextureTypes[tt].coordCount << " offset);\n";
844848
}
845849
}
846850

@@ -911,12 +915,12 @@ for (int tt = 0; tt < kBaseTextureTypeCount; ++tt)
911915
sb << "__target_intrinsic(glsl, \"textureGatherOffset($p, $2, $3, " << componentIndex << ")\")\n";
912916
sb << "vector<T, 4> Gather" << componentName << "(SamplerState s, ";
913917
sb << "float" << kBaseTextureTypes[tt].coordCount << " location, ";
914-
sb << "int" << kBaseTextureTypes[tt].coordCount << " offset);\n";
918+
sb << "constexpr int" << kBaseTextureTypes[tt].coordCount << " offset);\n";
915919

916920
EMIT_LINE_DIRECTIVE();
917921
sb << "vector<T, 4> Gather" << componentName << "(SamplerState s, ";
918922
sb << "float" << kBaseTextureTypes[tt].coordCount << " location, ";
919-
sb << "int" << kBaseTextureTypes[tt].coordCount << " offset, ";
923+
sb << "constexpr int" << kBaseTextureTypes[tt].coordCount << " offset, ";
920924
sb << "out uint status);\n";
921925

922926
EMIT_LINE_DIRECTIVE();

source/slang/diagnostic-defs.h

+2
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,8 @@ DIAGNOSTIC(40005, Error, topLevelModuleUsedWithoutSpecifyingBinding, "top level
286286

287287
DIAGNOSTIC(49999, Error, unknownSystemValueSemantic, "unknown system-value semantic '$0'")
288288

289+
DIAGNOSTIC(40006, Error, needCompileTimeConstant, "expected a compile-time constant");
290+
289291
//
290292
// 5xxxx - Target code generation.
291293
//

0 commit comments

Comments
 (0)