[PATCH] D74183: [IRGen] Add an alignment attribute to underaligned sret parameters

[Nikita Popov via Phabricator via cfe-commits]

nikic added a comment.

In D74183#1941741 <https://reviews.llvm.org/D74183#1941741>, @efriedma wrote:

> If it's just tramp3d-v4, I'm not that concerned... but that's a weird result.  On x86 in particular, alignment markings have almost no effect on optimization, generally.


I've just looked at the IR diff for tramp3d-v4 and it turns out that the root cause is an old friend of mine: The insertion of alignment assumptions during inlining (https://github.com/llvm/llvm-project/blob/b58902bc72c2b479b5ed27ec0d3422ba9782edbb/llvm/lib/Transforms/Utils/InlineFunction.cpp#L1139-L1173). That is, the IR now contains many instances of this sequence:

  %ptrint = ptrtoint %class.GuardLayers* %guards_m to i64
  %maskedptr = and i64 %ptrint, 3
  %maskcond = icmp eq i64 %maskedptr, 0
  tail call void @llvm.assume(i1 %maskcond)

to preserve the alignment information. From a cursory look I cannot tell whether these additional assumes also regress optimization (due to multi-use), but given the size increase on the final binary it seems pretty likely that this is the case.

This preservation of alignment during inlining is the reason why we used to not emit alignment information for pointer arguments in Rust for a long time: It caused serious regressions in optimization and increased compile-time. Nowadays we do emit alignment information, but set `-preserve-alignment-assumptions-during-inlining=false` to prevent this inlining behavior.

I think for the purposes of this revision, this means that we should probably either a) default `preserve-alignment-assumptions-during-inlining` to false (I would prefer this) or b) not emit the alignment unless it is smaller than the ABI alignment (I guess this was what this patch originally did?)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74183/new/

https://reviews.llvm.org/D74183