[PATCH] D150033: fix stack probe lowering for x86_intrcc

Tom Dohrmann via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun May 7 03:53:10 PDT 2023


Freax13 added a comment.

In D150033#4324920 <https://reviews.llvm.org/D150033#4324920>, @pengfei wrote:

> This approach look great.
>
> In D150033#4324861 <https://reviews.llvm.org/D150033#4324861>, @Freax13 wrote:
>
>> In D150033#4324769 <https://reviews.llvm.org/D150033#4324769>, @pengfei wrote:
>>
>>>> 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.
>>
>> So it turns out that we currently emit `push rax` for the stack update. This instruction obviously writes to the page and would trigger faults if it landed on a guard page. We can probably get away with just hard-coding the stack update to `push rax` and avoid any bad interactions with stack probing that way.
>
> You missed my point. `push rax` won't trigger a page fault if it's used for stack alignment, because the current value of `RSP` must be 0xXXXXXXX8, the instruction cannot cross the page boundary.

`RSP` won't be 0xXXXXXXX8, it will be 16-byte aligned. When handling an exception with an error code the CPU aligns the stack to 16 bytes and then pushes 6 8-bytes values, which will result in the stack still being 16-byte aligned.


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

https://reviews.llvm.org/D150033



More information about the llvm-commits mailing list