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.
