On Thu, Oct 11, 2012 at 1:20 AM, Marc Glisse <marc.gli...@inria.fr> wrote: > On Mon, 8 Oct 2012, Richard Guenther wrote: > >> On Mon, Oct 8, 2012 at 11:34 AM, Marc Glisse <marc.gli...@inria.fr> wrote: >>> >>> On Mon, 8 Oct 2012, Richard Guenther wrote: >>> >>>>> VEC_COND_EXPR is more complicated. We could for instance require that >>>>> it >>>>> takes as first argument a vector of -1 and 0 (thus <0, !=0 and the neon >>>>> thing are equivalent). Which would leave to decide what the expansion >>>>> of >>>>> vec_cond_expr passes to the targets when the first argument is not a >>>>> comparison, between !=0, <0, ==-1 or others (I vote for <0 because of >>>>> opencl). One issue is that targets wouldn't know if it was a dummy >>>>> comparison that can safely be ignored because the other part is the >>>>> result >>>>> of logical operations on comparisons (thus composed of -1 and 0) or a >>>>> genuine comparison with an arbitrary vector, so a new optimization >>>>> would >>>>> be >>>>> needed (in the back-end I guess or we would need an alternate >>>>> instruction >>>>> to >>>>> vcond) to detect if a vector is a "signed boolean" vector. >>>>> We could instead say that vec_cond_expr really follows OpenCL's >>>>> semantics >>>>> and looks at the MSB of each element. I am not sure that would change >>>>> much, >>>>> it would mostly delay the apparition of <0 to RTL expansion time (and >>>>> thus >>>>> make gimple slightly lighter). >>>> >>>> >>>> >>>> I think we should delay the decision on how to optimize this. It's >>>> indeed >>>> not trivial and the GIMPLE middle-end aggressively forwards feeding >>>> comparisons into the VEC_COND_EXPR expressions already (somewhat >>>> defeating any CSE that might be possible here) in forwprop. >>> >>> >>> >>> Thanks for going through the long email :-) >>> >>> What does that imply for the first argument of VEC_COND_EXPR? Currently, >>> the >>> expander asserts that it is a comparison, but that is not reflected in >>> the >>> gimple checkers. >> >> >> And I don't think we should reflect that in the gimple checkers rather >> fixup the >> expander (transparently use p != 0 or p < 0). > > > I guess I'll pick p < 0 then (just because I am more interested in x86 and > it makes the optimization easier on x86). Having another expander than vcond > (one that takes the mask directly instead of a comparison, and for which we > promise that the argument will be a vector of -1/0) would be convenient... > > >>> So is the best choice to document that VEC_COND_EXPR takes as >>> first argument a comparison and make gimple checking reflect that? (seems >>> sad, but at least that would tell me what I can/can't do) >> >> >> No, that would just mean that in GIMPLE you'd add this p != 0 or p < 0. >> And at some point in the future I really really want to push this embedded >> expression to a separate statement so I have a SSA definition for it. > > > Once the expander is ready to accept it, ok. It seems to me that the scalar > COND_EXPR may also have an embedded expression, so I assume COND_EXPR and > VEC_COND_EXPR are meant to diverge (or maybe you also want to do the same > for COND_EXPR?).
Yes, I want the same for COND_EXPR and even GIMPLE_COND. I had patches to do this about two years ago but was too lazy to fixup all the fallout. My plan was to eventually return to this and first tackle COND_EXPR and VEC_COND_EXRP only, leaving GIMPLE_COND in place. >>> By the way, since we are documenting comparisons as returning 0 and -1, >>> does >>> that bring back the integer_truep predicate? >> >> >> Not sure, true would still be != 0 or all_onesp (all bits of the >> precision are 1), no? > > > I was going to make truep equivalent to onep for scalars and all_onesp for > vectors (since -1 will be the only value documented as "true" for vectors). > I guess it can wait, I can manually inline it for now. Yes please. > Since we are documenting that comparisons of vectors return -1 and 0 in the > middle-end, I was wondering whether the comparison expanders would need > updating so they forward to vcond(...,-1,0), at least on platforms that > don't define VECTOR_STORE_FLAG_VALUE to constm1_rtx for this mode. But a > simple test on sparc shows it is already fine :-) Heh, good. Richard. > -- > Marc Glisse
