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