From 58ab94c30c4283e3e4e9179a5b565208143b76fb Mon Sep 17 00:00:00 2001 From: Dentomologist Date: Mon, 21 Aug 2023 11:33:24 -0700 Subject: [PATCH] GCC: Suppress PPCSTATE_OFF invalid-offsetof warnings Modify PPCSTATE_OFF and PPCSTATE_OFF_ARRAY macros when using GCC to avoid useless log spam. Specifically, use a consteval lambda with gcc _Pragma statements to disable the -Winvalid-offsetof warning inside the macros. Each successful build (and many failing ones) on the Android buildbot generates almost 300 cases of -Winvalid-offsetof, resulting in thousands of lines of log spam per build. In addition to bloating the log filesize these spurious warnings make it harder to find actual warnings. These warnings are generated by calls to the macros PPCSTATE_OFF and PPCSTATE_OFF_ARRAY, which in turn are used by many other macros used by the JIT. The ultimate cause is that offsetof is only conditionally supported on non-standard-layout types, which includes the PowerPCState struct. To address potential questions of whether there's a better way to handle this: The obvious solution would be to modify PowerPCState so that it does have a standard layout. This is unfortunately impractical. To have a standard layout a type can only contain other types with standard layouts. None of the stl containers are guaranteed to have standard layouts, and PowerPCState contains a std::tuple and std::array. PowerPCState also contains a PowerPC::Cache and InstructionCache which themselves contain std:arrays and std::vectors. Furthermore InstructionCache derives from Cache, and a derived class can only have standard layout if at most one class in its hierarchy has a non-static data member, but both classes have such members. Making InstructionCache have a standard layout would require duplicating all the functionality of Cache so it no longer derived from it, as well as replacing the stl containers. This might require having a raw pointer to said containers, with the manual memory management that implies. All of that would be much more disruptive than would be justified to get rid of some warnings (however annoying they might be). This is compounded by the fact that PowerPCState hasn't had a standard layout for a long time, if ever, and if the PPCSTATE_OFF macros weren't working reliably it would have become obvious a long time ago. As to why I picked the lambda solution over other potential changes: - Keeping the define as-is and wrapping some gcc #pragmas around it doesn't work because the pragmas don't get included when the define is substituted to the call site. - Keeping the define as a non-lambda expression and using inline _Pragma() statements would ideally be better and works fine for msvc, but fails for GCC with "'#pragma' is not allowed here". - Turning off -Winvalid-offsetof globally for gcc would work, but there might be other contexts where offsetof is problematic and GCC seems to be the only compiler warning about it. --- .../Core/PowerPC/Jit64Common/Jit64PowerPCState.h | 15 +++++++++++++++ .../Core/PowerPC/JitArm64/JitArm64_RegCache.h | 13 +++++++++++++ 2 files changed, 28 insertions(+) diff --git a/Source/Core/Core/PowerPC/Jit64Common/Jit64PowerPCState.h b/Source/Core/Core/PowerPC/Jit64Common/Jit64PowerPCState.h index f534c853c8..692129e0ab 100644 --- a/Source/Core/Core/PowerPC/Jit64Common/Jit64PowerPCState.h +++ b/Source/Core/Core/PowerPC/Jit64Common/Jit64PowerPCState.h @@ -11,11 +11,26 @@ // We offset by 0x80 because the range of one byte memory offsets is // -0x80..0x7f. +#ifdef __GNUC__ +#define PPCSTATE_OFF(i) \ + ([]() consteval { \ + _Pragma("GCC diagnostic push") \ + _Pragma("GCC diagnostic ignored \"-Winvalid-offsetof\"") return static_cast( \ + offsetof(PowerPC::PowerPCState, i)) - \ + 0x80; \ + _Pragma("GCC diagnostic pop") \ + }()) + +#define PPCSTATE_OFF_ARRAY(elem, i) \ + (PPCSTATE_OFF(elem[0]) + static_cast(sizeof(PowerPC::PowerPCState::elem[0]) * (i))) +#else #define PPCSTATE_OFF(i) (static_cast(offsetof(PowerPC::PowerPCState, i)) - 0x80) + #define PPCSTATE_OFF_ARRAY(elem, i) \ (static_cast(offsetof(PowerPC::PowerPCState, elem[0]) + \ sizeof(PowerPC::PowerPCState::elem[0]) * (i)) - \ 0x80) +#endif #define PPCSTATE_OFF_GPR(i) PPCSTATE_OFF_ARRAY(gpr, i) #define PPCSTATE_OFF_CR(i) PPCSTATE_OFF_ARRAY(cr.fields, i) diff --git a/Source/Core/Core/PowerPC/JitArm64/JitArm64_RegCache.h b/Source/Core/Core/PowerPC/JitArm64/JitArm64_RegCache.h index a8e63eb006..01339184ea 100644 --- a/Source/Core/Core/PowerPC/JitArm64/JitArm64_RegCache.h +++ b/Source/Core/Core/PowerPC/JitArm64/JitArm64_RegCache.h @@ -25,10 +25,23 @@ constexpr Arm64Gen::ARM64Reg PPC_REG = Arm64Gen::ARM64Reg::X29; // PC register when calling the dispatcher constexpr Arm64Gen::ARM64Reg DISPATCHER_PC = Arm64Gen::ARM64Reg::W26; +#ifdef __GNUC__ +#define PPCSTATE_OFF(elem) \ + ([]() consteval { \ + _Pragma("GCC diagnostic push") \ + _Pragma("GCC diagnostic ignored \"-Winvalid-offsetof\"") return offsetof( \ + PowerPC::PowerPCState, elem); \ + _Pragma("GCC diagnostic pop") \ + }()) + +#define PPCSTATE_OFF_ARRAY(elem, i) \ + (PPCSTATE_OFF(elem[0]) + sizeof(PowerPC::PowerPCState::elem[0]) * (i)) +#else #define PPCSTATE_OFF(elem) (offsetof(PowerPC::PowerPCState, elem)) #define PPCSTATE_OFF_ARRAY(elem, i) \ (offsetof(PowerPC::PowerPCState, elem[0]) + sizeof(PowerPC::PowerPCState::elem[0]) * (i)) +#endif #define PPCSTATE_OFF_GPR(i) PPCSTATE_OFF_ARRAY(gpr, i) #define PPCSTATE_OFF_CR(i) PPCSTATE_OFF_ARRAY(cr.fields, i)