Skip to content

Commit afb8146

Browse files
gtong-nvslangbot
gtong-nv
and
slangbot
authored
Diagnose error when the function args can't satisfy constexpr parameter requirements (#7269)
## Summary This PR enhances constexpr validation by adding proper error checking when function arguments cannot satisfy constexpr parameter requirements, addressing issue #6370. ## Problem Previously, when a function declared constexpr parameters, the compiler would attempt to propagate constexpr-ness to the call site arguments, but there was insufficient validation and error reporting when this propagation failed. This could lead silent failures where constexpr requirements weren't properly enforced ## Solution This PR adds checks that: 1. **Validates constexpr arguments**: When a function parameter is marked as `constexpr`, the compiler now explicitly checks that the corresponding argument can be marked as `constexpr` 2. **Issues clear compilation errors**: added `Diagnostics::argIsNotConstexpr`) 3. **Handles both call scenarios**: The validation works for both: - Direct function calls with IR-level function definitions - Calls to function from external modules Fixes #6370 --------- Co-authored-by: slangbot <ellieh+slangbot@nvidia.com> Co-authored-by: slangbot <186143334+slangbot@users.noreply.github.com>
1 parent 8ad0ae1 commit afb8146

6 files changed

Lines changed: 111 additions & 77 deletions

File tree

