Fix var-tracking ICE on ARM due to backend bug (PR target/89438)

Hi!

The following testcase ICEs on arm, because the backend creates
non-canonical SImode constants (0x80000000).  CONST_INTs always need to be
sign-extended from their corresponding mode to HOST_WIDE_INT.

Fixed thusly, bootstrapped/regtested on armv7hl-linux-gnueabi, ok for trunk?

2019-02-22  Jakub Jelinek  <jakub@redhat.com>

	PR target/89438
	* config/arm.vfp.md (*negdf2_vfp): Use HOST_WIDE_INT_C (-0x80000000)
	instead of 0x80000000.
	* config/arm/neon.md (neon_copysignf<mode>): Likewise.

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


	Jakub

Hi Jakub,

On 2/22/19 10:19 PM, Jakub Jelinek wrote:
> Hi!
>
> The following testcase ICEs on arm, because the backend creates
> non-canonical SImode constants (0x80000000).  CONST_INTs always need to be
> sign-extended from their corresponding mode to HOST_WIDE_INT.
>
> Fixed thusly, bootstrapped/regtested on armv7hl-linux-gnueabi, ok for 
> trunk?
>
> 2019-02-22  Jakub Jelinek  <jakub@redhat.com>
>
>         PR target/89438
>         * config/arm.vfp.md (*negdf2_vfp): Use HOST_WIDE_INT_C 
> (-0x80000000)
>         instead of 0x80000000.
>         * config/arm/neon.md (neon_copysignf<mode>): Likewise.
>
>         * gcc.dg/pr89438.c: New test.
>
> --- gcc/config/arm/vfp.md.jj    2019-02-18 20:48:32.642732323 +0100
> +++ gcc/config/arm/vfp.md       2019-02-22 00:37:36.730795663 +0100
> @@ -871,14 +871,15 @@ (define_insn_and_split "*negdf2_vfp"
>    if (REGNO (operands[0]) == REGNO (operands[1]))
>      {
>        operands[0] = gen_highpart (SImode, operands[0]);
> -      operands[1] = gen_rtx_XOR (SImode, operands[0], GEN_INT 
> (0x80000000));
> +      operands[1] = gen_rtx_XOR (SImode, operands[0],
> +                                GEN_INT (HOST_WIDE_INT_C (-0x80000000)));


const_int generation in RTL always subtly confuses me :(

So why is 0x80000000 now -0x80000000? Shouldn't we be using 
trunc_int_for_mode?

Thanks,

Kyrill


>      }
>    else
>      {
>        rtx in_hi, in_lo, out_hi, out_lo;
>
>        in_hi = gen_rtx_XOR (SImode, gen_highpart (SImode, operands[1]),
> -                          GEN_INT (0x80000000));
> +                          GEN_INT (HOST_WIDE_INT_C (-0x80000000)));
>        in_lo = gen_lowpart (SImode, operands[1]);
>        out_hi = gen_highpart (SImode, operands[0]);
>        out_lo = gen_lowpart (SImode, operands[0]);
> --- gcc/config/arm/neon.md.jj   2019-02-07 17:33:38.840669157 +0100
> +++ gcc/config/arm/neon.md      2019-02-22 00:40:15.243249783 +0100
> @@ -3610,7 +3610,7 @@ (define_expand "neon_copysignf<mode>"
>    "{
>       rtx v_bitmask_cast;
>       rtx v_bitmask = gen_reg_rtx (<VCVTF:V_cmp_result>mode);
> -     rtx c = GEN_INT (0x80000000);
> +     rtx c = GEN_INT (HOST_WIDE_INT_C (-0x80000000));
>
>       emit_move_insn (v_bitmask,
>                       gen_const_vec_duplicate 
> (<VCVTF:V_cmp_result>mode, c));
> --- gcc/testsuite/gcc.dg/pr89438.c.jj   2019-02-22 00:45:26.662292609 
> +0100
> +++ gcc/testsuite/gcc.dg/pr89438.c      2019-02-22 00:45:02.130682728 
> +0100
> @@ -0,0 +1,22 @@
> +/* PR target/89438 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -g -w" } */
> +
> +struct S { double b, c; struct T { double d, e; } f[16]; } g;
> +int h, i, j;
> +double k;
> +
> +double
> +foo (void)
> +{
> +  int m;
> +  if (j)
> +    return k;
> +  long a, p = a - 80;
> +  double b, n;
> +  n = b * h + g.f[p].e;
> +  m = n;
> +  double o = 1 ? m : 1.0;
> +  k = i ? -o : o;
> +  return k;
> +}
>
>         Jakub

On Mon, Feb 25, 2019 at 09:25:19AM +0000, Kyrill Tkachov wrote:
> > --- gcc/config/arm/vfp.md.jj    2019-02-18 20:48:32.642732323 +0100
> > +++ gcc/config/arm/vfp.md       2019-02-22 00:37:36.730795663 +0100
> > @@ -871,14 +871,15 @@ (define_insn_and_split "*negdf2_vfp"
> >    if (REGNO (operands[0]) == REGNO (operands[1]))
> >      {
> >        operands[0] = gen_highpart (SImode, operands[0]);
> > -      operands[1] = gen_rtx_XOR (SImode, operands[0], GEN_INT
> > (0x80000000));
> > +      operands[1] = gen_rtx_XOR (SImode, operands[0],
> > +                                GEN_INT (HOST_WIDE_INT_C (-0x80000000)));
> 
> 
> const_int generation in RTL always subtly confuses me :(
> 
> So why is 0x80000000 now -0x80000000? Shouldn't we be using
> trunc_int_for_mode?

We surely can, it will be tiny bit slower though.
HOST_WIDE_INT_C (-0x80000000) is basically inlining of what
trunc_int_for_mode (0x80000000, SImode) will do.

Or we could use gen_int_mode (0x80000000, SImode); instead of
the whole GEN_INT (HOST_WIDE_INT_C (-0x80000000)).

Whatever you prefer.

	Jakub

On 2/25/19 9:35 AM, Jakub Jelinek wrote:
> On Mon, Feb 25, 2019 at 09:25:19AM +0000, Kyrill Tkachov wrote:
>>> --- gcc/config/arm/vfp.md.jj    2019-02-18 20:48:32.642732323 +0100
>>> +++ gcc/config/arm/vfp.md       2019-02-22 00:37:36.730795663 +0100
>>> @@ -871,14 +871,15 @@ (define_insn_and_split "*negdf2_vfp"
>>>     if (REGNO (operands[0]) == REGNO (operands[1]))
>>>       {
>>>         operands[0] = gen_highpart (SImode, operands[0]);
>>> -      operands[1] = gen_rtx_XOR (SImode, operands[0], GEN_INT
>>> (0x80000000));
>>> +      operands[1] = gen_rtx_XOR (SImode, operands[0],
>>> +                                GEN_INT (HOST_WIDE_INT_C (-0x80000000)));
>>
>> const_int generation in RTL always subtly confuses me :(
>>
>> So why is 0x80000000 now -0x80000000? Shouldn't we be using
>> trunc_int_for_mode?
> We surely can, it will be tiny bit slower though.
> HOST_WIDE_INT_C (-0x80000000) is basically inlining of what
> trunc_int_for_mode (0x80000000, SImode) will do.
>
> Or we could use gen_int_mode (0x80000000, SImode); instead of
> the whole GEN_INT (HOST_WIDE_INT_C (-0x80000000)).

Let's use gen_int_mode. It looks the most clear and self-documenting to me.

Ok with that change.

Thanks,

Kyrill


>
> Whatever you prefer.
>
> 	Jakub