Re: [PATCH] rs6000: Split movsf_from_si from high word before reload[PR89310]

[Segher Boessenkool <segher@kernel.crashing.org>]

Hi!

On Thu, Jul 09, 2020 at 11:09:42AM +0800, luoxhu wrote:
> > Maybe change it back to just SI?  It won't match often at all for QI or
> > HI anyway, it seems.  Sorry for that detour.  Should be good with the
> > above nits fixed :-)
> 
> OK, if I see correctly, subreg of DImode should be SImode and I used
> subreg:SI to match only SI, so no need to consider mask for QI and HI? :)

My problem with it was that the shift amounts won't be 32 for QI etc.?

> +;;     {%1:SF=unspec[r122:DI>>0x20#0] 86;clobber scratch;}
> +;; split it before reload with "and mask" to avoid generating shift right
> +;; 32 bit then shift left 32 bit.
> +(define_insn_and_split "movsf_from_si2"
> +  [(set (match_operand:SF 0 "gpc_reg_operand" "=wa")
> +	    (unspec:SF [
> +	     (subreg:SI (
> +		 ashiftrt:DI (

Opening parens *start* a line, they never end it.  So:

(define_insn_and_split "movsf_from_si2"
  [(set (match_operand:SF 0 "gpc_reg_operand" "=wa")
	  (unspec:SF
	    [(subreg:SI
	       (ashiftrt:DI
		 (match_operand:DI 1 "input_operand" "r")
		 (const_int 32))
	      0)]
	    UNSPEC_SF_FROM_SI))
   (clobber (match_scratch:DI 2 "=r"))]

or something like that.  There aren't really any real rules...  The
important points are that nested things should be indented, and things
at the same level should have the same indent (like the outer set and
clobber).  The [ for an unspec is sometimes put at the end of a line,
that reads a little better perhaps.

> +  "TARGET_NO_SF_SUBREG"
> +  "#"
> +  "&& vsx_reg_sfsubreg_ok (operands[0], SFmode)"

Put this in the insn condition?  And since this is just a predicate,
you can just use it instead of gpc_reg_operand.

(The split condition becomes "&& 1" then, not "").


Segher