* [PATCH V3 0/2] Fix ICE with vwsll combine on 32bit targets @ 2024-06-17 18:33 Edwin Lu 2024-06-17 18:33 ` [PATCH V3 1/2] RISC-V: Fix vwsll combine on rv32 targets Edwin Lu 2024-06-17 18:33 ` [PATCH V3 2/2] RISC-V: Move mode assertion out of conditional branch in emit_insn Edwin Lu 0 siblings, 2 replies; 7+ messages in thread From: Edwin Lu @ 2024-06-17 18:33 UTC (permalink / raw) To: gcc-patches; +Cc: gnu-toolchain, Edwin Lu The following testcases have been failing on rv32 targets since r15-953-gaf4bf422a69: FAIL: gcc.target/riscv/rvv/autovec/binop/vwsll-1.c (internal compiler error: in maybe_legitimize_operand, at optabs.cc:8056) FAIL: gcc.target/riscv/rvv/autovec/binop/vwsll-1.c (test for excess errors) Fix the bug and also robustify our emit_insn by making an assertion check unconditional I'm not sure if this ICE warrants its own separate testcase since it is already being tested. I do have a minimal testcase on hand if we would like to add one. V2: Remove subreg condition and change assert to internal error V3: Update the _trunc_scalar splitter as well Edwin Lu (2): RISC-V: Fix vwsll combine on rv32 targets RISC-V: Move mode assertion out of conditional branch in emit_insn gcc/config/riscv/autovec-opt.md | 6 ++---- gcc/config/riscv/riscv-v.cc | 25 +++++++++++++++++++------ 2 files changed, 21 insertions(+), 10 deletions(-) -- 2.34.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH V3 1/2] RISC-V: Fix vwsll combine on rv32 targets 2024-06-17 18:33 [PATCH V3 0/2] Fix ICE with vwsll combine on 32bit targets Edwin Lu @ 2024-06-17 18:33 ` Edwin Lu 2024-06-17 23:31 ` Jeff Law 2024-06-17 18:33 ` [PATCH V3 2/2] RISC-V: Move mode assertion out of conditional branch in emit_insn Edwin Lu 1 sibling, 1 reply; 7+ messages in thread From: Edwin Lu @ 2024-06-17 18:33 UTC (permalink / raw) To: gcc-patches; +Cc: gnu-toolchain, Edwin Lu, Robin Dapp On rv32 targets, vwsll_zext1_scalar_<mode> would trigger an ice in maybe_legitimize_instruction when zero extending a uint32 to uint64 due to a mismatch between the input operand's mode (DI) and the expanded insn operand's mode (Pmode == SI). Ensure that mode of the operands match Tested on rv32/64 gcv newlib. Letting CI perform additional testing gcc/ChangeLog: * config/riscv/autovec-opt.md: Fix mode mismatch Signed-off-by: Edwin Lu <ewlu@rivosinc.com> Co-authored-by: Robin Dapp <rdapp@ventanamicro.com> --- V2: Remove subreg check V3: Update _trunc_scalar splitter as well --- gcc/config/riscv/autovec-opt.md | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/gcc/config/riscv/autovec-opt.md b/gcc/config/riscv/autovec-opt.md index 6a2eabbd854..d7a3cfd4602 100644 --- a/gcc/config/riscv/autovec-opt.md +++ b/gcc/config/riscv/autovec-opt.md @@ -1517,8 +1517,7 @@ (define_insn_and_split "*vwsll_zext1_scalar_<mode>" "&& 1" [(const_int 0)] { - if (GET_CODE (operands[2]) == SUBREG) - operands[2] = SUBREG_REG (operands[2]); + operands[2] = gen_lowpart (Pmode, operands[2]); insn_code icode = code_for_pred_vwsll_scalar (<MODE>mode); riscv_vector::emit_vlmax_insn (icode, riscv_vector::BINARY_OP, operands); DONE; @@ -1584,8 +1583,7 @@ (define_insn_and_split "*vwsll_zext1_trunc_scalar_<mode>" "&& 1" [(const_int 0)] { - if (GET_CODE (operands[2]) == SUBREG) - operands[2] = SUBREG_REG (operands[2]); + operands[2] = gen_lowpart (Pmode, operands[2]); insn_code icode = code_for_pred_vwsll_scalar (<V_DOUBLE_TRUNC>mode); riscv_vector::emit_vlmax_insn (icode, riscv_vector::BINARY_OP, operands); DONE; -- 2.34.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V3 1/2] RISC-V: Fix vwsll combine on rv32 targets 2024-06-17 18:33 ` [PATCH V3 1/2] RISC-V: Fix vwsll combine on rv32 targets Edwin Lu @ 2024-06-17 23:31 ` Jeff Law 2024-06-18 22:34 ` [Committed " Edwin Lu 0 siblings, 1 reply; 7+ messages in thread From: Jeff Law @ 2024-06-17 23:31 UTC (permalink / raw) To: Edwin Lu, gcc-patches; +Cc: gnu-toolchain, Robin Dapp On 6/17/24 12:33 PM, Edwin Lu wrote: > On rv32 targets, vwsll_zext1_scalar_<mode> would trigger an ice in > maybe_legitimize_instruction when zero extending a uint32 to uint64 due > to a mismatch between the input operand's mode (DI) and the expanded insn > operand's mode (Pmode == SI). Ensure that mode of the operands match > > Tested on rv32/64 gcv newlib. Letting CI perform additional testing > > gcc/ChangeLog: > > * config/riscv/autovec-opt.md: Fix mode mismatch OK jeff > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Committed V3 1/2] RISC-V: Fix vwsll combine on rv32 targets 2024-06-17 23:31 ` Jeff Law @ 2024-06-18 22:34 ` Edwin Lu 0 siblings, 0 replies; 7+ messages in thread From: Edwin Lu @ 2024-06-18 22:34 UTC (permalink / raw) To: Jeff Law, gcc-patches; +Cc: gnu-toolchain, Robin Dapp Committed. Thanks! Edwin On 6/17/2024 5:31 PM, Jeff Law wrote: > > > On 6/17/24 12:33 PM, Edwin Lu wrote: >> On rv32 targets, vwsll_zext1_scalar_<mode> would trigger an ice in >> maybe_legitimize_instruction when zero extending a uint32 to uint64 due >> to a mismatch between the input operand's mode (DI) and the expanded >> insn >> operand's mode (Pmode == SI). Ensure that mode of the operands match >> >> Tested on rv32/64 gcv newlib. Letting CI perform additional testing >> >> gcc/ChangeLog: >> >> * config/riscv/autovec-opt.md: Fix mode mismatch > OK > jeff >> ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH V3 2/2] RISC-V: Move mode assertion out of conditional branch in emit_insn 2024-06-17 18:33 [PATCH V3 0/2] Fix ICE with vwsll combine on 32bit targets Edwin Lu 2024-06-17 18:33 ` [PATCH V3 1/2] RISC-V: Fix vwsll combine on rv32 targets Edwin Lu @ 2024-06-17 18:33 ` Edwin Lu 2024-06-17 23:33 ` Jeff Law 1 sibling, 1 reply; 7+ messages in thread From: Edwin Lu @ 2024-06-17 18:33 UTC (permalink / raw) To: gcc-patches; +Cc: gnu-toolchain, Edwin Lu, Robin Dapp 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(-) 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); } -- 2.34.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V3 2/2] RISC-V: Move mode assertion out of conditional branch in emit_insn 2024-06-17 18:33 ` [PATCH V3 2/2] RISC-V: Move mode assertion out of conditional branch in emit_insn Edwin Lu @ 2024-06-17 23:33 ` Jeff Law 2024-06-18 22:34 ` [Committed " Edwin Lu 0 siblings, 1 reply; 7+ messages in thread From: Jeff Law @ 2024-06-17 23:33 UTC (permalink / raw) To: Edwin Lu, gcc-patches; +Cc: gnu-toolchain, Robin Dapp 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Committed V3 2/2] RISC-V: Move mode assertion out of conditional branch in emit_insn 2024-06-17 23:33 ` Jeff Law @ 2024-06-18 22:34 ` Edwin Lu 0 siblings, 0 replies; 7+ messages in thread From: Edwin Lu @ 2024-06-18 22:34 UTC (permalink / raw) To: Jeff Law, gcc-patches; +Cc: gnu-toolchain, Robin Dapp 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 > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-06-18 22:34 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-06-17 18:33 [PATCH V3 0/2] Fix ICE with vwsll combine on 32bit targets Edwin Lu 2024-06-17 18:33 ` [PATCH V3 1/2] RISC-V: Fix vwsll combine on rv32 targets Edwin Lu 2024-06-17 23:31 ` Jeff Law 2024-06-18 22:34 ` [Committed " Edwin Lu 2024-06-17 18:33 ` [PATCH V3 2/2] RISC-V: Move mode assertion out of conditional branch in emit_insn Edwin Lu 2024-06-17 23:33 ` Jeff Law 2024-06-18 22:34 ` [Committed " Edwin Lu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).