Allows defined constants as attribute arguments. #7439
Conversation
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
HLSL does not allow const vars to be evaluated in integer constant expressions - this change relaxes this restriction in the case of attribute integer parameter values. Specifically, a const var with integer constant expression initializer is accepted, and the value of the initializer is used as the value of the attribute parameter. This (expermental) behavior may be useful in cases where Vulkan specialization constants are used as an attribute parameter value.
danbrown-amd
force-pushed
the
attr-arg-consts
branch
2 times, most recently
from
a12ce71 to
9db3b6c
Compare
danbrown-amd
force-pushed
the
attr-arg-consts
branch
from
9db3b6c to
243acbc
Compare
| bool Expr::isVulkanSpecConstantExpr(const ASTContext &Ctx, | ||
| APValue *Result) const { | ||
| if (auto *D = dyn_cast<DeclRefExpr>(this)) { | ||
| if (auto *V = dyn_cast<VarDecl>(D->getDecl())) { | ||
| if (V->hasAttr<VKConstantIdAttr>()) { | ||
| if (const Expr *I = V->getAnyInitializer()) { | ||
| if (!I->isCXX11ConstantExpr(Ctx, Result)) | ||
| return false; | ||
| } | ||
| return true; | ||
| } | ||
| } | ||
| } | ||
| return false; | ||
| } | ||
|
|
There was a problem hiding this comment.
I might be clearer to require an individual if-statements to reduce the nesting.
auto *D = dyn_cast<DeclRefExpr>(this)
if (!D) return false;
auto *V = dyn_cast<VarDecl>(D->getDecl())
if (!V || !V->hasAttr<VkConstantIdAttr>()) return false;
...
| bool SpirvEmitter::processNumThreadsAttr(const FunctionDecl *decl) { | ||
| auto *numThreadsAttr = decl->getAttr<HLSLNumThreadsAttr>(); | ||
| if (!numThreadsAttr) | ||
| return false; | ||
|
|
||
| auto f = [](ASTContext &Ctx, Expr *E) { | ||
| if (E) { | ||
| llvm::APSInt apsInt; | ||
| APValue apValue; | ||
| if (E->isIntegerConstantExpr(apsInt, Ctx)) | ||
| return (uint32_t)apsInt.getSExtValue(); | ||
| if (E->isVulkanSpecConstantExpr(Ctx, &apValue) && apValue.isInt()) | ||
| return (uint32_t)apValue.getInt().getSExtValue(); | ||
| } | ||
| return 1U; | ||
| }; | ||
|
|
||
| spvBuilder.addExecutionMode(entryFunction, spv::ExecutionMode::LocalSize, | ||
| {f(astContext, numThreadsAttr->getX()), | ||
| f(astContext, numThreadsAttr->getY()), | ||
| f(astContext, numThreadsAttr->getZ())}, | ||
| decl->getLocation()); | ||
| return true; | ||
| } |
There was a problem hiding this comment.
If you have a spec constant, you need to use the LocalSizeId execution mode, with the id of the spec constant right?
We could implement this by always using LocalSizeId, and make the parameters the id of the expression in the attribute.
There was a problem hiding this comment.
I don't mind this change generally. It will be good for SPIR-V.
I don't know if this is complete yet (it is still a draft). I pointed out one issue with using LocalSize. You might also need to look at SpirvEmitter::addDerivativeGroupExecutionMode(). That function determines which derivative group execution mode to add based on the number of threas in the group. However, if you have a spec constant for the number of threads, we cannot do that.
No description provided.