On Tue, Dec 06, 2016 at 03:47:18PM -0600, Segher Boessenkool wrote:
> Hi Mike,
>
> On Tue, Dec 06, 2016 at 03:56:11PM -0500, Michael Meissner wrote:
> > PR target 78658 exposed a thinko that I had in extending the conversion of
> > QImode and HImode to floating point. If the QImode/HImode value was in a
> > vector register, the pattern didn't allocate the second pseudo register, but
> > the code generated a reference to that second register. The second register
> > was used to move the value from a GPR to the vector registers.
> >
> > I also modified the zero extend patterns to use ^ instead of ?* like I did
> > on
> > the November 21st patches so that the register allocator is more likely to
> > consider using the vector registers if it is useful to do so.
> >
> > I have done a bootstrap build and make check on a little endian power8
> > system
> > and there were no regressions. Can I check this into the trunk?
>
> Few things, most trivial (sorry)...
>
> > * config/rs6000/rs6000.md (zero_extendqi<mode>2): Use ^ instead of
> > ?* constraints for the ISA 3.0 patterns, so the register allocator
> > is more likely to allocate QImode/HImode to vector registers for
> > conversion to floating point point unless a reload is needed.
>
> "point point"
>
> > (zero_extendhi<mode>2): Likewise.
> > (float<QHI:mode><FP_ISA3:mode>2_internal): Properly deal with the
> > first alternative which is converting QImode/HImode to floating
> > point and the QImode/HImode value is in a vector register, and
> > does not allocate the second temorary.
Thanks.
> "temorary".
>
> > @@ -5413,8 +5413,11 @@ (define_insn_and_split "*float<QHI:mode>
> >
> > if (!MEM_P (input))
> > {
> > + rtx tmp = operands[3];
> > if (altivec_register_operand (input, <QHI:MODE>mode))
> > emit_insn (gen_extend<QHI:mode>di2 (di, input));
> > + else if (GET_CODE (tmp) == SCRATCH)
> > + emit_insn (gen_extend<QHI:mode>di2 (di, input));
> > else
> > {
> > rtx tmp = operands[3];
Thanks.
> Please remove the extra temporary... It isn't necessary, and now you
> have two variables named tmp (in nested scopes).
Thanks. I was going back and forth between float<QHI:mode>... and
floatuns<QHI:mode> and I didn't notice the second temporary.
> > @@ -5449,7 +5452,7 @@ (define_expand "floatuns<QHI:mode><FP_IS
> > (define_insn_and_split "*floatuns<QHI:mode><FP_ISA3:mode>2_internal"
> > [(set (match_operand:FP_ISA3 0 "vsx_register_operand" "=<Fv>,<Fv>,<Fv>")
> > (unsigned_float:FP_ISA3
> > - (match_operand:QHI 1 "reg_or_indexed_operand" "wJwK,r,Z")))
> > + (match_operand:QHI 1 "reg_or_indexed_operand" "wK,r,Z")))
> > (clobber (match_scratch:DI 2 "=wK,wi,wJwK"))
> > (clobber (match_scratch:DI 3 "=X,r,X"))]
> > "TARGET_P9_VECTOR && TARGET_DIRECT_MOVE && TARGET_POWERPC64
>
> Here you remove the wJ alternative, is that on purpose? The changelog
> does not mention it.
Whoops. I forgot to add the ChangeLog entry for that. I had the initial
version of these changes in a development branch, and I didn't copy over this
specific ChangeLog entry.
This fixes a potential bug. The extract byte/half-word instruction only
targets the traditional Altivec registers, so it is important that only Altivec
registers are allowed (i.e. wJ would allow traditional FPRs under ISA 3.0 and
wK would allow traditional Altivec registers under ISA 3.0). I have updated
the ChangeLog file to note this change.
> Okay for trunk with those things fixed. Thanks,
Committed, subversion id 243320.
--
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meiss...@linux.vnet.ibm.com, phone: +1 (978) 899-4797
