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 <nanley.g.chery@intel.com>
Tested-by: Marge Bot <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4174>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4174>

diff --git a/src/intel/isl/isl.h b/src/intel/isl/isl.h
index aabf980b86a6e48adbcbf4b17d8b1b047d4a35ff..3643dab97904ed5ccc273748f13632e297fa9fb4 100644 (file)
--- 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 1155fee03258e909abf9de5555e05a8b0c013a8e..6d12f3f4e55502e390b98c7848fd1ac3c2621762 100644 (file)
--- 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 7d3e4ffc9273bcd292bf0145892e2aec613ef438..91447614c9fb46e821483ea4319acb4f9806e5c4 100644 (file)
--- 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 47e41d8b8c61a8df2480d96825cd7e0d43c91ee8..d31d0434fdded608bf020b02a4e0419328799859 100644 (file)
--- 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);