Re: Re: [PATCH] RISC-V: Add opaque integer modes to fix ICE on DSE[PR111590]

[钟居哲 <juzhe.zhong@rivai.ai>]

[-- Attachment #1: Type: text/plain, Size: 4239 bytes --]

Thanks Richard.

Is it correct as follows ?

diff --git a/gcc/dse.cc b/gcc/dse.cc
index 8b07be17674..c58d3bf4e1b 100644
--- a/gcc/dse.cc
+++ b/gcc/dse.cc
@@ -1733,7 +1733,7 @@ find_shift_sequence (poly_int64 access_size,
   /* If a constant was stored into memory, try to simplify it here,
      otherwise the cost of the shift might preclude this optimization
      e.g. at -Os, even when no actual shift will be needed.  */
-  if (store_info->const_rhs)
+  if (store_info->const_rhs && known_le (access_size, GET_MODE_SIZE (MAX_MODE_INT)))

I failed to find native_encode_rtx and native_decode_rtx.



juzhe.zhong@rivai.ai
 
From: Richard Sandiford
Date: 2023-09-26 21:50
To: Juzhe-Zhong
CC: gcc-patches; kito.cheng; kito.cheng; jeffreyalaw; rdapp.gcc
Subject: Re: [PATCH] RISC-V: Add opaque integer modes to fix ICE on DSE[PR111590]
Juzhe-Zhong <juzhe.zhong@rivai.ai> writes:
> When doing fortran test with 'V' extension enabled on RISC-V port.
> I saw multiple ICE: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111590
>
> The root cause is on DSE:
>
> internal compiler error: in smallest_mode_for_size, at stor-layout.cc:356
> 0x1918f70 smallest_mode_for_size(poly_int<2u, unsigned long>, mode_class)
>         ../../../../gcc/gcc/stor-layout.cc:356
> 0x11f75bb smallest_int_mode_for_size(poly_int<2u, unsigned long>)
>         ../../../../gcc/gcc/machmode.h:916
> 0x3304141 find_shift_sequence
>         ../../../../gcc/gcc/dse.cc:1738
> 0x3304f1a get_stored_val
>         ../../../../gcc/gcc/dse.cc:1906
> 0x3305377 replace_read
>         ../../../../gcc/gcc/dse.cc:2010
> 0x3306226 check_mem_read_rtx
>         ../../../../gcc/gcc/dse.cc:2310
> 0x330667b check_mem_read_use
>         ../../../../gcc/gcc/dse.cc:2415
>
> After investigations, DSE is trying to do optimization like this following codes:
>
> (insn 86 85 87 9 (set (reg:V4DI 168)
>         (mem/u/c:V4DI (reg/f:DI 171) [0  S32 A128])) "bug.f90":6:18 discrim 6 1167 {*movv4di}
>      (expr_list:REG_EQUAL (const_vector:V4DI [
>                 (const_int 4 [0x4])
>                 (const_int 1 [0x1]) repeated x2
>                 (const_int 3 [0x3])
>             ])
>         (nil)))
>
> (set (mem) (reg:V4DI 168))
>
> Then it ICE on: auto new_mode = smallest_int_mode_for_size (access_size * BITS_PER_UNIT);
>
> The access_size may be 24 or 32. We don't have such integer modes with these size so it ICE.
>
> I saw both aarch64 and ARM has EI/OI/CI/XI opaque modes. 
>
> So I add it to walk around ICE on DCE, it works as all ICE are resolved.
>
> CC Richard to review to make sure I am doing the right thing to fix the bug.
>
> Hi, Richard, could you help me with this issue ? Thanks.
>
> gcc/ChangeLog:
>
> * config/riscv/riscv-modes.def (INT_MODE): Add opaque modes
 
I think it's a bug in dse.  The contract is:
 
/* Find the narrowest integer mode that contains at least SIZE bits.
   Such a mode must exist.  */
 
(emphasis on the last line).
 
The easy fix would be to add:
 
  && known_le (access_size, GET_MODE_SIZE (MAX_MODE_INT))
 
The better but more complex fix would be to make dse use native_encode_rtx/
native_decode_rtx (which IIRC didn't exist when the dse code was written).
 
Thanks,
Richard
 
 
>
> ---
>  gcc/config/riscv/riscv-modes.def | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/gcc/config/riscv/riscv-modes.def b/gcc/config/riscv/riscv-modes.def
> index e3c6ccb2809..ab86032c914 100644
> --- a/gcc/config/riscv/riscv-modes.def
> +++ b/gcc/config/riscv/riscv-modes.def
> @@ -393,6 +393,12 @@ VLS_MODES (1024); /* V1024QI  V512HI  V256SI V128DI  V512HF  V256SF V128DF */
>  VLS_MODES (2048); /* V2048QI V1024HI  V512SI V256DI V1024HF  V512SF V256DF */
>  VLS_MODES (4096); /* V4096QI V2048HI V1024SI V512DI V2048HF V1024SF V512DF */
>  
> +/* Opaque integer modes 3, 4, 6 or 8 general double registers.  */
> +INT_MODE (EI, 24);
> +INT_MODE (OI, 32);
> +INT_MODE (CI, 48);
> +INT_MODE (XI, 64);
> +
>  /* TODO: According to RISC-V 'V' ISA spec, the maximun vector length can
>     be 65536 for a single vector register which means the vector mode in
>     GCC can be maximum = 65536 * 8 bits (LMUL=8).