public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] aarch64: Fix ICE in aarch64_add_offset_1 [PR94121]
@ 2020-03-11  7:22 Jakub Jelinek
  2020-03-11  9:30 ` Kyrill Tkachov
  2020-03-12 11:27 ` Andreas Schwab
  0 siblings, 2 replies; 6+ messages in thread
From: Jakub Jelinek @ 2020-03-11  7:22 UTC (permalink / raw)
  To: Richard Earnshaw, Richard Sandiford, Kyrylo Tkachov; +Cc: gcc-patches

Hi!

abs_hwi asserts that the argument is not HOST_WIDE_INT_MIN and as the
(invalid) testcase shows, the function can be called with such an offset.
The following patch is IMHO minimal fix, absu_hwi unlike abs_hwi allows even
that value and will return (unsigned HOST_WIDE_INT) HOST_WIDE_INT_MIN
in that case.  The function then uses moffset in two spots which wouldn't
care if the value is (unsigned HOST_WIDE_INT) HOST_WIDE_INT_MIN or
HOST_WIDE_INT_MIN and wouldn't accept it (!moffset and
aarch64_uimm12_shift (moffset)), then in one spot where the signedness of
moffset does matter and using unsigned is the right thing -
moffset < 0x1000000 - and finally has code which will handle even this
value right; the assembler doesn't really care for DImode immediates if
	mov     x1, -9223372036854775808
or
	mov     x1, 9223372036854775808
is used and similarly it doesn't matter if we add or sub it in DImode.

Bootstrapped/regtested on aarch64-linux, ok for trunk?

2020-03-10  Jakub Jelinek  <jakub@redhat.com>

	PR target/94121
	* config/aarch64/aarch64.c (aarch64_add_offset_1): Use absu_hwi
	instead of abs_hwi, change moffset type to unsigned HOST_WIDE_INT.

	* gcc.dg/pr94121.c: New test.

--- gcc/config/aarch64/aarch64.c.jj	2020-02-28 17:33:03.414258503 +0100
+++ gcc/config/aarch64/aarch64.c	2020-03-10 17:01:39.435302124 +0100
@@ -3713,7 +3713,7 @@ aarch64_add_offset_1 (scalar_int_mode mo
   gcc_assert (emit_move_imm || temp1 != NULL_RTX);
   gcc_assert (temp1 == NULL_RTX || !reg_overlap_mentioned_p (temp1, src));
 
-  HOST_WIDE_INT moffset = abs_hwi (offset);
+  unsigned HOST_WIDE_INT moffset = absu_hwi (offset);
   rtx_insn *insn;
 
   if (!moffset)
--- gcc/testsuite/gcc.dg/pr94121.c.jj	2020-03-10 16:58:40.246974306 +0100
+++ gcc/testsuite/gcc.dg/pr94121.c	2020-03-10 16:58:40.246974306 +0100
@@ -0,0 +1,16 @@
+/* PR target/94121 */
+/* { dg-do compile { target pie } } */
+/* { dg-options "-O2 -fpie -w" } */
+
+#define DIFF_MAX __PTRDIFF_MAX__
+#define DIFF_MIN (-DIFF_MAX - 1)
+
+extern void foo (char *);
+extern char v[];
+
+void
+bar (void)
+{
+  char *p = v;
+  foo (&p[DIFF_MIN]);
+}

	Jakub


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] aarch64: Fix ICE in aarch64_add_offset_1 [PR94121]
  2020-03-11  7:22 [PATCH] aarch64: Fix ICE in aarch64_add_offset_1 [PR94121] Jakub Jelinek
@ 2020-03-11  9:30 ` Kyrill Tkachov
  2020-03-12 11:27 ` Andreas Schwab
  1 sibling, 0 replies; 6+ messages in thread
From: Kyrill Tkachov @ 2020-03-11  9:30 UTC (permalink / raw)
  To: Jakub Jelinek, Richard Earnshaw, Richard Sandiford; +Cc: gcc-patches

Hi Jakub,

