[SR-1502] Enabling whole-module optimization causes faulty control flow

[swift-ci]

Previous ID	SR-1502
Radar	rdar://problem/26255167
Original Reporter	andymatuschak (JIRA User)
Type	Bug
Status	Resolved
Resolution	Done

Additional Detail from JIRA

Votes	1
Component/s	Compiler
Labels	Bug, OptimizedOnly
Assignee	andymatuschak (JIRA)
Priority	Medium

md5: e1c2fd46de0482f4efa7bca7f3c4ff2c

Issue Description:

We recently tried turning on whole-module optimization on the Khan Academy project. Three tests newly started failing, apparently due to faulty switch pattern matching. Possibly related to [SR-1217]?

I tried and failed to create a reduced test case, but the following patch resolved the issue for us—perhaps that will be a clue?

diff --git a/KHATests/URLRoutingTest.swift b/KHATests/URLRoutingTest.swift
index 40702e5..6c6baae 100644
--- a/KHATests/URLRoutingTest.swift
+++ b/KHATests/URLRoutingTest.swift
@@ -83,7 +83,8 @@ class URLRoutingTest: XCTestCase {
                if let target = URLTarget(url: testCase.inputURL) {
                        switch target {
                        case .ContentItem: return true
-                       case .TopicSlug, .Domain: return false
+                       case .TopicSlug: return false
+                       case .Domain: return false
                        }
                } else {
                        return false
@@ -105,7 +106,8 @@ class URLRoutingTest: XCTestCase {
                if let target = URLTarget(url: testCase.inputURL) {
                        switch target {
                        case .Domain: return true
-                       case .TopicSlug, .ContentItem: return false
+                       case .TopicSlug: return false
+                       case .ContentItem: return false
                        }
                } else {
                        return false

If there's other useful diagnostic information I can generate or supply, please do let me know. We recently shared the KA source with @akyrtzi, so if an Apple-internal engineer takes a look at this, you can reproduce by running the unit tests with whole-module optimization enabled. -O alone does not produce the issue.

[rudkx]

Thanks for the report.

I'll take a look!

[rudkx]

andymatuschak (JIRA User), what version of Xcode were you using when you saw this behavior?

I ask because I am actually seeing a different bug that is blocking this from compiling when whole module optimization is enabled.

Feel free to get in touch with me at <first> dot <last> at apple.com.

[NachoSoto]

It reproduces in both 7.3 and 7.3.1. Forgot to mention that I had to make a few workarounds to get to compile (like nested types in extensions falling to link and something regarding Obj-C initializers without explicit nullability).

I can send you guys an updated project and the diff with the workarounds.

As a side note: I'm glad we're running our (limited) test suite in release mode. I wonder how many people are affected by this miss-compilation and they don't know.

[rudkx]

Yes, I was hitting the Obj-C initializer issue, and after commenting out the two places I hit that, I am now seeing a linking issue in an extension.

If you can get me a new project that works around these and demonstrates the control-flow issue, I can take a look at that and also try to get some traction on the initializer and linkage issues as I would not expect to see a difference here with whole module optimization enabled. If you want to file separate bugs for those issues to track progress and so that you can get notification when a fix goes in, please feel free.

[NachoSoto]

I just e-mailed you the updated project and the patch with all the work arounds. Thanks for your time! 🙂

[rudkx]

This turned out to be a bug in the LLVM MergeFunctions optimization pass, and that bug is still present in the LLVM repository.

I am preparing a patch to submit.

[rudkx]

Up for review: http://reviews.llvm.org/D20462

[NachoSoto]

That's huge, thanks Mark!

[rudkx]

Fixed in llvm.org trunk in r270250 and swift.org/swift-llvm swift-3.0-branch in 26ee5ef61d62b555f81f3a7373dc2479ede9ba70.

[jckarter]

andymatuschak (JIRA User) or @NachoSoto, when/if you make the jump to Swift 3, would you be able to verify this fix (if you haven't already)?

[NachoSoto]

We haven't yet, but we'll do soon 🙂