Re: [PATCH] PR78255: Make postreload aware of NO_FUNCTION_CSE

["Andre Vieira (lists)" <Andre dot SimoesDiasVieira at arm dot com>]

On 09/12/16 15:02, Bernd Schmidt wrote:
> On 12/09/2016 03:03 PM, Andre Vieira (lists) wrote:
>> This patch fixes the issue reported in PR78255 by making postreload
>> aware it should not be performing CSE on functions if NO_FUNCTION_CSE is
>> defined to true.
>>
>> Bootstrap and full regression on arm-none-linux-gnueabihf and
>> aarch64-unknown-linux-gnu.
>>
>> Also checked this fixed the reported issue on arm-none-eabi.
>>
>> Is this OK for trunk?
> 
> Hmm, it probably doesn't hurt, but looking at the PR I think the
> originally reported problem suggests you need a different fix: a
> separate register class to be used for indirect sibling calls. I
> remember seeing similar issues on other targets.
> 
> 
> Bernd

I agree that even though this "fixes" the PR issue, this change is
fixing more than just that.

As for your suggestion to use a separate register class for indirect
sibling calls. We already do, we use CALLER_SAVE_REGS. However, 'r3' is
also allowed by that scheme as it should. Since if we don't use 'r3' to
either pass an argument or align the stack, then it is perfectly valid
to use it for indirect sibling calls.

The problem is at the time where we decide whether it is safe to use
'r3' we expect the assigned registers not to change and postreload does,
when it shouldn't. Hence why I am now telling it to not do that. Now it
could be that there are other cases in which the register allocation
would change after reload and before the pro and epilogue pass. Maybe we
shouldn't be making the decision quite so early. This is a bit of a can
of worms though...

Regardless, the other testcases I add in this patch show a sub-optimal
transformation done by postreload, turning direct calls into indirect
calls, for targets which have specifically pointed out that no CSE
should be done on functions through 'NO_FUNCTION_CSE'.  Maybe it would
make more sense to split this up into two PR's, though by fixing
postreload I wouldn't be able to reproduce the failure mentioned in PR78255.

Would you prefer I create a new PR for the problem this is actually
fixing and refile this PATCH under that PR?

Cheers,
Andre