[AArch64] Add missing SEH_Nop when aligning the stack

[mstorsjo]

Summary

This makes sure that the instructions of the prologue matches the
SEH opcodes.

Also remove a couple redundant cases of setting HasWinCFI; it was
already set unconditionally after the conditional cases.

[efriedma]

llvm/test/CodeGen/AArch64/wineh-align-stack.ll

24

The prologue should end after the set_set_fp; it makes no sense to try to represent the "and" in the SEH unwind tables. (I thought we discussed this before at some point, but maybe I'm confusing it with something else...)

[mstorsjo]

llvm/test/CodeGen/AArch64/wineh-align-stack.ll

24

Yes, we discussed that for the ARM implementation of SEH, and I changed it that way there in the end.

For AArch64, the SEH prologue currently includes all of the realigning; I agree that it would be more correct to omit that from the prologue entirely. The AArch64 SEH insertion is a bit more annoying to fix though, as it currently adds all these individual SEH opcodes along with the prologue instructions, while the ARM implementation adds SEH opcodes for all instructions in a range in an MBB later, making it easier to include or leave out parts of it.

So for this case, I mostly wanted to fix mismatches to make D131394 work - I'd prefer to leave out this (trimming out unnecessary parts of the prologue in these cases) as a task up for grabs for someone else.

[efriedma]

LGTM

llvm/test/CodeGen/AArch64/wineh-align-stack.ll

24

Please leave a FIXME in the testcase, for whoever is updating it in the future.

llvm/lib/Target/AArch64/AArch64InstrInfo.cpp

if (EmitCFAOffset && DestReg == TmpReg) {
           .setMIFlags(Flag);
     }
 
     if (NeedsWinCFI) {
       assert(Sign == 1 && "SEH directives should always have a positive sign");
       int Imm = (int)(ThisVal << LocalShiftSize);
       if ((DestReg == AArch64::FP && SrcReg == AArch64::SP) ||
           (SrcReg == AArch64::FP && DestReg == AArch64::SP)) {
-        if (HasWinCFI)
-          *HasWinCFI = true;
         if (Imm == 0)
           BuildMI(MBB, MBBI, DL, TII->get(AArch64::SEH_SetFP)).setMIFlag(Flag);
         else
           BuildMI(MBB, MBBI, DL, TII->get(AArch64::SEH_AddFP))
               .addImm(Imm)
               .setMIFlag(Flag);
         assert(Offset == 0 && "Expected remaining offset to be zero to "
                               "emit a single SEH directive");
       } else if (DestReg == AArch64::SP) {
-        if (HasWinCFI)
-          *HasWinCFI = true;
         assert(SrcReg == AArch64::SP && "Unexpected SrcReg for SEH_StackAlloc");
         BuildMI(MBB, MBBI, DL, TII->get(AArch64::SEH_StackAlloc))
             .addImm(Imm)
             .setMIFlag(Flag);
+      } else {
+        BuildMI(MBB, MBBI, DL, TII->get(AArch64::SEH_Nop))
+            .setMIFlag(Flag);
       }
       if (HasWinCFI)
         *HasWinCFI = true;
     }
 
     SrcReg = TmpReg;
   } while (Offset);
 }

llvm/test/CodeGen/AArch64/wineh-align-stack.ll

+; RUN: llc < %s -mtriple=aarch64-windows | FileCheck %s
+
+define dso_local void @func() {
+entry:
+  %buf = alloca [64 x i8], align 32
+  %arraydecay = getelementptr inbounds [64 x i8], ptr %buf, i64 0, i64 0
+  call void @other(ptr noundef %arraydecay)
+  ret void
+}
+
+declare dso_local void @other(ptr noundef)
+
+; CHECK-LABEL: func:
+; CHECK-NEXT: .seh_proc func
+; CHECK-NEXT: // %bb.0:
+; CHECK-NEXT: stp x29, x30, [sp, #-16]!
+; CHECK-NEXT: .seh_save_fplr_x 16
+; CHECK-NEXT: mov x29, sp
+; CHECK-NEXT: .seh_set_fp
+; CHECK-NEXT: sub x9, sp, #80
+; CHECK-NEXT: .seh_nop
+; CHECK-NEXT: and sp, x9, #0xffffffffffffffe0
+; CHECK-NEXT: .seh_stackalloc 80
+; CHECK-NEXT: .seh_endprologue
+
+; FIXME: Ideally, the SEH prologue wouldn't include the stack realigning
+; instructions at all, but it's enough to end the prologue after .seh_set_fp.