Skip to content

Commit b49e543

Browse files
csyonghecsyonghe
committed
Add diagnostic to prevent defining unsized static variables.
1 parent 291b4cd commit b49e543

6 files changed

+146
-64
lines changed

‎source/slang/slang-check-decl.cpp

+23-4
Original file line numberDiff line numberDiff line change
@@ -2040,11 +2040,20 @@ namespace Slang
20402040
}
20412041
}
20422042

2043+
std::optional<TypeTag> varTypeTags;
2044+
auto getVarTypeTags = [&]()
2045+
{
2046+
if (!varTypeTags.has_value())
2047+
{
2048+
varTypeTags = getTypeTags(varDecl->getType());
2049+
}
2050+
return varTypeTags.value();
2051+
};
20432052
if (auto parentDecl = as<AggTypeDecl>(getParentDecl(varDecl)))
20442053
{
2045-
auto typeTags = getTypeTags(varDecl->getType());
2046-
parentDecl->addTag(typeTags);
2047-
if ((int)typeTags & (int)TypeTag::Unsized)
2054+
parentDecl->addTag(getVarTypeTags());
2055+
bool isUnsized = (((int)getVarTypeTags() & (int)TypeTag::Unsized) != 0);
2056+
if (isUnsized)
20482057
{
20492058
// Unsized decl must appear as the last member of the struct.
20502059
for (auto memberIdx = parentDecl->members.getCount() - 1; memberIdx >= 0; memberIdx--)
@@ -2064,7 +2073,17 @@ namespace Slang
20642073
}
20652074
}
20662075
}
2067-
2076+
bool isGlobalVar = (isGlobalDecl(varDecl) && !isGlobalShaderParameter(varDecl))
2077+
|| (!as<ParamDecl>(varDecl) && varDecl->hasModifier<HLSLStaticModifier>());
2078+
if (isGlobalVar)
2079+
{
2080+
bool isUnsized = (((int)getVarTypeTags() & (int)TypeTag::Unsized) != 0);
2081+
if (isUnsized)
2082+
{
2083+
getSink()->diagnose(varDecl, Diagnostics::globalVarCannotBeUnsized);
2084+
}
2085+
}
2086+
20682087
if (auto elementType = getConstantBufferElementType(varDecl->getType()))
20692088
{
20702089
if (doesTypeHaveTag(elementType, TypeTag::Incomplete))

‎source/slang/slang-check-overload.cpp

+8-2
Original file line numberDiff line numberDiff line change
@@ -475,8 +475,14 @@ namespace Slang
475475
break;
476476

477477
case OverloadCandidate::Flavor::Generic:
478-
return TryCheckGenericOverloadCandidateTypes(context, candidate);
479-
478+
{
479+
bool succ = TryCheckGenericOverloadCandidateTypes(context, candidate);
480+
if (!succ && context.mode != OverloadResolveContext::Mode::JustTrying)
481+
{
482+
getSink()->diagnose(context.loc, Diagnostics::cannotSpecializeGeneric, candidate.item.declRef);
483+
}
484+
return succ;
485+
}
480486
default:
481487
SLANG_UNEXPECTED("unknown flavor of overload candidate");
482488
break;

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

+3
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,9 @@ DIAGNOSTIC(30067, Error, mutatingMethodOnFunctionInputParameterError, "mutating
326326
DIAGNOSTIC(30068, Warning, mutatingMethodOnFunctionInputParameterWarning, "mutating method '$0' called on `in` parameter '$1'; changes will not be visible to caller. copy the parameter into a local variable if this behavior is intended")
327327

328328
DIAGNOSTIC(30070, Error, unsizedMemberMustAppearLast, "unsized member can only appear as the last member in a composite type.")
329+
DIAGNOSTIC(30071, Error, globalVarCannotBeUnsized, "global or static variable cannot not be unsized.")
330+
331+
DIAGNOSTIC(30075, Error, cannotSpecializeGeneric, "cannot specialize generic '$0' with the provided arguments.")
329332

330333
DIAGNOSTIC(30100, Error, staticRefToNonStaticMember, "type '$0' cannot be used to refer to non-static member '$1'")
331334
DIAGNOSTIC(30101, Error, cannotDereferenceType, "cannot dereference type '$0', do you mean to use '.'?")

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

+75-58
Original file line numberDiff line numberDiff line change
@@ -7768,6 +7768,33 @@ struct DeclLoweringVisitor : DeclVisitor<DeclLoweringVisitor, LoweredValInfo>
77687768
return false;
77697769
}
77707770

7771+
struct NestedContext
7772+
{
7773+
IRGenEnv subEnvStorage;
7774+
IRBuilder subBuilderStorage;
7775+
IRGenContext subContextStorage;
7776+
7777+
NestedContext(DeclLoweringVisitor* outer)
7778+
: subBuilderStorage(*outer->getBuilder())
7779+
, subContextStorage(*outer->context)
7780+
{
7781+
auto outerContext = outer->context;
7782+
7783+
subEnvStorage.outer = outerContext->env;
7784+
7785+
subContextStorage.irBuilder = &subBuilderStorage;
7786+
subContextStorage.env = &subEnvStorage;
7787+
7788+
subContextStorage.thisType = outerContext->thisType;
7789+
subContextStorage.thisTypeWitness = outerContext->thisTypeWitness;
7790+
7791+
subContextStorage.returnDestination = LoweredValInfo();
7792+
}
7793+
7794+
IRBuilder* getBuilder() { return &subBuilderStorage; }
7795+
IRGenContext* getContext() { return &subContextStorage; }
7796+
};
7797+
77717798
LoweredValInfo lowerGlobalShaderParam(VarDecl* decl)
77727799
{
77737800
IRType* paramType = lowerType(context, decl->getType());
@@ -7924,86 +7951,66 @@ struct DeclLoweringVisitor : DeclVisitor<DeclLoweringVisitor, LoweredValInfo>
79247951
return lowerGlobalConstantDecl(decl);
79257952
}
79267953

7927-
IRType* varType = lowerType(context, decl->getType());
7954+
NestedContext nested(this);
7955+
auto subBuilder = nested.getBuilder();
7956+
auto subContext = nested.getContext();
79287957

7929-
auto builder = getBuilder();
7958+
IRGeneric* outerGeneric = nullptr;
7959+
7960+
// If we are static, then we need to insert the declaration before the parent.
7961+
// This tries to match the behavior of previous `lowerFunctionStaticConstVarDecl` functionality
7962+
if (isFunctionStaticVarDecl(decl))
7963+
{
7964+
// We need to insert the constant at a level above
7965+
// the function being emitted. This will usually
7966+
// be the global scope, but it might be an outer
7967+
// generic if we are lowering a generic function.
7968+
subBuilder->setInsertBefore(subBuilder->getFunc());
7969+
}
7970+
else if (!isFunctionVarDecl(decl))
7971+
{
7972+
outerGeneric = emitOuterGenerics(subContext, decl, decl);
7973+
}
7974+
7975+
IRType* varType = lowerType(subContext, decl->getType());
79307976

79317977
// TODO(JS): Do we create something derived from IRGlobalVar? Or do we use
79327978
// a decoration to identify an *actual* global?
79337979

7934-
IRGlobalValueWithCode* irGlobal = builder->createGlobalVar(varType);
7935-
LoweredValInfo globalVal = LoweredValInfo::ptr(irGlobal);
7980+
IRGlobalValueWithCode* irGlobal = subBuilder->createGlobalVar(varType);
79367981

7937-
addLinkageDecoration(context, irGlobal, decl);
7938-
addNameHint(context, irGlobal, decl);
7982+
addLinkageDecoration(subContext, irGlobal, decl);
7983+
addNameHint(subContext, irGlobal, decl);
79397984

7940-
maybeSetRate(context, irGlobal, decl);
7985+
maybeSetRate(subContext, irGlobal, decl);
79417986

7942-
addVarDecorations(context, irGlobal, decl);
7943-
maybeAddDebugLocationDecoration(context, irGlobal);
7987+
addVarDecorations(subContext, irGlobal, decl);
7988+
maybeAddDebugLocationDecoration(subContext, irGlobal);
79447989

79457990
if (decl)
79467991
{
7947-
builder->addHighLevelDeclDecoration(irGlobal, decl);
7992+
subBuilder->addHighLevelDeclDecoration(irGlobal, decl);
79487993
}
79497994

7950-
// A global variable's SSA value is a *pointer* to
7951-
// the underlying storage.
7952-
context->setGlobalValue(decl, globalVal);
7953-
79547995
if( auto initExpr = decl->initExpr )
79557996
{
7956-
IRBuilder subBuilderStorage = *getBuilder();
7957-
IRBuilder* subBuilder = &subBuilderStorage;
7958-
79597997
subBuilder->setInsertInto(irGlobal);
79607998

7961-
IRGenContext subContextStorage = *context;
7962-
IRGenContext* subContext = &subContextStorage;
7963-
7964-
subContext->irBuilder = subBuilder;
7965-
7966-
// TODO: set up a parent IR decl to put the instructions into
7967-
79687999
IRBlock* entryBlock = subBuilder->emitBlock();
79698000
subBuilder->setInsertInto(entryBlock);
79708001

79718002
LoweredValInfo initVal = lowerLValueExpr(subContext, initExpr);
79728003
subContext->irBuilder->emitReturn(getSimpleVal(subContext, initVal));
79738004
}
79748005

7975-
irGlobal->moveToEnd();
8006+
// A global variable's SSA value is a *pointer* to
8007+
// the underlying storage.
8008+
auto loweredValue = LoweredValInfo::ptr(finishOuterGenerics(subBuilder, irGlobal, outerGeneric));
8009+
context->setGlobalValue(decl, loweredValue);
79768010

7977-
return globalVal;
8011+
return loweredValue;
79788012
}
79798013

