Re: [PATCH-1, rs6000] Enable SImode in FP register on P7 [PR88558]

["Kewen.Lin" <linkw@linux.ibm.com>]

Hi Haochen,

on 2023/8/25 14:44, HAO CHEN GUI wrote:
> Hi,
>   This patch enables SImode in FP register on P7. Instruction "fctiw"
> stores its integer output in an FP register. So SImode in FP register
> needs be enabled on P7 if we want support "fctiw" on P7.
> 

It sounds reasonable to support SImode in fpr with lfiwzx and stfiwx
supports, I'd like to hear from Segher/David/Mike/Peter on what they
think of this.


>   The test case is in the second patch which implements 32bit inline
> lrint.
> 
>   Bootstrapped and tested on powerpc64-linux BE and LE with no regressions.
> 
> Thanks
> Gui Haochen
> 
> ChangeLog
> rs6000: enable SImode in FP register on P7
> 
> gcc/
> 	PR target/88558
> 	* config/rs6000/rs6000.cc (rs6000_hard_regno_mode_ok_uncached):
> 	Enable Simode in FP register for P7.

Nit: s/Simode/SImode/

> 	* config/rs6000/rs6000.md (*movsi_internal1): Add fmr for SImode
> 	move between FP register.  Set attribute isa of stfiwx to "*"

... "between FP register", s/register/registers/.

> 	and attribute of stxsiwx to "p7".
> 
> patch.diff
> diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
> index 44b448d2ba6..99085c2cdd7 100644
> --- a/gcc/config/rs6000/rs6000.cc
> +++ b/gcc/config/rs6000/rs6000.cc
> @@ -1903,7 +1903,7 @@ rs6000_hard_regno_mode_ok_uncached (int regno, machine_mode mode)
>  	  if(GET_MODE_SIZE (mode) == UNITS_PER_FP_WORD)
>  	    return 1;
> 
> -	  if (TARGET_P8_VECTOR && (mode == SImode))
> +	  if (TARGET_POPCNTD && mode == SImode)
>  	    return 1;
> 
>  	  if (TARGET_P9_VECTOR && (mode == QImode || mode == HImode))
> diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
> index cdab49fbb91..ac5d29a2cf8 100644
> --- a/gcc/config/rs6000/rs6000.md
> +++ b/gcc/config/rs6000/rs6000.md
> @@ -7566,7 +7566,7 @@ (define_split
> 
>  (define_insn "*movsi_internal1"
>    [(set (match_operand:SI 0 "nonimmediate_operand"
> -	  "=r,         r,
> +	  "=r,         r,          ^d,

One justification is needed on why we need this disparaging, I guess
you want it prefer xxlor over fmr when the former is supported?  There
is a related discussion on fmr vs. xxlor, the original thread is:
https://gcc.gnu.org/pipermail/gcc-patches/2023-February/612821.html

>  	   r,          d,          v,
>  	   m,          ?Z,         ?Z,
>  	   r,          r,          r,          r,
> @@ -7575,7 +7575,7 @@ (define_insn "*movsi_internal1"
>  	   wa,         r,
>  	   r,          *h,         *h")
>  	(match_operand:SI 1 "input_operand"
> -	  "r,          U,
> +	  "r,          U,          ^d,

This seems to have the effect that double disparaging on this alternative,
it also needs a justification why one time isn't enough.

BR,
Kewen

>  	   m,          ?Z,         ?Z,
>  	   r,          d,          v,
>  	   I,          L,          eI,         n,
> @@ -7588,6 +7588,7 @@ (define_insn "*movsi_internal1"
>    "@
>     mr %0,%1
>     la %0,%a1
> +   fmr %0,%1
>     lwz%U1%X1 %0,%1
>     lfiwzx %0,%y1
>     lxsiwzx %x0,%y1
> @@ -7611,7 +7612,7 @@ (define_insn "*movsi_internal1"
>     mt%0 %1
>     nop"
>    [(set_attr "type"
> -	  "*,          *,
> +	  "*,          *,          fpsimple,
>  	   load,       fpload,     fpload,
>  	   store,      fpstore,    fpstore,
>  	   *,          *,          *,          *,
> @@ -7620,7 +7621,7 @@ (define_insn "*movsi_internal1"
>  	   mtvsr,      mfvsr,
>  	   *,          *,          *")
>     (set_attr "length"
> -	  "*,          *,
> +	  "*,          *,          *,
>  	   *,          *,          *,
>  	   *,          *,          *,
>  	   *,          *,          *,          8,
> @@ -7629,9 +7630,9 @@ (define_insn "*movsi_internal1"
>  	   *,          *,
>  	   *,          *,          *")
>     (set_attr "isa"
> -	  "*,          *,
> -	   *,          p8v,        p8v,
> -	   *,          p8v,        p8v,
> +	  "*,          *,          *,
> +	   *,          p7,         p8v,
> +	   *,          *,          p8v,
>  	   *,          *,          p10,        *,
>  	   p8v,        p9v,        p9v,        p8v,
>  	   p9v,        p8v,        p9v,