[Driver][Frontend] Introduce `load-pass-plugin` option #68985

antoniofrighetto · 2023-10-05T14:44:04Z

Allow dynamic loading of LLVM passes via load-pass-plugin option passed to the Swift compiler driver, similarly to current Apple Clang -fpass-plugin option.

Previous discussion: https://forums.swift.org/t/load-external-llvm-pass-plugins-via-swift-frontend/67596

antoniofrighetto · 2023-10-05T14:47:19Z

Please, let me know if anything needs to be improved as far as style conventions et alia is concerned, I'd be happy to do so.

antoniofrighetto · 2023-10-05T15:21:56Z

(cc/ @DougGregor)

antoniofrighetto · 2023-10-17T16:08:47Z

Gentle ping. Any feedback or suggestions would be duly appreciated. Alongside this, I believe the option should be added as part of Options.swift as well. May that be the case? (cc/ @DougGregor, @rintaro).

jslegendre · 2023-10-19T03:02:33Z

Hey, this may not be getting much attention because the internal driver (this one) is considered legacy. The default driver as of Xcode 13 swift-driver. You might have better luck over there.

Note: I'm not affiliated with the Swift project in any way. Just a guy who'd love to see this merged

artemcm · 2023-10-19T17:33:55Z

There was some discussion on this previously in:
#38017

The driver and frontend pieces are separate and yes, proper driver support would mean adding the logic in https://github.com/apple/swift-driver. Legacy (C++-based) driver support being optional/discouraged.

@lhames @aschwaighofer can comment better on the architecture of loading such passes in IRGen.

antoniofrighetto · 2023-10-23T16:26:10Z

Thanks a lot to both for the helpful insights. Support in the new swift-driver should be addressed by swiftlang/swift-driver#1472. The implementation should aim to ensure a clear separation between the driver and the frontend. Also, thanks for sharing #38017, was not aware of this. Reviewed the discussion there carefully, it was very informative.

antoniofrighetto · 2023-10-27T14:52:33Z

Fixed conflicts by rebasing to main, and changed test library type from MODULE to SHARED, as CMake seems to be defaulting to generate the plugin with .so extension on macOS too (rather than .dylib, this was previously leading the unittest to fail).

weliveindetail

The way this patch proposes to load the plugin pass in IRGen looks good. It is exactly what we do in Clang and Flang, and what the documentation recommends.

weliveindetail · 2023-11-06T14:22:34Z

lib/IRGen/IRGen.cpp

