[SPIR-V] Add descriptor heap -fvk-resource-heap-stride / -fvk-sampler-heap-stride CLI flags#8519
Open
jzakharovnv wants to merge 7 commits into
Open
[SPIR-V] Add descriptor heap -fvk-resource-heap-stride / -fvk-sampler-heap-stride CLI flags#8519jzakharovnv wants to merge 7 commits into
jzakharovnv wants to merge 7 commits into
Conversation
Contributor
You can test this locally with the following command:git-clang-format --diff 02e491eb019766b866bcf09c427365713353841b 9c5badc64f7c13c517694a59a9aa068d7d198ac0 -- include/dxc/Support/SPIRVOptions.h lib/DxcSupport/HLSLOptions.cpp tools/clang/include/clang/SPIRV/AstTypeProbe.h tools/clang/include/clang/SPIRV/SpirvBuilder.h tools/clang/include/clang/SPIRV/SpirvContext.h tools/clang/include/clang/SPIRV/SpirvInstruction.h tools/clang/include/clang/SPIRV/SpirvType.h tools/clang/include/clang/SPIRV/SpirvVisitor.h tools/clang/lib/SPIRV/AstTypeProbe.cpp tools/clang/lib/SPIRV/CapabilityVisitor.cpp tools/clang/lib/SPIRV/DeclResultIdMapper.cpp tools/clang/lib/SPIRV/DeclResultIdMapper.h tools/clang/lib/SPIRV/EmitVisitor.cpp tools/clang/lib/SPIRV/EmitVisitor.h tools/clang/lib/SPIRV/LowerTypeVisitor.cpp tools/clang/lib/SPIRV/SpirvBuilder.cpp tools/clang/lib/SPIRV/SpirvContext.cpp tools/clang/lib/SPIRV/SpirvEmitter.cpp tools/clang/lib/SPIRV/SpirvEmitter.h tools/clang/lib/SPIRV/SpirvInstruction.cpp tools/clang/lib/SPIRV/SpirvType.cpp tools/clang/unittests/SPIRV/SpirvContextTest.cppView the diff from clang-format here.diff --git a/lib/DxcSupport/HLSLOptions.cpp b/lib/DxcSupport/HLSLOptions.cpp
index 3ef3e334..72b17886 100644
--- a/lib/DxcSupport/HLSLOptions.cpp
+++ b/lib/DxcSupport/HLSLOptions.cpp
@@ -384,7 +384,8 @@ static bool handleHeapStride(const InputArgList &args, OptSpecifier id,
}
// Power of 2 in [8, 256] inclusive.
if (number < 8 || number > 256 || (number & (number - 1)) != 0) {
- errors << name << " must be a power of 2 between 8 and 256 (inclusive); got "
+ errors << name
+ << " must be a power of 2 between 8 and 256 (inclusive); got "
<< value;
return false;
}
diff --git a/tools/clang/lib/SPIRV/SpirvBuilder.cpp b/tools/clang/lib/SPIRV/SpirvBuilder.cpp
index a146a5dd..9fce2c57 100644
--- a/tools/clang/lib/SPIRV/SpirvBuilder.cpp
+++ b/tools/clang/lib/SPIRV/SpirvBuilder.cpp
@@ -2012,7 +2012,8 @@ SpirvConstant *SpirvBuilder::getConstantNull(QualType type) {
return nullConst;
}
-SpirvConstant *SpirvBuilder::getConstantSizeOfEXT(const SpirvType *operandType) {
+SpirvConstant *
+SpirvBuilder::getConstantSizeOfEXT(const SpirvType *operandType) {
// Reuse the existing instruction for a given descriptor type; multiple heap
// accesses of the same element type share one OpConstantSizeOfEXT.
auto found = constantSizeOfEXTMap.find(operandType);
diff --git a/tools/clang/lib/SPIRV/SpirvEmitter.cpp b/tools/clang/lib/SPIRV/SpirvEmitter.cpp
index c61a572a..4d023a88 100644
--- a/tools/clang/lib/SPIRV/SpirvEmitter.cpp
+++ b/tools/clang/lib/SPIRV/SpirvEmitter.cpp
@@ -5106,20 +5106,20 @@ SpirvEmitter::processStructuredBufferLoad(const CXXMemberCallExpr *expr) {
auto *zero = spvBuilder.getConstantInt(astContext.IntTy, llvm::APInt(32, 0));
auto *index = doExpr(expr->getArg(0));
- auto *result = derefOrCreatePointerToValue(buffer->getType(), info, structType,
- {zero, index}, buffer->getExprLoc(),
- range);
+ auto *result =
+ derefOrCreatePointerToValue(buffer->getType(), info, structType,
+ {zero, index}, buffer->getExprLoc(), range);
- // derefOrCreatePointerToValue returns an lvalue (AccessChain) when the base is
- // an lvalue. This covers descriptor-heap buffers reached either directly
+ // derefOrCreatePointerToValue returns an lvalue (AccessChain) when the base
+ // is an lvalue. This covers descriptor-heap buffers reached either directly
// (ResourceDescriptorHeap[i].Load()) or through a local alias var.
// StructuredBuffer::Load semantically returns a value, and the AST emits no
// LValueToRValue cast for the call expression, so emit the load explicitly.
// (Verified required: scoping this to alias vars only regresses the direct
// heap-access tests; non-heap callers are unaffected in the existing suite.)
if (result && !result->isRValue())
- result = spvBuilder.createLoad(structType, result, buffer->getExprLoc(),
- range);
+ result =
+ spvBuilder.createLoad(structType, result, buffer->getExprLoc(), range);
return result;
}
@@ -5171,8 +5171,8 @@ bool SpirvEmitter::tryToAssignDescriptorHeapImageAlias(
return false;
const auto *dstVar = dyn_cast<VarDecl>(dstDecl);
- if (!dstVar || (!isRWTexture(dstVar->getType()) &&
- !isRWBuffer(dstVar->getType())))
+ if (!dstVar ||
+ (!isRWTexture(dstVar->getType()) && !isRWBuffer(dstVar->getType())))
return false;
const auto *src = srcExpr->IgnoreParenCasts();
@@ -5270,7 +5270,7 @@ SpirvInstruction *SpirvEmitter::emitDescriptorHeapImageTexelPointer(
// Descriptor-heap buffers: ConstantBuffer is a UBO (Uniform); every other
// buffer resource (Structured/RW/ByteAddress, TextureBuffer) is an SSBO
-// (StorageBuffer). Here because the opaque OpTypeBufferEXT descriptor
+// (StorageBuffer). Here because the opaque OpTypeBufferEXT descriptor
// carries no pointee interface type, so RemoveBufferBlockVisitor
// cannot infer/correct its storage class post-lowering.
static spv::StorageClass
@@ -5292,8 +5292,9 @@ SpirvInstruction *SpirvEmitter::emitDescriptorHeapBufferAccess(
const SpirvPointerType *bufferDataPointerType = nullptr;
SpirvLayoutRule layoutRule = spirvOptions.sBufferLayoutRule;
if (isConstantTextureBuffer(resourceType)) {
- layoutRule = isConstantBuffer(resourceType) ? spirvOptions.cBufferLayoutRule
- : spirvOptions.tBufferLayoutRule;
+ layoutRule = isConstantBuffer(resourceType)
+ ? spirvOptions.cBufferLayoutRule
+ : spirvOptions.tBufferLayoutRule;
bufferDataPointerType = spvContext.getPointerType(
bufferDataType, getDescriptorHeapBufferStorageClass(resourceType));
} else {
@@ -5313,13 +5314,15 @@ SpirvInstruction *SpirvEmitter::emitDescriptorHeapBufferAccess(
const spv::StorageClass bufferExtSC = isConstantBuffer(resourceType)
? spv::StorageClass::Uniform
: spv::StorageClass::StorageBuffer;
- const SpirvType *bufferDescriptorType = spvContext.getBufferEXTType(bufferExtSC);
+ const SpirvType *bufferDescriptorType =
+ spvContext.getBufferEXTType(bufferExtSC);
const SpirvType *arrayType =
getDescriptorHeapRuntimeArrayType(bufferDescriptorType,
/*onSamplerHeap=*/false);
- SpirvInstruction *untypedAccessChainPtr = spvBuilder.createUntypedAccessChainKHR(
- untypedUniformConstantType, arrayType, heapVar, index,
- baseExpr->getExprLoc());
+ SpirvInstruction *untypedAccessChainPtr =
+ spvBuilder.createUntypedAccessChainKHR(untypedUniformConstantType,
+ arrayType, heapVar, index,
+ baseExpr->getExprLoc());
SpirvInstruction *bufferDataPtr = spvBuilder.createUnaryOp(
spv::Op::OpBufferPointerEXT, bufferDataPointerType, untypedAccessChainPtr,
baseExpr->getExprLoc());
@@ -5332,9 +5335,9 @@ SpirvInstruction *SpirvEmitter::emitDescriptorHeapBufferAccess(
declIdMapper.getInterlockExecutionMode(), {},
baseExpr->getExprLoc());
}
- descriptorHeapBufferAccesses[expr] = {bufferDataPointerType, arrayType,
- heapVar, index,
- indexExpr->getType(), layoutRule};
+ descriptorHeapBufferAccesses[expr] = {
+ bufferDataPointerType, arrayType, heapVar, index,
+ indexExpr->getType(), layoutRule};
return bufferDataPtr;
}
@@ -5370,7 +5373,8 @@ SpirvEmitter::incDecRWACSBufferCounter(const CXXMemberCallExpr *expr,
(isAppendStructuredBuffer(object->getType()) ||
isConsumeStructuredBuffer(object->getType()))) {
emitError("append/consume structured buffers are not supported with "
- "SPV_EXT_descriptor_heap", expr->getCallee()->getExprLoc());
+ "SPV_EXT_descriptor_heap",
+ expr->getCallee()->getExprLoc());
return nullptr;
}
@@ -6024,18 +6028,18 @@ SpirvEmitter::processIntrinsicMemberCall(const CXXMemberCallExpr *expr,
retVal = processTextureLevelOfDetail(expr, /* unclamped */ true);
break;
case IntrinsicOp::MOP_IncrementCounter:
- if (SpirvInstruction *counter = incDecRWACSBufferCounter(expr, /*isInc*/ true))
- retVal = spvBuilder.createUnaryOp(spv::Op::OpBitcast,
- astContext.UnsignedIntTy, counter,
- expr->getCallee()->getExprLoc(),
- expr->getCallee()->getSourceRange());
+ if (SpirvInstruction *counter =
+ incDecRWACSBufferCounter(expr, /*isInc*/ true))
+ retVal = spvBuilder.createUnaryOp(
+ spv::Op::OpBitcast, astContext.UnsignedIntTy, counter,
+ expr->getCallee()->getExprLoc(), expr->getCallee()->getSourceRange());
break;
case IntrinsicOp::MOP_DecrementCounter:
- if (SpirvInstruction *counter = incDecRWACSBufferCounter(expr, /*isInc*/ false))
- retVal = spvBuilder.createUnaryOp(spv::Op::OpBitcast,
- astContext.UnsignedIntTy, counter,
- expr->getCallee()->getExprLoc(),
- expr->getCallee()->getSourceRange());
+ if (SpirvInstruction *counter =
+ incDecRWACSBufferCounter(expr, /*isInc*/ false))
+ retVal = spvBuilder.createUnaryOp(
+ spv::Op::OpBitcast, astContext.UnsignedIntTy, counter,
+ expr->getCallee()->getExprLoc(), expr->getCallee()->getSourceRange());
break;
case IntrinsicOp::MOP_Append:
if (hlsl::IsHLSLStreamOutputType(
@@ -6992,8 +6996,9 @@ SpirvEmitter::doCXXOperatorCallExpr(const CXXOperatorCallExpr *expr,
return nullptr;
}
QualType resourceType = parentExpr->getType();
- // The heap object must be a direct reference to the builtin heap variable.
- // Anything else (e.g. a non-variable expression) has no backing VarDecl.
+ // The heap object must be a direct reference to the builtin heap
+ // variable. Anything else (e.g. a non-variable expression) has no backing
+ // VarDecl.
const auto *declRefExpr = dyn_cast<DeclRefExpr>(baseExpr->IgnoreCasts());
const auto *decl =
declRefExpr ? dyn_cast<VarDecl>(declRefExpr->getDecl()) : nullptr;
@@ -7013,7 +7018,8 @@ SpirvEmitter::doCXXOperatorCallExpr(const CXXOperatorCallExpr *expr,
if (isAppendStructuredBuffer(resourceType) ||
isConsumeStructuredBuffer(resourceType)) {
emitError("append/consume structured buffers are not supported with "
- "SPV_EXT_descriptor_heap", expr->getExprLoc());
+ "SPV_EXT_descriptor_heap",
+ expr->getExprLoc());
return nullptr;
}
@@ -7033,16 +7039,16 @@ SpirvEmitter::doCXXOperatorCallExpr(const CXXOperatorCallExpr *expr,
const SpirvType *arrayType = getDescriptorHeapRuntimeArrayType(
handleType, isSamplerDescriptorHeap(decl));
SpirvInstruction *untypedAccessChainPtr =
- spvBuilder.createUntypedAccessChainKHR(
- untypedUniformConstantType, arrayType, var, index,
- baseExpr->getExprLoc());
+ spvBuilder.createUntypedAccessChainKHR(untypedUniformConstantType,
+ arrayType, var, index,
+ baseExpr->getExprLoc());
if (isRasterizerOrderedView(resourceType))
- spvBuilder.addExecutionMode(
- entryFunction, declIdMapper.getInterlockExecutionMode(), {},
- baseExpr->getExprLoc());
- descriptorHeapImageAccesses[expr] = {untypedAccessChainPtr, handleType,
- arrayType, var, index,
- indexExpr->getType()};
+ spvBuilder.addExecutionMode(entryFunction,
+ declIdMapper.getInterlockExecutionMode(),
+ {}, baseExpr->getExprLoc());
+ descriptorHeapImageAccesses[expr] = {
+ untypedAccessChainPtr, handleType, arrayType, var, index,
+ indexExpr->getType()};
return spvBuilder.createLoad(resourceType, untypedAccessChainPtr,
baseExpr->getExprLoc(), range);
}
diff --git a/tools/clang/lib/SPIRV/SpirvEmitter.h b/tools/clang/lib/SPIRV/SpirvEmitter.h
index ec650c24..7966afea 100644
--- a/tools/clang/lib/SPIRV/SpirvEmitter.h
+++ b/tools/clang/lib/SPIRV/SpirvEmitter.h
@@ -412,12 +412,9 @@ private:
/// descriptorHeapBufferAccesses[expr] and returns the buffer-data pointer, or
/// nullptr (after emitting an error) on type-lowering failure. Caller must
/// have already checked the resource is buffer-like.
- SpirvInstruction *emitDescriptorHeapBufferAccess(QualType resourceType,
- SpirvInstruction *heapVar,
- SpirvInstruction *index,
- const Expr *expr,
- const Expr *baseExpr,
- const Expr *indexExpr);
+ SpirvInstruction *emitDescriptorHeapBufferAccess(
+ QualType resourceType, SpirvInstruction *heapVar, SpirvInstruction *index,
+ const Expr *expr, const Expr *baseExpr, const Expr *indexExpr);
/// Generates the necessary instructions for conducting the given binary
/// operation on lhs and rhs.
@@ -1226,14 +1223,16 @@ private:
/// \brief Marks an alias resource as heap-loaded with no associated counter.
void markDescriptorHeapCounterUnsupported(const DeclaratorDecl *decl);
- /// \brief Returns true if counter operations on the resource expression are known to
- /// be unsupported because the resource came from ResourceDescriptorHeap.
+ /// \brief Returns true if counter operations on the resource expression are
+ /// known to be unsupported because the resource came from
+ /// ResourceDescriptorHeap.
bool isDescriptorHeapCounterUnsupported(const Expr *expr) const;
- /// \brief Records the descriptor heap index assigned to a local image resource
- /// alias, if the source expression came directly from a descriptor heap. This
- /// mirrors the normal resource handle store while preserving enough
- /// information to recreate OpUntypedImageTexelPointerEXT after reassignment.
+ /// \brief Records the descriptor heap index assigned to a local image
+ /// resource alias, if the source expression came directly from a descriptor
+ /// heap. This mirrors the normal resource handle store while preserving
+ /// enough information to recreate OpUntypedImageTexelPointerEXT after
+ /// reassignment.
bool tryToAssignDescriptorHeapImageAlias(const DeclaratorDecl *dstDecl,
const Expr *srcExpr);
bool tryToAssignDescriptorHeapImageAlias(const Expr *dstExpr,
@@ -1243,8 +1242,8 @@ private:
bool tryToAssignDescriptorHeapBufferAlias(const Expr *dstExpr,
const Expr *srcExpr);
- /// \brief Creates the "<name>.descriptor.index" function variable used to remember
- /// the descriptor heap index of a local resource alias dstVar.
+ /// \brief Creates the "<name>.descriptor.index" function variable used to
+ /// remember the descriptor heap index of a local resource alias dstVar.
SpirvVariable *createDescriptorHeapIndexVar(const VarDecl *dstVar);
/// \brief If decl is a function-local variable initialized directly from a
@@ -1253,8 +1252,8 @@ private:
/// descriptor-heap alias and should be emitted as a normal variable.
bool tryToCreateDescriptorHeapAlias(const VarDecl *decl, const Expr *init);
- /// \brief Handles a buffer = ResourceDescriptorHeap[i] assignment. Returns None if
- /// assignExpr is not such an assignment (caller should fall back to a
+ /// \brief Handles a buffer = ResourceDescriptorHeap[i] assignment. Returns
+ /// None if assignExpr is not such an assignment (caller should fall back to a
/// normal assignment). Otherwise the alias was created and the wrapped value
/// is the result of the assignment expression (possibly nullptr).
llvm::Optional<SpirvInstruction *>
@@ -1267,16 +1266,16 @@ private:
SpirvInstruction *emitDescriptorHeapBufferPointer(const VarDecl *decl,
SourceLocation loc);
- /// \brief Emits an OpUntypedImageTexelPointerEXT for a descriptor-heap image alias
- /// decl (OpLoad of the saved index, then OpUntypedAccessChainKHR feeding
- /// the texel pointer). Returns nullptr if decl is not a recorded heap
+ /// \brief Emits an OpUntypedImageTexelPointerEXT for a descriptor-heap image
+ /// alias decl (OpLoad of the saved index, then OpUntypedAccessChainKHR
+ /// feeding the texel pointer). Returns nullptr if decl is not a recorded heap
/// image alias. Symmetric with emitDescriptorHeapBufferPointer.
SpirvInstruction *emitDescriptorHeapImageTexelPointer(
const VarDecl *decl, SpirvInstruction *coordinate,
SpirvInstruction *sample, QualType resultType, SourceLocation loc);
- /// \brief Emits OpLoad of indexVar then OpUntypedAccessChainKHR into the heap,
- /// yielding the per-descriptor pointer shared by the buffer/image alias
+ /// \brief Emits OpLoad of indexVar then OpUntypedAccessChainKHR into the
+ /// heap, yielding the per-descriptor pointer shared by the buffer/image alias
/// re-derivation paths above.
SpirvInstruction *emitDescriptorHeapAccessChain(const SpirvType *arrayType,
SpirvInstruction *heap,
diff --git a/tools/clang/lib/SPIRV/SpirvInstruction.cpp b/tools/clang/lib/SPIRV/SpirvInstruction.cpp
index 0668f247..0f39e24d 100644
--- a/tools/clang/lib/SPIRV/SpirvInstruction.cpp
+++ b/tools/clang/lib/SPIRV/SpirvInstruction.cpp
@@ -724,8 +724,8 @@ SpirvConstantSizeOfEXT::SpirvConstantSizeOfEXT(QualType resultType,
bool SpirvConstantSizeOfEXT::operator==(
const SpirvConstantSizeOfEXT &that) const {
- return resultType == that.resultType &&
- astResultType == that.astResultType && operandType == that.operandType;
+ return resultType == that.resultType && astResultType == that.astResultType &&
+ operandType == that.operandType;
}
SpirvConstantNull::SpirvConstantNull(QualType type)
@@ -987,8 +987,8 @@ SpirvImageTexelPointer::SpirvImageTexelPointer(QualType resultType,
SpirvUntypedImageTexelPointerEXT::SpirvUntypedImageTexelPointerEXT(
QualType resultType, SourceLocation loc, const SpirvType *spvImageType,
- SpirvInstruction *imageInst,
- SpirvInstruction *coordinateInst, SpirvInstruction *sampleInst)
+ SpirvInstruction *imageInst, SpirvInstruction *coordinateInst,
+ SpirvInstruction *sampleInst)
: SpirvInstruction(IK_UntypedImageTexelPointerEXT,
spv::Op::OpUntypedImageTexelPointerEXT, resultType, loc),
imageType(spvImageType), image(imageInst), coordinate(coordinateInst),
|
jzakharovnv
force-pushed
the
pr3-cli-stride-flags
branch
3 times, most recently
from
1e2a278 to
5e7b928
Compare
Collaborator
Author
|
@microsoft-github-policy-service agree company="NVIDIA" |
… CLI flags [SPIR-V] Fix getDescriptorHeapRuntimeArrayType signature mismatch
jzakharovnv
force-pushed
the
pr3-cli-stride-flags
branch
from
5e7b928 to
9c5badc
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Building off of #8518, this PR adds two new command-line flags that override the ArrayStride of the descriptor heap runtime arrays emitted by -fspv-use-descriptor-heap. It is part 3/4 in a series.
-fvk-resource-heap-stride and -fvk-sampler-heap-stride sets the stride for ResourceDescriptorHeap SamplerDescriptorHeap arrays respectively. N and M must be a power of two in [8, 256]. When set, the CLI value takes the highest precedence.
Assisted by an AI agent.
@dnovillo