On Mon, Aug 22, 2016 at 9:10 AM, Richard Biener
<[email protected]> wrote:
> On Sat, Aug 20, 2016 at 1:38 AM, Patrick Palka <[email protected]> wrote:
>> On Fri, Aug 19, 2016 at 7:30 PM, Patrick Palka <[email protected]> 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...
>
> From its documentation it looks like integer_nonzerop would be the
> same as ! integer_zerop if you pass in a tree code that is handled
> by integer_zerop. So I guess it doesn't really make sense to ask
> this for a vector?
Thus eventually a cleanup would be to share a common worker for
most of the predicates, returning a enum which also contains UNHANDLED
and wrap all of the predicates around that...
Richard.
> Richard.
>
>>>
>>> gcc/ChangeLog:
>>>
>>> * tree.c (integer_nonzerop): Rewrite to use a switch. Handle
>>> VECTOR_CSTs.
>>> (test_vector_constants): New static function.
>>> (tree_c_tests): Call it.
>>> ---
>>> gcc/tree.c | 43 ++++++++++++++++++++++++++++++++++++++-----
>>> 1 file changed, 38 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/gcc/tree.c b/gcc/tree.c
>>> index 33e6f97..48795c7 100644
>>> --- a/gcc/tree.c
>>> +++ b/gcc/tree.c
>>> @@ -2439,11 +2439,24 @@ integer_pow2p (const_tree expr)
>>> int
>>> integer_nonzerop (const_tree expr)
>>> {
>>> - return ((TREE_CODE (expr) == INTEGER_CST
>>> - && !wi::eq_p (expr, 0))
>>> - || (TREE_CODE (expr) == COMPLEX_CST
>>> - && (integer_nonzerop (TREE_REALPART (expr))
>>> - || integer_nonzerop (TREE_IMAGPART (expr)))));
>>> + switch (TREE_CODE (expr))
>>> + {
>>> + case INTEGER_CST:
>>> + return !wi::eq_p (expr, 0);
>>> + case COMPLEX_CST:
>>> + return (integer_nonzerop (TREE_REALPART (expr))
>>> + || integer_nonzerop (TREE_IMAGPART (expr)));
>>> + case VECTOR_CST:
>>> + {
>>> + unsigned i;
>>> + for (i = 0; i < VECTOR_CST_NELTS (expr); ++i)
>>> + if (integer_nonzerop (VECTOR_CST_ELT (expr, i)))
>>> + return true;
>>> + return false;
>>> + }
>>> + default:
>>> + return false;
>>> + }
>>> }
>>>
>>> /* Return 1 if EXPR is the integer constant one. For vector,
>>> @@ -14230,6 +14243,25 @@ test_integer_constants ()
>>> ASSERT_EQ (type, TREE_TYPE (zero));
>>> }
>>>
>>> +/* Verify various predicates and operations on vector constants. */
>>> +
>>> +static void
>>> +test_vector_constants ()
>>> +{
>>> + tree inner_type = integer_type_node;
>>> + tree type = build_vector_type (inner_type, 8);
>>> + tree zero = build_zero_cst (type);
>>> + tree one = build_one_cst (type);
>>> +
>>> + ASSERT_TRUE (integer_zerop (zero));
>>> + ASSERT_FALSE (integer_onep (zero));
>>> + ASSERT_FALSE (integer_nonzerop (zero));
>>> +
>>> + ASSERT_FALSE (integer_zerop (one));
>>> + ASSERT_TRUE (integer_onep (one));
>>> + ASSERT_TRUE (integer_nonzerop (one));
>>> +}
>>> +
>>> /* Verify identifiers. */
>>>
>>> static void
>>> @@ -14258,6 +14290,7 @@ void
>>> tree_c_tests ()
>>> {
>>> test_integer_constants ();
>>> + test_vector_constants ();
>>> test_identifiers ();
>>> test_labels ();
>>> }
>>> --
>>> 2.9.3.650.g20ba99f
>>>
