Page MenuHomePhabricator

[SelectionDAG] Don't promote the alignment of allocas beyond the stack alignment.

Authored by efriedma on May 6 2020, 5:28 PM.



allocas in LLVM IR have a specified alignment. When that alignment is specified, the alloca has at least that alignment at runtime.

If the specified type of the alloca has a higher preferred alignment, SelectionDAG currently ignores that specified alignment, and increases the alignment. It does this even if it would trigger stack realignment. I don't think this makes sense, so this patch changes that.

I was looking into this for SVE in particular: for SVE, overaligning vscale'ed types is extra expensive because it requires realigning the stack multiple times, or using dynamic allocation. (This currently isn't implemented.)

I updated the expected assembly for a couple tests; in particular, for arg-copy-elide.ll, the optimization in question does not increase the alignment the way SelectionDAG normally would. For the rest, I just increased the specified alignment on the allocas to match what SelectionDAG was inferring.

Diff Detail

Event Timeline

efriedma created this revision.May 6 2020, 5:28 PM
Herald added a project: Restricted Project. · View Herald Transcript
sdesmalen added inline comments.May 7 2020, 12:11 PM

nit: AI->getAlign().getValueOr(TyPrefAlign)


The LangRef says:

On every specification that takes a <abi>:<pref>, specifying the <pref> alignment is optional. If omitted, the preceding : should be omitted too and <pref> will be equal to <abi>.

Does this need an assert to ensure that the type's ABI alignment <= StackAlign?

efriedma marked an inline comment as done.May 7 2020, 12:38 PM
efriedma added inline comments.

The use of the preferred alignment here is purely an optimization. If the IR specifies the alignment it wants, the datalayout rules aren't relevant.

The alignment of the alloca could end up less than the ABI alignment, and that's fine; if the frontend needed an ABI-aligned alloca, it wouldn't have specified a lower alignment.

sdesmalen accepted this revision.May 7 2020, 1:13 PM



Okay I wasn't sure about that, thanks for clarifying.

This revision is now accepted and ready to land.May 7 2020, 1:13 PM
This revision was automatically updated to reflect the committed changes.