[PATCH, rs6000] Remove mode promotion of SSA variables

[HAO CHEN GUI]

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