[AArch64] Async unwind - do not schedule frame setup/destroy

[efriedma]

llvm/lib/Target/AArch64/AArch64InstrInfo.cpp

1102

Not sure I understand the logic here. Can we just make CFI directives a scheduling barrier, like we do for SEH directives? Why are instructions that modify the frame pointer special?

[chill]

llvm/lib/Target/AArch64/AArch64InstrInfo.cpp

1102

The CFI instructions are already a scheduling barrier (MachineIntsr::isPosition()), but that's not enough, as the PostRA scheduler can still reorder non-CFI instructions in a way that makes the unwind info not instruction precise. As an example, on

int g(int);

int f(int x) {
	__asm__(""::: "x20", "x21", "x22");
	return 1 + g(x - 1);
}

After PEI we get

bb.0.entry:
  liveins: $w0, $lr, $x22, $x20, $x21
  early-clobber $sp = frame-setup STPXpre killed $fp, killed $lr, $sp(tied-def 0), -6 :: (store (s64) into %stack.4), (store (s64) into %stack.3)
  frame-setup STRXui killed $x22, $sp, 2 :: (store (s64) into %stack.2)
  frame-setup STPXi killed $x21, killed $x20, $sp, 4 :: (store (s64) into %stack.1), (store (s64) into %stack.0)
  $fp = frame-setup ADDXri $sp, 0, 0
  frame-setup CFI_INSTRUCTION def_cfa $w29, 48
...

but then after the PostRA scheduling one of the stores and the assignment to FP get reordered

bb.0.entry:
  liveins: $w0, $lr, $x22, $x20, $x21
  early-clobber $sp = frame-setup STPXpre $fp, killed $lr, $sp(tied-def 0), -6 :: (store (s64) into %stack.4), (store (s64) into %stack.3)
  frame-setup STRXui killed $x22, $sp, 2 :: (store (s64) into %stack.2)
  $fp = frame-setup ADDXri $sp, 0, 0
  frame-setup STPXi killed $x21, killed $x20, $sp, 4 :: (store (s64) into %stack.1), (store (s64) into %stack.0)
  frame-setup CFI_INSTRUCTION def_cfa $w29, 48
...

or

f:                                      // @f
	.cfi_startproc
