[SUA][IRGen] Add stub for swift_coroFrameAlloc that weakly links against the runtime function #79889

t-rasmud · 2025-03-10T19:11:05Z

This PR modifies IRGen to emit a stub function __swift_coroFrameAllocStub instead of the
newly introduced swift-rt function swift_coroFrameAlloc. The stub checks whether the runtime has the symbol
swift_coroFrameAlloc and dispatches to it if it exists, uses malloc otherwise. This ensures the
ability to back deploy the feature to older OS targets.

rdar://145239850

t-rasmud · 2025-03-11T22:05:46Z

@swift-ci test

t-rasmud · 2025-03-12T04:26:01Z

Please test with following pull request:
swiftlang/llvm-project#10236

@swift-ci test

t-rasmud · 2025-03-12T15:18:34Z

Please test with following pull request:
swiftlang/llvm-project#10239

@swift-ci test

t-rasmud · 2025-03-18T21:05:14Z

@swift-ci test

t-rasmud · 2025-03-19T18:44:01Z

@swift-ci test

t-rasmud · 2025-03-19T20:59:18Z

@swift-ci test

t-rasmud · 2025-03-21T19:23:04Z

@swift-ci test

t-rasmud · 2025-03-24T20:49:40Z

@swift-ci test

…nst the runtime function This commit modifies IRGen to emit a stub function `__swift_coroFrameAllocStub` instead of the newly introduced swift-rt function `swift_coroFrameAlloc`. The stub checks whether the runtime has the symbol `swift_coroFrameAlloc` and dispatches to it if it exists, uses `malloc` otherwise. This ensures the ability to back deploy the feature to older OS targets. rdar://145239850

t-rasmud · 2025-03-25T18:48:15Z

@swift-ci test

compnerd · 2025-03-25T22:50:00Z

lib/IRGen/GenFunc.h

+      auto coroAllocPtr = IGF.IGM.getCoroFrameAllocFn();
+      auto coroAllocFn = dyn_cast<llvm::Function>(coroAllocPtr);
+      if (!llvm::Triple(IGM.Triple).isOSWindows())
+        coroAllocFn->setLinkage(llvm::GlobalValue::ExternalWeakLinkage);


This makes the symbol strong on windows and weak on everything else. Is this intentional?

@compnerd

@t-rasmud saw errors of the form:

conflicting weak extern definition ... New default ... conflicts with old default

I think we have to answer first why we are getting this error message to decide that disabling weak linkage is the right thing to do here.
I am doubtful, without further information of why marking the declaration weak caused the failure.

Ideally, we can weakly link the symbol also on windows.

Here's the exact failure: https://ci-external.swift.org/job/swift-PR-windows/38338/consoleText

FAILED: lib/lib_FoundationCollections.lib C:\Windows\system32\cmd.exe /C "cd . && T:\5\bin\swiftc.exe -target x86_64-unknown-windows-msvc -emit-library -static -o lib\lib_FoundationCollections.lib @CMakeFiles\_FoundationCollections.rsp && cd ." Rope+_Node.swift.obj : fatal error LNK1227: conflicting weak extern definition for 'swift_coroFrameAlloc'. New default '.weak.swift_coroFrameAlloc.default.$s22_FoundationCollections4RopeV5_NodeV6objectyXlvs' conflicts with old default '.weak.swift_coroFrameAlloc.default.$s22_FoundationCollections4RopeV7isValidySbAC5IndexVyx_GF' in Rope+Collection.swift.obj

@compnerd I don't know why a weak external reference would result in an error message like that. But it seems that others ran into a similar issue:

swift/lib/IRGen/IRGenModule.cpp

Line 1179 in 98fc0a9

// Windows does not allow multiple definitions of weak symbols.

Hmm

https://maskray.me/blog/2021-04-25-weak-symbol

Is the problem that entry in the stdlib runtime is not marked weak?

swift/include/swift/Runtime/Heap.h

Line 44 in 98fc0a9

void *swift_coroFrameAlloc(size_t bytes, MallocTypeId typeId);

What happens if we change that line to use SWIFT_RUNTIME_WEAK_IMPORT?

Using SWIFT_RUNTIME_WEAK_IMPORT results in linker error: Undefined symbols for architecture x86_64: "_swift_coroFrameAlloc"

Sorry, I only now realize that typed malloc is not available on windows (or any non apple platform for that matter).

You can disable coroFrameAlloc emission on windows.

swift/include/swift/Runtime/Config.h

Line 132 in ecc9093

#if defined(__APPLE__) && defined(__LP64__) && __has_include(<malloc_type_private.h>) && SWIFT_STDLIB_HAS_DARWIN_LIBMALLOC

#if defined(__APPLE__) && defined(__LP64__) && __has_include(<malloc_type_private.h>) && SWIFT_STDLIB_HAS_DARWIN_LIBMALLOC # include <TargetConditionals.h> # if TARGET_OS_IOS && !TARGET_OS_SIMULATOR ... # define SWIFT_STDLIB_HAS_MALLOC_TYPE 1 # endif #endif #ifndef SWIFT_STDLIB_HAS_MALLOC_TYPE # define SWIFT_STDLIB_HAS_MALLOC_TYPE 0 #endif

You would achieve that with something similar in spirit to the following change:

