On Mon, 19 Sep 2016, Patrick Palka wrote:

On Wed, Sep 14, 2016 at 1:58 AM, Marc Glisse <marc.gli...@inria.fr> wrote:
On Fri, 19 Aug 2016, Patrick Palka wrote:

On Fri, Aug 19, 2016 at 7:30 PM, Patrick Palka <patr...@parcs.ath.cx>

integer_nonzerop() currently unconditionally returns false for a
VECTOR_CST argument.  This is confusing because one would expect that
integer_onep(x) => integer_nonzerop(x) for all x but that is currently
not the case.  For a VECTOR_CST of all ones i.e. {1,1,1,1},
integer_onep() returns true but integer_nonzerop() returns false.

This patch makes integer_nonzerop() handle VECTOR_CSTs in the obvious
way and also adds some self tests (the last of which fails without the
change).  Does this look OK to commit afetr bootstrap + regtesting on

Actually I guess there is some ambiguity as to whether
integer_nonzerop() should return true for a VECTOR_CST only if it has
at least one non-zero element or only if all of its elements are

In my opinion, the second one would make a lot more sense. I think most
(all?) current uses are protected by checks that mean it is never called on
vectors (where did you notice this issue?). But if we wanted to generalize,

Yeah I don't think integer_onep is called anywhere on a VECTOR_CST
currently. I ran into this issue because I tried using integer_onep on
a VECTOR_CST and found that it didn't work as expected.

I thought we were talking about integer_nonzerop? integer_onep is used on vectors just fine...

looking for instance at fold-const.c (A & C) == D where D & ~C != 0, we
would want that no element of D&~C is 0 to be able to simplify the whole
vector to 0. If we want an OR as in your patch, in most cases we could use
!integer_zerop, possibly with extra checks that it is indeed a constant.
Maybe we could solve this with more explicit names? integer_each_nonzerop vs

More precise naming would be nice.  Note that integer_all_onesp() and
integer_truep() already exist which is another reason I leaned towards
making integer_onep behave as an OR for VECTOR_CSTs since the AND
behavior is already covered by those aforementioned two predicates.
But since there is no consensus and no such user of integer_onep I
will just drop this patch.

Marc Glisse

Reply via email to