From 7cf34c3637b85feeba2cf1ac67fc3baf9803804b Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 3 Dec 2020 15:59:38 -0500 Subject: [PATCH 1/2] node: Eliminate variable shadowing --- src/video_core/shader/node.h | 96 ++++++++++++++++++------------------ 1 file changed, 49 insertions(+), 47 deletions(-) diff --git a/src/video_core/shader/node.h b/src/video_core/shader/node.h index 8f230d57a..8b081030f 100644 --- a/src/video_core/shader/node.h +++ b/src/video_core/shader/node.h @@ -284,24 +284,26 @@ using TrackSampler = std::shared_ptr; struct Sampler { /// Bound samplers constructor - constexpr explicit Sampler(u32 index, u32 offset, Tegra::Shader::TextureType type, - bool is_array, bool is_shadow, bool is_buffer, bool is_indexed) - : index{index}, offset{offset}, type{type}, is_array{is_array}, is_shadow{is_shadow}, - is_buffer{is_buffer}, is_indexed{is_indexed} {} + constexpr explicit Sampler(u32 index_, u32 offset_, Tegra::Shader::TextureType type_, + bool is_array_, bool is_shadow_, bool is_buffer_, bool is_indexed_) + : index{index_}, offset{offset_}, type{type_}, is_array{is_array_}, is_shadow{is_shadow_}, + is_buffer{is_buffer_}, is_indexed{is_indexed_} {} /// Separate sampler constructor - constexpr explicit Sampler(u32 index, std::pair offsets, std::pair buffers, - Tegra::Shader::TextureType type, bool is_array, bool is_shadow, - bool is_buffer) - : index{index}, offset{offsets.first}, secondary_offset{offsets.second}, - buffer{buffers.first}, secondary_buffer{buffers.second}, type{type}, is_array{is_array}, - is_shadow{is_shadow}, is_buffer{is_buffer}, is_separated{true} {} + constexpr explicit Sampler(u32 index_, std::pair offsets, std::pair buffers, + Tegra::Shader::TextureType type, bool is_array_, bool is_shadow_, + bool is_buffer_) + : index{index_}, offset{offsets.first}, secondary_offset{offsets.second}, + buffer{buffers.first}, secondary_buffer{buffers.second}, type{type}, is_array{is_array_}, + is_shadow{is_shadow_}, is_buffer{is_buffer_}, is_separated{true} {} /// Bindless samplers constructor - constexpr explicit Sampler(u32 index, u32 offset, u32 buffer, Tegra::Shader::TextureType type, - bool is_array, bool is_shadow, bool is_buffer, bool is_indexed) - : index{index}, offset{offset}, buffer{buffer}, type{type}, is_array{is_array}, - is_shadow{is_shadow}, is_buffer{is_buffer}, is_bindless{true}, is_indexed{is_indexed} {} + constexpr explicit Sampler(u32 index_, u32 offset_, u32 buffer_, + Tegra::Shader::TextureType type, bool is_array_, bool is_shadow_, + bool is_buffer_, bool is_indexed_) + : index{index_}, offset{offset_}, buffer{buffer_}, type{type}, is_array{is_array_}, + is_shadow{is_shadow_}, is_buffer{is_buffer_}, is_bindless{true}, is_indexed{is_indexed_} { + } u32 index = 0; ///< Emulated index given for the this sampler. u32 offset = 0; ///< Offset in the const buffer from where the sampler is being read. @@ -341,12 +343,12 @@ struct BindlessSamplerNode { struct Image { public: /// Bound images constructor - constexpr explicit Image(u32 index, u32 offset, Tegra::Shader::ImageType type) - : index{index}, offset{offset}, type{type} {} + constexpr explicit Image(u32 index_, u32 offset_, Tegra::Shader::ImageType type_) + : index{index_}, offset{offset_}, type{type_} {} /// Bindless samplers constructor - constexpr explicit Image(u32 index, u32 offset, u32 buffer, Tegra::Shader::ImageType type) - : index{index}, offset{offset}, buffer{buffer}, type{type}, is_bindless{true} {} + constexpr explicit Image(u32 index_, u32 offset_, u32 buffer_, Tegra::Shader::ImageType type_) + : index{index_}, offset{offset_}, buffer{buffer_}, type{type_}, is_bindless{true} {} void MarkWrite() { is_written = true; @@ -437,20 +439,20 @@ private: /// Holds any kind of operation that can be done in the IR class OperationNode final : public AmendNode { public: - explicit OperationNode(OperationCode code) : OperationNode(code, Meta{}) {} + explicit OperationNode(OperationCode code_) : OperationNode(code_, Meta{}) {} - explicit OperationNode(OperationCode code, Meta meta) - : OperationNode(code, std::move(meta), std::vector{}) {} + explicit OperationNode(OperationCode code_, Meta meta_) + : OperationNode(code_, std::move(meta_), std::vector{}) {} - explicit OperationNode(OperationCode code, std::vector operands) - : OperationNode(code, Meta{}, std::move(operands)) {} + explicit OperationNode(OperationCode code_, std::vector operands_) + : OperationNode(code_, Meta{}, std::move(operands_)) {} - explicit OperationNode(OperationCode code, Meta meta, std::vector operands) - : code{code}, meta{std::move(meta)}, operands{std::move(operands)} {} + explicit OperationNode(OperationCode code_, Meta meta_, std::vector operands_) + : code{code_}, meta{std::move(meta_)}, operands{std::move(operands_)} {} template - explicit OperationNode(OperationCode code, Meta meta, Args&&... operands) - : code{code}, meta{std::move(meta)}, operands{operands...} {} + explicit OperationNode(OperationCode code_, Meta meta_, Args&&... operands_) + : code{code_}, meta{std::move(meta_)}, operands{operands_...} {} OperationCode GetCode() const { return code; @@ -477,8 +479,8 @@ private: /// Encloses inside any kind of node that returns a boolean conditionally-executed code class ConditionalNode final : public AmendNode { public: - explicit ConditionalNode(Node condition, std::vector&& code) - : condition{std::move(condition)}, code{std::move(code)} {} + explicit ConditionalNode(Node condition_, std::vector&& code_) + : condition{std::move(condition_)}, code{std::move(code_)} {} const Node& GetCondition() const { return condition; @@ -496,7 +498,7 @@ private: /// A general purpose register class GprNode final { public: - explicit constexpr GprNode(Tegra::Shader::Register index) : index{index} {} + explicit constexpr GprNode(Tegra::Shader::Register index_) : index{index_} {} u32 GetIndex() const { return static_cast(index); @@ -509,7 +511,7 @@ private: /// A custom variable class CustomVarNode final { public: - explicit constexpr CustomVarNode(u32 index) : index{index} {} + explicit constexpr CustomVarNode(u32 index_) : index{index_} {} constexpr u32 GetIndex() const { return index; @@ -522,7 +524,7 @@ private: /// A 32-bits value that represents an immediate value class ImmediateNode final { public: - explicit constexpr ImmediateNode(u32 value) : value{value} {} + explicit constexpr ImmediateNode(u32 value_) : value{value_} {} u32 GetValue() const { return value; @@ -535,7 +537,7 @@ private: /// One of Maxwell's internal flags class InternalFlagNode final { public: - explicit constexpr InternalFlagNode(InternalFlag flag) : flag{flag} {} + explicit constexpr InternalFlagNode(InternalFlag flag_) : flag{flag_} {} InternalFlag GetFlag() const { return flag; @@ -548,8 +550,8 @@ private: /// A predicate register, it can be negated without additional nodes class PredicateNode final { public: - explicit constexpr PredicateNode(Tegra::Shader::Pred index, bool negated) - : index{index}, negated{negated} {} + explicit constexpr PredicateNode(Tegra::Shader::Pred index_, bool negated_) + : index{index_}, negated{negated_} {} Tegra::Shader::Pred GetIndex() const { return index; @@ -568,12 +570,12 @@ private: class AbufNode final { public: // Initialize for standard attributes (index is explicit). - explicit AbufNode(Tegra::Shader::Attribute::Index index, u32 element, Node buffer = {}) - : buffer{std::move(buffer)}, index{index}, element{element} {} + explicit AbufNode(Tegra::Shader::Attribute::Index index_, u32 element_, Node buffer_ = {}) + : buffer{std::move(buffer_)}, index{index_}, element{element_} {} // Initialize for physical attributes (index is a variable value). - explicit AbufNode(Node physical_address, Node buffer = {}) - : physical_address{std::move(physical_address)}, buffer{std::move(buffer)} {} + explicit AbufNode(Node physical_address_, Node buffer_ = {}) + : physical_address{std::move(physical_address_)}, buffer{std::move(buffer_)} {} Tegra::Shader::Attribute::Index GetIndex() const { return index; @@ -605,7 +607,7 @@ private: /// Patch memory (used to communicate tessellation stages). class PatchNode final { public: - explicit PatchNode(u32 offset) : offset{offset} {} + explicit PatchNode(u32 offset_) : offset{offset_} {} u32 GetOffset() const { return offset; @@ -618,7 +620,7 @@ private: /// Constant buffer node, usually mapped to uniform buffers in GLSL class CbufNode final { public: - explicit CbufNode(u32 index, Node offset) : index{index}, offset{std::move(offset)} {} + explicit CbufNode(u32 index_, Node offset_) : index{index_}, offset{std::move(offset_)} {} u32 GetIndex() const { return index; @@ -636,7 +638,7 @@ private: /// Local memory node class LmemNode final { public: - explicit LmemNode(Node address) : address{std::move(address)} {} + explicit LmemNode(Node address_) : address{std::move(address_)} {} const Node& GetAddress() const { return address; @@ -649,7 +651,7 @@ private: /// Shared memory node class SmemNode final { public: - explicit SmemNode(Node address) : address{std::move(address)} {} + explicit SmemNode(Node address_) : address{std::move(address_)} {} const Node& GetAddress() const { return address; @@ -662,9 +664,9 @@ private: /// Global memory node class GmemNode final { public: - explicit GmemNode(Node real_address, Node base_address, const GlobalMemoryBase& descriptor) - : real_address{std::move(real_address)}, base_address{std::move(base_address)}, - descriptor{descriptor} {} + explicit GmemNode(Node real_address_, Node base_address_, const GlobalMemoryBase& descriptor_) + : real_address{std::move(real_address_)}, base_address{std::move(base_address_)}, + descriptor{descriptor_} {} const Node& GetRealAddress() const { return real_address; @@ -687,7 +689,7 @@ private: /// Commentary, can be dropped class CommentNode final { public: - explicit CommentNode(std::string text) : text{std::move(text)} {} + explicit CommentNode(std::string text_) : text{std::move(text_)} {} const std::string& GetText() const { return text; From edd8208779051dfd62382f9ed1f896a43f1b3f7c Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 3 Dec 2020 16:03:31 -0500 Subject: [PATCH 2/2] node: Mark member functions as [[nodiscard]] where applicable Prevents logic bugs from accidentally ignoring the return value. --- src/video_core/shader/node.h | 58 ++++++++++++++++++------------------ 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/src/video_core/shader/node.h b/src/video_core/shader/node.h index 8b081030f..a1e2c4d8e 100644 --- a/src/video_core/shader/node.h +++ b/src/video_core/shader/node.h @@ -379,7 +379,7 @@ struct GlobalMemoryBase { u32 cbuf_index = 0; u32 cbuf_offset = 0; - bool operator<(const GlobalMemoryBase& rhs) const { + [[nodiscard]] bool operator<(const GlobalMemoryBase& rhs) const { return std::tie(cbuf_index, cbuf_offset) < std::tie(rhs.cbuf_index, rhs.cbuf_offset); } }; @@ -416,7 +416,7 @@ using Meta = class AmendNode { public: - std::optional GetAmendIndex() const { + [[nodiscard]] std::optional GetAmendIndex() const { if (amend_index == amend_null_index) { return std::nullopt; } @@ -454,19 +454,19 @@ public: explicit OperationNode(OperationCode code_, Meta meta_, Args&&... operands_) : code{code_}, meta{std::move(meta_)}, operands{operands_...} {} - OperationCode GetCode() const { + [[nodiscard]] OperationCode GetCode() const { return code; } - const Meta& GetMeta() const { + [[nodiscard]] const Meta& GetMeta() const { return meta; } - std::size_t GetOperandsCount() const { + [[nodiscard]] std::size_t GetOperandsCount() const { return operands.size(); } - const Node& operator[](std::size_t operand_index) const { + [[nodiscard]] const Node& operator[](std::size_t operand_index) const { return operands.at(operand_index); } @@ -482,11 +482,11 @@ public: explicit ConditionalNode(Node condition_, std::vector&& code_) : condition{std::move(condition_)}, code{std::move(code_)} {} - const Node& GetCondition() const { + [[nodiscard]] const Node& GetCondition() const { return condition; } - const std::vector& GetCode() const { + [[nodiscard]] const std::vector& GetCode() const { return code; } @@ -500,7 +500,7 @@ class GprNode final { public: explicit constexpr GprNode(Tegra::Shader::Register index_) : index{index_} {} - u32 GetIndex() const { + [[nodiscard]] constexpr u32 GetIndex() const { return static_cast(index); } @@ -513,7 +513,7 @@ class CustomVarNode final { public: explicit constexpr CustomVarNode(u32 index_) : index{index_} {} - constexpr u32 GetIndex() const { + [[nodiscard]] constexpr u32 GetIndex() const { return index; } @@ -526,7 +526,7 @@ class ImmediateNode final { public: explicit constexpr ImmediateNode(u32 value_) : value{value_} {} - u32 GetValue() const { + [[nodiscard]] constexpr u32 GetValue() const { return value; } @@ -539,7 +539,7 @@ class InternalFlagNode final { public: explicit constexpr InternalFlagNode(InternalFlag flag_) : flag{flag_} {} - InternalFlag GetFlag() const { + [[nodiscard]] constexpr InternalFlag GetFlag() const { return flag; } @@ -553,11 +553,11 @@ public: explicit constexpr PredicateNode(Tegra::Shader::Pred index_, bool negated_) : index{index_}, negated{negated_} {} - Tegra::Shader::Pred GetIndex() const { + [[nodiscard]] constexpr Tegra::Shader::Pred GetIndex() const { return index; } - bool IsNegated() const { + [[nodiscard]] constexpr bool IsNegated() const { return negated; } @@ -577,23 +577,23 @@ public: explicit AbufNode(Node physical_address_, Node buffer_ = {}) : physical_address{std::move(physical_address_)}, buffer{std::move(buffer_)} {} - Tegra::Shader::Attribute::Index GetIndex() const { + [[nodiscard]] Tegra::Shader::Attribute::Index GetIndex() const { return index; } - u32 GetElement() const { + [[nodiscard]] u32 GetElement() const { return element; } - const Node& GetBuffer() const { + [[nodiscard]] const Node& GetBuffer() const { return buffer; } - bool IsPhysicalBuffer() const { + [[nodiscard]] bool IsPhysicalBuffer() const { return static_cast(physical_address); } - const Node& GetPhysicalAddress() const { + [[nodiscard]] const Node& GetPhysicalAddress() const { return physical_address; } @@ -607,9 +607,9 @@ private: /// Patch memory (used to communicate tessellation stages). class PatchNode final { public: - explicit PatchNode(u32 offset_) : offset{offset_} {} + explicit constexpr PatchNode(u32 offset_) : offset{offset_} {} - u32 GetOffset() const { + [[nodiscard]] constexpr u32 GetOffset() const { return offset; } @@ -622,11 +622,11 @@ class CbufNode final { public: explicit CbufNode(u32 index_, Node offset_) : index{index_}, offset{std::move(offset_)} {} - u32 GetIndex() const { + [[nodiscard]] u32 GetIndex() const { return index; } - const Node& GetOffset() const { + [[nodiscard]] const Node& GetOffset() const { return offset; } @@ -640,7 +640,7 @@ class LmemNode final { public: explicit LmemNode(Node address_) : address{std::move(address_)} {} - const Node& GetAddress() const { + [[nodiscard]] const Node& GetAddress() const { return address; } @@ -653,7 +653,7 @@ class SmemNode final { public: explicit SmemNode(Node address_) : address{std::move(address_)} {} - const Node& GetAddress() const { + [[nodiscard]] const Node& GetAddress() const { return address; } @@ -668,15 +668,15 @@ public: : real_address{std::move(real_address_)}, base_address{std::move(base_address_)}, descriptor{descriptor_} {} - const Node& GetRealAddress() const { + [[nodiscard]] const Node& GetRealAddress() const { return real_address; } - const Node& GetBaseAddress() const { + [[nodiscard]] const Node& GetBaseAddress() const { return base_address; } - const GlobalMemoryBase& GetDescriptor() const { + [[nodiscard]] const GlobalMemoryBase& GetDescriptor() const { return descriptor; } @@ -691,7 +691,7 @@ class CommentNode final { public: explicit CommentNode(std::string text_) : text{std::move(text_)} {} - const std::string& GetText() const { + [[nodiscard]] const std::string& GetText() const { return text; }