Skip to content

Conversation

@aidanfnv
Copy link
Contributor

@aidanfnv aidanfnv commented Nov 8, 2025

Fixes #8894

Adds handling of TypeCastIntVal in GetElementCount, recursively calling GetElementCount on its base IntVal, allowing a SpecializationConstant to be used as an array size.

Also adds a check for specialization constants in GetElementCount so that if a DeclRefIntVal has a VarDeclBase with the SpecializationConstantAttribute, GetElementCount will return LayoutSize::infinite() to ensure that the array gets a binding but does not get a size at compile-time. In this case the DeclRefIntVal is the base IntVal of the TypeCastIntVal that we recursively check.

@aidanfnv aidanfnv requested a review from kaizhangNV November 8, 2025 01:42
@aidanfnv aidanfnv requested a review from a team as a code owner November 8, 2025 01:42
@aidanfnv aidanfnv added the pr: non-breaking PRs without breaking changes label Nov 8, 2025
kaizhangNV
kaizhangNV previously approved these changes Nov 8, 2025
Copy link
Contributor

@kaizhangNV kaizhangNV left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@aidanfnv
Copy link
Contributor Author

The recursive call to GetElementCount returns 0 for SpecializationConstants, which means that SpecializationConstant-sized arrays do not get binding layouts, which leads to the reproducer shader (and the test based on it) to fail SPIR-V validation due to the missing binding.
Since the specialization constant's value can be changed at runtime (the value set in the shader, e.g. MAX_SAMPLES = 32, is just a default), we cannot ensure the array's size at compile time, so we must treat it as unbounded at that time.

I have added a check for specialization constants in GetElementCount so that if a DeclRefIntVal has a VarDeclBase with the SpecializationConstantAttribute, GetElementCount will return LayoutSize::infinite() to ensure that the array gets a binding but does not get a size at compile-time. In this case the DeclRefIntVal is the base IntVal of the TypeCastIntVal that we recursively check.

@aidanfnv aidanfnv requested a review from kaizhangNV November 10, 2025 23:31
// emission, then evaluated to concrete bytes at pipeline-creation time
//
// This differs from generic parameters, which are resolved at Slang compile-time
return LayoutSize::infinite();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is unnecessarily true, if downstream doesn't provide the value via pipeline, it will just use the default value. So I don't think this change is correct.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to differentiate it from generic parameter?

Both of them cannot be determined at front-end, so I don't know why there is difference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr: non-breaking PRs without breaking changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Compiler crash when using [SpecializationConstant] as an array size

2 participants