[PATCH] D71754: [AArch64] Fix save register pairing for Windows AAPCS

Mon Dec 23 09:25:05 PST 2019

sanwou01 marked 2 inline comments as done.
sanwou01 added inline comments.

================
Comment at: llvm/lib/Target/AArch64/AArch64FrameLowering.cpp:1877
   // the MCLayer.  We need to add support for the save_lrpair unwind code.
+  if (Reg2 == AArch64::FP)
+    return true;
----------------
rengolin wrote:
> This seems awfully permissive. Shouldn't you check that the pairs are explicitly:
> 
>     FP + LR
>     LR + FP
Hm, this invalidates any pair with FP as the second register, so it follows that FP will be the first register of a pair (on Windows/WinAAPCS). The assert below (line 2028) checks that when FP is the first register of a pair, LR is the second.

Maybe I misunderstood your comment, but that seems quite precise to me!

================
Comment at: llvm/lib/Target/AArch64/AArch64FrameLowering.cpp:1986
         if (AArch64::GPR64RegClass.contains(NextReg) &&
-            !invalidateRegisterPairing(RPI.Reg1, NextReg, NeedsWinCFI,
+            !invalidateRegisterPairing(RPI.Reg1, NextReg, IsWindows, NeedsWinCFI,
                                        NeedsFrameRecord))
----------------
rengolin wrote:
> Is Windows' only ABI WindowsAAPCS?
Good point. It might be so in the AArch64 backend, but who knows when a new ABI comes around.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71754