On 3/11/20 7:22 AM, Jakub Jelinek wrote:
> Hi!
>
> abs_hwi asserts that the argument is not HOST_WIDE_INT_MIN and as the
> (invalid) testcase shows, the function can be called with such an offset.
> The following patch is IMHO minimal fix, absu_hwi unlike abs_hwi 
> allows even
> that value and will return (unsigned HOST_WIDE_INT) HOST_WIDE_INT_MIN
> in that case.  The function then uses moffset in two spots which wouldn't
> care if the value is (unsigned HOST_WIDE_INT) HOST_WIDE_INT_MIN or
> HOST_WIDE_INT_MIN and wouldn't accept it (!moffset and
> aarch64_uimm12_shift (moffset)), then in one spot where the signedness of
> moffset does matter and using unsigned is the right thing -
> moffset < 0x1000000 - and finally has code which will handle even this
> value right; the assembler doesn't really care for DImode immediates if
>         mov     x1, -9223372036854775808
> or
>         mov     x1, 9223372036854775808
> is used and similarly it doesn't matter if we add or sub it in DImode.
>
> Bootstrapped/regtested on aarch64-linux, ok for trunk?

Ok.

Thanks,

Kyrill

>
> 2020-03-10  Jakub Jelinek  <jakub@redhat.com>
>
>         PR target/94121
>         * config/aarch64/aarch64.c (aarch64_add_offset_1): Use absu_hwi
>         instead of abs_hwi, change moffset type to unsigned HOST_WIDE_INT.
>
>         * gcc.dg/pr94121.c: New test.
>
> --- gcc/config/aarch64/aarch64.c.jj     2020-02-28 17:33:03.414258503 
> +0100
> +++ gcc/config/aarch64/aarch64.c        2020-03-10 17:01:39.435302124 
> +0100
> @@ -3713,7 +3713,7 @@ aarch64_add_offset_1 (scalar_int_mode mo
>    gcc_assert (emit_move_imm || temp1 != NULL_RTX);
>    gcc_assert (temp1 == NULL_RTX || !reg_overlap_mentioned_p (temp1, 
> src));
>
> -  HOST_WIDE_INT moffset = abs_hwi (offset);
> +  unsigned HOST_WIDE_INT moffset = absu_hwi (offset);
>    rtx_insn *insn;
>
>    if (!moffset)
> --- gcc/testsuite/gcc.dg/pr94121.c.jj   2020-03-10 16:58:40.246974306 
> +0100
> +++ gcc/testsuite/gcc.dg/pr94121.c      2020-03-10 16:58:40.246974306 
> +0100
> @@ -0,0 +1,16 @@
> +/* PR target/94121 */
> +/* { dg-do compile { target pie } } */
> +/* { dg-options "-O2 -fpie -w" } */
> +
> +#define DIFF_MAX __PTRDIFF_MAX__
> +#define DIFF_MIN (-DIFF_MAX - 1)
> +
> +extern void foo (char *);
> +extern char v[];
> +
> +void
> +bar (void)
> +{
> +  char *p = v;
> +  foo (&p[DIFF_MIN]);
> +}
>
>         Jakub
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] aarch64: Fix ICE in aarch64_add_offset_1 [PR94121]
  2020-03-11  7:22 [PATCH] aarch64: Fix ICE in aarch64_add_offset_1 [PR94121] Jakub Jelinek
  2020-03-11  9:30 ` Kyrill Tkachov
@ 2020-03-12 11:27 ` Andreas Schwab
  2020-03-12 14:24   ` [PATCH] aarch64: Fix another bug " Jakub Jelinek
  1 sibling, 1 reply; 6+ messages in thread
From: Andreas Schwab @ 2020-03-12 11:27 UTC (permalink / raw)
  To: Jakub Jelinek via Gcc-patches
  Cc: Richard Earnshaw, Richard Sandiford, Kyrylo Tkachov, Jakub Jelinek

I'm getting this ICE with -mabi=ilp32:

during RTL pass: fwprop1
/opt/gcc/gcc-20200312/gcc/testsuite/gcc.dg/pr94121.c: In function 'bar':
/opt/gcc/gcc-20200312/gcc/testsuite/gcc.dg/pr94121.c:16:1: internal compiler error: in decompose, at rtl.h:2279
0xca5063 wi::int_traits<std::pair<rtx_def*, machine_mode> >::decompose(long*, unsigned int, std::pair<rtx_def*, machine_mode> const&)
	../../gcc/rtl.h:2279
0xca5063 wide_int_ref_storage<false, false>::wide_int_ref_storage<std::pair<rtx_def*, machine_mode> >(std::pair<rtx_def*, machine_mode> const&)
	../../gcc/wide-int.h:1024
0xca5063 generic_wide_int<wide_int_ref_storage<false, false> >::generic_wide_int<std::pair<rtx_def*, machine_mode> >(std::pair<rtx_def*, machine_mode> const&)
	../../gcc/wide-int.h:782
0xca5063 poly_int<2u, generic_wide_int<wide_int_ref_storage<false, false> > >::poly_int<std::pair<rtx_def*, machine_mode> >(std::pair<rtx_def*, machine_mode> const&)
	../../gcc/poly-int.h:670
0xca5063 wi::to_poly_wide(rtx_def const*, machine_mode)
	../../gcc/rtl.h:2364
0xca5063 neg_poly_int_rtx
	../../gcc/simplify-rtx.c:64
0xcab637 simplify_binary_operation_1
	../../gcc/simplify-rtx.c:2677
0xcacc87 simplify_binary_operation(rtx_code, machine_mode, rtx_def*, rtx_def*)
	../../gcc/simplify-rtx.c:2291
0xcacd33 simplify_gen_binary(rtx_code, machine_mode, rtx_def*, rtx_def*)
	../../gcc/simplify-rtx.c:189
0x163797f propagate_rtx_1
	../../gcc/fwprop.c:520
0x16384b3 propagate_rtx
	../../gcc/fwprop.c:752
0x1639b83 forward_propagate_and_simplify
	../../gcc/fwprop.c:1421
0x1639b83 forward_propagate_into
	../../gcc/fwprop.c:1490
0x163a74f fwprop
	../../gcc/fwprop.c:1580

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] aarch64: Fix another bug in aarch64_add_offset_1 [PR94121]
  2020-03-12 11:27 ` Andreas Schwab
