[PATCH] D152626: Add support for probestack on ARM targets

[Ryan Summers via Phabricator via llvm-commits]

ryan-summers added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMFrameLowering.cpp:591
-  return (StackSizeInBytes >= StackProbeSize) &&
-         !F.hasFnAttribute("no-stack-arg-probe");
-}
----------------
The `no-stack-arg-probe` FnAttribute is no longer present for Windows, which isn't desirable as the removal would imply a break in backwards compatibility.


================
Comment at: llvm/lib/Target/ARM/ARMFrameLowering.cpp:765
   if (!AFI->hasStackFrame() &&
-      (!STI.isTargetWindows() || !WindowsRequiresStackProbe(MF, NumBytes))) {
+      !(EmitStackProbeCall && NumBytes >= StackProbeSize)) {
     if (NumBytes != 0) {
----------------
It looks like this unintentionally is removing the check for WIndows stack probe requirements. I'll need to reinclude this condition


================
Comment at: llvm/lib/Target/ARM/ARMFrameLowering.cpp:967
 
-  if (STI.isTargetWindows() && WindowsRequiresStackProbe(MF, NumBytes)) {
+  if (NumBytes >= StackProbeSize && EmitStackProbeCall) {
     uint32_t NumWords = NumBytes >> 2;
----------------
Similar to the above, this changes the conditional for Windows requiring stack probes, which it shouldn't.


================
Comment at: llvm/lib/Target/ARM/ARMFrameLowering.cpp:2301
   // this estimate also includes the varargs store size.
-  if (STI.isTargetWindows() &&
-      WindowsRequiresStackProbe(MF, MFI.estimateStackSize(MF))) {
+  if (MFI.estimateStackSize(MF) >= StackProbeSize && EmitStackProbeCall) {
     SavedRegs.set(ARM::R4);
----------------
Same as earlier - we need to preserve stack probe emission for Windows


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152626