[llvm-commits] [PATCH] Segmented stack fixes, support for Mac, Win32, FreeBSD

[Rafael Ávila de Espíndola]

On 05/01/12 06:54 PM, Brian Anderson wrote:
> This series provides fixes to segmented stacks and adds support for
> x86 32-bit and 64-bit Mac, 32-bit Win32 and 64-bit FreeBSD. It builds
> on the __morestack mechanism implemented for Linux by gcc and LLVM.
> 
> For each platform I had to pick a slot in the TCB to store the stack
> boundary. This is a difficult decision since that is precious real
> estate. On Win32 I am using the pvArbitrary slot, which is reserved
> for application use. On FreeBSD it uses tcb_spare.
> 
> On Mac I have provisionally picked slot 90, which appears to already
> be reserved for JavaScript Core. I welcome any suggestions for a
> better location to store the stack boundary.
> 
> The new operating systems only support frames of static size, as that
> is all that is needed by the Rust language.

On patch 01:
What was failing? Do you have a testcase that can be added?

On patch 02:
OK with me.

On patch 03:
X86_FastCall is x86_fastcallcc in the .ll. If I remember correctly, the
CallingConv::Fast calling conversion can be anything we want, so the
patch is OK.

On patch 04:
OK.

On patch 05:
Please name variables according to:
http://llvm.org/docs/CodingStandards.html#ll_naming
What is the "primary" argument for? Please add a comment.

On patch 06:
Saving the scratch register is safe because you known there is always
some reserve space left?
Needs a test.

On patch 07:
Is the assert
assert(false && "Unhandled case in adjustForSegmentedStacks");
user visible? i.e., can I hit it running llc targeting an unsupported OS?
Needs a test.

On patch 08:
Needs a test.

And now I see the last patches are tests :-) It is normal to add the
tests in the same patch that fixes the problem or implements the feature?

Sanjoy, any other comments?

> Regards, Brian Anderson

Thanks,
Rafael