7980-
struct NestedContext
7981-
{
7982-
IRGenEnv subEnvStorage;
7983-
IRBuilder subBuilderStorage;
7984-
IRGenContext subContextStorage;
7985-
7986-
NestedContext(DeclLoweringVisitor* outer)
7987-
: subBuilderStorage(*outer->getBuilder())
7988-
, subContextStorage(*outer->context)
7989-
{
7990-
auto outerContext = outer->context;
7991-
7992-
subEnvStorage.outer = outerContext->env;
7993-
7994-
subContextStorage.irBuilder = &subBuilderStorage;
7995-
subContextStorage.env = &subEnvStorage;
7996-
7997-
subContextStorage.thisType = outerContext->thisType;
7998-
subContextStorage.thisTypeWitness = outerContext->thisTypeWitness;
7999-
8000-
subContextStorage.returnDestination = LoweredValInfo();
8001-
}
8002-
8003-
IRBuilder* getBuilder() { return &subBuilderStorage; }
8004-
IRGenContext* getContext() { return &subContextStorage; }
8005-
};
8006-
80078014
LoweredValInfo lowerFunctionStaticConstVarDecl(
80088015
VarDeclBase* decl)
80098016
{
@@ -10221,10 +10228,10 @@ bool canDeclLowerToAGeneric(Decl* decl)
1022110228
// a generic that returns a type (a simple type-level function).
1022210229
if(as<TypeDefDecl>(decl)) return true;
1022310230

10224-
// If we have a variable declaration that is *static* and *const* we can lower to a generic
10231+
// A static member variable declaration can be lowered into a generic.
1022510232
if (auto varDecl = as<VarDecl>(decl))
1022610233
{
10227-
if (varDecl->hasModifier<HLSLStaticModifier>() && varDecl->hasModifier<ConstModifier>())
10234+
if (varDecl->hasModifier<HLSLStaticModifier>())
1022810235
{
1022910236
return !isFunctionVarDecl(varDecl);
1023010237
}
@@ -10346,7 +10353,9 @@ LoweredValInfo emitDeclRef(
1034610353
// We can only really specialize things that map to single values.
1034710354
// It would be an error if we got a non-`None` value that
1034810355
// wasn't somehow a single value.
10349-
auto irGenericVal = getSimpleVal(context, genericVal);
10356+
genericVal = materialize(context, genericVal);
10357+
auto irGenericVal = genericVal.val;
10358+
SLANG_ASSERT(irGenericVal);
1035010359

1035110360
// We have the IR value for the generic we'd like to specialize,
1035210361
// and now we need to get the value for the arguments.
@@ -10378,8 +10387,16 @@ LoweredValInfo emitDeclRef(
1037810387
irGenericVal,
1037910388
irArgs.getCount(),
1038010389
irArgs.getBuffer());
10381-
10382-
return LoweredValInfo::simple(irSpecializedVal);
10390+
switch (genericVal.flavor)
10391+
{
10392+
case LoweredValInfo::Flavor::Simple:
10393+
return LoweredValInfo::simple(irSpecializedVal);
10394+
case LoweredValInfo::Flavor::Ptr:
10395+
return LoweredValInfo::ptr(irSpecializedVal);
10396+
default:
10397+
SLANG_UNEXPECTED("unhandled lowered value flavor");
10398+
UNREACHABLE_RETURN(LoweredValInfo());
10399+
}
1038310400
}
1038410401
else if(auto thisTypeSubst = as<LookupDeclRef>(subst))
1038510402
{

‎source/slang/slang-syntax.cpp

+2
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ void printDiagnosticArg(StringBuilder& sb, Decl* decl)
2525

2626
void printDiagnosticArg(StringBuilder& sb, DeclRefBase* declRefBase)
2727
{
28+
if (!declRefBase)
29+
return;
2830
printDiagnosticArg(sb, declRefBase->getDecl());
2931
}
3032

‎tests/bugs/gh-4150.slang

+35
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
//TEST:SIMPLE(filecheck=CHECK1):-target spirv -DERROR1
2+
//TEST:SIMPLE(filecheck=CHECK2):-target spirv -DERROR2
3+
4+
// CHECK1: error 30075
5+
// CHECK2: error 30071
6+
7+
struct RWTex<T : __BuiltinRealType, let N : uint>
8+
{
9+
#ifdef ERROR2
10+
// expect error: cannot define static member with unsized type.
11+
static [[vk::binding(0, 0)]] vector<T,N> rwtable[];
12+
#else
13+
static [[vk::binding(0, 0)]] vector<T,N> rwtable[4];
14+
#endif
15+
static vector<T, N> get(uint image_index) { return rwtable[image_index]; }
16+
}
17+
18+
struct Push
19+
{
20+
uint image_id;
21+
};
22+
[[vk::push_constant]] Push p;
23+
RWStructuredBuffer<float3> output;
24+
[shader("compute")]
25+
[numthreads(8, 8, 1)]
26+
void main(uint3 pixel_i : SV_DispatchThreadID)
27+
{
28+
output[0] =
29+
#ifdef ERROR1
30+
// expect error: trying to specialize RWTex, which has two arguments, with only one argument.
31+
RWTex<float3>::get(p.image_id);
32+
#else
33+
RWTex<float,3>::get(p.image_id);
34+
#endif
35+
}

0 commit comments

Comments
 (0)