+++ b/lib/Frontend/CompilerInvocation.cpp @@ -3766,6 +3766,10 @@ static bool ParseIRGenArgs(IRGenOptions &Opts, ArgList &Args, Args.hasFlag(OPT_enable_async_frame_push_pop_metadata, OPT_disable_async_frame_push_pop_metadata, Opts.EmitAsyncFramePushPopMetadata); + + if (Opts.EmitTypeMallocForCoroFrame && (Triple.isOSLinux() || Triple.isOSWindows())) { + Opts.EmitTypeMallocForCoroFrame = false; + } Opts.EmitTypeMallocForCoroFrame = Args.hasFlag(OPT_enable_emit_type_malloc_for_coro_frame, OPT_disable_emit_type_malloc_for_coro_frame,

t-rasmud · 2025-03-26T17:52:36Z

@swift-ci test

t-rasmud · 2025-03-27T17:25:35Z

@swift-ci test

t-rasmud · 2025-03-28T00:00:39Z

@swift-ci test

aschwaighofer · 2025-03-28T14:43:00Z

You can restrict a test to run only on say macOS and iOS by adding the following line to the test:

// REQUIRES: OS=macosx || OS=iOS
...

t-rasmud · 2025-04-01T18:33:17Z

@swift-ci test

t-rasmud · 2025-04-07T21:59:39Z

@swift-ci test

aschwaighofer

LGTM

…nst the runtime function (#79889) * [SUA][IRGen] Add stub for swift_coroFrameAlloc that weakly links against the runtime function This commit modifies IRGen to emit a stub function `__swift_coroFrameAllocStub` instead of the newly introduced swift-rt function `swift_coroFrameAlloc`. The stub checks whether the runtime has the symbol `swift_coroFrameAlloc` and dispatches to it if it exists, uses `malloc` otherwise. This ensures the ability to back deploy the feature to older OS targets. rdar://145239850 (cherry picked from commit 5e2f20b)

…nst the runtime function (#79889) (#80769) * [SUA][IRGen] Add stub for swift_coroFrameAlloc that weakly links against the runtime function This commit modifies IRGen to emit a stub function `__swift_coroFrameAllocStub` instead of the newly introduced swift-rt function `swift_coroFrameAlloc`. The stub checks whether the runtime has the symbol `swift_coroFrameAlloc` and dispatches to it if it exists, uses `malloc` otherwise. This ensures the ability to back deploy the feature to older OS targets. rdar://145239850 (cherry picked from commit 5e2f20b)

t-rasmud force-pushed the swift-coro-stub branch from 7e88407 to 3dd9282 Compare March 11, 2025 22:04

t-rasmud marked this pull request as ready for review March 11, 2025 22:06

t-rasmud requested a review from rjmccall as a code owner March 11, 2025 22:06

t-rasmud requested review from aschwaighofer, devincoughlin, ojhunt, yln and fbigarella March 11, 2025 22:07

t-rasmud changed the title ~~[SUA][IRGen] Add stub for swift_coroFrameAlloc~~ [SUA][IRGen] Add stub for swift_coroFrameAlloc that weakly links against the runtime function Mar 11, 2025

t-rasmud marked this pull request as draft March 12, 2025 02:11

t-rasmud marked this pull request as ready for review March 21, 2025 21:39

t-rasmud requested review from hborla, slavapestov and xedin as code owners March 21, 2025 21:39

t-rasmud added 7 commits March 25, 2025 11:45

Enable typed malloc

d15c897

test changes

20a7fb9

Attach artificial debug location to retcon intrinstic call with stub

a82b07b

fix test

554f9f8

Fix test

387f2af

Fix external linkage on windows

27347f7

t-rasmud force-pushed the swift-coro-stub branch from ef351da to 27347f7 Compare March 25, 2025 18:48

t-rasmud requested a review from compnerd as a code owner March 25, 2025 18:48

compnerd reviewed Mar 25, 2025

View reviewed changes

Fix test

54075ca

t-rasmud added 2 commits March 26, 2025 16:21

Minor test improvements

74619f3

[NFC] Test weak linking on windows

721da48

Do not emit swift_coroFrameAlloc on linux and windows

e20af08

Run test only on macos and ios

9337e07

aschwaighofer approved these changes Apr 7, 2025

View reviewed changes

t-rasmud merged commit 5e2f20b into swiftlang:main Apr 8, 2025
5 checks passed

t-rasmud mentioned this pull request Apr 15, 2025

[6.2][SUA][IRGen] Add stub for swift_coroFrameAlloc that weakly links against the runtime function #80769

Merged

[SUA][IRGen] Add stub for swift_coroFrameAlloc that weakly links against the runtime function #79889

[SUA][IRGen] Add stub for swift_coroFrameAlloc that weakly links against the runtime function #79889

Uh oh!

Conversation

t-rasmud commented Mar 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

t-rasmud commented Mar 11, 2025

Uh oh!

t-rasmud commented Mar 12, 2025

Uh oh!

t-rasmud commented Mar 12, 2025

Uh oh!

t-rasmud commented Mar 18, 2025

Uh oh!

t-rasmud commented Mar 19, 2025

Uh oh!

t-rasmud commented Mar 19, 2025

Uh oh!

t-rasmud commented Mar 21, 2025

Uh oh!

t-rasmud commented Mar 24, 2025

Uh oh!

t-rasmud commented Mar 25, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

t-rasmud commented Mar 26, 2025

Uh oh!

t-rasmud commented Mar 27, 2025

Uh oh!

t-rasmud commented Mar 28, 2025

Uh oh!

aschwaighofer commented Mar 28, 2025

Uh oh!

t-rasmud commented Apr 1, 2025

Uh oh!

t-rasmud commented Apr 7, 2025

Uh oh!

aschwaighofer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

t-rasmud commented Mar 10, 2025 •

edited

Loading