[PATCH] D81358: [PowerPC] Implement probing for dynamic stack allocation

[Kai Luo via Phabricator via llvm-commits]

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


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:11701
+  // attribute.
+  unsigned StackProbeSize = 4096;
+  const Function &Fn = MF.getFunction();
----------------
jsji wrote:
> Can we set the size to `sys::Process::getPageSizeEstimate()`? Typical systems are all giving me 65536, are we probing too much if we set it to 4096 here?
Yes, `sysconf` gives `65536` on my rhel POWER machine and `4096` on my rhel X86 machine. I'm quite concerned if users do cross compilation if the host has large page size than the target.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:11714
+// Lower dynamic stack allocation with probing. It's not implemented like
+// PPCRegisterInfo::lowerDynamicAlloc, since it involves loop and RegScavenger
+// is unable to allocate registers for loops in prologepilog.
----------------
jsji wrote:
> I don't quite understand this. Why we can NOT do this similar to x86 or SystemZ? using PHI nodes.
The comment to compare with `PPCRegisterInfo::lowerDynamicAlloc` might be a little bit misleading. My point is `PPCTargetLowering::emitProbedAlloca` is unable to expaned in `prologepilog` like `PPCRegisterInfo::lowerDynamicAlloc`. Should I delete this comparison?

Looks we don't need phi nodes in the loop, only `r1`, which is a physical register, is updated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81358