From efbce87192ce73ba577684369f759693dccec67c Mon Sep 17 00:00:00 2001 From: Mikko Rasa Date: Thu, 11 Nov 2021 12:00:07 +0200 Subject: [PATCH] Improve layout qualifier handling with some more utility functions --- source/glsl/finalize.cpp | 49 ++++++++++------------------------------ source/glsl/finalize.h | 1 - source/glsl/reflect.cpp | 7 +----- source/glsl/spirv.cpp | 29 ++++++------------------ source/glsl/syntax.cpp | 23 ++++++++++++++++--- source/glsl/syntax.h | 4 +++- source/glsl/validate.cpp | 31 ++++++++----------------- source/glsl/validate.h | 2 -- 8 files changed, 53 insertions(+), 93 deletions(-) diff --git a/source/glsl/finalize.cpp b/source/glsl/finalize.cpp index 3bdca5cb..7eb0165d 100644 --- a/source/glsl/finalize.cpp +++ b/source/glsl/finalize.cpp @@ -46,12 +46,7 @@ void StructOrganizer::visit(VariableDeclaration &var) if(layout_offset) *layout_offset = offset; else - { - if(!var.layout) - var.layout = new Layout; - - var.layout->qualifiers.push_back(Layout::Qualifier("offset", offset)); - } + add_layout_qualifier(var.layout, Layout::Qualifier("offset", offset)); if(!has_matrix_order) { @@ -59,7 +54,7 @@ void StructOrganizer::visit(VariableDeclaration &var) while(basic && basic->kind==BasicTypeDeclaration::ARRAY) basic = dynamic_cast(basic->base_type); if(basic && basic->kind==BasicTypeDeclaration::MATRIX) - var.layout->qualifiers.push_back(Layout::Qualifier("column_major")); + add_layout_qualifier(var.layout, Layout::Qualifier("column_major")); } offset += mem_reqs.size; @@ -111,7 +106,7 @@ void LocationAllocator::allocate_locations(const string &iface) auto j = uniforms.find((*i)->name); if(j!=uniforms.end() && j->second.location>=0) { - add_layout_value((*i)->layout, "location", j->second.location); + add_layout_qualifier((*i)->layout, Layout::Qualifier("location", j->second.location)); continue; } } @@ -130,7 +125,7 @@ void LocationAllocator::allocate_locations(const string &iface) next = blocking+1; } - add_layout_value((*i)->layout, "location", next); + add_layout_qualifier((*i)->layout, Layout::Qualifier("location", next)); if((*i)->interface=="uniform") uniforms[(*i)->name].location = next; @@ -146,7 +141,7 @@ void LocationAllocator::bind_uniform(RefPtr &layout, const string &name, { auto i = uniforms.find(name); if(i!=uniforms.end() && i->second.bind_point>=0) - add_layout_value(layout, "binding", i->second.bind_point); + add_layout_qualifier(layout, Layout::Qualifier("binding", i->second.bind_point)); else { set &used = used_bindings[0]; @@ -155,23 +150,15 @@ void LocationAllocator::bind_uniform(RefPtr &layout, const string &name, while(used.count(bind_point)) bind_point = (bind_point+1)%range; - add_layout_value(layout, "binding", bind_point); + add_layout_qualifier(layout, Layout::Qualifier("binding", bind_point)); uniforms[name].bind_point = bind_point; used.insert(bind_point); } } -void LocationAllocator::add_layout_value(RefPtr &layout, const string &name, unsigned value) -{ - if(!layout) - layout = new Layout; - - layout->qualifiers.push_back(Layout::Qualifier(name, value)); -} - bool LocationAllocator::visit_uniform(const string &name, RefPtr &layout) { - int bind_point = (layout ? get_layout_value(*layout, "binding") : -1); + int bind_point = get_layout_value(layout.get(), "binding"); if(bind_point>=0) { used_bindings[0].insert(bind_point); @@ -187,13 +174,13 @@ void LocationAllocator::visit(VariableDeclaration &var) if(!var.interface.empty()) { - int location = (var.layout ? get_layout_value(*var.layout, "location") : -1); + int location = get_layout_value(var.layout.get(), "location"); if(location<0 && var.linked_declaration && var.linked_declaration->layout) { - location = get_layout_value(*var.linked_declaration->layout, "location"); + location = get_layout_value(var.linked_declaration->layout.get(), "location"); if(location>=0) - add_layout_value(var.layout, "location", location); + add_layout_qualifier(var.layout, Layout::Qualifier("location", location)); } if(location>=0) @@ -223,13 +210,7 @@ void LocationAllocator::visit(InterfaceBlock &iface) if(iface.interface=="uniform") { - bool push_constant = false; - if(iface.layout) - { - auto i = find_member(iface.layout->qualifiers, string("push_constant"), &Layout::Qualifier::name); - push_constant = (i!=iface.layout->qualifiers.end()); - } - + bool push_constant = has_layout_qualifier(iface.layout.get(), "push_constant"); if(!push_constant && !visit_uniform(iface.block_name, iface.layout)) unbound_blocks.push_back(&iface); } @@ -512,13 +493,7 @@ bool StructuralFeatureConverter::supports_interface_blocks(const string &iface) void StructuralFeatureConverter::visit(InterfaceBlock &iface) { - bool push_constant = false; - if(iface.layout) - { - auto i = find_member(iface.layout->qualifiers, string("push_constant"), &Layout::Qualifier::name); - push_constant = (i!=iface.layout->qualifiers.end()); - } - + bool push_constant = has_layout_qualifier(iface.layout.get(), "push_constant"); if((!supports_interface_blocks(iface.interface) || (push_constant && features.target_api!=VULKAN)) && iface.type_declaration) { if(!iface.instance_name.empty()) diff --git a/source/glsl/finalize.h b/source/glsl/finalize.h index 17430748..bde0618a 100644 --- a/source/glsl/finalize.h +++ b/source/glsl/finalize.h @@ -50,7 +50,6 @@ private: void allocate_locations(const std::string &); void bind_uniform(RefPtr &, const std::string &, unsigned); - void add_layout_value(RefPtr &, const std::string &, unsigned); bool visit_uniform(const std::string &, RefPtr &); virtual void visit(VariableDeclaration &); diff --git a/source/glsl/reflect.cpp b/source/glsl/reflect.cpp index 811679f3..a2f43a9d 100644 --- a/source/glsl/reflect.cpp +++ b/source/glsl/reflect.cpp @@ -311,12 +311,7 @@ void MemoryRequirementsCalculator::visit(StructDeclaration &strct) void MemoryRequirementsCalculator::visit(VariableDeclaration &var) { - if(var.layout) - { - auto i = find_member(var.layout->qualifiers, string("offset"), &Layout::Qualifier::name); - if(i!=var.layout->qualifiers.end()) - r_offset = i->value; - } + r_offset = get_layout_value(var.layout.get(), "offset"); if(var.type_declaration) var.type_declaration->visit(*this); diff --git a/source/glsl/spirv.cpp b/source/glsl/spirv.cpp index 20345e20..40e731fe 100644 --- a/source/glsl/spirv.cpp +++ b/source/glsl/spirv.cpp @@ -1582,16 +1582,6 @@ void SpirVGenerator::visit(StructDeclaration &strct) void SpirVGenerator::visit(VariableDeclaration &var) { - const vector *layout_ql = (var.layout ? &var.layout->qualifiers : 0); - - int spec_id = -1; - if(layout_ql) - { - auto i = find_member(*layout_ql, string("constant_id"), &Layout::Qualifier::name); - if(i!=layout_ql->end()) - spec_id = i->value; - } - Id type_id = get_variable_type_id(var); Id var_id; @@ -1600,6 +1590,8 @@ void SpirVGenerator::visit(VariableDeclaration &var) if(!var.init_expression) throw internal_error("const variable without initializer"); + int spec_id = get_layout_value(var.layout.get(), "constant_id"); + SetFlag set_const(constant_expression); SetFlag set_spec(spec_constant, spec_id>=0); r_expression_result_id = 0; @@ -1650,9 +1642,9 @@ void SpirVGenerator::visit(VariableDeclaration &var) writer.write(init_id); writer.end_op(OP_VARIABLE); - if(layout_ql) + if(var.layout) { - for(const Layout::Qualifier &q: *layout_ql) + for(const Layout::Qualifier &q: var.layout->qualifiers) { if(q.name=="location") writer.write_op_decorate(var_id, DECO_LOCATION, q.value); @@ -1680,14 +1672,7 @@ void SpirVGenerator::visit(VariableDeclaration &var) void SpirVGenerator::visit(InterfaceBlock &iface) { - const vector *layout_ql = (iface.layout ? &iface.layout->qualifiers : 0); - - bool push_const = false; - if(layout_ql) - { - auto i = find_member(*layout_ql, string("push_constant"), &Layout::Qualifier::name); - push_const = (i!=layout_ql->end()); - } + bool push_const = has_layout_qualifier(iface.layout.get(), "push_constant"); StorageClass storage = (push_const ? STORAGE_PUSH_CONSTANT : get_interface_storage(iface.interface, true)); Id type_id; @@ -1715,9 +1700,9 @@ void SpirVGenerator::visit(InterfaceBlock &iface) writer.write_op(content.globals, OP_VARIABLE, ptr_type_id, block_id, storage); - if(layout_ql) + if(iface.layout) { - for(const Layout::Qualifier &q: *layout_ql) + for(const Layout::Qualifier &q: iface.layout->qualifiers) { if(q.name=="set") writer.write_op_decorate(block_id, DECO_DESCRIPTOR_SET, q.value); diff --git a/source/glsl/syntax.cpp b/source/glsl/syntax.cpp index 40f6d4f6..2ab4d1b6 100644 --- a/source/glsl/syntax.cpp +++ b/source/glsl/syntax.cpp @@ -415,10 +415,27 @@ const TypeDeclaration *get_ultimate_base_type(const TypeDeclaration *type) return type; } -int get_layout_value(const Layout &layout, const string &name, int def_value) +bool has_layout_qualifier(const Layout *layout, const string &name) { - auto i = find_member(layout.qualifiers, name, &Layout::Qualifier::name); - return (i!=layout.qualifiers.end() ? i->value : def_value); + if(!layout) + return false; + auto i = find_member(layout->qualifiers, name, &Layout::Qualifier::name); + return i!=layout->qualifiers.end(); +} + +int get_layout_value(const Layout *layout, const string &name, int def_value) +{ + if(!layout) + return def_value; + auto i = find_member(layout->qualifiers, name, &Layout::Qualifier::name); + return (i!=layout->qualifiers.end() ? i->value : def_value); +} + +void add_layout_qualifier(RefPtr &layout, const Layout::Qualifier &q) +{ + if(!layout) + layout = new Layout; + layout->qualifiers.push_back(q); } void add_to_chain(Assignment::Target &target, Assignment::Target::ChainType type, unsigned index) diff --git a/source/glsl/syntax.h b/source/glsl/syntax.h index 499779d9..a1c62d4b 100644 --- a/source/glsl/syntax.h +++ b/source/glsl/syntax.h @@ -553,7 +553,9 @@ struct Module std::string get_unused_variable_name(const Block &, const std::string &); const TypeDeclaration *get_ultimate_base_type(const TypeDeclaration *); -int get_layout_value(const Layout &, const std::string &, int = -1); +bool has_layout_qualifier(const Layout *, const std::string &); +int get_layout_value(const Layout *, const std::string &, int = -1); +void add_layout_qualifier(RefPtr &, const Layout::Qualifier &); void add_to_chain(Assignment::Target &, Assignment::Target::ChainType, unsigned); bool targets_overlap(const Assignment::Target &, const Assignment::Target &); diff --git a/source/glsl/validate.cpp b/source/glsl/validate.cpp index df3312e1..6a4b8eb0 100644 --- a/source/glsl/validate.cpp +++ b/source/glsl/validate.cpp @@ -526,8 +526,7 @@ void ExpressionValidator::visit(VariableReference &var) error(var, format("Reference to non-constant variable '%s' in a constant expression", var.name)); else if(var.declaration->layout && constant_expression==FIXED_CONSTANT) { - auto i = find_member(var.declaration->layout->qualifiers, string("constant_id"), &Layout::Qualifier::name); - if(i!=var.declaration->layout->qualifiers.end()) + if(has_layout_qualifier(var.declaration->layout.get(), "constant_id")) error(var, format("Reference to specialization constant '%s' in a fixed constant expression", var.name)); } } @@ -662,13 +661,8 @@ void ExpressionValidator::visit(VariableDeclaration &var) var.layout->visit(*this); if(var.init_expression) { - ConstantKind const_kind = (var.constant ? SPEC_CONSTANT : NOT_CONSTANT); - if(var.layout) - { - auto i = find_member(var.layout->qualifiers, string("constant_id"), &Layout::Qualifier::name); - if(i!=var.layout->qualifiers.end()) - const_kind = FIXED_CONSTANT; - } + ConstantKind const_kind = (!var.constant ? NOT_CONSTANT : + has_layout_qualifier(var.layout.get(), "constant_id") ? FIXED_CONSTANT : SPEC_CONSTANT); SetForScope set_const(constant_expression, const_kind); TraversingVisitor::visit(var.init_expression); @@ -770,18 +764,13 @@ void FlowControlValidator::visit(Iteration &iter) } -int StageInterfaceValidator::get_location(const Layout &layout) -{ - return get_layout_value(layout, "location", -1); -} - void StageInterfaceValidator::visit(VariableDeclaration &var) { - int location = (var.layout ? get_location(*var.layout) : -1); + int location = get_layout_value(var.layout.get(), "location"); if(var.interface=="in" && var.linked_declaration) { const Layout *linked_layout = var.linked_declaration->layout.get(); - int linked_location = (linked_layout ? get_location(*linked_layout) : -1); + int linked_location = get_layout_value(linked_layout, "location"); if(linked_location!=location) { error(var, format("Mismatched location %d for 'in %s'", location, var.name)); @@ -910,10 +899,10 @@ void GlobalInterfaceValidator::visit(VariableDeclaration &var) uni.name = var.name; if(var.layout) { - uni.location = get_layout_value(*var.layout, "location"); + uni.location = get_layout_value(var.layout.get(), "location"); uni.loc_count = LocationCounter().apply(var); - uni.desc_set = get_layout_value(*var.layout, "set", 0); - uni.bind_point = get_layout_value(*var.layout, "binding"); + uni.desc_set = get_layout_value(var.layout.get(), "set", 0); + uni.bind_point = get_layout_value(var.layout.get(), "binding"); } uniforms.push_back(uni); @@ -931,8 +920,8 @@ void GlobalInterfaceValidator::visit(InterfaceBlock &iface) uni.name = iface.block_name; if(iface.layout) { - uni.desc_set = get_layout_value(*iface.layout, "set", 0); - uni.bind_point = get_layout_value(*iface.layout, "binding"); + uni.desc_set = get_layout_value(iface.layout.get(), "set", 0); + uni.bind_point = get_layout_value(iface.layout.get(), "binding"); } uniforms.push_back(uni); diff --git a/source/glsl/validate.h b/source/glsl/validate.h index 558803ff..a09f9f64 100644 --- a/source/glsl/validate.h +++ b/source/glsl/validate.h @@ -175,8 +175,6 @@ public: void apply(Stage &s) { stage = &s; s.content.visit(*this); } private: - int get_location(const Layout &); - virtual void visit(VariableDeclaration &); virtual void visit(FunctionDeclaration &) { } }; -- 2.43.0