On Sat, 2 Nov 2013, Marc Glisse wrote: > On Sat, 2 Nov 2013, Jakub Jelinek wrote: > > > On Sat, Nov 02, 2013 at 05:38:53PM +0000, Richard Sandiford wrote: > > > OK, thanks. I should have realised this earlier, but we have: > > > > > > /* Return 1 if EXPR is the integer constant one or the corresponding > > > complex constant. */ > > > > > > int > > > integer_onep (const_tree expr) > > > ... > > > /* Return 1 if EXPR is the integer constant minus one. */ > > > > > > int > > > integer_minus_onep (const_tree expr) > > > > > > which makes them sound like a pair. But integer_minus_onep returns > > > true for any all-ones INTEGER_CST (regardless of sign): > > > > > > if (TREE_CODE (expr) == COMPLEX_CST) > > > return (integer_all_onesp (TREE_REALPART (expr)) > > > && integer_zerop (TREE_IMAGPART (expr))); > > > else > > > return integer_all_onesp (expr); > > > > > > So a nonzero 1-bit unsigned bitfield is both integer_onep and > > > integer_minus_onep, but a 1-bit signed bitfield is only > > > integer_minus_onep. Should integer_minus_onep be changed so > > > that it always returns false for unsigned types? > > > > Yeah, I was wondering about that too. I think integer_minus_onep > > is a fairly new thing, I guess if you want to change it, somebody has to go > > through all users and see what they want. > > Returning false for unsigned types is natural thing to do though. > > See the conversation around: > http://gcc.gnu.org/ml/gcc-patches/2013-05/msg00053.html > for the introduction of this function. > > Richard already asked the same question then, and the patch renaming some > integer_all_onesp to integer_minus_onep was forgotten. Personnally, I am fine > with integer_minus_onep returning true on -1U. Without the renaming patch, > there is a single use of integer_minus_onep (in fold-const.c), so it shouldn't > be hard to check all users, and IIRC we want it to return true for -1U in this > case (turning x*-1 into -x).
Well - it was introduced because integer_all_onesp was "wrong" for complex types. But now -1U is more integer_all_onesp than integer_one_p ... The single user doesn't care, so we either adjust the function comment to not say "minus one" but to relate it to the arithmetic operation it is supposed to be a complement / invariant of. Richard.
