Yeah, talking about this with robin offline for this fix.
> Yes, but what we set tieable is e.g. V4QI and V2SF.
That comes from different code lines.
Jeff would like to learn more about extract_low_bits, it will first convert to
int_mode and then call the tieable_p.
And I bet the V4QI and V2SF comes from the if condition for gen_lowpart.
--- a/gcc/dse.cc
+++ b/gcc/dse.cc
@@ -1946,7 +1946,9 @@ get_stored_val (store_info *store_info, machine_mode
read_mode,
copy_rtx (store_info->const_rhs));
else if (VECTOR_MODE_P (read_mode) && VECTOR_MODE_P (store_mode)
&& known_le (GET_MODE_BITSIZE (read_mode), GET_MODE_BITSIZE (store_mode))
- && targetm.modes_tieable_p (read_mode, store_mode)) // <= V4QI and V2SF
here.
+ && targetm.modes_tieable_p (read_mode, store_mode)
+ && validate_subreg (read_mode, store_mode, copy_rtx (store_info->rhs),
+ subreg_lowpart_offset (read_mode, store_mode)))
read_reg = gen_lowpart (read_mode, copy_rtx (store_info->rhs));
else
read_reg = extract_low_bits (read_mode, store_mode,
Pan
-----Original Message-----
From: Robin Dapp <rdapp....@gmail.com>
Sent: Thursday, February 29, 2024 9:29 PM
To: Li, Pan2 <pan2...@intel.com>; Jeff Law <jeffreya...@gmail.com>;
gcc-patches@gcc.gnu.org
Cc: rdapp....@gmail.com; juzhe.zh...@rivai.ai; kito.ch...@gmail.com;
richard.guent...@gmail.com; Wang, Yanzhang <yanzhang.w...@intel.com>; Liu,
Hongtao <hongtao....@intel.com>
Subject: Re: [PATCH v2] DSE: Bugfix ICE after allow vector type in
get_stored_val
On 2/29/24 02:38, Li, Pan2 wrote:
>> So it's going to check if V2SF can be tied to DI and V4QI with SI. I
>> suspect those are going to fail for RISC-V as those aren't tieable.
>
> Yes, you are right. Different REG_CLASS are not allowed to be tieable in
> RISC-V.
>
> static bool
> riscv_modes_tieable_p (machine_mode mode1, machine_mode mode2)
> {
> /* We don't allow different REG_CLASS modes tieable since it
> will cause ICE in register allocation (RA).
> E.g. V2SI and DI are not tieable. */
> if (riscv_v_ext_mode_p (mode1) != riscv_v_ext_mode_p (mode2))
> return false;
> return (mode1 == mode2
> || !(GET_MODE_CLASS (mode1) == MODE_FLOAT
> && GET_MODE_CLASS (mode2) == MODE_FLOAT));
> }
Yes, but what we set tieable is e.g. V4QI and V2SF.
I suggested a target band-aid before:
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 799d7919a4a..982ca1a4250 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -8208,6 +8208,11 @@ riscv_modes_tieable_p (machine_mode mode1, machine_mode
mode2)
E.g. V2SI and DI are not tieable. */
if (riscv_v_ext_mode_p (mode1) != riscv_v_ext_mode_p (mode2))
return false;
+ if (GET_MODE_CLASS (GET_MODE_INNER (mode1)) == MODE_INT
+ && GET_MODE_CLASS (GET_MODE_INNER (mode2)) == MODE_FLOAT
+ && GET_MODE_SIZE (GET_MODE_INNER (mode1))
+ != GET_MODE_SIZE (GET_MODE_INNER (mode2)))
+ return false;
return (mode1 == mode2
|| !(GET_MODE_CLASS (mode1) == MODE_FLOAT
&& GET_MODE_CLASS (mode2) == MODE_FLOAT));
but I don't like that as it just works around something
that I didn't even understand fully...
Regards
Robin
