[PATCH] D150033: fix stack probe lowering for x86_intrcc

[Phoebe Wang via Phabricator via llvm-commits]

pengfei added a comment.

> IIUC in the context of `x86_intrcc` the 8 byte stack pointer change should never cause any stack probing to be emitted because it's smaller than the stack probe size (4096 bytes)

I have the same conclusion but for a different reason. It is not because 8 bytes < 4096. We still have chance to enter a guard page if the current `RSP` is near the end a a page, e.g., 4095. The reason is no matter what the size we want for stack realignment, the rest space we used to align is always in the same page unless the alignment requirement > 4096 bytes, which I don't think existing in reality.
Given that, I think should prevent to build the first `STACKALLOC_W_PROBING` rather than patch it later.

> `x86_intrcc` is used for exception/interrupt handlers on x86... The probing is done by writing to the memory to see if it will trigger a fault.

IIUC, the exception/interrupt handlers are under ring 0 mode, do we really have a chance to trigger fault? Maybe we shouldn't do `probe-stack` with `x86_intrcc`?



================
Comment at: llvm/lib/Target/X86/X86FrameLowering.cpp:655
+  while (1) {
+    auto Where = llvm::find_if(PrologMBB, [](MachineInstr &MI) {
+      return MI.getOpcode() == X86::STACKALLOC_W_PROBING;
----------------
Freax13 wrote:
> pengfei wrote:
> > Use `for (MachineInstr &MI : PrologMBB) {` to iterate rather than loop `find_if`.
> Hmm, this doesn't work and I think my whole approach is flawed: Initially I was afraid to use a for loop because I was afraid that the iterator would get invalidated by the call to `emitStackProbeInline` and `eraseFromParent`. The invalidation caused `eraseFromParent` isn't a huge problem and could probably be bypassed by using `make_early_inc_range`. The real problem is that `emitStackProbeInline` will split the basic block when it actually inserts stack probe if it chooses to insert a loop. Crucially this means that if the first `STACKALLOC_W_PROBING` machine instruction causes one of these stack probe loops to be created all other `STACKALLOC_W_PROBING`s will be moved into a new basic block after the loop. This means that `inlineStackProbe` will no longer find them and we're back to where we started with `STACKALLOC_W_PROBING` not being inlined.
> I'm not yet sure how to fix this, I'd welcome suggestions.
I think this can be solved by recording `MI` in a loop first and then emitting stack probe later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150033