From: Jeff Law <law@redhat.com>
To: Segher Boessenkool <segher@kernel.crashing.org>
Cc: gcc-patches <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH][RFA/RFC] Stack clash mitigation patch 05/08
Date: Mon, 31 Jul 2017 15:53:00 -0000 [thread overview]
Message-ID: <9f927f5d-8290-d5d7-dee1-4a964e35fa10@redhat.com> (raw)
In-Reply-To: <20170721202533.GF13471@gate.crashing.org>
On 07/21/2017 02:25 PM, Segher Boessenkool wrote:
> On Thu, Jul 20, 2017 at 08:20:52AM -0600, Jeff Law wrote:
>>> Can only combine-stack-adjustments do this? It seems like something
>>> many passes could do, and then your new note doesn't help.
>> SO far it's only been observed with c-s-a, but further auditing is
>> certainly desirable here, particularly with the upcoming changes to the
>> generic dynamic alloca handling.
>>
>> In the V2 patch only backends would emit unrolled inline alloca/probe
>> sequences like what you see above and only for prologues. Thus there
>> were a very limited number of passes to be concerned about.
>>
>> In the V3 patch we have unrolled inline probing for the dynamic space as
>> well, so this kind of sequence is exposed to everything after
>> gimple->rtl expansion.
>>
>> Unfortunately, the most thorough checker we have is x86 and on that
>> target, because of stack alignment issues, we'll never see a constant
>> size in the dynamic space and thus no unrolled inlined alloca/probe
>> sequences.
>>
>> In reality I suspect that with teh hard register references, most passes
>> are going to leave those insns alone, but some auditing is necessary.
>
> This is similar to what rs6000 uses stack_tie for. You want the
> prevent a store to the stack (the probe) from being moved after a
> later stack pointer update. By pretending (in the insn pattern)
> there is a store to stack with that stack pointer update, nothing
> can move stores after it.
FWIW, we found a case where the scheduler would muck things up.
Essentially it has code rewrites address computations and memory
references in the hopes of breaking dependency chains. In many ways
it's a lot like c-s-a.
Anyway, it broke the dependency chain and as a result we ended up with
unsafe sequences on aarch64. THe V3 patch posted last night addresses
that in two ways. First the scheduler knows about the magic note in the
same manner as c-s-a. And we emit scheduling barriers in the probing
code to prevent undesirable movement.
Jeff
next prev parent reply other threads:[~2017-07-31 15:53 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-19 5:17 Jeff Law
2017-07-20 13:14 ` Segher Boessenkool
2017-07-20 14:20 ` Jeff Law
2017-07-21 20:25 ` Segher Boessenkool
2017-07-31 15:53 ` Jeff Law [this message]
-- strict thread matches above, loose matches on Subject: below --
2017-07-11 21:21 Jeff Law
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=9f927f5d-8290-d5d7-dee1-4a964e35fa10@redhat.com \
--to=law@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=segher@kernel.crashing.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).