[PATCH] D96004: [AArch64] Stack probing for function prologues

[serge via Phabricator via llvm-commits]

serge-sans-paille added a comment.

@ostannard  I've done the best I can for this review, but I have no ARM background so I can't be sure for the arch-specific parts @kristof.beyls can you please have a look?



================
Comment at: llvm/lib/Target/AArch64/AArch64FrameLowering.h:153
+  /// Replace a StackProbe stub (if any) with the actual probe code inline
+  virtual void inlineStackProbe(MachineFunction &MF,
+                                MachineBasicBlock &PrologueMBB) const override;
----------------
Why did you set that one virtual?


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:16487
+  if (MF.getFunction().hasFnAttribute("probe-stack"))
+    return MF.getFunction().getFnAttribute("probe-stack").getValueAsString() ==
+           "inline-asm";
----------------
Should you warn at some point if another (unsupported) scheme is used?


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:16515
+  // more than 1024 bytes above SP.
+  return 1024;
+}
----------------
Can you give any hint about why 1024 was chosen, and why it's independent from `getStackProbeSize` ?


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

https://reviews.llvm.org/D96004