[PATCH] D13361: Support for emitting inline stack probes

[Andy Ayers via llvm-commits]

AndyAyers added a comment.

Thanks, I'll do some cleanup and post a revision.

Answered (hopefully) a few of your other questions inline.


================
Comment at: lib/Target/X86/X86FrameLowering.cpp:502
@@ +501,3 @@
+  //    LimitReg = gs magic thread env access
+  //    if FinalReg >= LimitReg goto ContinueMBB
+  // RoundBB:
----------------
rnk wrote:
> Is this check correct for a downwards growing stack? LimitReg here is StackLimit in NT_TIB, which is the minimum allowed value for this thread's SP, right? I think we want to enter the loop if FinalReg >= LimitReg.
> 
> What are we supposed to do anyway if we attempt to probe beyond StackLimit? I would expect normal code that doesn't probe will crash when accessing beyond StackLimit, so why can't we skip this check and crash somewhere in the loop?
The StackLimit field of the TIB has a confusing name. It points at the lowest touched page of the stack -- not the lower limit on how far the stack can grow before the OS raises stack overflow.

The "true" lower stack limit is found in the DeallocationStack field in the TEB.

The idea here is to only touch pages that the OS hasn't seen this thread use before, rather than just touch all the pages in the range between the current SP and the desired new SP. So we only need to probe if the new SP lies beyond (less than, since the stack is growing down) StackLimit.

================
Comment at: lib/Target/X86/X86FrameLowering.cpp:635-636
@@ +634,4 @@
+      .addReg(0);
+  // Probe by storing a byte onto the stack.
+  BuildMI(LoopMBB, DL, TII.get(X86::MOV8mi))
+      .addReg(ProbeReg)
----------------
rnk wrote:
> MSVC CRT and mingw CRT both use TEST for this purpose. Is there a reason why the CLR wants to do a write here instead of a read?
In the Windows and VC CRT sources for x64, we always seem to do a write. Probably just paranoia to make sure the OS flips the page protection to R/W instead of just R?


Repository:
  rL LLVM

http://reviews.llvm.org/D13361