Perform the override check early, so that it is still done
even when we decide to discard an unreachable cpreg.

Use assert not printf+abort.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/helper.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index f1fbbdb9e0..2ed07795d8 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -8538,6 +8538,14 @@ static void add_cpreg_to_hashtable(ARMCPU *cpu, const ARMCPRegInfo *r,
         g_assert_not_reached();
     }
 
+    /* Overriding of an existing definition must be explicitly requested. */
+    if (!(r->type & ARM_CP_OVERRIDE)) {
+        const ARMCPRegInfo *oldreg = get_arm_cp_reginfo(cpu->cp_regs, key);
+        if (oldreg) {
+            assert(oldreg->type & ARM_CP_OVERRIDE);
+        }
+    }
+
     /* Combine cpreg and name into one allocation. */
     name_len = strlen(name) + 1;
     r2 = g_malloc(sizeof(*r2) + name_len);
@@ -8622,20 +8630,6 @@ static void add_cpreg_to_hashtable(ARMCPU *cpu, const ARMCPRegInfo *r,
         assert(!raw_accessors_invalid(r2));
     }
 
-    /* Overriding of an existing definition must be explicitly
-     * requested.
-     */
-    if (!(r->type & ARM_CP_OVERRIDE)) {
-        const ARMCPRegInfo *oldreg = get_arm_cp_reginfo(cpu->cp_regs, key);
-        if (oldreg && !(oldreg->type & ARM_CP_OVERRIDE)) {
-            fprintf(stderr, "Register redefined: cp=%d %d bit "
-                    "crn=%d crm=%d opc1=%d opc2=%d, "
-                    "was %s, now %s\n", r2->cp, 32 + 32 * is64,
-                    r2->crn, r2->crm, r2->opc1, r2->opc2,
-                    oldreg->name, r2->name);
-            g_assert_not_reached();
-        }
-    }
     g_hash_table_insert(cpu->cp_regs, (gpointer)(uintptr_t)key, r2);
 }
 
-- 
2.34.1

On Sun, 1 May 2022 at 07:17, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Perform the override check early, so that it is still done
> even when we decide to discard an unreachable cpreg.
>
> Use assert not printf+abort.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/helper.c | 22 ++++++++--------------
>  1 file changed, 8 insertions(+), 14 deletions(-)

The reason I put the printf in here originally was that I
felt the conditionality with which we define cpregs was
complicated enough that conflicting cpreg definitions might
be found not by the developer writing new code but perhaps
by users in the field who used rarer CPU types or oddball
feature enable combinations, and I wanted the reports to
provide enough detail to identify the conflicting registers
without having to repro running under a debugger to print out
the values of oldreg and r2.

Still, I suppose it's N years later and we haven't in fact
particularly had that kind of bug report, so it's not strictly
necessary.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM