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