// %bb.0:                               // %entry
	stp	x29, x30, [sp, #-48]!           // 16-byte Folded Spill
	str	x22, [sp, #16]                  // 8-byte Folded Spill
	mov	x29, sp
	stp	x21, x20, [sp, #32]             // 16-byte Folded Spill
	.cfi_def_cfa w29, 48
...

so now while the PC is at the `stp	x21, x20, [sp, #32]` the unwind info tells that
the CFA register is still SP and the FP has its original value.

Instructions that modify FP (and SP, but it's already handled) are special in that they (potentially) affect
what the current CFA is.

[chill]

Ping?

[efriedma]

Oh, I understand the issue; we need to ensure that any instruction annotated by a CFI directive is not rescheduled away from the directive. (This isn't a problem for Windows EH because there's never more than one instruction between two SEH markings.)

I don't see how this is specific to instructions that modify fp, though. We end up with inaccurate CFI if we put any instruction between an instruction and its associated CFI directive, whether or not that instruction modifies fp. Can we explicitly check for instructions followed by a CFI directive?

[chill]

In D112326#3256195, @efriedma wrote:

Oh, I understand the issue; we need to ensure that any instruction annotated by a CFI directive is not rescheduled away from the directive. (This isn't a problem for Windows EH because there's never more than one instruction between two SEH markings.)

I don't see how this is specific to instructions that modify fp, though.

We end up with inaccurate CFI if we put any instruction between an instruction and its associated CFI directive, whether or not that instruction modifies fp.

No, not necessarily, but, that said ...

Can we explicitly check for instructions followed by a CFI directive?

... I think this would work. I'll give it a try.

[efriedma]

LGTM

[chill]

Just rebasing the whole chain for the sake of the remaining unreviewed patches.

llvm/lib/Target/AArch64/AArch64InstrInfo.cpp

if (MI.getOperand(0).getImm() == 0x14)
       return true;
     break;
   case AArch64::DSB:
   case AArch64::ISB:
     // DSB and ISB also are scheduling barriers.
     return true;
   default:;
   }
-  return isSEHInstruction(MI);
+  if (isSEHInstruction(MI))
+    return true;
+  auto Next = std::next(MI.getIterator());
+  return Next != MBB->end() && Next->isCFIInstruction();
 }
 
 /// analyzeCompare - For a comparison instruction, return the source registers
 /// in SrcReg and SrcReg2, and the value it compares against in CmpValue.
 /// Return true if the comparison instruction can be analyzed.
 bool AArch64InstrInfo::analyzeCompare(const MachineInstr &MI, Register &SrcReg,
                                       Register &SrcReg2, int64_t &CmpMask,
                                       int64_t &CmpValue) const {
   // The first operand can be a frame index where we'd normally expect a
   // register.

llvm/test/CodeGen/AArch64/GlobalISel/byval-call.ll

 
 declare void @byval_a64i32([64 x i32]* byval([64 x i32]) %ptr)
 
 define void @call_byval_a64i32([64 x i32]* %incoming) {
 ; CHECK-LABEL: call_byval_a64i32:
 ; CHECK:       // %bb.0:
 ; CHECK-NEXT:    sub sp, sp, #288
 ; CHECK-NEXT:    stp x29, x30, [sp, #256] // 16-byte Folded Spill
-; CHECK-NEXT:    add x29, sp, #256
 ; CHECK-NEXT:    str x28, [sp, #272] // 8-byte Folded Spill
+; CHECK-NEXT:    add x29, sp, #256
 ; CHECK-NEXT:    .cfi_def_cfa w29, 32
 ; CHECK-NEXT:    .cfi_offset w28, -16
 ; CHECK-NEXT:    .cfi_offset w30, -24
 ; CHECK-NEXT:    .cfi_offset w29, -32
 ; CHECK-NEXT:    ldr q0, [x0]
 ; CHECK-NEXT:    str q0, [sp]
 ; CHECK-NEXT:    ldr q0, [x0, #16]
 ; CHECK-NEXT:    str q0, [sp, #16]

llvm/test/CodeGen/AArch64/arm64-homogeneous-prolog-epilog-no-helper.ll

 ; CHECK-NEXT:    ldp d11, d10, [sp], #64
 ; CHECK-NEXT:    ret
 ;
 ; CHECK-LINUX-LABEL: _Z3foofffi:
 ; CHECK-LINUX:       // %bb.0: // %entry
 ; CHECK-LINUX-NEXT:    stp d11, d10, [sp, #-64]!
 ; CHECK-LINUX-NEXT:    stp d9, d8, [sp, #16]
 ; CHECK-LINUX-NEXT:    stp x29, x30, [sp, #32]
-; CHECK-LINUX-NEXT:    add x29, sp, #32
 ; CHECK-LINUX-NEXT:    stp x20, x19, [sp, #48]
+; CHECK-LINUX-NEXT:    add x29, sp, #32
 ; CHECK-LINUX-NEXT:    .cfi_def_cfa w29, 32
 ; CHECK-LINUX-NEXT:    .cfi_offset w19, -8
 ; CHECK-LINUX-NEXT:    .cfi_offset w20, -16
 ; CHECK-LINUX-NEXT:    .cfi_offset w30, -24
 ; CHECK-LINUX-NEXT:    .cfi_offset w29, -32
 ; CHECK-LINUX-NEXT:    .cfi_offset b8, -40
 ; CHECK-LINUX-NEXT:    .cfi_offset b9, -48
 ; CHECK-LINUX-NEXT:    .cfi_offset b10, -56

llvm/test/CodeGen/AArch64/framelayout-frame-record.mir

bb.0:
     $d9 = IMPLICIT_DEF
     $x19 = IMPLICIT_DEF
     RET_ReallyLR
 
 # CHECK-LABEL: TestFrameRecordLocation
 
 # CHECK: stp d9, d8, [sp, #-48]!
 # CHECK: stp x29, x30, [sp, #16]
-# CHECK: add x29, sp, #16
 # CHECK: str x19, [sp, #32]
+# CHECK: add x29, sp, #16
 
 # CHECK: .cfi_def_cfa w29, 32
 # CHECK: .cfi_offset w19, -16
 # CHECK: .cfi_offset w30, -24
 # CHECK: .cfi_offset w29, -32
 # CHECK: .cfi_offset b8, -40
 # CHECK: .cfi_offset b9, -48
 ...

llvm/test/CodeGen/AArch64/framelayout-unaligned-fp.ll

entry:
   %call2 = call i64 @f(i64 %2)
   %conv3 = fptosi float %conv to i64
   ret i64 %conv3
 }
 
 ; CHECK-LABEL: b:
 ; CHECK:       str     d8, [sp, #-32]!
 ; CHECK-NEXT:  stp     x29, x30, [sp, #8]
-; CHECK-NEXT:  add     x29, sp, #8
 ; CHECK-NEXT:  str     x19, [sp, #24]
+; CHECK-NEXT:  add     x29, sp, #8
 
 ; CHECK:       sub     sp, x29, #8
 ; CHECK-NEXT:  ldp     x29, x30, [sp, #8]
 ; CHECK-NEXT:  ldr     x19, [sp, #24]
 ; CHECK-NEXT:  ldr     d8, [sp], #32
 ; CHECK-NEXT:  ret
 
 declare i64 @d()
 declare i64 @e()
 declare i64 @f(i64)