This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]