[PATCH] D77016: Change AArch64 Windows EH UnwindHelp object to be a fixed object

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

If you're not using arcanist, please upload patches with full context (-U10000).



================
Comment at: llvm/lib/Target/AArch64/AArch64FrameLowering.cpp:219
 
+/// Returns the size of fixed object (to determine the prologue size).
+static unsigned getFixedObjectSize(const MachineFunction &MF,
----------------
Maybe elaborate a bit; this is specifically the first space allocated in the function, next to sp on entry.


================
Comment at: llvm/lib/Target/AArch64/AArch64FrameLowering.cpp:1011
       Subtarget.isCallingConvWin64(MF.getFunction().getCallingConv());
   // Var args are accounted for in the containing function, so don't
   // include them for funclets.
----------------
Maybe move the "Var args are accounted for in the containing function" comment into getFixedObjectSize?


================
Comment at: llvm/lib/Target/AArch64/AArch64FrameLowering.cpp:2683
+  for (int I = MFI.getObjectIndexBegin(); I < 0; ++I)
+    MinFixedObjOffset = std::min(MinFixedObjOffset, MFI.getObjectOffset(I));
+
----------------
This loop is strange. There's space explicitly allocated for the object in getFixedObjectSize, so you should be able to directly compute its offset.


================
Comment at: llvm/lib/Target/AArch64/AArch64FrameLowering.cpp:2690
+  // Ensure alignment.
+  UnwindHelpOffset = -alignTo(-UnwindHelpOffset, 16);
+
----------------
Why does UnwindHelp need to be 16-byte aligned?


================
Comment at: llvm/test/CodeGen/AArch64/wineh-unwindhelp-via-fp.ll:10
+; CHECK: mov x[[#SCRATCH_REG:]], #-2
+; CHECK: stur x[[#SCRATCH_REG:]], [x29, #[[#]]]
+;
----------------
This testcase seems overly complicated; if you're only checking the assembly output for ?func2@@YAXXZ, no need to include the other definitions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77016