Hi!

On Wed, May 19, 2021 at 04:36:00PM +0800, HAO CHEN GUI wrote:
> On 19/5/2021 下午 4:33, HAO CHEN GUI wrote:
> >    This patch removes mode promotion of SSA variables on rs6000 
> >platform.

It isn't "promotion of SSA variables".  At the point where this code
applies we are generating RTL, which doesn't do SSA.  It has what is
called "pseudo-registers" (or short, "pseudos"), which will be assigned
hard registers later.

> >    Bootstrapped and tested on powerppc64le and powerppc64be (with 
> >m32) with no regressions. Is this okay for trunk? Any recommendations? 
> >Thanks a lot.

powerpc64-linux and powerpc64le-linux I guess?

>       * config/rs6000/rs6000-call.c (rs6000_promote_function_mode):
>       Replace PROMOTE_MODE marco with its content.
>       * config/rs6000/rs6000.h (PROMOTE_MODE): Remove.

Please split this into two?  The first is obvious, the second much less
so; we'll need to see justification for it.  I know it helps greatly,
but please record that in the commit message :-)

> diff --git a/gcc/config/rs6000/rs6000-call.c b/gcc/config/rs6000/rs6000-call.c
> index f5676255387..dca139b2ecf 100644
> --- a/gcc/config/rs6000/rs6000-call.c
> +++ b/gcc/config/rs6000/rs6000-call.c
> @@ -6646,7 +6646,9 @@ rs6000_promote_function_mode (const_tree type 
> ATTRIBUTE_UNUSED,
>                             int *punsignedp ATTRIBUTE_UNUSED,
>                             const_tree, int for_return ATTRIBUTE_UNUSED)
>  {
> -  PROMOTE_MODE (mode, *punsignedp, type);
> +  if (GET_MODE_CLASS (mode) == MODE_INT
> +      && GET_MODE_SIZE (mode) < (TARGET_32BIT ? 4 : 8))
> +    mode = TARGET_32BIT ? SImode : DImode;
>  
>    return mode;
>  }

So this part is pre-approved as a separate patch.

> -/* Define this macro if it is advisable to hold scalars in registers
> -   in a wider mode than that declared by the program.  In such cases,
> -   the value is constrained to be within the bounds of the declared
> -   type, but kept valid in the wider mode.  The signedness of the
> -   extension may differ from that of the type.  */
> -
> -#define PROMOTE_MODE(MODE,UNSIGNEDP,TYPE)    \
> -  if (GET_MODE_CLASS (MODE) == MODE_INT              \
> -      && GET_MODE_SIZE (MODE) < (TARGET_32BIT ? 4 : 8)) \
> -    (MODE) = TARGET_32BIT ? SImode : DImode;
> -

And this part needs some more words in the commit message :-)

Since we do have instructions that can do (almost) everything 32-bit at
least as efficiently as the corresponding 64-bit things, it helps a lot
to not promote so many things to 64 bits.  We didn't realise that before
because that TARGET_PROMOTE_FUNCTION_MODE thing was in the way, since
the very early days of the rs6000 port even, so everyone (well, at least
me :-) ) was tricked into thinking this is an ABI requirement and we
cannot touch it.  But of course it is not, and we can :-)

Some examples of how this improves generated code, or even some
benchmark results, would be good to have.

Also, how about something like

#define PROMOTE_MODE(MODE,UNSIGNEDP,TYPE)       \
  if (GET_MODE_CLASS (MODE) == MODE_INT         \
      && GET_MODE_SIZE (MODE) < 4)              \
    (MODE) = SImode;

(that is, promoting modes smaller than SImode to SImode).  How does that
compare?

Thanks!


Segher

Reply via email to