Disallow 'binding' and 'set' on push constants

These aren't allowed on push constants (it's a validation error now), so
we at least catch it in the SPIRV generator and emit an error. Fixed two
places where we were breaking this rule when automatically adjusting
layouts for interface blocks.

Bug: skia:12670
Change-Id: I08b88f4c2844da77239207817f49510118be4e60
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/474976
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
This commit is contained in:
Brian Osman 2021-11-22 12:51:49 -05:00 committed by SkCQ
parent 4fa40eb741
commit e051cd35e4
3 changed files with 35 additions and 24 deletions

View File

@ -1,4 +1,4 @@
layout(push_constant, binding=456) uniform testBlock {
layout(push_constant) uniform testBlock {
layout(offset=16) half2x2 m1;
layout(offset=32) half2x2 m2;
};

View File

@ -498,7 +498,7 @@ void SPIRVCodeGenerator::writeStruct(const Type& type, const MemoryLayout& memor
}
}
this->writeInstruction(SpvOpMemberName, resultId, i, field.fName, fNameBuffer);
this->writeLayout(fieldLayout, resultId, i);
this->writeFieldLayout(fieldLayout, resultId, i);
if (field.fModifiers.fLayout.fBuiltin < 0) {
this->writeInstruction(SpvOpMemberDecorate, resultId, (SpvId) i, SpvDecorationOffset,
(SpvId) offset, fDecorationBuffer);
@ -2950,23 +2950,31 @@ SpvId SPIRVCodeGenerator::writeFunction(const FunctionDefinition& f, OutputStrea
return result;
}
void SPIRVCodeGenerator::writeLayout(const Layout& layout, SpvId target) {
void SPIRVCodeGenerator::writeLayout(const Layout& layout, SpvId target, int line) {
bool isPushConstant = (layout.fFlags & Layout::kPushConstant_Flag);
if (layout.fLocation >= 0) {
this->writeInstruction(SpvOpDecorate, target, SpvDecorationLocation, layout.fLocation,
fDecorationBuffer);
}
if (layout.fBinding >= 0 && !isPushConstant) {
this->writeInstruction(SpvOpDecorate, target, SpvDecorationBinding, layout.fBinding,
fDecorationBuffer);
if (layout.fBinding >= 0) {
if (isPushConstant) {
fContext.fErrors->error(line, "Can't apply 'binding' to push constants");
} else {
this->writeInstruction(SpvOpDecorate, target, SpvDecorationBinding, layout.fBinding,
fDecorationBuffer);
}
}
if (layout.fIndex >= 0) {
this->writeInstruction(SpvOpDecorate, target, SpvDecorationIndex, layout.fIndex,
fDecorationBuffer);
}
if (layout.fSet >= 0 && !isPushConstant) {
this->writeInstruction(SpvOpDecorate, target, SpvDecorationDescriptorSet, layout.fSet,
fDecorationBuffer);
if (layout.fSet >= 0) {
if (isPushConstant) {
fContext.fErrors->error(line, "Can't apply 'set' to push constants");
} else {
this->writeInstruction(SpvOpDecorate, target, SpvDecorationDescriptorSet, layout.fSet,
fDecorationBuffer);
}
}
if (layout.fInputAttachmentIndex >= 0) {
this->writeInstruction(SpvOpDecorate, target, SpvDecorationInputAttachmentIndex,
@ -2979,7 +2987,7 @@ void SPIRVCodeGenerator::writeLayout(const Layout& layout, SpvId target) {
}
}
void SPIRVCodeGenerator::writeLayout(const Layout& layout, SpvId target, int member) {
void SPIRVCodeGenerator::writeFieldLayout(const Layout& layout, SpvId target, int member) {
// 'binding' and 'set' can not be applied to struct members
SkASSERT(layout.fBinding == -1);
SkASSERT(layout.fSet == -1);
@ -3069,10 +3077,10 @@ SpvId SPIRVCodeGenerator::writeInterfaceBlock(const InterfaceBlock& intf, bool a
this->writeInstruction(SpvOpTypePointer, ptrType, storageClass, typeId, fConstantBuffer);
this->writeInstruction(SpvOpVariable, ptrType, result, storageClass, fConstantBuffer);
Layout layout = intfModifiers.fLayout;
if (intfModifiers.fFlags & Modifiers::kUniform_Flag && layout.fSet == -1) {
layout.fSet = 0;
if (storageClass == SpvStorageClassUniform && layout.fSet < 0) {
layout.fSet = fProgram.fConfig->fSettings.fDefaultUniformSet;
}
this->writeLayout(layout, result);
this->writeLayout(layout, result, intfVar.fLine);
fVariableMap[&intfVar] = result;
return result;
}
@ -3129,7 +3137,7 @@ void SPIRVCodeGenerator::writeGlobalVar(ProgramKind kind, const VarDeclaration&
this->writeInstruction(SpvOpStore, id, value, fGlobalInitializersBuffer);
fCurrentBlock = 0;
}
this->writeLayout(layout, id);
this->writeLayout(layout, id, var.fLine);
if (var.modifiers().fFlags & Modifiers::kFlat_Flag) {
this->writeInstruction(SpvOpDecorate, id, SpvDecorationFlat, fDecorationBuffer);
}
@ -3483,15 +3491,18 @@ void SPIRVCodeGenerator::addRTFlipUniform(int line) {
skstd::string_view name = "sksl_synthetic_uniforms";
const Type* intfStruct = fSynthetics.takeOwnershipOfSymbol(
Type::MakeStructType(/*line=*/-1, name, fields, /*interfaceBlock=*/true));
int binding = fProgram.fConfig->fSettings.fRTFlipBinding;
if (binding == -1) {
fContext.fErrors->error(line, "layout(binding=...) is required in SPIR-V");
}
int set = fProgram.fConfig->fSettings.fRTFlipSet;
if (set == -1) {
fContext.fErrors->error(line, "layout(set=...) is required in SPIR-V");
}
bool usePushConstants = fProgram.fConfig->fSettings.fUsePushConstants;
int binding = -1, set = -1;
if (!usePushConstants) {
binding = fProgram.fConfig->fSettings.fRTFlipBinding;
if (binding == -1) {
fContext.fErrors->error(line, "layout(binding=...) is required in SPIR-V");
}
set = fProgram.fConfig->fSettings.fRTFlipSet;
if (set == -1) {
fContext.fErrors->error(line, "layout(set=...) is required in SPIR-V");
}
}
int flags = usePushConstants ? Layout::Flag::kPushConstant_Flag : 0;
const Modifiers* modsPtr;
{

View File

@ -201,9 +201,9 @@ private:
std::vector<SpvId> getAccessChain(const Expression& expr, OutputStream& out);
void writeLayout(const Layout& layout, SpvId target);
void writeLayout(const Layout& layout, SpvId target, int line);
void writeLayout(const Layout& layout, SpvId target, int member);
void writeFieldLayout(const Layout& layout, SpvId target, int member);
void writeStruct(const Type& type, const MemoryLayout& layout, SpvId resultId);