@ 2020-03-12 14:24   ` Jakub Jelinek
  2020-03-13  8:34     ` Jakub Jelinek
  2020-03-13 10:06     ` Richard Earnshaw (lists)
  0 siblings, 2 replies; 6+ messages in thread
From: Jakub Jelinek @ 2020-03-12 14:24 UTC (permalink / raw)
  To: Richard Earnshaw, Richard Sandiford, Kyrylo Tkachov
  Cc: gcc-patches, Andreas Schwab

On Thu, Mar 12, 2020 at 12:27:48PM +0100, Andreas Schwab wrote:
> I'm getting this ICE with -mabi=ilp32:
> 
> during RTL pass: fwprop1
> /opt/gcc/gcc-20200312/gcc/testsuite/gcc.dg/pr94121.c: In function 'bar':
> /opt/gcc/gcc-20200312/gcc/testsuite/gcc.dg/pr94121.c:16:1: internal compiler error: in decompose, at rtl.h:2279

That is a preexisting issue, caused by another bug in the same function.
When mode is SImode and moffset is 0x80000000 (or anything else with the
bit 31 set), we need to sign-extend it.

Fixed thusly, ok for trunk if it passes bootstrap/regtest on aarch64-linux?

2020-03-12  Jakub Jelinek  <jakub@redhat.com>

	PR target/94121
	* config/aarch64/aarch64.c (aarch64_add_offset_1): Use gen_int_mode
	instead of GEN_INT.

--- gcc/config/aarch64/aarch64.c.jj	2020-03-12 15:05:20.610726090 +0100
+++ gcc/config/aarch64/aarch64.c	2020-03-12 15:18:35.390025244 +0100
@@ -3757,7 +3757,8 @@ aarch64_add_offset_1 (scalar_int_mode mo
   if (emit_move_imm)
     {
       gcc_assert (temp1 != NULL_RTX || can_create_pseudo_p ());
-      temp1 = aarch64_force_temporary (mode, temp1, GEN_INT (moffset));
+      temp1 = aarch64_force_temporary (mode, temp1,
+				       gen_int_mode (moffset, mode));
     }
   insn = emit_insn (offset < 0
 		    ? gen_sub3_insn (dest, src, temp1)


	Jakub


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] aarch64: Fix another bug in aarch64_add_offset_1 [PR94121]
  2020-03-12 14:24   ` [PATCH] aarch64: Fix another bug " Jakub Jelinek
@ 2020-03-13  8:34     ` Jakub Jelinek
  2020-03-13 10:06     ` Richard Earnshaw (lists)
  1 sibling, 0 replies; 6+ messages in thread
From: Jakub Jelinek @ 2020-03-13  8:34 UTC (permalink / raw)
  To: Richard Earnshaw, Richard Sandiford, Kyrylo Tkachov,
	Andreas Schwab, gcc-patches

On Thu, Mar 12, 2020 at 03:24:31PM +0100, Jakub Jelinek via Gcc-patches wrote:
> On Thu, Mar 12, 2020 at 12:27:48PM +0100, Andreas Schwab wrote:
> > I'm getting this ICE with -mabi=ilp32:
> > 
> > during RTL pass: fwprop1
> > /opt/gcc/gcc-20200312/gcc/testsuite/gcc.dg/pr94121.c: In function 'bar':
> > /opt/gcc/gcc-20200312/gcc/testsuite/gcc.dg/pr94121.c:16:1: internal compiler error: in decompose, at rtl.h:2279
> 
> That is a preexisting issue, caused by another bug in the same function.
> When mode is SImode and moffset is 0x80000000 (or anything else with the
> bit 31 set), we need to sign-extend it.
> 
> Fixed thusly, ok for trunk if it passes bootstrap/regtest on aarch64-linux?

