Re: [PATCH 2/2] RISC-V: Move mode assertion out of conditional branch in emit_insn

[Edwin Lu]

On 6/12/2024 12:39 AM, Robin Dapp wrote:
Hi Edwin,
this LGTM but I just remembered I intended to turn the assert
into a more descriptive error.

The attached patch has been sitting on my local branch for a
while.  Maybe we should rather fold yours into it?

That's fine with me! Having more descriptive errors would have helped a 
lot during the triaging/debugging step :)

Edwin

Regards
  Robin

 From d164403ef577917f905c1690f2199fab330f05e2 Mon Sep 17 00:00:00 2001
From: Robin Dapp <rd...@ventanamicro.com>
Date: Fri, 31 May 2024 14:51:17 +0200
Subject: [PATCH] RISC-V: Use descriptive errors instead of asserts.

In emit_insn we forestall possible ICEs in maybe_legitimize_operand by
asserting.  This patch replaces the assertions by more descriptive
internal errors.

gcc/ChangeLog:

        * config/riscv/riscv-v.cc: Replace asserts by internal errors.
---
  gcc/config/riscv/riscv-v.cc | 27 ++++++++++++++++++++++-----
  1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/gcc/config/riscv/riscv-v.cc b/gcc/config/riscv/riscv-v.cc
index 8911f5783c8..810203b8ba5 100644
--- a/gcc/config/riscv/riscv-v.cc
+++ b/gcc/config/riscv/riscv-v.cc
@@ -50,6 +50,7 @@
  #include "rtx-vector-builder.h"
  #include "targhooks.h"
  #include "predict.h"
+#include "errors.h"
  
  using namespace riscv_vector;
  
@@ -291,10 +292,20 @@ public:
        if (mode == VOIDmode)
          mode = Pmode;
        else
-         /* Early assertion ensures same mode since maybe_legitimize_operand
-            will check this.  */
-         gcc_assert (GET_MODE (ops[opno]) == VOIDmode
-                     || GET_MODE (ops[opno]) == mode);
+         {
+           /* Early assertion ensures same mode since maybe_legitimize_operand
+              will check this.  */
+           machine_mode required_mode = GET_MODE (ops[opno]);
+           if (required_mode != VOIDmode && required_mode != mode)
+             {
+               internal_error ("expected mode %s for operand %d of "
+                               "insn %s but got mode %s.\n",
+                               GET_MODE_NAME (mode),
+                               opno,
+                               insn_data[(int) icode].name,
+                               GET_MODE_NAME (required_mode));
+             }
+         }
  
  	add_input_operand (ops[opno], mode);
        }
@@ -346,7 +357,13 @@ public:
      else if (m_insn_flags & VXRM_RDN_P)
        add_rounding_mode_operand (VXRM_RDN);
  
-    gcc_assert (insn_data[(int) icode].n_operands == m_opno);
+
+    if (insn_data[(int) icode].n_operands != m_opno)
+      internal_error ("invalid number of operands for insn %s, "
+                     "expected %d but got %d.\n",
+                     insn_data[(int) icode].name,
+                     insn_data[(int) icode].n_operands, m_opno);
+
      expand (icode, any_mem_p);
    }