From 5ac4636a1477fcb458f18de9781b026eb91aa1f4 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sat, 18 Apr 2020 20:51:17 -0400 Subject: [PATCH 1/2] pica_state: Make use of std::array Same behavior, stronger typing. --- src/video_core/command_processor.cpp | 5 +++-- src/video_core/pica.cpp | 9 ++++++--- src/video_core/pica_state.h | 15 +++++++++------ 3 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/video_core/command_processor.cpp b/src/video_core/command_processor.cpp index 1c633cf5c..7c5b1c4e5 100644 --- a/src/video_core/command_processor.cpp +++ b/src/video_core/command_processor.cpp @@ -31,7 +31,7 @@ namespace Pica::CommandProcessor { // Expand a 4-bit mask to 4-byte mask, e.g. 0b0101 -> 0x00FF00FF -static const u32 expand_bits_to_bytes[] = { +constexpr std::array expand_bits_to_bytes{ 0x00000000, 0x000000ff, 0x0000ff00, 0x0000ffff, 0x00ff0000, 0x00ff00ff, 0x00ffff00, 0x00ffffff, 0xff000000, 0xff0000ff, 0xff00ff00, 0xff00ffff, 0xffff0000, 0xffff00ff, 0xffffff00, 0xffffffff, }; @@ -62,7 +62,8 @@ static void WriteUniformIntReg(Shader::ShaderSetup& setup, unsigned index, } static void WriteUniformFloatReg(ShaderRegs& config, Shader::ShaderSetup& setup, - int& float_regs_counter, u32 uniform_write_buffer[4], u32 value) { + int& float_regs_counter, std::array& uniform_write_buffer, + u32 value) { auto& uniform_setup = config.uniform_setup; // TODO: Does actual hardware indeed keep an intermediate buffer or does diff --git a/src/video_core/pica.cpp b/src/video_core/pica.cpp index 5e0c2d480..d2f8874fe 100644 --- a/src/video_core/pica.cpp +++ b/src/video_core/pica.cpp @@ -3,6 +3,7 @@ // Refer to the license.txt file included. #include +#include #include "core/global.h" #include "video_core/geometry_pipeline.h" #include "video_core/pica.h" @@ -31,6 +32,8 @@ void Shutdown() { template void Zero(T& o) { + static_assert(std::is_trivially_copyable_v, + "It's undefined behavior to memset a non-trivially copyable type"); memset(&o, 0, sizeof(o)); } @@ -59,10 +62,10 @@ void State::Reset() { Zero(immediate); primitive_assembler.Reconfigure(PipelineRegs::TriangleTopology::List); vs_float_regs_counter = 0; - Zero(vs_uniform_write_buffer); + vs_uniform_write_buffer.fill(0); gs_float_regs_counter = 0; - Zero(gs_uniform_write_buffer); + gs_uniform_write_buffer.fill(0); default_attr_counter = 0; - Zero(default_attr_write_buffer); + default_attr_write_buffer.fill(0); } } // namespace Pica diff --git a/src/video_core/pica_state.h b/src/video_core/pica_state.h index 3c29ff84e..95f449676 100644 --- a/src/video_core/pica_state.h +++ b/src/video_core/pica_state.h @@ -197,13 +197,13 @@ struct State { PrimitiveAssembler primitive_assembler; int vs_float_regs_counter = 0; - u32 vs_uniform_write_buffer[4]{}; + std::array vs_uniform_write_buffer{}; int gs_float_regs_counter = 0; - u32 gs_uniform_write_buffer[4]{}; + std::array gs_uniform_write_buffer{}; int default_attr_counter = 0; - u32 default_attr_write_buffer[3]{}; + std::array default_attr_write_buffer{}; private: friend class boost::serialization::access; @@ -223,11 +223,14 @@ private: ar& geometry_pipeline; ar& primitive_assembler; ar& vs_float_regs_counter; - ar& vs_uniform_write_buffer; + ar& boost::serialization::make_array(vs_uniform_write_buffer.data(), + vs_uniform_write_buffer.size()); ar& gs_float_regs_counter; - ar& gs_uniform_write_buffer; + ar& boost::serialization::make_array(gs_uniform_write_buffer.data(), + gs_uniform_write_buffer.size()); ar& default_attr_counter; - ar& default_attr_write_buffer; + ar& boost::serialization::make_array(default_attr_write_buffer.data(), + default_attr_write_buffer.size()); boost::serialization::split_member(ar, *this, file_version); } From 41b7df4a32ebb1dc44d96ecdebe545069f98c639 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sat, 18 Apr 2020 21:00:13 -0400 Subject: [PATCH 2/2] command_processor: Resolve undefined behavior type punning We can use std::memcpy to achieve the same behavior without undefined behavior. Once Citra moves to C++20 we can convert this over to std::bit_cast. --- src/video_core/command_processor.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/video_core/command_processor.cpp b/src/video_core/command_processor.cpp index 7c5b1c4e5..e3b136091 100644 --- a/src/video_core/command_processor.cpp +++ b/src/video_core/command_processor.cpp @@ -4,6 +4,7 @@ #include #include +#include #include #include #include "common/assert.h" @@ -86,8 +87,11 @@ static void WriteUniformFloatReg(ShaderRegs& config, Shader::ShaderSetup& setup, // NOTE: The destination component order indeed is "backwards" if (uniform_setup.IsFloat32()) { - for (auto i : {0, 1, 2, 3}) - uniform[3 - i] = float24::FromFloat32(*(float*)(&uniform_write_buffer[i])); + for (auto i : {0, 1, 2, 3}) { + float buffer_value; + std::memcpy(&buffer_value, &uniform_write_buffer[i], sizeof(float)); + uniform[3 - i] = float24::FromFloat32(buffer_value); + } } else { // TODO: Untested uniform.w = float24::FromRaw(uniform_write_buffer[0] >> 8);