From b93a1952258ebef6319fd4f4186d704e04b3064c Mon Sep 17 00:00:00 2001 From: Matt Turner Date: Thu, 12 Mar 2020 14:44:46 -0700 Subject: [PATCH] isl: Avoid EXPECT_DEATH in unit tests EXPECT_DEATH works by forking the process and letting the forked process fail with an assertion. This process is evidently incredibly expensive, taking ~30 seconds to run the whole isl_aux_info_test on a 2.8GHz Skylake. Annoyingly all of the (expected) assertion failures also leaves lots of messages in dmesg and potentially generates lots of coredumps. Instead, avoid the expense of fork/exec by redefining assert() and unreachable() in the code we're testing to return a unit-test-only value. With this patch, the test takes ~1ms. Also, while modifying the EXPECT_EQ() calls, reverse the arguments so that the expected value comes first, as is intended. Otherwise gtest failure messages don't make much sense. Fixes: https://gitlab.freedesktop.org/mesa/mesa/issues/2567 Reviewed-by: Nanley Chery Tested-by: Marge Bot Part-of: --- src/intel/isl/isl.h | 9 +++- src/intel/isl/isl_aux_info.c | 34 ++++++++++++++ src/intel/isl/meson.build | 8 +++- src/intel/isl/tests/isl_aux_info_test.cpp | 56 ++++++----------------- 4 files changed, 61 insertions(+), 46 deletions(-) diff --git a/src/intel/isl/isl.h b/src/intel/isl/isl.h index aabf980b86a..3643dab9790 100644 --- a/src/intel/isl/isl.h +++ b/src/intel/isl/isl.h @@ -811,7 +811,10 @@ enum isl_aux_usage { * the CCS and filling it with zeros. */ enum isl_aux_state { - ISL_AUX_STATE_CLEAR = 0, +#ifdef IN_UNIT_TEST + ISL_AUX_STATE_ASSERT, +#endif + ISL_AUX_STATE_CLEAR, ISL_AUX_STATE_PARTIAL_CLEAR, ISL_AUX_STATE_COMPRESSED_CLEAR, ISL_AUX_STATE_COMPRESSED_NO_CLEAR, @@ -824,6 +827,10 @@ enum isl_aux_state { * Enum which describes explicit aux transition operations. */ enum isl_aux_op { +#ifdef IN_UNIT_TEST + ISL_AUX_OP_ASSERT, +#endif + ISL_AUX_OP_NONE, /** Fast Clear diff --git a/src/intel/isl/isl_aux_info.c b/src/intel/isl/isl_aux_info.c index 1155fee0325..6d12f3f4e55 100644 --- a/src/intel/isl/isl_aux_info.c +++ b/src/intel/isl/isl_aux_info.c @@ -23,6 +23,24 @@ #include "isl/isl.h" +#ifdef IN_UNIT_TEST +/* STATIC_ASSERT is a do { ... } while(0) statement */ +UNUSED static void static_assert_func(void) { + STATIC_ASSERT(ISL_AUX_OP_ASSERT == ((enum isl_aux_op) 0)); + STATIC_ASSERT(ISL_AUX_STATE_ASSERT == ((enum isl_aux_state) 0)); +} + +#undef unreachable +#define unreachable(str) return 0 + +#undef assert +#define assert(cond) do { \ + if (!(cond)) { \ + return 0; \ + } \ +} while (0) +#endif + /* How writes with an isl_aux_usage behave. */ enum write_behavior { /* Writes only touch the main surface. */ @@ -93,6 +111,10 @@ aux_state_possible(enum isl_aux_state state, case ISL_AUX_STATE_PASS_THROUGH: case ISL_AUX_STATE_AUX_INVALID: return true; +#ifdef IN_UNIT_TEST + case ISL_AUX_STATE_ASSERT: + break; +#endif } unreachable("Invalid aux state."); @@ -130,6 +152,10 @@ isl_aux_prepare_access(enum isl_aux_state initial_state, case ISL_AUX_STATE_AUX_INVALID: return info[usage].write_behavior == WRITES_ONLY_TOUCH_MAIN ? ISL_AUX_OP_NONE : ISL_AUX_OP_AMBIGUATE; +#ifdef IN_UNIT_TEST + case ISL_AUX_STATE_ASSERT: + break; +#endif } unreachable("Invalid aux state."); @@ -163,6 +189,10 @@ isl_aux_state_transition_aux_op(enum isl_aux_state initial_state, ISL_AUX_STATE_PASS_THROUGH : ISL_AUX_STATE_RESOLVED; case ISL_AUX_OP_AMBIGUATE: return ISL_AUX_STATE_PASS_THROUGH; +#if IN_UNIT_TEST + case ISL_AUX_OP_ASSERT: + break; +#endif } unreachable("Invalid aux op."); @@ -203,6 +233,10 @@ isl_aux_state_transition_write(enum isl_aux_state initial_state, case ISL_AUX_STATE_COMPRESSED_NO_CLEAR: case ISL_AUX_STATE_AUX_INVALID: return initial_state; +#ifdef IN_UNIT_TEST + case ISL_AUX_STATE_ASSERT: + break; +#endif } unreachable("Invalid aux state."); diff --git a/src/intel/isl/meson.build b/src/intel/isl/meson.build index 7d3e4ffc927..91447614c9f 100644 --- a/src/intel/isl/meson.build +++ b/src/intel/isl/meson.build @@ -143,10 +143,14 @@ if with_tests 'isl_aux_info', executable( 'isl_aux_info_test', - 'tests/isl_aux_info_test.cpp', + [ + 'tests/isl_aux_info_test.cpp', + 'isl_aux_info.c', + ], dependencies : [dep_m, idep_gtest, idep_mesautil], include_directories : [inc_common, inc_intel], - link_with : [libisl], + c_args : '-DIN_UNIT_TEST', + cpp_args : '-DIN_UNIT_TEST', ), suite : ['intel'], ) diff --git a/src/intel/isl/tests/isl_aux_info_test.cpp b/src/intel/isl/tests/isl_aux_info_test.cpp index 47e41d8b8c6..d31d0434fdd 100644 --- a/src/intel/isl/tests/isl_aux_info_test.cpp +++ b/src/intel/isl/tests/isl_aux_info_test.cpp @@ -24,18 +24,10 @@ #include "gtest/gtest.h" #include "isl/isl.h" -#define ISL_AUX_OP_ASSERT ((enum isl_aux_op) 100) -#define ISL_AUX_STATE_ASSERT ((enum isl_aux_state) 100) - -#ifndef NDEBUG -#define ASSERTS_ENABLED true -#else -#define ASSERTS_ENABLED false -#endif - void PrintTo(const enum isl_aux_op &op, ::std::ostream* os) { *os << (const char *[]) { + [ISL_AUX_OP_ASSERT ] = "ISL_AUX_OP_ASSERT", [ISL_AUX_OP_NONE ] = "ISL_AUX_OP_NONE", [ISL_AUX_OP_FAST_CLEAR ] = "ISL_AUX_OP_FAST_CLEAR", [ISL_AUX_OP_FULL_RESOLVE ] = "ISL_AUX_OP_FULL_RESOLVE", @@ -45,16 +37,9 @@ PrintTo(const enum isl_aux_op &op, ::std::ostream* os) { } #define E(state, usage, fc, op) \ -do { \ - if (ISL_AUX_OP_ ## op != ISL_AUX_OP_ASSERT) { \ - EXPECT_EQ(isl_aux_prepare_access(ISL_AUX_STATE_ ## state, \ - ISL_AUX_USAGE_ ## usage, fc), \ - ISL_AUX_OP_ ## op); \ - } else if (ASSERTS_ENABLED) { \ - EXPECT_DEATH(isl_aux_prepare_access(ISL_AUX_STATE_ ## state, \ - ISL_AUX_USAGE_ ## usage, fc), ""); \ - } \ -} while (0) + EXPECT_EQ(ISL_AUX_OP_ ## op, \ + isl_aux_prepare_access(ISL_AUX_STATE_ ## state, \ + ISL_AUX_USAGE_ ## usage, fc)) TEST(PrepareAccess, CompressedFalseFastClearFalsePartialResolveFalse) { E(CLEAR, NONE, false, FULL_RESOLVE); @@ -144,6 +129,7 @@ TEST(PrepareAccess, CompressedTrueFastClearTruePartialResolveTrue) { void PrintTo(const enum isl_aux_state &state, ::std::ostream* os) { *os << (const char *[]) { + [ISL_AUX_STATE_ASSERT ] = "ISL_AUX_STATE_ASSERT", [ISL_AUX_STATE_CLEAR ] = "ISL_AUX_STATE_CLEAR", [ISL_AUX_STATE_PARTIAL_CLEAR ] = "ISL_AUX_STATE_PARTIAL_CLEAR", [ISL_AUX_STATE_COMPRESSED_CLEAR ] = "ISL_AUX_STATE_COMPRESSED_CLEAR", @@ -156,18 +142,10 @@ PrintTo(const enum isl_aux_state &state, ::std::ostream* os) { #undef E #define E(state1, usage, op, state2) \ -do { \ - if (ISL_AUX_STATE_ ## state2 != ISL_AUX_STATE_ASSERT) { \ - EXPECT_EQ(isl_aux_state_transition_aux_op(ISL_AUX_STATE_ ## state1, \ - ISL_AUX_USAGE_ ## usage, \ - ISL_AUX_OP_ ## op), \ - ISL_AUX_STATE_ ## state2); \ - } else if (ASSERTS_ENABLED) { \ - EXPECT_DEATH(isl_aux_state_transition_aux_op(ISL_AUX_STATE_ ## state1, \ - ISL_AUX_USAGE_ ## usage, \ - ISL_AUX_OP_ ## op), ""); \ - } \ -} while (0) + EXPECT_EQ(ISL_AUX_STATE_ ## state2, \ + isl_aux_state_transition_aux_op(ISL_AUX_STATE_ ## state1, \ + ISL_AUX_USAGE_ ## usage, \ + ISL_AUX_OP_ ## op)) /* The usages used in each test of this suite represent all combinations of * ::fast_clear and ::full_resolves_ambiguate. @@ -344,18 +322,10 @@ TEST(StateTransitionAuxOp, Ambiguate) { #undef E #define E(state1, usage, full_surface, state2) \ -do { \ - if (ISL_AUX_STATE_ ## state2 != ISL_AUX_STATE_ASSERT) { \ - EXPECT_EQ(isl_aux_state_transition_write(ISL_AUX_STATE_ ## state1, \ - ISL_AUX_USAGE_ ## usage, \ - full_surface), \ - ISL_AUX_STATE_ ## state2); \ - } else if (ASSERTS_ENABLED) { \ - EXPECT_DEATH(isl_aux_state_transition_write(ISL_AUX_STATE_ ## state1, \ - ISL_AUX_USAGE_ ## usage, \ - full_surface), ""); \ - } \ -} while (0) + EXPECT_EQ(ISL_AUX_STATE_ ## state2, \ + isl_aux_state_transition_write(ISL_AUX_STATE_ ## state1, \ + ISL_AUX_USAGE_ ## usage, \ + full_surface)) TEST(StateTransitionWrite, WritesOnlyTouchMain) { E(CLEAR, NONE, false, ASSERT); -- 2.30.2