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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to