Re: [PATCH] Auto vectorize cond expr (remaining patches)

[Geoffrey Keating <geoffk at apple dot com>]

On 11/04/2005, at 11:23 AM, Devang Patel wrote:

>         * config/rs6000.c (rs6000_emit_vector_select): Fix vector 
> select
>         operand ordering.
>
>
> > > > *************** rs6000_emit_vector_select (rtx dest, rtx
> > > > *** 10667,10673 ****
> > > >
> > > >     t = gen_rtx_fmt_ee (SET, VOIDmode, temp,
> > > >                       gen_rtx_fmt_Ei (UNSPEC, dest_mode,
> > > > !                                     gen_rtvec (3, op1, op2, mask),
> > > >                                       vsel_insn_index));
> > > >     emit_insn (t);
> > > >     emit_move_insn (dest, temp);
> > > > --- 10667,10673 ----
> > > >
> > > >     t = gen_rtx_fmt_ee (SET, VOIDmode, temp,
> > > >                       gen_rtx_fmt_Ei (UNSPEC, dest_mode,
> > > > !                                     gen_rtvec (3, op2, op1, mask),
> > > >                                       vsel_insn_index));
> > >
> > > I don't have enough context to know whether this is correct,
> > > but you should pass this separately though an rs6000 maintainer.
> >
> > OK. It was supplying operands in wrong order to vector select
> > instruction. When I posted patch for rs6000_emit_vector_select (),
> > test case only checked compilation. Obviously it did not catch the 
> > change
> > of behavior in vectorized loop because of this bug.
>
> rs6000 maintainers, what do you think?

I think this code would be much clearer if it was using gen_rtx_SET and 
gen_rtx_UNSPEC, instead of the gen_rtx_fmt_* functions, and if the 
functions involved mentioned in their comments what the actual 
operation to be generated was, rather than just "emit a vector select 
insn... using op1, op2, and mask".

I think the change is correct.

Attachment: smime.p7s
Description: S/MIME cryptographic signature