* [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
* [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 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: [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 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
* 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).