From 6c71858c5c698a718045ba44eda495080417a84b Mon Sep 17 00:00:00 2001 From: MerryMage Date: Thu, 11 Feb 2016 17:41:15 +0000 Subject: [PATCH] BitField: Make trivially copyable and remove assignment operator --- src/common/bit_field.h | 44 ++++++++++------------ src/common/emu_window.cpp | 4 +- src/core/hle/kernel/process.cpp | 2 +- src/core/hle/result.h | 8 ++-- src/core/hle/service/cfg/cfg.cpp | 6 +-- src/core/hle/service/gsp_gpu.cpp | 6 +-- src/core/hle/service/hid/hid.cpp | 2 +- src/core/hle/service/ptm/ptm.cpp | 4 +- src/core/hle/service/soc_u.cpp | 12 +++--- src/core/hw/gpu.cpp | 16 ++++---- src/video_core/command_processor.cpp | 4 +- src/video_core/debug_utils/debug_utils.cpp | 8 ++-- 12 files changed, 56 insertions(+), 60 deletions(-) diff --git a/src/common/bit_field.h b/src/common/bit_field.h index 66689f398..600e0c70c 100644 --- a/src/common/bit_field.h +++ b/src/common/bit_field.h @@ -115,29 +115,24 @@ template struct BitField { private: - // This constructor might be considered ambiguous: - // Would it initialize the storage or just the bitfield? - // Hence, delete it. Use the assignment operator to set bitfield values! - BitField(T val) = delete; + // We hide the copy assigment operator here, because the default copy + // assignment would copy the full storage value, rather than just the bits + // relevant to this particular bit field. + // We don't delete it because we want BitField to be trivially copyable. + BitField& operator=(const BitField&) = default; public: + // This constructor and assignment operator might be considered ambiguous: + // Would they initialize the storage or just the bitfield? + // Hence, delete them. Use the Assign method to set bitfield values! + BitField(T val) = delete; + BitField& operator=(T val) = delete; + // Force default constructor to be created // so that we can use this within unions BitField() = default; - // We explicitly delete the copy assigment operator here, because the - // default copy assignment would copy the full storage value, rather than - // just the bits relevant to this particular bit field. - BitField& operator=(const BitField&) = delete; - - FORCE_INLINE BitField& operator=(T val) - { - Assign(val); - return *this; - } - - FORCE_INLINE operator T() const - { + FORCE_INLINE operator T() const { return Value(); } @@ -145,8 +140,7 @@ public: storage = (storage & ~GetMask()) | (((StorageType)value << position) & GetMask()); } - FORCE_INLINE T Value() const - { + FORCE_INLINE T Value() const { if (std::numeric_limits::is_signed) { std::size_t shift = 8 * sizeof(T)-bits; @@ -159,8 +153,7 @@ public: } // TODO: we may want to change this to explicit operator bool() if it's bug-free in VS2015 - FORCE_INLINE bool ToBool() const - { + FORCE_INLINE bool ToBool() const { return Value() != 0; } @@ -176,8 +169,7 @@ private: // Unsigned version of StorageType typedef typename std::make_unsigned::type StorageTypeU; - FORCE_INLINE StorageType GetMask() const - { + FORCE_INLINE StorageType GetMask() const { return (((StorageTypeU)~0) >> (8 * sizeof(T)-bits)) << position; } @@ -189,6 +181,10 @@ private: static_assert(position < 8 * sizeof(T), "Invalid position"); static_assert(bits <= 8 * sizeof(T), "Invalid number of bits"); static_assert(bits > 0, "Invalid number of bits"); - static_assert(std::is_standard_layout::value, "Invalid base type"); + static_assert(std::is_pod::value, "Invalid base type"); }; #pragma pack() + +#if (__GNUC__ >= 5) || defined __clang__ || defined _MSC_VER +static_assert(std::is_trivially_copyable>::value, "BitField must be trivially copyable"); +#endif diff --git a/src/common/emu_window.cpp b/src/common/emu_window.cpp index b69b05cb9..b2807354a 100644 --- a/src/common/emu_window.cpp +++ b/src/common/emu_window.cpp @@ -55,14 +55,14 @@ void EmuWindow::TouchPressed(unsigned framebuffer_x, unsigned framebuffer_y) { (framebuffer_layout.bottom_screen.bottom - framebuffer_layout.bottom_screen.top); touch_pressed = true; - pad_state.touch = 1; + pad_state.touch.Assign(1); } void EmuWindow::TouchReleased() { touch_pressed = false; touch_x = 0; touch_y = 0; - pad_state.touch = 0; + pad_state.touch.Assign(0); } void EmuWindow::TouchMoved(unsigned framebuffer_x, unsigned framebuffer_y) { diff --git a/src/core/hle/kernel/process.cpp b/src/core/hle/kernel/process.cpp index d148efde2..16eb972fb 100644 --- a/src/core/hle/kernel/process.cpp +++ b/src/core/hle/kernel/process.cpp @@ -35,7 +35,7 @@ SharedPtr Process::Create(SharedPtr code_set) { process->codeset = std::move(code_set); process->flags.raw = 0; - process->flags.memory_region = MemoryRegion::APPLICATION; + process->flags.memory_region.Assign(MemoryRegion::APPLICATION); Memory::InitLegacyAddressSpace(process->vm_manager); return process; diff --git a/src/core/hle/result.h b/src/core/hle/result.h index ea3abb5f6..0fce5988b 100644 --- a/src/core/hle/result.h +++ b/src/core/hle/result.h @@ -193,10 +193,10 @@ union ResultCode { explicit ResultCode(u32 raw) : raw(raw) {} ResultCode(ErrorDescription description_, ErrorModule module_, ErrorSummary summary_, ErrorLevel level_) : raw(0) { - description = description_; - module = module_; - summary = summary_; - level = level_; + description.Assign(description_); + module.Assign(module_); + summary.Assign(summary_); + level.Assign(level_); } ResultCode& operator=(const ResultCode& o) { raw = o.raw; return *this; } diff --git a/src/core/hle/service/cfg/cfg.cpp b/src/core/hle/service/cfg/cfg.cpp index 633fe19eb..7556aa6a5 100644 --- a/src/core/hle/service/cfg/cfg.cpp +++ b/src/core/hle/service/cfg/cfg.cpp @@ -293,8 +293,8 @@ ResultCode DeleteConfigNANDSaveFile() { ResultCode UpdateConfigNANDSavegame() { FileSys::Mode mode = {}; - mode.write_flag = 1; - mode.create_flag = 1; + mode.write_flag.Assign(1); + mode.create_flag.Assign(1); FileSys::Path path("config"); @@ -405,7 +405,7 @@ void Init() { FileSys::Path config_path("config"); FileSys::Mode open_mode = {}; - open_mode.read_flag = 1; + open_mode.read_flag.Assign(1); auto config_result = Service::FS::OpenFileFromArchive(*archive_result, config_path, open_mode); diff --git a/src/core/hle/service/gsp_gpu.cpp b/src/core/hle/service/gsp_gpu.cpp index 98b11c798..5838b6d71 100644 --- a/src/core/hle/service/gsp_gpu.cpp +++ b/src/core/hle/service/gsp_gpu.cpp @@ -347,7 +347,7 @@ void SignalInterrupt(InterruptId interrupt_id) { FrameBufferUpdate* info = GetFrameBufferInfo(thread_id, screen_id); if (info->is_dirty) { SetBufferSwap(screen_id, info->framebuffer_info[info->index]); - info->is_dirty = false; + info->is_dirty.Assign(false); } } } @@ -499,7 +499,7 @@ static void SetLcdForceBlack(Service::Interface* self) { // Since data is already zeroed, there is no need to explicitly set // the color to black (all zero). - data.is_enabled = enable_black; + data.is_enabled.Assign(enable_black); LCD::Write(HW::VADDR_LCD + 4 * LCD_REG_INDEX(color_fill_top), data.raw); // Top LCD LCD::Write(HW::VADDR_LCD + 4 * LCD_REG_INDEX(color_fill_bottom), data.raw); // Bottom LCD @@ -521,7 +521,7 @@ static void TriggerCmdReqQueue(Service::Interface* self) { ExecuteCommand(command_buffer->commands[i], thread_id); // Indicates that command has completed - command_buffer->number_commands = command_buffer->number_commands - 1; + command_buffer->number_commands.Assign(command_buffer->number_commands - 1); } } diff --git a/src/core/hle/service/hid/hid.cpp b/src/core/hle/service/hid/hid.cpp index 0bed0ce36..11d7e69a1 100644 --- a/src/core/hle/service/hid/hid.cpp +++ b/src/core/hle/service/hid/hid.cpp @@ -105,7 +105,7 @@ void Update() { bool pressed = false; std::tie(touch_entry->x, touch_entry->y, pressed) = VideoCore::g_emu_window->GetTouchState(); - touch_entry->valid = pressed ? 1 : 0; + touch_entry->valid.Assign(pressed ? 1 : 0); // TODO(bunnei): We're not doing anything with offset 0xA8 + 0x18 of HID SharedMemory, which // supposedly is "Touch-screen entry, which contains the raw coordinate data prior to being diff --git a/src/core/hle/service/ptm/ptm.cpp b/src/core/hle/service/ptm/ptm.cpp index 22c1093ff..6bdee4d9e 100644 --- a/src/core/hle/service/ptm/ptm.cpp +++ b/src/core/hle/service/ptm/ptm.cpp @@ -110,8 +110,8 @@ void Init() { FileSys::Path gamecoin_path("gamecoin.dat"); FileSys::Mode open_mode = {}; - open_mode.write_flag = 1; - open_mode.create_flag = 1; + open_mode.write_flag.Assign(1); + open_mode.create_flag.Assign(1); // Open the file and write the default gamecoin information auto gamecoin_result = Service::FS::OpenFileFromArchive(*archive_result, gamecoin_path, open_mode); if (gamecoin_result.Succeeded()) { diff --git a/src/core/hle/service/soc_u.cpp b/src/core/hle/service/soc_u.cpp index 822b093f4..e603bf794 100644 --- a/src/core/hle/service/soc_u.cpp +++ b/src/core/hle/service/soc_u.cpp @@ -178,17 +178,17 @@ struct CTRPollFD { static Events TranslateTo3DS(u32 input_event) { Events ev = {}; if (input_event & POLLIN) - ev.pollin = 1; + ev.pollin.Assign(1); if (input_event & POLLPRI) - ev.pollpri = 1; + ev.pollpri.Assign(1); if (input_event & POLLHUP) - ev.pollhup = 1; + ev.pollhup.Assign(1); if (input_event & POLLERR) - ev.pollerr = 1; + ev.pollerr.Assign(1); if (input_event & POLLOUT) - ev.pollout = 1; + ev.pollout.Assign(1); if (input_event & POLLNVAL) - ev.pollnval = 1; + ev.pollnval.Assign(1); return ev; } diff --git a/src/core/hw/gpu.cpp b/src/core/hw/gpu.cpp index 4bd3a632d..c60310586 100644 --- a/src/core/hw/gpu.cpp +++ b/src/core/hw/gpu.cpp @@ -146,8 +146,8 @@ inline void Write(u32 addr, const T data) { // Reset "trigger" flag and set the "finish" flag // NOTE: This was confirmed to happen on hardware even if "address_start" is zero. - config.trigger = 0; - config.finished = 1; + config.trigger.Assign(0); + config.finished.Assign(1); } break; } @@ -444,16 +444,16 @@ void Init() { framebuffer_sub.address_left1 = 0x1848F000; framebuffer_sub.address_left2 = 0x184C7800; - framebuffer_top.width = 240; - framebuffer_top.height = 400; + framebuffer_top.width.Assign(240); + framebuffer_top.height.Assign(400); framebuffer_top.stride = 3 * 240; - framebuffer_top.color_format = Regs::PixelFormat::RGB8; + framebuffer_top.color_format.Assign(Regs::PixelFormat::RGB8); framebuffer_top.active_fb = 0; - framebuffer_sub.width = 240; - framebuffer_sub.height = 320; + framebuffer_sub.width.Assign(240); + framebuffer_sub.height.Assign(320); framebuffer_sub.stride = 3 * 240; - framebuffer_sub.color_format = Regs::PixelFormat::RGB8; + framebuffer_sub.color_format.Assign(Regs::PixelFormat::RGB8); framebuffer_sub.active_fb = 0; last_skip_frame = false; diff --git a/src/video_core/command_processor.cpp b/src/video_core/command_processor.cpp index 5dfedfe31..ed20057b5 100644 --- a/src/video_core/command_processor.cpp +++ b/src/video_core/command_processor.cpp @@ -429,7 +429,7 @@ static void WritePicaReg(u32 id, u32 value, u32 mask) { uniform.w.ToFloat32()); // TODO: Verify that this actually modifies the register! - uniform_setup.index = uniform_setup.index + 1; + uniform_setup.index.Assign(uniform_setup.index + 1); } break; } @@ -478,7 +478,7 @@ static void WritePicaReg(u32 id, u32 value, u32 mask) { ASSERT_MSG(lut_config.index < 256, "lut_config.index exceeded maximum value of 255!"); g_state.lighting.luts[lut_config.type][lut_config.index].raw = value; - lut_config.index = lut_config.index + 1; + lut_config.index.Assign(lut_config.index + 1); break; } diff --git a/src/video_core/debug_utils/debug_utils.cpp b/src/video_core/debug_utils/debug_utils.cpp index 4f66dbd65..6e6fd7335 100644 --- a/src/video_core/debug_utils/debug_utils.cpp +++ b/src/video_core/debug_utils/debug_utils.cpp @@ -201,11 +201,11 @@ void DumpShader(const std::string& filename, const Regs::ShaderConfig& config, c if (it == output_info_table.end()) { output_info_table.emplace_back(); - output_info_table.back().type = type; - output_info_table.back().component_mask = component_mask; - output_info_table.back().id = i; + output_info_table.back().type.Assign(type); + output_info_table.back().component_mask.Assign(component_mask); + output_info_table.back().id.Assign(i); } else { - it->component_mask = it->component_mask | component_mask; + it->component_mask.Assign(it->component_mask | component_mask); } } catch (const std::out_of_range& ) { DEBUG_ASSERT_MSG(false, "Unknown output attribute mapping");