public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Richard Biener <richard.guenther@gmail.com>
To: Jeff Law <jlaw@tachyum.com>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>
Subject: Re: Aligning stack offsets for spills
Date: Tue, 8 Jun 2021 08:56:38 +0200	[thread overview]
Message-ID: <CAFiYyc2rTxGhWAv_q=Q8EnC7sQtDSxagk6OzeXjZ==8YV3CtHQ@mail.gmail.com> (raw)
In-Reply-To: <98179c8e-bcec-83ed-5b99-6f54791bd7cd@tachyum.com>

On Mon, Jun 7, 2021 at 9:00 PM Jeff Law <jlaw@tachyum.com> wrote:
>
>
> So, as many of you know I left Red Hat a while ago and joined Tachyum.
> We're building a new processor and we've come across an issue where I
> think we need upstream discussion.
>
> I can't divulge many of the details right now, but one of the quirks of
> our architecture is that reg+d addressing modes for our vector
> loads/stores require the displacement to be aligned.  This is an
> artifact of how these instructions are encoded.
>
> Obviously we can emit a load of the address into a register when the
> displacement isn't aligned.  From a correctness point that works
> perfectly.  Unfortunately, it's a significant performance hit on some
> standard benchmarks (spec) where we have a great number of spills of
> vector objects into the stack at unaligned offsets in the hot parts of
> the code.
>
>
> We've considered 3 possible approaches to solve this problem.
>
> 1. When the displacement isn't properly aligned, allocate more space in
> assign_stack_local so that we can make the offset aligned.  The downside
> is this potentially burns a lot of stack space, but in practice the cost
> was minimal (16 bytes in a 9k frame)  From a performance standpoint this
> works perfectly.
>
> 2. Abuse the register elimination code to create a second pointer into
> the stack.  Spills would start as <virtual> + offset, then either get
> eliminated to sp+offset' when the offset is aligned or gpr+offset'' when
> the offset wasn't properly aligned. We started a bit down this path, but
> with #1 working so well, we didn't get this approach to proof-of-concept.
>
> 3. Hack up the post-reload optimizers to fix things up as best as we
> can.  This may still be advantageous, but again with #1 working so well,
> we didn't explore this in any significant way.  We may still look at
> this at some point in other contexts.
>

So just as extra info - you're pre-allocating the frame (including for spills)
and not using push/pop?

> Here's what we're playing with.  Obviously we'd need a target hook to
> drive this behavior.  I was thinking that we'd pass in any slot offset
> alignment requirements (from the target hook) to assign_stack_local and
> that would bubble down to this point in try_fit_stack_local:
>
> diff --git a/gcc/function.c b/gcc/function.c
> index d616f5f64f4..7f441b87a63 100644
> --- a/gcc/function.c
> +++ b/gcc/function.c
> @@ -307,6 +307,14 @@ try_fit_stack_local (poly_int64 start, poly_int64
> length,
>     frame_off = targetm.starting_frame_offset () % frame_alignment;
>     frame_phase = frame_off ? frame_alignment - frame_off : 0;
>
> +  if (known_eq (size, 64) && alignment < 64)
> +    alignment = 64;
> +

I'm not familiar with the spill slot allocation code in GCC (I assume the above
is part of it) - do we in any way "sort" the spill slots so the extra required
padding is minimal?  Does the above guarantee that in the end the
offset will be aligned?  I assume IRA/LRA can still choose to eliminate
the respective frame pointer to sth else that ends up misaligning the offset
again?  Thus is it a real fix or a heuristic that ends up working most of
the time?

The actual alignment value should be dependent on the mode and
target preference and thus a target hook I suppose (you mention
this applies to vector loads/stores only).

Don't you have the very same issue with non-stack accesses?

Richard.

>     /* Round the frame offset to the specified alignment.  */
>
>     if (FRAME_GROWS_DOWNWARD)
>
> Thoughts?
>
> Jeff

  reply	other threads:[~2021-06-08  6:56 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-07 19:00 Jeff Law
2021-06-08  6:56 ` Richard Biener [this message]
2021-06-08 15:00   ` Jeff Law
2021-06-08 14:08 ` Michael Matz
2021-06-08 14:47   ` Jeff Law
2021-06-08 14:55     ` Jakub Jelinek
2021-06-08 15:06       ` H.J. Lu
2021-06-08 15:18         ` Jeff Law
2021-06-08 15:56           ` Michael Matz
2021-06-10 22:49       ` Jeff Law
2021-06-10 19:28 ` Peter Bergner
2021-06-10 21:34   ` Segher Boessenkool

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='CAFiYyc2rTxGhWAv_q=Q8EnC7sQtDSxagk6OzeXjZ==8YV3CtHQ@mail.gmail.com' \
    --to=richard.guenther@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jlaw@tachyum.com \
    /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).