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>
>> wrote:
>>>
>>> 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
>>> x86_64-pc-linux-gnu?
>>
>>
>> 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
>> non-zero...
>
>
> 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.
> 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
> integer_not_all_zerop?
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
