Page MenuHomePhabricator

Allow /STACK in #pragma comment(linker, ...)
ClosedPublic

Authored by alexreinking on Mar 31 2021, 1:03 PM.

Details

Summary

The Halide project uses #pragma comment(linker, "/STACK:...") to set the stack size high enough for our embedded compiler to run in end-user programs on Windows.

Unfortunately, lld-link.exe breaks on this when embedded in a COFF object, despite supporting the flag on the command line. MSVC's link.exe supports this fine. This patch extends support for this to lld-link.exe for better compatibility with MSVC projects.

Diff Detail

Event Timeline

alexreinking requested review of this revision.Mar 31 2021, 1:03 PM
alexreinking created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptMar 31 2021, 1:03 PM

Fix formatting.

This looks sensible to me (especially assuming that link.exe allows /stack options in directives - I didn't verify this myself). It will need a testcase added before it can be landed though.

srj added a subscriber: srj.Apr 1 2021, 9:51 AM
rnk added a comment.Apr 1 2021, 2:00 PM

I verified MSVC supports this. This does need a test, and one can be made by combining the techniques in the lld/test/COFF/stack.test test and the directives.s test.

thakis requested changes to this revision.Tue, Apr 13, 4:06 PM

(marking this "Request Changes" for the missing test)

This revision now requires changes to proceed.Tue, Apr 13, 4:06 PM
This revision was not accepted when it landed; it landed in state Needs Revision.Wed, May 5, 4:01 PM
This revision was automatically updated to reflect the committed changes.
rnk added a comment.Wed, May 5, 4:01 PM

I found a spare moment to add the test and went ahead and pushed this. Thanks!

No, thank you, @rnk, for adding the test! I've been busy with my grad school obligations lately and this wound up pretty far down my to-do list. I'm happy this could get merged 🙂