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

2023-09-26 Thread Richard Sandiford
钟居哲  writes:
> 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)))

Yes, but the "&& ..." needs to be on a new line because of the 80-character
limit.

> I failed to find native_encode_rtx and native_decode_rtx.

See simplify-rtx.cc.

Richard


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

2023-09-26 Thread 钟居哲
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.zh...@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  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).
 


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

2023-09-26 Thread Richard Sandiford
Juzhe-Zhong  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).


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

2023-09-25 Thread Juzhe-Zhong
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

---
 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).
-- 
2.36.3