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] Move ~X | X -> -1 folding


On Tue, 30 Jun 2015, Marek Polacek wrote:

> On Tue, Jun 30, 2015 at 03:59:23PM +0200, Richard Biener wrote:
> > On Tue, 30 Jun 2015, Marek Polacek wrote:
> > 
> > > On Tue, Jun 30, 2015 at 03:13:14PM +0200, Richard Biener wrote:
> > > > On Tue, 30 Jun 2015, Marek Polacek wrote:
> > > > 
> > > > > On Tue, Jun 30, 2015 at 02:47:49PM +0200, Richard Biener wrote:
> > > > > > On Tue, 30 Jun 2015, Marek Polacek wrote:
> > > > > > 
> > > > > > > On Tue, Jun 30, 2015 at 01:39:29PM +0200, Marc Glisse wrote:
> > > > > > > > Does my suggestion to "build the all_ones constant in TREE_TYPE (@0) and
> > > > > > > > convert that to type" help for that?
> > > > > > > 
> > > > > > > It appears to work, but it seems weird to me to create a integer constant
> > > > > > > in one type and then immediately cast it to another type.
> > > > > > 
> > > > > > Yes.  Do you have a testcase now that fails using bools?
> > > > > 
> > > > > I don't have a testcase that fails with the pattern we currently have, i.e.
> > > > > the one with tree_nop_conversion_p.
> > > > 
> > > > I mean with removing tree_nop_conversion_p.
> > > 
> > > Aha.  With tree_nop_conversion_p removed, gcc.dg/binop-notor2.c fails,
> > > because there we optimize the return statement to "return -1" instead
> > > of "return 1".
> > > <https://gcc.gnu.org/ml/gcc-patches/2015-06/msg02179.html>
> > 
> > Hmm ok.  That testcase is basically
> > 
> > int foo (_Bool a)
> > {
> >   return ((int) a) | ((int) ~a);
> > }
> > 
> > where indeed with unsigned bool (yeah, our bool is unsigned) we
> > get zero-extension on both arms.  Similar issue would show up with
> > 
> > int foo (unsigned char a)
> > {
> >   return ((int) a) | ((int) ~a);
> > }
> > 
> > so it's not specific to bools.  So yes, the suggestion to
> > do
> > 
> >   (convert { build_all_ones_cst (TREE_TYPE (@0)); })
> > 
> > would work here.
> 
> Ok, so do you want me to change that pattern to use this
> (convert { build_all_ones_cst (TREE_TYPE (@0)); })
> (along with a new test containing those two functions you mentioned)?
> 
> If so, is such a patch preapproved provided it passes the usual testing?

Yes.

Thanks,
Richard.


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