+    } else {
+      diagnoseSync(Diags, DiagMutex, SourceLoc(),
+                   diag::unable_to_load_pass_plugin, PluginFile,
+                   toString(PassPlugin.takeError()));


I wonder if the diagnostic handling should remain in performLLVM(). Passing Diags and DiagMutex to performLLVMOptimizations() seems a bit unfortunate. We could instead just return the llvm::Error from here and let the callers take care of it. That would change the return type of the function. A lot of code in include/swift is using llvm::Error already, but I am not sure what the conventions are in Swift. @lhames What do you think?

antoniofrighetto · 2023-11-14T20:29:16Z

Gentle ping (cc/ @lhames).

wsmoses · 2023-11-22T19:36:54Z

Just wanted to express support for this here as it also helps with adding plugins for AD

jslegendre · 2023-11-23T01:08:02Z

Also expressing support for this!

antoniofrighetto · 2024-01-22T16:14:29Z

Rebased to main, dropped unittest in Driver as part of recent legacy driver deprecation, added test coverage exercizing performLLVMOptimization. New driver support ready at swiftlang/swift-driver#1472. Kind ping @lhames, @aschwaighofer, @artemcm, @benlangmuir, @weliveindetail.

antoniofrighetto · 2024-03-25T20:16:39Z

@lhames, @airspeedswift Gentle ping. Any chance of this getting reviewed?

antoniofrighetto · 2024-04-16T09:20:27Z

Kind ping (maybe cc/ @adrian-prantl).

lib/IRGen/IRGen.cpp

test/Frontend/Inputs/TestPlugin.cpp

adrian-prantl

LGTM with a couple of minor comments inside.

adrian-prantl · 2024-05-01T00:55:30Z

@swift-ci test

antoniofrighetto · 2024-05-02T09:35:11Z

Rebased to main, opened sister PR @ swiftlang/llvm-project#8678, reflecting changes (not sure if we should have targeted a different branch instead).

antoniofrighetto · 2024-05-16T16:12:15Z

@adrian-prantl Kind ping on this, CI was previously failing.

antoniofrighetto · 2024-07-31T17:31:31Z

Rebased to main (fixed conflicts), swiftlang/llvm-project#8678 PR rebased to stable/20230725, which should be the expected branch. Very gentle remainder. I do not have commit access, could anyone kindly merge this?

antoniofrighetto · 2024-07-31T19:06:07Z

I assume this should be tested with:

Please test with following PR:
https://github.com/swiftlang/llvm-project/pull/8678
https://github.com/swiftlang/swift-driver/pull/1472

@swift-ci Please test

JDevlieghere · 2024-09-09T15:43:03Z

swiftlang/llvm-project#8678
swiftlang/swift-driver#1472

@swift-ci test

JDevlieghere · 2024-09-10T02:39:04Z

The macOS failure is:

/Users/ec2-user/jenkins/workspace/swift-PR-macos/branch-main/swift/stdlib/private/SwiftPrivate/IO.swift:21:8: error: failed to build module 'Darwin'; this SDK is not supported by the compiler (the SDK is built with 'Apple Swift version 5.9.2 (swiftlang-5.9.2.2.11 clang-1500.1.0.2.2)', while this compiler is 'Apple Swift version 6.1-dev effective-5.10 (LLVM 97c65ef47934098, Swift c06508954544f05)'). Please select a toolchain which matches the SDK.
 19 | #else
 20 | #if canImport(Darwin)
 21 | import Darwin
    |        `- error: failed to build module 'Darwin'; this SDK is not supported by the compiler (the SDK is built with 'Apple Swift version 5.9.2 (swiftlang-5.9.2.2.11 clang-1500.1.0.2.2)', while this compiler is 'Apple Swift version 6.1-dev effective-5.10 (LLVM 97c65ef47934098, Swift c06508954544f05)'). Please select a toolchain which matches the SDK.

edit: should have been fixed by #76344

JDevlieghere · 2024-09-10T02:39:54Z

swiftlang/llvm-project#8678
swiftlang/swift-driver#1472

@swift-ci test macos

weliveindetail · 2024-10-10T12:31:58Z

@JDevlieghere This will be affected by the rebranch isn't it? Any chance it can land before?

artemcm · 2024-10-23T21:13:33Z

@adrian-prantl @JDevlieghere is this good to merge?

adrian-prantl · 2024-10-25T18:30:16Z

I don't have any further concerns with this patch.

Allow dynamic loading of LLVM passes via `load-pass-plugin` option passed to the Swift compiler driver.

antoniofrighetto · 2024-11-07T16:33:29Z

Rebased to main, fixed conflicts (to be tested again as above otherwise CI fails), kind ping for merging this.

aschwaighofer · 2024-11-12T17:46:03Z

@swift-ci test

antoniofrighetto · 2024-11-12T19:47:39Z

@swift-ci test

Thanks, the other two PRs (swiftlang/llvm-project#8678, swiftlang/swift-driver#1472) are also required for the CI to pass.

aschwaighofer · 2024-11-12T19:51:00Z

swiftlang/llvm-project#8678
swiftlang/swift-driver#1472
@swift-ci test

aschwaighofer · 2024-11-12T19:53:24Z

@antoniofrighetto Can you put up a PR against stable/20240723. Swift has rebased against that newer llvm-project branch.

antoniofrighetto · 2024-11-12T20:48:00Z

Done at swiftlang/llvm-project#9586, thanks.

aschwaighofer · 2024-11-14T18:25:36Z

swiftlang/swift-driver#1472
swiftlang/llvm-project#9586
@swift-ci test

antoniofrighetto · 2024-11-19T16:08:25Z

Kind ping (before other rebases conflicts pop up :P)

al45tair · 2024-11-21T14:58:57Z

@antoniofrighetto @aschwaighofer
The test for this is failing under ASAN because of an ODR rule violation (there are two vtables for llvm::circular_raw_ostream). See #77771.

paukala · 2024-12-05T09:07:13Z

Great to see this merged!

Plugins (at least the one that I am using) usually rely on libLLVM.dylib to be linked with swiftc / clang, which seems to be an issue as libLLVM.dylib is not linked per default.

@antoniofrighetto did you experience something similar during development? If so, what would be a good way to include the needed lib?

In my quick test, I set LLVM_BUILD_LLVM_DYLIB and LLVM_LINK_LLVM_DYLIB to true.

However, this resulted in `Library not loaded: @rpath/libLLVM.dylib` multiple errors while building the toolchain, such as:

dyld[43617]: Library not loaded: @rpath/libLLVM.dylib\n 
Referenced from: <...> /Users/user/swift-project/build/buildbot_osx/swift-macosx-arm64/bin/swift-frontend\n 
Reason: tried: \'/usr/lib/swift/libLLVM.dylib\' (no such file, not in dyld cache),
\'/System/Volumes/Preboot/Cryptexes/OS/usr/lib/swift/libLLVM.dylib\' (no such file),
\'/Users/user/swift-project/build/buildbot_osx/swift-macosx-arm64/lib/swift/host/compiler/libLLVM.dylib\' (no such file), 
\'/usr/lib/swift/libLLVM.dylib\' (no such file, not in dyld cache),
\'/System/Volumes/Preboot/Cryptexes/OS/usr/lib/swift/libLLVM.dylib\' (no such file),
\'/Users/user/swift-project/build/buildbot_osx/swift-macosx-arm64/lib/swift/host/compiler/libLLVM.dylib\' (no such file)\n")

antoniofrighetto requested review from artemcm, tshortli, hborla, slavapestov and xedin as code owners October 5, 2023 14:44

xedin removed their request for review October 5, 2023 23:05

antoniofrighetto force-pushed the feature/load-pass-plugin branch from 09d9fde to 2293235 Compare October 6, 2023 08:31

antoniofrighetto mentioned this pull request Oct 23, 2023

Add support for load-pass-plugin option swiftlang/swift-driver#1472

Merged

antoniofrighetto force-pushed the feature/load-pass-plugin branch from 2293235 to 177dc53 Compare October 27, 2023 14:51

weliveindetail reviewed Nov 6, 2023

View reviewed changes

antoniofrighetto force-pushed the feature/load-pass-plugin branch from 177dc53 to 7055ddf Compare January 22, 2024 16:12

antoniofrighetto force-pushed the feature/load-pass-plugin branch from 7055ddf to 564940c Compare January 29, 2024 13:56

marcobrador mentioned this pull request Feb 16, 2024

Support Swift code open-obfuscator/o-mvll#9

Open

adrian-prantl reviewed Apr 30, 2024

View reviewed changes

lib/IRGen/IRGen.cpp Outdated Show resolved Hide resolved

adrian-prantl reviewed Apr 30, 2024

View reviewed changes

test/Frontend/Inputs/TestPlugin.cpp Outdated Show resolved Hide resolved

adrian-prantl approved these changes Apr 30, 2024

View reviewed changes

antoniofrighetto force-pushed the feature/load-pass-plugin branch from 564940c to bfc69de Compare April 30, 2024 18:47

antoniofrighetto force-pushed the feature/load-pass-plugin branch from bfc69de to f4d021f Compare May 2, 2024 06:55

antoniofrighetto mentioned this pull request May 2, 2024

[lldb][Plugins][Swift] Reflect API changes in SwiftExpressionParser swiftlang/llvm-project#8678

Closed

antoniofrighetto force-pushed the feature/load-pass-plugin branch from f4d021f to 0489398 Compare July 31, 2024 17:28

antoniofrighetto requested a review from rjmccall as a code owner July 31, 2024 17:28

[Driver][Frontend] Introduce load-pass-plugin option

377c03f

Allow dynamic loading of LLVM passes via `load-pass-plugin` option passed to the Swift compiler driver.

antoniofrighetto force-pushed the feature/load-pass-plugin branch from 0489398 to 377c03f Compare November 7, 2024 16:32

antoniofrighetto mentioned this pull request Nov 12, 2024

[lldb][Plugins][Swift] Reflect API changes in SwiftExpressionParser swiftlang/llvm-project#9586

Merged

aschwaighofer merged commit c5463bd into swiftlang:main Nov 20, 2024
5 checks passed

al45tair mentioned this pull request Nov 21, 2024

🟠 OSS Swift CI: oss-swift-incremental-ASAN-RA-macos failed: test: Frontend/load-pass-plugin.swift (exit code 1) #77771

Closed

[Driver][Frontend] Introduce load-pass-plugin option #68985

[Driver][Frontend] Introduce load-pass-plugin option #68985

Conversation

antoniofrighetto commented Oct 5, 2023

antoniofrighetto commented Oct 5, 2023

antoniofrighetto commented Oct 5, 2023

antoniofrighetto commented Oct 17, 2023

jslegendre commented Oct 19, 2023

artemcm commented Oct 19, 2023

antoniofrighetto commented Oct 23, 2023

antoniofrighetto commented Oct 27, 2023

weliveindetail left a comment

Choose a reason for hiding this comment

weliveindetail Nov 6, 2023

Choose a reason for hiding this comment

antoniofrighetto commented Nov 14, 2023

wsmoses commented Nov 22, 2023

jslegendre commented Nov 23, 2023

antoniofrighetto commented Jan 22, 2024

antoniofrighetto commented Mar 25, 2024

antoniofrighetto commented Apr 16, 2024

adrian-prantl left a comment

Choose a reason for hiding this comment

adrian-prantl commented May 1, 2024

antoniofrighetto commented May 2, 2024

antoniofrighetto commented May 16, 2024

antoniofrighetto commented Jul 31, 2024 • edited Loading

antoniofrighetto commented Jul 31, 2024 • edited Loading

JDevlieghere commented Sep 9, 2024

JDevlieghere commented Sep 10, 2024 • edited Loading

JDevlieghere commented Sep 10, 2024

weliveindetail commented Oct 10, 2024

artemcm commented Oct 23, 2024

adrian-prantl commented Oct 25, 2024

antoniofrighetto commented Nov 7, 2024

aschwaighofer commented Nov 12, 2024

antoniofrighetto commented Nov 12, 2024

aschwaighofer commented Nov 12, 2024

aschwaighofer commented Nov 12, 2024

antoniofrighetto commented Nov 12, 2024

aschwaighofer commented Nov 14, 2024

antoniofrighetto commented Nov 19, 2024

al45tair commented Nov 21, 2024

paukala commented Dec 5, 2024

[Driver][Frontend] Introduce `load-pass-plugin` option #68985

[Driver][Frontend] Introduce `load-pass-plugin` option #68985

antoniofrighetto commented Jul 31, 2024 •

edited

Loading

antoniofrighetto commented Jul 31, 2024 •

edited

Loading

JDevlieghere commented Sep 10, 2024 •

edited

Loading