Skip to content

[windows] Temporarily guard lifetime dependence diagnostics #74599

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 21, 2024

Conversation

atrick
Copy link
Contributor

@atrick atrick commented Jun 20, 2024

by -enable-experimental-feature NonescapableTypes
on the Windows platform

These passes do nothing unless the above feature flag is enabled, so the only reason to run the pass is to exercise SwiftCompilerSources and catch invalid SIL.

These passes rely on fundamental SwiftCompilerSources abstractions which have not yet been tested outside of the passes. They don't yet handle all SIL patterns, and SIL continues to evolve. We would like to can these issues quickly as we hit them, but only if we have a way of reproducing the failure. Currently, we don't have a way of reproducing Windows-arm64 failures.

Workaround for:
rdar://128434000 ([nonescapable] [LifetimeDependenceInsertion] Package resolution fails with arm64 Windows toolchain)

by -enable-experimental-feature NonescapableTypes
on the Windows platform

These passes do nothing unless the above feature flag is enabled, so
the only reason to run the pass is to exercise SwiftCompilerSources
and catch invalid SIL.

These passes rely on fundamental SwiftCompilerSources abstractions
which have not yet been tested outside of the passes. They don't yet
handle all SIL patterns, and SIL continues to evolve. We would like to
can these issues quickly as we hit them, but only if we have a way of
reproducing the failure. Currently, we don't have a way of reproducing
Windows-arm64 failures.

Workaround for:
rdar://128434000 ([nonescapable] [LifetimeDependenceInsertion]
Package resolution fails with arm64 Windows toolchain)
@atrick atrick requested review from compnerd and etcwilde June 20, 2024 23:53
@atrick atrick requested a review from eeckstein as a code owner June 20, 2024 23:53
@atrick
Copy link
Contributor Author

atrick commented Jun 20, 2024

@swift-ci test

@@ -29,6 +29,11 @@ private func log(prefix: Bool = true, _ message: @autoclosure () -> String) {
let lifetimeDependenceDiagnosticsPass = FunctionPass(
name: "lifetime-dependence-diagnostics")
{ (function: Function, context: FunctionPassContext) in
#if os(Windows)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we consider limiting this to os(Windows) && arch(arm64) and same throughout? Is there any thing that would be useful that we could gather to help isolate this issue?

Copy link
Contributor Author

@atrick atrick Jun 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we consider limiting this to os(Windows) && arch(arm64) and same throughout?

Not much point. These passes literally do nothing at the moment. They just process the SIL using a few SwiftCompilerSources helpers and crash if the SIL is invalid according to those helpers.

Is there any thing that would be useful that we could gather to help isolate this issue?

For the crashing function, identifying the difference in -emit-silgen between x86 and arm64 would be a good start. --Note that the pretty stack trace should print the name of the function being compiled.--
I can't imagine why there's any difference in SIL.

A symbolicated stack trace might be nice.

In general, I don't want to get any compiler crashers for projects that haven't tried to build with an asserts compiler.

But since I won't be able to look at it for a week, there's no reason to hold up any toolchain builds.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, sounds like the value of the additional SIL validation is low?

Copy link
Member

@compnerd compnerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM assuming the additional SIL validation is not very useful.

@atrick atrick enabled auto-merge June 21, 2024 04:32
@atrick atrick disabled auto-merge June 21, 2024 16:14
@atrick
Copy link
Contributor Author

atrick commented Jun 21, 2024

@swift-ci smoke test

@atrick atrick enabled auto-merge June 21, 2024 16:15
@atrick atrick merged commit c5abca2 into swiftlang:main Jun 21, 2024
4 of 5 checks passed
@atrick atrick deleted the windows-disable-lifedep branch July 5, 2024 06:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants