Re: [patch] Support vectorization of min/max location pattern - take 2

[Ira Rosen <IRAR at il dot ibm dot com>]

Richard Guenther <richard.guenther@gmail.com> wrote on 09/08/2010 02:00:59
PM:

> On Mon, Aug 9, 2010 at 12:53 PM, Ira Rosen <IRAR@il.ibm.com> wrote:
> >
> >
> > Richard Guenther <richard.guenther@gmail.com> wrote on 09/08/2010
12:50:14
> > PM:
> >> > I implemented VEC_COND_EXPR extension in the attached patch.
> >> >
> >> > For reduction epilogue I defined new tree codes
> >> > REDUC_MIN/MAX_FIRST/LAST_LOC_EXPR.
> >>
> >> Why do you need new tree codes here?
> >
> > After vector loop we have two vectors one with four minimums and the
second
> > with four corresponding array indexes. The extraction of the correct
index
> > out of four can be done differently on each platform (including
problematic
> > vector comparisons).
>
> So the tree code is just to tie those two operations together?

It is not to tie MIN/MAX extraction and index extraction together. It is to
extract scalar value from a vector of indexes. To do that we need both
vectors (minimums and indexes). We already have tree codes for other
reduction epilogues (like REDUC_PLUS_EXPR).

>
> >> They btw need
> >> documentation - just stating the new operand is a vector isn't
> >> very informative. ?They need documentation in generic.texi.
> >
> > Sorry about that, I'll add documentation for both.
>
> Thanks.
>
> >>
> >> Likewise the new RTX codes (what are they for??)
> >
> > Probably there is a better way to do that, but I needed to map new
vector
> > comparison instructions that compare floats and return ints.
>
> So you just need this at expansion time then and the RTXen
> will never appear in RTL code?

It will appear in RTL code. AFAIU, current RTX codes for vector comparison
require same types for input and output, so I had to add new codes.

> Why not use a target hook for
> expanding those comparisons then?  Btw, my GSoC student
> implemented lowering of generic vector comparisons resulting
> in a mask in tree-vect-generic.c using a target hook that eventually
> uses target specific builtins.  I attached the latest patch for that.

I used a target hook in the original patch, but I inserted calls it in the
vectorizer.
BTW, I don't understand how your hook will work for mips:

> >> >>
> >> >> (2) The mips C.cond.PS instruction does *not* produce a bitmask
> >> >> ? ? like altivec or sse do. ?Instead it sets multiple condition
> >> >> ? ? codes. ?One then uses MOV[TF].PS to merge the elements based
> >> >> ? ? on the individual condition codes. ?While there's no direct
> >> >> ? ? corresponding instruction that will operate on integers, I
> >> >> ? ? don't think it would be too difficult to use MOV[TF].G or
> >> >> ? ? BC1AND2[FT] instructions to emulate it. ?In any case, this
> >> >> ? ? is again a case where you don't want to expose any part of
> >> >> ? ? the VEC_COND at the gimple level.
> >> >>
> >> >>
> >> >> r~
> >


If VECT_COND_EXPR and REDUC_..._LOC_EXPR are used, why do we need a target
hook? To avoid new RTX codes?

>
> >> need documentation
> >> in rtl.texi.
> >>
> >> Btw, you still don't adjust if-conversion to fold the COND_EXPR
> >> it generates - that would generate the MIN/MAX expressions
> >> directly and you wouldn't have to pattern match the COND_EXPR.
> >
> > I don't see how it can help to avoid pattern matching. We will still
need
> > to match MIN/MAX's arguments with the COND_EXPR arguments.
>
> True, but you need to match MIN/MAX instead.  Well, my point
> is that if-convert shouldn't create a COND_EXPR in that case.

OK, I'll try to fix if-convert (and my patch accordingly).

Thanks,
Ira

>
> Richard.
>