[PATCH, rs6000] Remove mode promotion of SSA variables
HAO CHEN GUI
guihaoc@linux.ibm.com
Thu May 20 08:29:07 GMT 2021
On 19/5/2021 下午 9:20, Segher Boessenkool wrote:
> 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?
It hits an ICE when assigning a function return value to a variable. The
mode of variable is promoted to SImode, but the mode of return value is
promoted to DImode. Current GCC logical is either the mode of set_dest
is unchanged or the mode of set_dest matches the mode of function return
value.
By the way, I tested the patch on ppc64 with m32 option. The SPECint
shows a little improvement(0.4%). So it's better not do any mode promotions.
>
> Thanks!
>
>
> Segher
More information about the Gcc-patches
mailing list