From 084d7d6b014443be7625fb9d8f1ddd309a22f6f4 Mon Sep 17 00:00:00 2001 From: Liam Date: Tue, 7 Jun 2022 17:02:29 -0400 Subject: common: Change semantics of UNREACHABLE to unconditionally crash --- src/common/assert.cpp | 5 +++++ src/common/assert.h | 14 ++++++++++++-- src/common/settings.cpp | 2 +- 3 files changed, 18 insertions(+), 3 deletions(-) (limited to 'src/common') diff --git a/src/common/assert.cpp b/src/common/assert.cpp index b44570528..a27a025ae 100644 --- a/src/common/assert.cpp +++ b/src/common/assert.cpp @@ -11,3 +11,8 @@ void assert_handle_failure() { Crash(); } } + +[[noreturn]] void unreachable_impl() { + Crash(); + throw std::runtime_error("Unreachable code"); +} diff --git a/src/common/assert.h b/src/common/assert.h index dbfd8abaf..478bfa856 100644 --- a/src/common/assert.h +++ b/src/common/assert.h @@ -11,6 +11,8 @@ // everywhere. So let's just move the handling of the failed assert to a single cpp file. void assert_handle_failure(); +[[noreturn]] void unreachable_impl(); + // For asserts we'd like to keep all the junk executed when an assert happens away from the // important code in the function. One way of doing this is to put all the relevant code inside a // lambda and force the compiler to not inline it. Unfortunately, MSVC seems to have no syntax to @@ -44,9 +46,17 @@ assert_noinline_call(const Fn& fn) { } \ while (0) -#define UNREACHABLE() assert_noinline_call([] { LOG_CRITICAL(Debug, "Unreachable code!"); }) +#define UNREACHABLE() \ + do { \ + assert_noinline_call([] { LOG_CRITICAL(Debug, "Unreachable code!"); }); \ + unreachable_impl(); \ + } while (0) + #define UNREACHABLE_MSG(...) \ - assert_noinline_call([&] { LOG_CRITICAL(Debug, "Unreachable code!\n" __VA_ARGS__); }) + do { \ + assert_noinline_call([&] { LOG_CRITICAL(Debug, "Unreachable code!\n" __VA_ARGS__); }); \ + unreachable_impl(); \ + } while (0) #ifdef _DEBUG #define DEBUG_ASSERT(_a_) ASSERT(_a_) diff --git a/src/common/settings.cpp b/src/common/settings.cpp index 6ffab63af..751549583 100644 --- a/src/common/settings.cpp +++ b/src/common/settings.cpp @@ -147,7 +147,7 @@ void UpdateRescalingInfo() { info.down_shift = 0; break; default: - UNREACHABLE(); + ASSERT(false); info.up_scale = 1; info.down_shift = 0; } -- cgit v1.2.3 From 58fea44eb5bfe268c1ddd2ea063744eb7bbe7e44 Mon Sep 17 00:00:00 2001 From: Liam Date: Tue, 7 Jun 2022 18:05:32 -0400 Subject: common: Don't test ASSERT conditions inline --- src/common/assert.cpp | 10 ++++++--- src/common/assert.h | 58 +++++++++++++++++++++++++-------------------------- 2 files changed, 36 insertions(+), 32 deletions(-) (limited to 'src/common') diff --git a/src/common/assert.cpp b/src/common/assert.cpp index a27a025ae..b20c19123 100644 --- a/src/common/assert.cpp +++ b/src/common/assert.cpp @@ -6,9 +6,13 @@ #include "common/settings.h" -void assert_handle_failure() { - if (Settings::values.use_debug_asserts) { - Crash(); +void assert_check_condition(bool cond, std::function&& on_failure) { + if (!cond) { + on_failure(); + + if (Settings::values.use_debug_asserts) { + Crash(); + } } } diff --git a/src/common/assert.h b/src/common/assert.h index 478bfa856..fb7808657 100644 --- a/src/common/assert.h +++ b/src/common/assert.h @@ -4,57 +4,57 @@ #pragma once +#include + #include "common/logging/log.h" // Sometimes we want to try to continue even after hitting an assert. // However touching this file yields a global recompilation as this header is included almost // everywhere. So let's just move the handling of the failed assert to a single cpp file. -void assert_handle_failure(); - -[[noreturn]] void unreachable_impl(); // For asserts we'd like to keep all the junk executed when an assert happens away from the // important code in the function. One way of doing this is to put all the relevant code inside a -// lambda and force the compiler to not inline it. Unfortunately, MSVC seems to have no syntax to -// specify __declspec on lambda functions, so what we do instead is define a noinline wrapper -// template that calls the lambda. This seems to generate an extra instruction at the call-site -// compared to the ideal implementation (which wouldn't support ASSERT_MSG parameters), but is good -// enough for our purposes. -template -#if defined(_MSC_VER) -[[msvc::noinline]] -#elif defined(__GNUC__) -[[gnu::cold, gnu::noinline]] -#endif -static void -assert_noinline_call(const Fn& fn) { - fn(); - assert_handle_failure(); -} +// lambda and force the compiler to not inline it. +void assert_check_condition(bool cond, std::function&& on_failure); + +[[noreturn]] void unreachable_impl(); #define ASSERT(_a_) \ - do \ - if (!(_a_)) { \ - assert_noinline_call([] { LOG_CRITICAL(Debug, "Assertion Failed!"); }); \ + do { \ + if (std::is_constant_evaluated()) { \ + if (!(_a_)) { \ + /* Will trigger compile error here */ \ + assert_check_condition(bool(_a_), \ + [] { LOG_CRITICAL(Debug, "Assertion Failed!"); }); \ + } \ + } else { \ + assert_check_condition(bool(_a_), [] { LOG_CRITICAL(Debug, "Assertion Failed!"); }); \ } \ - while (0) + } while (0) #define ASSERT_MSG(_a_, ...) \ - do \ - if (!(_a_)) { \ - assert_noinline_call([&] { LOG_CRITICAL(Debug, "Assertion Failed!\n" __VA_ARGS__); }); \ + do { \ + if (std::is_constant_evaluated()) { \ + if (!(_a_)) { \ + /* Will trigger compile error here */ \ + assert_check_condition(bool(_a_), \ + [] { LOG_CRITICAL(Debug, "Assertion Failed!"); }); \ + } \ + } else { \ + assert_check_condition( \ + bool(_a_), [&] { LOG_CRITICAL(Debug, "Assertion Failed!\n" __VA_ARGS__); }); \ } \ - while (0) + } while (0) #define UNREACHABLE() \ do { \ - assert_noinline_call([] { LOG_CRITICAL(Debug, "Unreachable code!"); }); \ + LOG_CRITICAL(Debug, "Unreachable code!"); \ unreachable_impl(); \ } while (0) #define UNREACHABLE_MSG(...) \ do { \ - assert_noinline_call([&] { LOG_CRITICAL(Debug, "Unreachable code!\n" __VA_ARGS__); }); \ + LOG_CRITICAL(Debug, "Unreachable code!\n" __VA_ARGS__); \ unreachable_impl(); \ } while (0) -- cgit v1.2.3 From a29ddcee40d78b1c1f7f64855331888777add4f4 Mon Sep 17 00:00:00 2001 From: Liam Date: Tue, 7 Jun 2022 19:46:10 -0400 Subject: common/assert: add unlikely --- src/common/assert.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/common') diff --git a/src/common/assert.cpp b/src/common/assert.cpp index b20c19123..1a85faccf 100644 --- a/src/common/assert.cpp +++ b/src/common/assert.cpp @@ -7,7 +7,7 @@ #include "common/settings.h" void assert_check_condition(bool cond, std::function&& on_failure) { - if (!cond) { + if (!cond) [[unlikely]] { on_failure(); if (Settings::values.use_debug_asserts) { -- cgit v1.2.3 From feaf010fa2c2952eaa6659ea2decaaa2493d4bb8 Mon Sep 17 00:00:00 2001 From: Liam Date: Sun, 12 Jun 2022 18:11:02 -0400 Subject: common/assert: rework ASSERT handling to avoid std::function usage --- src/common/assert.cpp | 10 +++------- src/common/assert.h | 45 +++++++++++++++++---------------------------- 2 files changed, 20 insertions(+), 35 deletions(-) (limited to 'src/common') diff --git a/src/common/assert.cpp b/src/common/assert.cpp index 1a85faccf..6026b7dc2 100644 --- a/src/common/assert.cpp +++ b/src/common/assert.cpp @@ -6,13 +6,9 @@ #include "common/settings.h" -void assert_check_condition(bool cond, std::function&& on_failure) { - if (!cond) [[unlikely]] { - on_failure(); - - if (Settings::values.use_debug_asserts) { - Crash(); - } +void assert_fail_impl() { + if (Settings::values.use_debug_asserts) { + Crash(); } } diff --git a/src/common/assert.h b/src/common/assert.h index fb7808657..8c927fcc0 100644 --- a/src/common/assert.h +++ b/src/common/assert.h @@ -4,47 +4,36 @@ #pragma once -#include - #include "common/logging/log.h" // Sometimes we want to try to continue even after hitting an assert. // However touching this file yields a global recompilation as this header is included almost // everywhere. So let's just move the handling of the failed assert to a single cpp file. -// For asserts we'd like to keep all the junk executed when an assert happens away from the -// important code in the function. One way of doing this is to put all the relevant code inside a -// lambda and force the compiler to not inline it. -void assert_check_condition(bool cond, std::function&& on_failure); - +void assert_fail_impl(); [[noreturn]] void unreachable_impl(); +#ifdef _MSC_VER +#define YUZU_NO_INLINE __declspec(noinline) +#else +#define YUZU_NO_INLINE __attribute__((noinline)) +#endif + #define ASSERT(_a_) \ - do { \ - if (std::is_constant_evaluated()) { \ - if (!(_a_)) { \ - /* Will trigger compile error here */ \ - assert_check_condition(bool(_a_), \ - [] { LOG_CRITICAL(Debug, "Assertion Failed!"); }); \ - } \ - } else { \ - assert_check_condition(bool(_a_), [] { LOG_CRITICAL(Debug, "Assertion Failed!"); }); \ + ([&]() YUZU_NO_INLINE { \ + if (!(_a_)) [[unlikely]] { \ + LOG_CRITICAL(Debug, "Assertion Failed!"); \ + assert_fail_impl(); \ } \ - } while (0) + }()) #define ASSERT_MSG(_a_, ...) \ - do { \ - if (std::is_constant_evaluated()) { \ - if (!(_a_)) { \ - /* Will trigger compile error here */ \ - assert_check_condition(bool(_a_), \ - [] { LOG_CRITICAL(Debug, "Assertion Failed!"); }); \ - } \ - } else { \ - assert_check_condition( \ - bool(_a_), [&] { LOG_CRITICAL(Debug, "Assertion Failed!\n" __VA_ARGS__); }); \ + ([&]() YUZU_NO_INLINE { \ + if (!(_a_)) [[unlikely]] { \ + LOG_CRITICAL(Debug, "Assertion Failed!\n" __VA_ARGS__); \ + assert_fail_impl(); \ } \ - } while (0) + }()) #define UNREACHABLE() \ do { \ -- cgit v1.2.3