‎source/slang/glsl.meta.slang‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1914,7 +1914,7 @@ public vector<T.Element,4> texture(Sampler1D<T> sampler, float p)
19141914
__generic<T:ITexelElement>
19151915
[ForceInline]
19161916
[require(cpp_glsl_hlsl_spirv, texture_sm_4_0_fragment)]
1917-
public vector<T.Element,4> texture(Sampler1D<T> sampler, float p, constexpr float bias)
1917+
public vector<T.Element,4> texture(Sampler1D<T> sampler, float p, float bias)
19181918
{
19191919
return __vectorReshape2<T.Element, 4>(sampler.SampleBias(p, bias));
19201920
}
@@ -1950,7 +1950,7 @@ public vector<T.Element,4> texture(_Texture<
19501950
0, // isShadow
19511951
1, // isCombined
19521952
format
1953-
> sampler, vector<float,Shape.dimensions+isArray> p, constexpr float bias)
1953+
> sampler, vector<float,Shape.dimensions+isArray> p, float bias)
19541954
{
19551955
return __vectorReshape2<T.Element,4>(sampler.SampleBias(p, bias));
19561956
}
@@ -3891,7 +3891,7 @@ public vector<T.Element,4> textureGatherOffset(_Texture<
38913891
0, // isShadow
38923892
1, // isCombined
38933893
format
3894-
> sampler, vector<float,2+isArray> p, constexpr vector<int,2> offset, int comp = 0)
3894+
> sampler, vector<float,2+isArray> p, vector<int,2> offset, int comp = 0)
38953895
{
38963896
switch (comp)
38973897
{
@@ -3915,11 +3915,12 @@ public vec4 textureGatherOffset(_Texture<
39153915
1, // isShadow
39163916
1, // isCombined
39173917
format
3918-
> sampler, vector<float,2+isArray> p, float refZ, constexpr vector<int,2> offset)
3918+
> sampler, vector<float,2+isArray> p, float refZ, vector<int,2> offset)
39193919
{
39203920
return sampler.GatherCmp(p, refZ, offset);
39213921
}
39223922

3923+
39233924
// -------------------
39243925
// textureGatherOffsets
39253926
// -------------------

‎source/slang/hlsl.meta.slang‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1064,7 +1064,7 @@ extension _Texture<T,Shape,isArray,isMS,sampleCount,0,isShadow,1,format>
10641064
[ForceInline]
10651065
__glsl_extension(GL_ARB_sparse_texture_clamp)
10661066
[require(cpp_glsl_hlsl_metal_spirv, texture_sm_4_1_clamp_fragment)]
1067-
T Sample(vector<float, Shape.dimensions+isArray> location, vector<int, Shape.planeDimensions> offset, float clamp)
1067+
T Sample(vector<float, Shape.dimensions+isArray> location, constexpr vector<int, Shape.planeDimensions> offset, float clamp)
10681068
{
10691069
__requireComputeDerivative();
10701070
__target_switch
@@ -1091,7 +1091,7 @@ extension _Texture<T,Shape,isArray,isMS,sampleCount,0,isShadow,1,format>
10911091

10921092
[__readNone]
10931093
[ForceInline]
1094-
T Sample(vector<float, Shape.dimensions+isArray> location, vector<int, Shape.planeDimensions> offset, float clamp, out uint status)
1094+
T Sample(vector<float, Shape.dimensions+isArray> location, constexpr vector<int, Shape.planeDimensions> offset, float clamp, out uint status)
10951095
{
10961096
__requireComputeDerivative();
10971097
__target_switch
@@ -3135,7 +3135,7 @@ vector<T.Element,4> __texture_gather_offset(
31353135
_Texture<T, Shape, isArray, 0, sampleCount, access, isShadow, 0, format> texture,
31363136
SamplerState s,
31373137
vector<float, Shape.dimensions+isArray> location,
3138-
constexpr vector<int, Shape.planeDimensions> offset,
3138+
vector<int, Shape.planeDimensions> offset,
31393139
int component)
31403140
{
31413141
__target_switch
@@ -3196,7 +3196,7 @@ __generic<T:ITexelElement, Shape: __ITextureShape, let isArray:int, let sampleCo
31963196
vector<T.Element,4> __texture_gather_offset(
31973197
_Texture<T, Shape, isArray, 0, sampleCount, access, isShadow, 1, format> sampler,
31983198
vector<float, Shape.dimensions+isArray> location,
3199-
constexpr vector<int, Shape.planeDimensions> offset,
3199+
vector<int, Shape.planeDimensions> offset,
32003200
int component)
32013201
{
32023202
__target_switch
@@ -3340,7 +3340,7 @@ vector<T.Element,4> __texture_gatherCmp_offset(
33403340
SamplerComparisonState s,
33413341
vector<float, Shape.dimensions+isArray> location,
33423342
T.Element compareValue,
3343-
constexpr vector<int, Shape.planeDimensions> offset)
3343+
vector<int, Shape.planeDimensions> offset)
33443344
{
33453345
__target_switch
33463346
{
@@ -3386,7 +3386,7 @@ vector<T.Element,4> __texture_gatherCmp_offset(
33863386
_Texture<T, Shape, isArray, 0, sampleCount, access, isShadow, 1, format> sampler,
33873387
vector<float, Shape.dimensions+isArray> location,
33883388
T.Element compareValue,
3389-
constexpr vector<int, Shape.planeDimensions> offset)
3389+
vector<int, Shape.planeDimensions> offset)
33903390
{
33913391
__target_switch
33923392
{
@@ -3532,7 +3532,7 @@ ${{{{
35323532
$(samplerParam)
35333533
vector<float, Shape.dimensions+isArray> location
35343534
$(compareParam),
3535-
constexpr vector<int, Shape.planeDimensions> offset)
3535+
vector<int, Shape.planeDimensions> offset)
35363536
{
35373537
__target_switch
35383538
{
@@ -3558,6 +3558,7 @@ ${{{{
35583558
}
35593559
}
35603560

3561+
35613562
[ForceInline]
35623563
[require(hlsl, texture_gather)]
35633564
vector<T.Element,4> Gather$(compareFunc)$(componentFunc)(
@@ -3628,6 +3629,7 @@ ${{{{
36283629
${{{{
36293630
} // for (isScalarTexture)
36303631
}}}}
3632+
36313633
// End of all Texture Gather
36323634

36333635

‎source/slang/slang-diagnostic-defs.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2442,11 +2442,8 @@ DIAGNOSTIC(
24422442
"\"index\"] attribute to provide a binding location.")
24432443
DIAGNOSTIC(40006, Error, unimplementedSystemValueSemantic, "unknown system-value semantic '$0'")
24442444

2445-
24462445
DIAGNOSTIC(49999, Error, unknownSystemValueSemantic, "unknown system-value semantic '$0'")
24472446

2448-
DIAGNOSTIC(40006, Error, needCompileTimeConstant, "expected a compile-time constant")
2449-
24502447
DIAGNOSTIC(40007, Internal, irValidationFailed, "IR validation failed: $0")
24512448

24522449
DIAGNOSTIC(
@@ -2469,6 +2466,9 @@ DIAGNOSTIC(
24692466
unconstrainedGenericParameterNotAllowedInDynamicFunction,
24702467
"unconstrained generic paramter '$0' is not allowed in a dynamic function.")
24712468

2469+
DIAGNOSTIC(40012, Error, needCompileTimeConstant, "expected a compile-time constant")
2470+
2471+
DIAGNOSTIC(40013, Error, argIsNotConstexpr, "arg $0 in '$1' is not a compile-time constant")
24722472

24732473
DIAGNOSTIC(
24742474
40020,

‎source/slang/slang-ir-constexpr.cpp‎

Lines changed: 32 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
#include "slang-ir-dominators.h"
55
#include "slang-ir-insts.h"
6+
#include "slang-ir-util.h"
67
#include "slang-ir.h"
78

89
namespace Slang
@@ -59,8 +60,12 @@ bool isConstExpr(IRInst* value)
5960
case kIROp_StructKey:
6061
case kIROp_WitnessTable:
6162
case kIROp_Generic:
63+
case kIROp_GlobalConstant:
6264
return true;
63-
65+
case kIROp_Param:
66+
if (isGenericParam(value))
67+
return true;
68+
break;
6469
default:
6570
break;
6671
}
@@ -156,8 +161,10 @@ bool opCanBeConstExprByForwardPass(IRInst* value)
156161
{
157162
// TODO: handle call inst here.
158163

159-
if (value->getOp() == kIROp_Param)
164+
if (value->getOp() == kIROp_Param || value->getOp() == kIROp_Specialize)
165+
{
160166
return false;
167+
}
161168
return opCanBeConstExpr(value->getOp());
162169
}
163170

@@ -393,12 +400,6 @@ bool propagateConstExprBackward(PropagateConstExprContext* context, IRGlobalValu
393400
// the callee for this call statically, and if so try to propagate
394401
// constexpr from the parameters back to the arguments.
395402
auto callInst = (IRCall*)ii;
396-
397-
UInt operandCount = callInst->getOperandCount();
398-
399-
UInt firstCallArg = 1;
400-
UInt callArgCount = operandCount - firstCallArg;
401-
402403
auto callee = callInst->getOperand(0);
403404

404405
// If we are calling a generic operation, then
@@ -423,64 +424,35 @@ bool propagateConstExprBackward(PropagateConstExprContext* context, IRGlobalValu
423424
}
424425

425426
auto calleeFunc = as<IRFunc>(callee);
426-
if (calleeFunc && isDefinition(calleeFunc))
427+
auto calleeType = callee->getDataType();
428+
if (auto caleeFuncType = as<IRFuncType>(calleeType))
427429
{
428-
// We have an IR-level function definition we are calling,
429-
// and thus we can propagate `constexpr` information
430-
// through its `IRParam`s.
431-
432-
auto calleeFuncType = calleeFunc->getDataType();
433-
434-
UInt callParamCount = calleeFuncType->getParamCount();
435-
SLANG_RELEASE_ASSERT(callParamCount == callArgCount);
436-
437-
// If the callee has a definition, then we can read `constexpr`
438-
// information off of the parameters of its first IR block.
439-
if (auto calleeFirstBlock = calleeFunc->getFirstBlock())
430+
UInt operandCount = callInst->getOperandCount();
431+
UInt firstCallArg = 1;
432+
UInt callArgCount = operandCount - firstCallArg;
433+
auto paramCount = caleeFuncType->getParamCount();
434+
SLANG_RELEASE_ASSERT(paramCount == callArgCount);
435+
for (UInt pp = 0; pp < paramCount; ++pp)
440436
{
441-
UInt paramCounter = 0;
442-
for (auto pp = calleeFirstBlock->getFirstParam(); pp;
443-
pp = pp->getNextParam())
437+
auto paramType = caleeFuncType->getParamType(pp);
438+
if (isConstExpr(paramType))
444439
{
445-
UInt paramIndex = paramCounter++;
446-
447-
auto param = pp;
448-
auto arg = callInst->getOperand(firstCallArg + paramIndex);
449-
450-
if (isConstExpr(param))
440+
auto arg = callInst->getOperand(firstCallArg + pp);
441+
if (maybeMarkConstExprBackwardPass(context, arg))
451442
{
452-
if (maybeMarkConstExprBackwardPass(context, arg))
453-
{
454-
changedThisIteration = true;
455-
}
443+
changedThisIteration = true;
456444
}
457-
}
458-
}
459-
}
460-
else
461-
{
462-
// If we don't have a concrete callee function
463-
// definition, then we need to extract the
464-
// type of the callee instruction, and try to work
465-
// with that.
466-
//
467-
// Note that this does not allow us to propagate
468-
// `constexpr` information from the body of a callee
469-
// back to call sites.
470-
auto calleeType = callee->getDataType();
471-
if (auto caleeFuncType = as<IRFuncType>(calleeType))
472-
{
473-
auto paramCount = caleeFuncType->getParamCount();
474-
for (UInt pp = 0; pp < paramCount; ++pp)
475-
{
476-
auto paramType = caleeFuncType->getParamType(pp);
477-
auto arg = callInst->getOperand(firstCallArg + pp);
478-
if (isConstExpr(paramType))
445+
// If arg is not constexpr after this, meaning it can't be
446+
// marked constexpr for some reason, but the param requires
447+
// that. This is not expected.
448+
if (!isConstExpr(arg))
479449
{
480-
if (maybeMarkConstExprBackwardPass(context, arg))
481-
{
482-
changedThisIteration = true;
483-
}
450+
context->getSink()->diagnose(
451+
callInst->sourceLoc,
452+
Diagnostics::argIsNotConstexpr,
453+
pp + 1,
454+
calleeFunc);
455+
return false;
484456
}
485457
}
486458
}

‎tests/diagnostics/constexpr-error.slang‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ float4 main() : SV_Target
2424
result += t.Sample(s, uv, int2(0,0));
2525

2626
// Error: data passed through cbuffer isn't compile-time constant
27-
// CHECK: ([[# @LINE+1]]): error 40006:
27+
// CHECK: ([[# @LINE+1]]): error 40012:
2828
result += t.Sample(s, uv, offset);
2929

3030
// Error: data computed via conditional isn't compile-time cosntant
@@ -33,16 +33,19 @@ float4 main() : SV_Target
3333
{
3434
ii = 1;
3535
}
36-
// CHECK: ([[# @LINE+1]]): error 40006:
36+
// CHECK: ([[# @LINE+1]]): error 40012:
3737
result += t.Sample(s, uv, int2(ii));
3838

3939
// Error: data computed in loop isn't compile-time constant
4040
// (and loop isn't unroll-able)
41-
// CHECK: ([[# @LINE+1]]): error 40006:
41+
// CHECK: ([[# @LINE+1]]): error 40012:
4242
for(uint jj = 0; jj < uv.y; jj++)
4343
{
4444
result += t.Sample(s, uv, int2(jj));
4545
}
4646

47+
48+
49+
4750
return result;
4851
}

‎tests/diagnostics/constexpr-param.slang‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
//DIAGNOSTIC_TEST:SIMPLE(filecheck=CHECK):
2+
//DIAGNOSTIC_TEST:SIMPLE(filecheck=CHECK1):
3+
//DIAGNOSTIC_TEST:SIMPLE(filecheck=CHECK2):
4+
5+
// Failure to pass compile-time-constant argument
6+
// where it is expected.
7+
//
8+
// Test both callee and nested callee that require constexpr parameters.
9+
// Test both function defined in the same module and external function.
10+
11+
Texture2D<int> input;
12+
RWTexture2D<int> output;
13+
14+
int func_with_constexpr_params(constexpr int const_a)
15+
{
16+
return const_a + const_a;
17+
}
18+
19+
int func_internal(int a)
20+
{
21+
// Error: arg passed through isn't compile-time constant
22+
// CHECK: ([[# @LINE+1]]): error 40013:
23+
return func_with_constexpr_params(a);
24+
}
25+
26+
int func_external(int a)
27+
{
28+
// CHECK1: ([[# @LINE+1]]): error 40012:
29+
int2 offset = int2(a, a);
30+
int3 location = int3(0, 0, 0);
31+
// Load requires constexpr offset
32+
return input.Load(location, offset);
33+
}
34+
35+
[shader("compute")]
36+
void Test(uint3 thread_index : SV_DispatchThreadID)
37+
{
38+
int result = 0;
39+
40+
// Okay, immediate constant
41+
result += func_with_constexpr_params(1);
42+
43+
44+
int ii = 0;
45+
if (thread_index.x > 0)
46+
{
47+
ii = 1;
48+
}
49+
// Pass non-compile-time-constant argument to function that requires constexpr parameter
50+
// CHECK2: ([[# @LINE+1]]): error 40013:
51+
result += func_with_constexpr_params(ii);
52+
53+
// The nested function calls should issue compile errors
54+
result += func_internal(ii);
55+
result += func_external(ii);
56+
}

0 commit comments

Comments
 (0)