| Message ID | 20240617183317.2656046-3-ewlu@rivosinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | Fix ICE with vwsll combine on 32bit targets | expand |
On 6/17/24 12:33 PM, Edwin Lu wrote: > When emitting insns, we have an early assertion to ensure the input > operand's mode and the expanded operand's mode are the same; however, it > does not perform this check if the pattern does not have an explicit > machine mode specifying the operand. In this scenario, it will always > assume that mode = Pmode to correctly satisfy the > maybe_legitimize_operand check, however, there may be problems when > working in 32 bit environments. > > Make the assert unconditional and replace it with an internal error for > more descriptive logging > > gcc/ChangeLog: > > * config/riscv/riscv-v.cc: Move assert out of conditional block OK. Jeff
Thanks! Edwin On 6/17/2024 5:33 PM, Jeff Law wrote: > > > On 6/17/24 12:33 PM, Edwin Lu wrote: >> When emitting insns, we have an early assertion to ensure the input >> operand's mode and the expanded operand's mode are the same; however, it >> does not perform this check if the pattern does not have an explicit >> machine mode specifying the operand. In this scenario, it will always >> assume that mode = Pmode to correctly satisfy the >> maybe_legitimize_operand check, however, there may be problems when >> working in 32 bit environments. >> >> Make the assert unconditional and replace it with an internal error for >> more descriptive logging >> >> gcc/ChangeLog: >> >> * config/riscv/riscv-v.cc: Move assert out of conditional block > OK. > > Jeff >
diff --git a/gcc/config/riscv/riscv-v.cc b/gcc/config/riscv/riscv-v.cc index 8911f5783c8..5306711c1b7 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; @@ -290,11 +291,17 @@ public: always Pmode. */ 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 +353,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); }
When emitting insns, we have an early assertion to ensure the input operand's mode and the expanded operand's mode are the same; however, it does not perform this check if the pattern does not have an explicit machine mode specifying the operand. In this scenario, it will always assume that mode = Pmode to correctly satisfy the maybe_legitimize_operand check, however, there may be problems when working in 32 bit environments. Make the assert unconditional and replace it with an internal error for more descriptive logging gcc/ChangeLog: * config/riscv/riscv-v.cc: Move assert out of conditional block Signed-off-by: Edwin Lu <ewlu@rivosinc.com> Co-authored-by: Robin Dapp <rdapp@ventanamicro.com> --- V2: change assert to internal error V3: No change --- gcc/config/riscv/riscv-v.cc | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-)