From: "Andre Vieira (lists)" <Andre.SimoesDiasVieira@arm.com>
To: Bernd Schmidt <bschmidt@redhat.com>,
GCC Patches <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH] PR78255: Make postreload aware of NO_FUNCTION_CSE
Date: Fri, 09 Dec 2016 15:34:00 -0000 [thread overview]
Message-ID: <584ACF02.9070101@arm.com> (raw)
In-Reply-To: <334ff580-3e7d-22fb-83da-da18acd84244@redhat.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
next prev parent reply other threads:[~2016-12-09 15:34 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-12-09 14:03 Andre Vieira (lists)
2016-12-09 15:02 ` Bernd Schmidt
2016-12-09 15:34 ` Andre Vieira (lists) [this message]
2016-12-09 15:58 ` Bernd Schmidt
2016-12-09 16:02 ` Ramana Radhakrishnan
2016-12-09 16:16 ` Andre Vieira (lists)
2016-12-09 16:31 ` Bernd Schmidt
2016-12-09 17:22 ` [arm-embedded][committed] " Andre Vieira (lists)
2017-01-06 10:53 ` [PATCH] " Andre Vieira (lists)
2017-01-06 15:47 ` Jeff Law
2017-01-11 15:09 ` Andre Vieira (lists)
2016-12-12 9:05 ` Christophe Lyon
2016-12-20 16:48 ` [ARM][committed] Fix for PR78255-2.c testism for targets that do not optimize for tailcall Andre Vieira (lists)
2016-12-09 16:01 ` [PATCH] PR78255: Make postreload aware of NO_FUNCTION_CSE Jeff Law
2016-12-09 15:47 Wilco Dijkstra
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=584ACF02.9070101@arm.com \
--to=andre.simoesdiasvieira@arm.com \
--cc=bschmidt@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).