Re: Remove redundant AND from count reduction loop

[Richard Biener <richard.guenther@gmail.com>]

On Wed, Jun 24, 2015 at 1:10 PM, Richard Sandiford
<richard.sandiford@arm.com> wrote:
> Richard Biener <richard.guenther@gmail.com> writes:
>>>> I'm fine with using tree_nop_conversion_p for now.
>>>
>>> I like the suggestion about checking TYPE_VECTOR_SUBPARTS and the element
>>> mode.  How about:
>>>
>>>  (if (VECTOR_INTEGER_TYPE_P (type)
>>>       && TYPE_VECTOR_SUBPARTS (type) == TYPE_VECTOR_SUBPARTS (TREE_TYPE (@0))
>>>       && (TYPE_MODE (TREE_TYPE (type))
>>>           == TYPE_MODE (TREE_TYPE (TREE_TYPE (@0)))))
>>>
>>> (But is it really OK to be adding more mode-based compatibility checks?
>>> I thought you were hoping to move away from modes in the middle end.)
>>
>> The TYPE_MODE check makes the VECTOR_INTEGER_TYPE_P check redundant
>> (the type of a comparison is always a signed vector integer type).
>
> OK, will just use VECTOR_TYPE_P then.

Given we're in a VEC_COND_EXPR that's redundant as well.

>>>>>> +/* We could instead convert all instances of the vec_cond to negate,
>>>>>> +   but that isn't necessarily a win on its own.  */
>>>>
>>>> so p ? 1 : 0 -> -p?  Why isn't that a win on its own?  It looks more compact
>>>> at least ;)  It would also simplify the patterns below.
>>>
>>> In the past I've dealt with processors where arithmetic wasn't handled
>>> as efficiently as logical ops.  Seems like an especial risk for 64-bit
>>> elements, from a quick scan of the i386 scheduling models.
>>
>> But then expansion could undo this ...
>
> So do the inverse fold and convert (neg (cond)) to (vec_cond cond 1 0)?
> Is there precendent for doing that kind of thing?

Expanding it as this, yes.  Whether there is precedence no idea, but
surely the expand_unop path could, if there is no optab for neg:vector_mode,
try expanding as vec_cond .. 1 0.  There is precedence for different
expansion paths dependent on optabs (or even rtx cost?).  Of course
expand_unop doesn't get the original tree ops (expand_expr.c does,
where some special-casing using get_gimple_for_expr is).  Not sure
if expand_unop would get 'cond' in a form where it can recognize
the result is either -1 or 0.

>>> I also realised later that:
>>>
>>> /* Vector comparisons are defined to produce all-one or all-zero results.  */
>>> (simplify
>>>  (vec_cond @0 integer_all_onesp@1 integer_zerop@2)
>>>  (if (tree_nop_conversion_p (type, TREE_TYPE (@0)))
>>>    (convert @0)))
>>>
>>> is redundant with some fold-const.c code.
>>
>> If so then you should remove the fold-const.c at the time you add the pattern.
>
> Can I just drop that part of the patch instead?  The fold-const.c
> code handles COND_EXPR and VEC_COND_EXPR analogously, so I'd have
> to move COND_EXPR at the same time.  And then the natural follow-on
> would be: why not move the other COND_EXPR and VEC_COND_EXPR folds too? :-)

Yes, why not? ;)  But sure, you can also drop the case for now.

>> Note that ISTR code performing exactly the opposite transform in
>> fold-const.c ...
>
> That's another reason why I'm worried about just doing the (negate ...)
> thing without knowing whether the negate can be folded into anything else.

I'm not aware of anything here.

Richard.

> Thanks,
> Richard
>