On Wed, Jun 24, 2015 at 1:10 PM, Richard Sandiford <richard.sandif...@arm.com> wrote: > Richard Biener <richard.guent...@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 >