Now successfully bootstrapped/regtested on aarch64-linux.

> 2020-03-12  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR target/94121
> 	* config/aarch64/aarch64.c (aarch64_add_offset_1): Use gen_int_mode
> 	instead of GEN_INT.
> 
> --- gcc/config/aarch64/aarch64.c.jj	2020-03-12 15:05:20.610726090 +0100
> +++ gcc/config/aarch64/aarch64.c	2020-03-12 15:18:35.390025244 +0100
> @@ -3757,7 +3757,8 @@ aarch64_add_offset_1 (scalar_int_mode mo
>    if (emit_move_imm)
>      {
>        gcc_assert (temp1 != NULL_RTX || can_create_pseudo_p ());
> -      temp1 = aarch64_force_temporary (mode, temp1, GEN_INT (moffset));
> +      temp1 = aarch64_force_temporary (mode, temp1,
> +				       gen_int_mode (moffset, mode));
>      }
>    insn = emit_insn (offset < 0
>  		    ? gen_sub3_insn (dest, src, temp1)
> 

	Jakub


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] aarch64: Fix another bug in aarch64_add_offset_1 [PR94121]
  2020-03-12 14:24   ` [PATCH] aarch64: Fix another bug " Jakub Jelinek
  2020-03-13  8:34     ` Jakub Jelinek
@ 2020-03-13 10:06     ` Richard Earnshaw (lists)
  1 sibling, 0 replies; 6+ messages in thread
From: Richard Earnshaw (lists) @ 2020-03-13 10:06 UTC (permalink / raw)
  To: Jakub Jelinek, Richard Sandiford, Kyrylo Tkachov
  Cc: Andreas Schwab, gcc-patches

On 12/03/2020 14:24, Jakub Jelinek via Gcc-patches wrote:
> On Thu, Mar 12, 2020 at 12:27:48PM +0100, Andreas Schwab wrote:
>> I'm getting this ICE with -mabi=ilp32:
>>
>> during RTL pass: fwprop1
>> /opt/gcc/gcc-20200312/gcc/testsuite/gcc.dg/pr94121.c: In function 'bar':
>> /opt/gcc/gcc-20200312/gcc/testsuite/gcc.dg/pr94121.c:16:1: internal compiler error: in decompose, at rtl.h:2279
> 
> That is a preexisting issue, caused by another bug in the same function.
> When mode is SImode and moffset is 0x80000000 (or anything else with the
> bit 31 set), we need to sign-extend it.
> 
> Fixed thusly, ok for trunk if it passes bootstrap/regtest on aarch64-linux?
> 
> 2020-03-12  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR target/94121
> 	* config/aarch64/aarch64.c (aarch64_add_offset_1): Use gen_int_mode
> 	instead of GEN_INT.

OK.

R.
> 
> --- gcc/config/aarch64/aarch64.c.jj	2020-03-12 15:05:20.610726090 +0100
> +++ gcc/config/aarch64/aarch64.c	2020-03-12 15:18:35.390025244 +0100
> @@ -3757,7 +3757,8 @@ aarch64_add_offset_1 (scalar_int_mode mo
>     if (emit_move_imm)
>       {
>         gcc_assert (temp1 != NULL_RTX || can_create_pseudo_p ());
> -      temp1 = aarch64_force_temporary (mode, temp1, GEN_INT (moffset));
> +      temp1 = aarch64_force_temporary (mode, temp1,
> +				       gen_int_mode (moffset, mode));
>       }
>     insn = emit_insn (offset < 0
>   		    ? gen_sub3_insn (dest, src, temp1)
> 
> 
> 	Jakub
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-03-13 10:06 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-11  7:22 [PATCH] aarch64: Fix ICE in aarch64_add_offset_1 [PR94121] Jakub Jelinek
2020-03-11  9:30 ` Kyrill Tkachov
2020-03-12 11:27 ` Andreas Schwab
2020-03-12 14:24   ` [PATCH] aarch64: Fix another bug " Jakub Jelinek
2020-03-13  8:34     ` Jakub Jelinek
2020-03-13 10:06     ` Richard Earnshaw (lists)

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