[PATCH] D88994: Fix the default alignment of i1 vectors.

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/test/CodeGen/Thumb2/mve-pred-bitcast.ll:133
 ; CHECK-LE:       @ %bb.0: @ %entry
-; CHECK-LE-NEXT:    .save {r4, r6, r7, lr}
-; CHECK-LE-NEXT:    push {r4, r6, r7, lr}
-; CHECK-LE-NEXT:    .setfp r7, sp, #8
-; CHECK-LE-NEXT:    add r7, sp, #8
-; CHECK-LE-NEXT:    .pad #16
-; CHECK-LE-NEXT:    sub sp, #16
-; CHECK-LE-NEXT:    mov r4, sp
-; CHECK-LE-NEXT:    bfc r4, #0, #4
-; CHECK-LE-NEXT:    mov sp, r4
+; CHECK-LE-NEXT:    .pad #4
+; CHECK-LE-NEXT:    sub sp, #4
----------------
dmgreen wrote:
> efriedma wrote:
> > sdesmalen wrote:
> > > Not sure if this matters, but I can't see any operations (select) going through memory, so don't know why the code on the left was so convoluted or how your change affects this.
> > Type legalization is creating a stack temporary to lower the bitcast.  The reason it doesn't show up in the assembly is that we optimize it out later: the resulting `<16 x i1>` load gets legalized to an i16 load followed by a PREDICATE_CAST, and then the whole store+load sequence gets optimized out.  But the alignment requirement of the stack temporary sticks around.
> > 
> > This patch avoids the issue by lowering the alignment of the temporary, so it doesn't trigger stack realignment.
> > 
> > There's probably room for improvement here.
> Yeah, I'm a little surprised that it's doing that. There's no reason to go via a stack slot to begin with, and that dead stack slot are not removed...
> 
> I was trying to custom lower it but running into legalizer problems. I may keep looking, but it's not an problem with this patch.
Thanks for confirming, I suspected is was something like that!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88994