[PATCH] D150033: fix stack probe lowering for x86_intrcc

[Phoebe Wang via Phabricator via llvm-commits]

pengfei added a comment.

In D150033#4324928 <https://reviews.llvm.org/D150033#4324928>, @Freax13 wrote:

> 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.

Thanks! I got the point now. This is used to make `RSP` be 0xXXXXXXX8 again to match with normal functions. I was confused it with stack realignment. So yeah, there might be a chance to trigger faults. It looks to me we have confidence to use `push` in prologue without prob, so I'm inclined the reason is they are small than 4096.


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

https://reviews.llvm.org/D150033