[PATCH] D88543: [AArch64] Match the windows canonical callee saved register order

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64FrameLowering.cpp:2018
+    if (N % 2 != 0)
+      return true;
+  } else if (Reg1 == AArch64::FP && Reg2 == AArch64::LR) {
----------------
efriedma wrote:
> This if ends up being a little confusing to read... maybe worth duplicating the `if (Reg2 == Reg1 + 1)` check into both the integer and fp branches of the if statement.
Hmm, maybe...

I'm a bit dissatisfied with this solution, as it misses some pairing opportunities: E.g. if saving only x20 and x21, but not x19, this code wouldn't manage to pair x20 and x21 - as when iterating over them in the top-to-bottom order, when inspecting x21, it'll choose not to pair it with x20, as it expects x20 to be paired with x19.

But I'm a bit out of ideas of a good way of achieving that, without making a huge mess in the generic code in computeCalleeSaveRegisterPairs...

On the other hand, I'm not sure if that's much of a practical issue that happens often, other than in pathological cases?


================
Comment at: llvm/lib/Target/AArch64/AArch64FrameLowering.cpp:2211
+      bool DoFixup = false;
+      if (!NeedsWinCFI) {
+        DoFixup = !RPI.isPaired();
----------------
efriedma wrote:
> Do we really need to check NeedsWinCFI here?  It seems like the rounding should be a property of the chosen stack layout, which should already be determined at this point.
We do: If we need to save the set of x19, d8 and d9, the windows stack layout of that is (bottom-up) x19, (d8, d9), gap. When iterating backwards starting from the top, we start out with d9 and pair it with d8 - but we need to add the alignment (to the d9 stack object) to get the gap here at the top. But the pair d8,d9 aren't aligned to a 16 byte boundary themselves.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88543