[avr,committed] : Remove flag_strict_overflow from avr.md

Applied this addendum to r247495 which removed flag_strict_overflow. 
There were remains of the flag in avr.md which broke the avr build.

Committed as r247632.


Johann


	* config/avr/avr.md [flag_strict_overflow]: Remove any occurence
	of this flag from insn conditions due to removal from r247495.

On Fri, 5 May 2017, Georg-Johann Lay wrote:

> Applied this addendum to r247495 which removed flag_strict_overflow. There
> were remains of the flag in avr.md which broke the avr build.
> 
> Committed as r247632.

Whoops - sorry for not grepping besides .[ch] files...

But... these patterns very much look like premature optimization
and/or bugs.  combine is supposed to handle this via simplify_rtx.
Also note that on RTL we generally assume overflow wraps as we lose
signedness of operands.  Not sure what 'compare' in your patterns
will end up with.

The only flag_wrapv checks in RTL otherwise are in simplify-rtx.c
for ABS which seems to be a singed RTL op.

That said, I suggest to get rid of the avr.md patterns and instead
move functionality to simplify-rtx.c (if they still trigger).

Richard.

> 
> Johann
> 
> 
> 	* config/avr/avr.md [flag_strict_overflow]: Remove any occurence
> 	of this flag from insn conditions due to removal from r247495.
> 
> 
> Index: config/avr/avr.md
> ===================================================================
> --- config/avr/avr.md   (revision 247631)
> +++ config/avr/avr.md   (working copy)
> @@ -4580,7 +4580,7 @@ (define_insn "*negated_tstqi"
>    [(set (cc0)
>          (compare (neg:QI (match_operand:QI 0 "register_operand" "r"))
>                   (const_int 0)))]
> -  "!flag_wrapv && !flag_trapv && flag_strict_overflow"
> +  "!flag_wrapv && !flag_trapv"
>    "cp __zero_reg__,%0"
>    [(set_attr "cc" "compare")
>     (set_attr "length" "1")])
> @@ -4598,7 +4598,7 @@ (define_insn "*negated_tsthi"
>    [(set (cc0)
>          (compare (neg:HI (match_operand:HI 0 "register_operand" "r"))
>                   (const_int 0)))]
> -  "!flag_wrapv && !flag_trapv && flag_strict_overflow"
> +  "!flag_wrapv && !flag_trapv"
>    "cp __zero_reg__,%A0
>         cpc __zero_reg__,%B0"
>  [(set_attr "cc" "compare")
> @@ -4621,7 +4621,7 @@ (define_insn "*negated_tstpsi"
>    [(set (cc0)
>          (compare (neg:PSI (match_operand:PSI 0 "register_operand" "r"))
>                   (const_int 0)))]
> -  "!flag_wrapv && !flag_trapv && flag_strict_overflow"
> +  "!flag_wrapv && !flag_trapv"
>    "cp __zero_reg__,%A0\;cpc __zero_reg__,%B0\;cpc __zero_reg__,%C0"
>    [(set_attr "cc" "compare")
>     (set_attr "length" "3")])
> @@ -4640,7 +4640,7 @@ (define_insn "*negated_tstsi"
>    [(set (cc0)
>          (compare (neg:SI (match_operand:SI 0 "register_operand" "r"))
>                   (const_int 0)))]
> -  "!flag_wrapv && !flag_trapv && flag_strict_overflow"
> +  "!flag_wrapv && !flag_trapv"
>    "cp __zero_reg__,%A0
>         cpc __zero_reg__,%B0
>         cpc __zero_reg__,%C0
> 
>

On 05.05.2017 13:04, Richard Biener wrote:
> On Fri, 5 May 2017, Georg-Johann Lay wrote:
>
>> Applied this addendum to r247495 which removed flag_strict_overflow. There
>> were remains of the flag in avr.md which broke the avr build.
>>
>> Committed as r247632.
>
> Whoops - sorry for not grepping besides .[ch] files...
>
> But... these patterns very much look like premature optimization
> and/or bugs.  combine is supposed to handle this via simplify_rtx.

Well, for now the patch just restores avr BE to be able to be build.

> Also note that on RTL we generally assume overflow wraps as we lose
> signedness of operands.  Not sure what 'compare' in your patterns
> will end up with.
>
> The only flag_wrapv checks in RTL otherwise are in simplify-rtx.c
> for ABS which seems to be a singed RTL op.

Which is a bug, IMO.  Letting undefined overflow propagate to RTL
renders some RTL as if it has undefined behaviour.  Consequence is
that testing the MSB must no more use signed comparisons on
less-zero resp. greater-or-equal-to-zero.

Cf. https://gcc.gnu.org/PR75964 for an example:


typedef __UINT8_TYPE__ uint8_t;

uint8_t abs8 (uint8_t x)
{
     if (x & 0x80)
         x = -x;

     if (x & 0x80)
         x = 0x7f;

     return x;
}

The first comparison is performed by a signed test against 0 (which
is reasonable and the best code in that case) but then we conclude
that the second test is always false, which is BUG.

IMO the culprit is to let slip undefined overflow to RTL.


Johann


> That said, I suggest to get rid of the avr.md patterns and instead
> move functionality to simplify-rtx.c (if they still trigger).
>
> Richard.
>

On Fri, 5 May 2017, Georg-Johann Lay wrote:

> On 05.05.2017 13:04, Richard Biener wrote:
> > On Fri, 5 May 2017, Georg-Johann Lay wrote:
> > 
> > > Applied this addendum to r247495 which removed flag_strict_overflow. There
> > > were remains of the flag in avr.md which broke the avr build.
> > > 
> > > Committed as r247632.
> > 
> > Whoops - sorry for not grepping besides .[ch] files...
> > 
> > But... these patterns very much look like premature optimization
> > and/or bugs.  combine is supposed to handle this via simplify_rtx.
> 
> Well, for now the patch just restores avr BE to be able to be build.

Sure.

> > Also note that on RTL we generally assume overflow wraps as we lose
> > signedness of operands.  Not sure what 'compare' in your patterns
> > will end up with.
> > 
> > The only flag_wrapv checks in RTL otherwise are in simplify-rtx.c
> > for ABS which seems to be a singed RTL op.
> 
> Which is a bug, IMO.  Letting undefined overflow propagate to RTL
> renders some RTL as if it has undefined behaviour.  Consequence is
> that testing the MSB must no more use signed comparisons on
> less-zero resp. greater-or-equal-to-zero.
> 
> Cf. https://gcc.gnu.org/PR75964 for an example:
> 
> 
> typedef __UINT8_TYPE__ uint8_t;
> 
> uint8_t abs8 (uint8_t x)
> {
>     if (x & 0x80)
>         x = -x;
> 
>     if (x & 0x80)
>         x = 0x7f;
> 
>     return x;
> }
> 
> The first comparison is performed by a signed test against 0 (which
> is reasonable and the best code in that case) but then we conclude
> that the second test is always false, which is BUG.
> 
> IMO the culprit is to let slip undefined overflow to RTL.

Yes.  I thought in RTL overflow is always well-defined (but then
as I said your patterns are equally bogus).

Richard.

> 
> Johann
> 
> 
> > That said, I suggest to get rid of the avr.md patterns and instead
> > move functionality to simplify-rtx.c (if they still trigger).
> > 
> > Richard.
> >