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.

"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];

Please remove the extra temporary...  It isn't necessary, and now you
have two variables named tmp (in nested scopes).

> @@ -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.

Okay for trunk with those things fixed.  Thanks,


Segher

Reply via email to