Add test fixture for experimental-lto-mode #6891

rauhul · 2023-09-12T16:50:25Z

Adds a test covering experimental lto mode and fixes a bug where the
lto mode was not passed to the linker.

rauhul · 2023-09-12T16:50:43Z

@swift-ci please smoke test

rauhul · 2023-09-12T16:53:14Z

rauhul · 2023-09-12T17:07:08Z

[12/14] Write Objects.LinkFileList
error: autolink-extract command failed with exit code 1 (use -v to see invocation)
<unknown>:0: error: error opening input file '/tmp/Miscellaneous_LTO_SwiftAndCTargets.GHkhKS/Miscellaneous_LTO_SwiftAndCTargets/.build/x86_64-unknown-linux-gnu/debug/cLib.build/cLib.c.o' (The file was not recognized as a valid object file)

@fabianfett it looks lto (as currently implemented) just doesn't work on linux! Thanks for the report!

rauhul · 2023-09-12T17:41:37Z

The error comes from swift-autolink-extract, which I know nothing about...

/usr/bin/swift-autolink-extract /swift-package-manager/Fixtures/Miscellaneous/LTO/SwiftAndCTargets/.build/aarch64-unknown-linux-gnu/debug/cLib.build/cLib.c.o /swift-package-manager/Fixtures/Miscellaneous/LTO/SwiftAndCTargets/.build/aarch64-unknown-linux-gnu/debug/exe.build/exe.swiftmodule.o /swift-package-manager/Fixtures/Miscellaneous/LTO/SwiftAndCTargets/.build/aarch64-unknown-linux-gnu/debug/swiftLib.build/swiftLib.swiftmodule.o -o /swift-package-manager/Fixtures/Miscellaneous/LTO/SwiftAndCTargets/.build/aarch64-unknown-linux-gnu/debug/cLib.build/exe.autolink
error: autolink-extract command failed with exit code 1 (use -v to see invocation)
<unknown>:0: error: error opening input file '/swift-package-manager/Fixtures/Miscellaneous/LTO/SwiftAndCTargets/.build/aarch64-unknown-linux-gnu/debug/cLib.build/cLib.c.o' (The file was not recognized as a valid object file

neonichu · 2023-09-12T17:59:50Z

Maybe @artemcm does

artemcm · 2023-09-12T18:03:51Z

swift-autolink-extract takes as input all the object files from compilation commands in order to extract auto-linking directives so that they can be specified as explicit flags to the linker, because non-Darwin linkers do not have auto-linking feature. It is strange why LLVM is not recognizing this object file as a valid .o.

rauhul · 2023-09-12T18:05:51Z

swift-autolink-extract takes as input all the object files from compilation commands in order to extract auto-linking directives so that they can be specified as explicit flags to the linker, because non-Darwin linkers do not have auto-linking feature. It is strange why LLVM is not recognizing this object file as a valid .o.

Well this file doesn't exist on disk because the file extension is .bc right?

./.build/aarch64-unknown-linux-gnu/debug/swiftLib.build/swiftLib.swift.bc
./.build/aarch64-unknown-linux-gnu/debug/exe.build/main.swift.bc

artemcm · 2023-09-12T18:08:21Z

swift-autolink-extract takes as input all the object files from compilation commands in order to extract auto-linking directives so that they can be specified as explicit flags to the linker, because non-Darwin linkers do not have auto-linking feature. It is strange why LLVM is not recognizing this object file as a valid .o.

Well this file doesn't exist on disk because the file extension is .bc right?
./.build/aarch64-unknown-linux-gnu/debug/swiftLib.build/swiftLib.swift.bc
./.build/aarch64-unknown-linux-gnu/debug/exe.build/main.swift.bc

Ah, then we need to teach the driver about how this works, I believe it is just forwarding compilation outputs to swift-autolink-extract.

rauhul · 2023-09-12T18:10:00Z

swift-autolink-extract takes as input all the object files from compilation commands in order to extract auto-linking directives so that they can be specified as explicit flags to the linker, because non-Darwin linkers do not have auto-linking feature. It is strange why LLVM is not recognizing this object file as a valid .o.

Well this file doesn't exist on disk because the file extension is .bc right?
./.build/aarch64-unknown-linux-gnu/debug/swiftLib.build/swiftLib.swift.bc
./.build/aarch64-unknown-linux-gnu/debug/exe.build/main.swift.bc
Ah, then we need to teach the driver about how this works, I believe it is just forwarding compilation outputs to swift-autolink-extract.

I'm a little confused because we use .bc because that's the extension the driver chooses

artemcm · 2023-09-12T18:14:50Z

swift-autolink-extract takes as input all the object files from compilation commands in order to extract auto-linking directives so that they can be specified as explicit flags to the linker, because non-Darwin linkers do not have auto-linking feature. It is strange why LLVM is not recognizing this object file as a valid .o.

Well this file doesn't exist on disk because the file extension is .bc right?
./.build/aarch64-unknown-linux-gnu/debug/swiftLib.build/swiftLib.swift.bc
./.build/aarch64-unknown-linux-gnu/debug/exe.build/main.swift.bc
Ah, then we need to teach the driver about how this works, I believe it is just forwarding compilation outputs to swift-autolink-extract.
I'm a little confused because we use .bc because that's the extension the driver chooses

This job must not be configured correctly. It cannot accept .bc files, so it must not have been taught about these compilation flows before now.

rauhul · 2023-09-18T17:23:58Z

@artemcm would teaching lib/DriverTool/autolink_extract_main.cpp about llvm bitcode files (how to open/load/search for the auto link section) be the right approach to fix this?

artemcm · 2023-09-18T20:03:04Z

@artemcm would teaching lib/DriverTool/autolink_extract_main.cpp about llvm bitcode files (how to open/load/search for the auto link section) be the right approach to fix this?

Reading the code, I get the impression that if we're using LTO, then we're using lld, which natively supports auto-linking with ELF.
(https://github.com/apple/swift/blob/3a05e798e6d045bb25e7539e07b870994bc88884/lib/IRGen/IRGenModule.cpp#L1701)

So I wonder if we can simply teach the driver that this flow does not require autolink-extract, and the rest of compilation downstream will "just work". Are you able to check this at desk? I won't be near a Linux box until tomorrow.

artemcm · 2023-09-18T20:09:18Z

Hmm, the driver is already supposed to skip swift-autolink-extract when it knows we're doing LTO.
https://github.com/apple/swift/blob/3a05e798e6d045bb25e7539e07b870994bc88884/lib/Driver/Driver.cpp#L2255
https://github.com/apple/swift-driver/blob/e48171a466d624abdeaee3ea5d7a8b7237fda1eb/Sources/SwiftDriver/Jobs/AutolinkExtractJob.swift#L21

So we need to figure out why we're still generating this job here.

rauhul · 2023-09-18T20:31:46Z

Hmm, the driver is already supposed to skip swift-autolink-extract when it knows we're doing LTO. https://github.com/apple/swift/blob/3a05e798e6d045bb25e7539e07b870994bc88884/lib/Driver/Driver.cpp#L2255 https://github.com/apple/swift-driver/blob/e48171a466d624abdeaee3ea5d7a8b7237fda1eb/Sources/SwiftDriver/Jobs/AutolinkExtractJob.swift#L21

So we need to figure out why we're still generating this job here.

Oh I see, that is interesting.

Are you able to check this at desk? I won't be near a Linux box until tomorrow.

I've been running under docker to repro this issue. But I guess the question is why the driver doesn't see this link command as an lto job.

Lemme try to get the full swiftc command and maybe that will provide some clues.

rauhul · 2023-09-18T20:42:01Z

@artemcm here's a verbose build log:
build.log

rauhul · 2023-09-18T20:45:49Z

Maybe its as simple as adding --lto <#LTOKind#> to the swiftc call which spm produces

rauhul · 2023-09-21T18:05:26Z

@swift-ci please test

rauhul · 2023-09-21T18:24:29Z

@swift-ci please smoke test windows

rauhul · 2023-11-03T21:13:50Z

@swift-ci please test

rauhul · 2023-11-03T21:13:52Z

@swift-ci please smoke test windows

rauhul · 2023-11-03T21:29:19Z

@swift-ci please test

rauhul · 2023-11-03T21:29:21Z

@swift-ci please smoke test windows

shahmishal · 2023-11-04T04:03:11Z

@swift-ci please smoke test windows

MaxDesiatov · 2023-11-05T20:01:27Z

@swift-ci test windows

add a verbose flag to be able to help debug why this fixture is failing on linux

rauhul · 2023-11-06T23:47:53Z

@swift-ci please test

rauhul · 2023-11-09T20:01:02Z

swiftlang/swift#69696
@swift-ci please test

rauhul · 2023-11-09T21:24:47Z

swiftlang/swift#69564
@swift-ci please test

rauhul · 2023-12-05T23:36:21Z

@swift-ci please test

rauhul · 2023-12-06T05:22:18Z

@swift-ci please test

rauhul · 2023-12-06T16:22:20Z

@swift-ci please test

MaxDesiatov · 2023-12-06T17:44:21Z

Tests/FunctionalTests/MiscellaneousTests.swift

+        try fixture(name: "Miscellaneous/LTO/SwiftAndCTargets") { fixturePath in
+            do {
+                let output = try executeSwiftBuild(
+                  fixturePath,


This seems to be using a mix of 4 and 2-space formatting. Additionally, why wrap this in do/catch if try fixture(name:) would be rethrowing the error anyway?

I had added XCTFail("\(error)") to help with debugging, removing and fixing indentation

fixed in 93f7309f8a58c7f6cce5827c254af5e8015d3eb2

rauhul · 2023-12-06T17:49:52Z

@swift-ci please test

MaxDesiatov

Thanks!

MaxDesiatov · 2023-12-06T17:51:58Z

@swift-ci test windows

Cherry-pick of #6891. Adds a test covering experimental LTO mode and fixes a bug where the LTO mode was not passed to the linker. Fixes #6888 --------- Co-authored-by: Rauhul Varma <[email protected]>

Add test fixture for experimental-lto-mode

1a50013

rauhul force-pushed the lto-test-fixture branch from 8c513a1 to 8c8dfef Compare November 3, 2023 21:11

rauhul marked this pull request as ready for review November 3, 2023 21:14

rauhul requested review from neonichu, tomerd and MaxDesiatov as code owners November 3, 2023 21:14

Add missing lto linker flags

26678f2

rauhul force-pushed the lto-test-fixture branch from 8c8dfef to 26678f2 Compare November 3, 2023 21:29

Add --verbose flag

3d76670

add a verbose flag to be able to help debug why this fixture is failing on linux

rauhul mentioned this pull request Nov 7, 2023

Update mixin_swiftpm_base to include swift-driver swiftlang/swift#69696

Closed

only run test on macOS to work around missing swift-driver

f59105c

rauhul enabled auto-merge (squash) December 6, 2023 05:28

MaxDesiatov reviewed Dec 6, 2023

View reviewed changes

review comments

93f7309

MaxDesiatov approved these changes Dec 6, 2023

View reviewed changes

rauhul merged commit 906afa4 into main Dec 6, 2023

rauhul deleted the lto-test-fixture branch December 6, 2023 20:27

MaxDesiatov restored the lto-test-fixture branch December 7, 2023 16:18

MaxDesiatov added bug build system Changes to interactions with build systems labels Dec 7, 2023

MaxDesiatov mentioned this pull request Dec 7, 2023

[5.10] Add test fixture for --experimental-lto-mode option #7166

Merged

MaxDesiatov deleted the lto-test-fixture branch December 7, 2023 16:25

Add test fixture for experimental-lto-mode #6891

Add test fixture for experimental-lto-mode #6891

Conversation

rauhul commented Sep 12, 2023 • edited Loading

rauhul commented Sep 12, 2023

rauhul commented Sep 12, 2023

rauhul commented Sep 12, 2023

rauhul commented Sep 12, 2023

neonichu commented Sep 12, 2023

artemcm commented Sep 12, 2023

rauhul commented Sep 12, 2023 • edited Loading

artemcm commented Sep 12, 2023

rauhul commented Sep 12, 2023

artemcm commented Sep 12, 2023

rauhul commented Sep 18, 2023

artemcm commented Sep 18, 2023

artemcm commented Sep 18, 2023

rauhul commented Sep 18, 2023

rauhul commented Sep 18, 2023

rauhul commented Sep 18, 2023

rauhul commented Sep 21, 2023

rauhul commented Sep 21, 2023

rauhul commented Nov 3, 2023

rauhul commented Nov 3, 2023

rauhul commented Nov 3, 2023

rauhul commented Nov 3, 2023

shahmishal commented Nov 4, 2023

MaxDesiatov commented Nov 5, 2023

rauhul commented Nov 6, 2023

rauhul commented Nov 9, 2023

rauhul commented Nov 9, 2023

rauhul commented Dec 5, 2023

rauhul commented Dec 6, 2023

rauhul commented Dec 6, 2023

MaxDesiatov Dec 6, 2023

Choose a reason for hiding this comment

rauhul Dec 6, 2023

Choose a reason for hiding this comment

rauhul Dec 6, 2023

Choose a reason for hiding this comment

rauhul commented Dec 6, 2023

MaxDesiatov left a comment

Choose a reason for hiding this comment

MaxDesiatov commented Dec 6, 2023

rauhul commented Sep 12, 2023 •

edited

Loading

rauhul commented Sep 12, 2023 •

edited

Loading