[DynamicCast] Rely on runtime when casts can't be optimized #33761

tbkka · 2020-09-02T18:18:13Z

The Swift compiler renders as? operations in Swift into variations of the checked_cast_br instructions in SIL. Subsequent optimization passes may alter or eliminate these instructions. Any remaining cast instructions after optimization are translated by IRGen into runtime calls.

At least, that's the theory. Unfortunately, the current IRGen logic does not recognize all of the casting instruction variants and translates one particular unrecognized variant into a simple nil load. This effectively converts an as? operation into one that will always fail. Specifically, this occurs for optimized casts of metatypes to AnyObject:

  let a = Int.self
  let b = a as? AnyObject
  // b should not be nil here

(Ironically, in this particular case, the frontend issues a diagnostic warning that the cast will always succeed.)

This PR changes this case in IRGen to instead generate a call to swift_dynamicCast, deferring this case to the runtime. (Yes, this particular example should get optimized into a simple store, but that's a future project.)

Test suite changes:

With this change, the "BoxingCasts" test suite (which exercises a variety of boxing and unboxing casts) now passes in both optimized and non-optimized builds, so I've enabled those versions.
There was a SIL test previously that constructed a checked_cast_br instruction with a thin metatype. The IRGen pass should never see instructions of this form, so I've changed that particular test to exercise a thick metatype instead.

tbkka · 2020-09-02T18:32:27Z

@swift-ci Please test

swift-ci · 2020-09-02T19:51:36Z

Build failed
Swift Test Linux Platform
Git Sha - 1e203eb3b79eefe3ca93b592f9e6e3a57138f6dd

swift-ci · 2020-09-02T20:37:31Z

Build failed
Swift Test OS X Platform
Git Sha - 1e203eb3b79eefe3ca93b592f9e6e3a57138f6dd

jckarter

Makes sense. We shouldn't try to peephole these in IRGen; we should let the cast optimizer do that if possible.

tbkka · 2020-09-08T20:00:45Z

@swift-ci Please test macOS

swift-ci · 2020-09-08T21:38:34Z

Build failed
Swift Test OS X Platform
Git Sha - 1e203eb3b79eefe3ca93b592f9e6e3a57138f6dd

tbkka · 2020-09-10T15:56:12Z

@swift-ci Please test

swift-ci · 2020-09-10T17:13:14Z

Build failed
Swift Test Linux Platform
Git Sha - 1e203eb3b79eefe3ca93b592f9e6e3a57138f6dd

swift-ci · 2020-09-10T17:41:20Z

Build failed
Swift Test OS X Platform
Git Sha - 1e203eb3b79eefe3ca93b592f9e6e3a57138f6dd

shahmishal · 2020-10-01T06:16:30Z

Please update the base branch to main by Oct 5th otherwise the pull request will be closed automatically.

How to change the base branch: (Link)
More detail about the branch update: (Link)

tbkka · 2020-10-05T18:04:09Z

Looks like the Linux test is failing in optimized builds due to some issue with ARC. I'm trying to isolate whether it's an issue with my change or is an existing bug in the compiler ARC optimizations.

tbkka · 2020-10-07T22:14:59Z

Thanks to @eeckstein for fixing the ARC bug I was tripping over. Now let's try those tests again!

tbkka · 2020-10-07T22:15:10Z

@swift-ci Please test

swift-ci · 2020-10-07T23:33:48Z

Build failed
Swift Test Linux Platform
Git Sha - 88fc76f450360365aec5b692376482e3bfd370c6

tbkka · 2020-10-08T17:54:02Z

Linux is crashing on the optimized form of this test:

[ RUN      ] BoxingCasts.test184(): Casting SwiftValueBox(AnyHashable(ClassInt)) to ClassInt
stderr>>> CRASHED: SIGSEGV
the test crashed unexpectedly
[     FAIL ] BoxingCasts.test184(): Casting SwiftValueBox(AnyHashable(ClassInt)) to ClassInt

tbkka · 2020-10-13T01:21:26Z

@swift-ci Please test Linux

tbkka · 2020-10-13T01:22:05Z

Re-ran the full Linux test locally and it's passing for me. Hmmm.... I'll try the CI bot again and see if that still fails before I spend time digging further.

swift-ci · 2020-10-13T02:42:58Z

Build failed
Swift Test Linux Platform
Git Sha - 88fc76f450360365aec5b692376482e3bfd370c6

tbkka · 2020-10-15T20:14:59Z

Re-ran the Linux test locally on Ubuntu 16.04 (required upgrading clang-3.8 to clang-12), and it all passes just fine. I guess I'll try again with the CI preset and see??

tbkka · 2020-10-16T16:20:21Z

Still having trouble reproducing this locally. I'll try again with the exact build options used on the CI bot.

tbkka · 2020-10-17T01:14:48Z

@swift-ci Please test

swift-ci · 2020-10-17T01:38:11Z

Build failed
Swift Test OS X Platform
Git Sha - e5381bddc6ba8a6c3584734390fd14e51ac0a99d

tbkka · 2020-10-17T01:40:07Z

@swift-ci Please test

tbkka · 2020-10-17T01:40:59Z

Let's see if the test that's been failing on Linux also fails on macOS when you enable it properly.

swift-ci · 2020-10-17T02:55:09Z

Build failed
Swift Test Linux Platform
Git Sha - 4c3818b895f866fc73259742b62134be91999ffe

tbkka · 2020-10-19T21:02:32Z

Same cast was in a disabled test in Casts.swift, so I also enabled that unconditionally.

With that, I can now consistently reproduce it on Linux. The root cause seems to be another variation of the ARC issue that @eeckstein fixed for me earlier. As before, it fails on Linux because it triggers __SwiftValue boxing which behaves slightly differently on Linux than on macOS.

tbkka · 2020-10-22T19:35:38Z

I believe #34383 will resolve the last test failure here.

tbkka · 2020-10-22T19:36:41Z

#34383
@swift-ci Please test

swift-ci · 2020-10-22T20:06:28Z

Build failed
Swift Test Linux Platform
Git Sha - 543d4fcb38e38ddf1109e983c9ed6f3da942d5a7

The Swift compiler renders `as?` operations in Swift into variations of the `checked_cast_br` instructions in SIL. Subsequent optimization passes may alter or eliminate these instructions. Any remaining instructions after optimization are translated by IRGen into runtime calls. At least, that's the theory. Unfortunately, the current IRGen logic does not recognize all of the casting instruction variants and renders one particular unrecognized variant as a simple nil load. Specifically, this occurs for optimized casts of metatypes to AnyObject: ``` let a = Int.self let b = a as? AnyObject // b should not be nil here ``` This PR changes this case in IRGen to instead generate a call to `swift_dynamicCast`, deferring this case to the runtime. Future: Someday, the compiler should be taught to correctly optimize this cast away.

I think this exercises the same optimizer path that's breaking another test I've been staring at.

tbkka · 2020-10-28T18:48:53Z

Now that #34383 has landed, let's try this again.

tbkka · 2020-10-28T18:48:58Z

@swift-ci Please test

swift-ci · 2020-10-28T19:10:11Z

Build failed
Swift Test Linux Platform
Git Sha - 35aa75a

tbkka · 2020-10-28T20:27:49Z

@swift-ci Please clean test Linux

* 'main' of github.com:apple/swift: [DynamicCast] Rely on runtime when casts can't be optimized (swiftlang#33761)

tbkka requested review from slavapestov and jckarter September 2, 2020 18:18

jckarter approved these changes Sep 8, 2020

View reviewed changes

tbkka changed the base branch from master to main October 1, 2020 22:12

tbkka force-pushed the tbkka/dynamicCastRework-IRGen-GenCast branch 2 times, most recently from 1f2335d to e5381bd Compare October 17, 2020 01:14

tbkka force-pushed the tbkka/dynamicCastRework-IRGen-GenCast branch from e5381bd to 318935e Compare October 17, 2020 01:38

tbkka force-pushed the tbkka/dynamicCastRework-IRGen-GenCast branch from 4c3818b to 543d4fc Compare October 21, 2020 20:33

tbkka mentioned this pull request Oct 24, 2020

RCIdentity: fix another case of not-RC-identity-preserving casts. #34383

Merged

tbkka added 2 commits October 28, 2020 10:57

Enable another casting test

35aa75a

I think this exercises the same optimizer path that's breaking another test I've been staring at.

tbkka force-pushed the tbkka/dynamicCastRework-IRGen-GenCast branch from 543d4fc to 35aa75a Compare October 28, 2020 18:02

tbkka merged commit b0675c0 into swiftlang:main Oct 28, 2020

ainu-bot added a commit to google/swift that referenced this pull request Oct 29, 2020

Merge branch 'main' of github.com:apple/swift into tensorflow-stage

a5700a7

* 'main' of github.com:apple/swift: [DynamicCast] Rely on runtime when casts can't be optimized (swiftlang#33761)

tbkka deleted the tbkka/dynamicCastRework-IRGen-GenCast branch August 1, 2024 16:36

[DynamicCast] Rely on runtime when casts can't be optimized #33761

[DynamicCast] Rely on runtime when casts can't be optimized #33761

Uh oh!

Conversation

tbkka commented Sep 2, 2020

Uh oh!

tbkka commented Sep 2, 2020

Uh oh!

swift-ci commented Sep 2, 2020

Uh oh!

swift-ci commented Sep 2, 2020

Uh oh!

jckarter left a comment

Choose a reason for hiding this comment

Uh oh!

tbkka commented Sep 8, 2020

Uh oh!

swift-ci commented Sep 8, 2020

Uh oh!

tbkka commented Sep 10, 2020

Uh oh!

swift-ci commented Sep 10, 2020

Uh oh!

swift-ci commented Sep 10, 2020

Uh oh!

shahmishal commented Oct 1, 2020

Uh oh!

tbkka commented Oct 5, 2020

Uh oh!

tbkka commented Oct 7, 2020

Uh oh!

tbkka commented Oct 7, 2020

Uh oh!

swift-ci commented Oct 7, 2020

Uh oh!

tbkka commented Oct 8, 2020

Uh oh!

tbkka commented Oct 13, 2020

Uh oh!

tbkka commented Oct 13, 2020

Uh oh!

swift-ci commented Oct 13, 2020

Uh oh!

tbkka commented Oct 15, 2020

Uh oh!

tbkka commented Oct 16, 2020

Uh oh!

tbkka commented Oct 17, 2020

Uh oh!

swift-ci commented Oct 17, 2020

Uh oh!

tbkka commented Oct 17, 2020

Uh oh!

tbkka commented Oct 17, 2020

Uh oh!

swift-ci commented Oct 17, 2020

Uh oh!

tbkka commented Oct 19, 2020

Uh oh!

tbkka commented Oct 22, 2020

Uh oh!

tbkka commented Oct 22, 2020

Uh oh!

swift-ci commented Oct 22, 2020

Uh oh!

tbkka commented Oct 28, 2020

Uh oh!

tbkka commented Oct 28, 2020

Uh oh!

swift-ci commented Oct 28, 2020

Uh oh!

tbkka commented Oct 28, 2020

Uh oh!

Uh oh!