Skip to content

[cast-opt] Change optimizeMetatypeConversion to return a value and us… #22361

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

Conversation

gottesmm
Copy link
Contributor

@gottesmm gottesmm commented Feb 5, 2019

…e the new

replace value uses action to make sure that we properly notify passes that we
made the change.

This fixes a latent bug where we were not notifying SILCombine about this
replacement.

Specifically:

-  auto replaceCast = [&](SingleValueInstruction *NewCast) {
-    assert(Ty.getAs<AnyMetatypeType>()->getRepresentation()
-           == NewCast->getType().getAs<AnyMetatypeType>()->getRepresentation());
-    MCI->replaceAllUsesWith(NewCast);
-    EraseInstAction(MCI);
-    return NewCast;
+  auto replaceCast = [&](SILValue newValue) -> SILValue {
+    assert(ty.getAs<AnyMetatypeType>()->getRepresentation() ==
+           newValue->getType().getAs<AnyMetatypeType>()->getRepresentation());
+    ReplaceValueUsesAction(mci, newValue);
+    EraseInstAction(mci);
+    return newValue;
   };

Notice how we use MCI->replaceAllUsesWith instead of one of our replace call
backs. SILCombine hooks these to know if it should re-run users.

…e the new

replace value uses action to make sure that we properly notify passes that we
made the change.

This fixes a latent bug where we were not notifying SILCombine about this
replacement.

Specifically:

-  auto replaceCast = [&](SingleValueInstruction *NewCast) {
-    assert(Ty.getAs<AnyMetatypeType>()->getRepresentation()
-           == NewCast->getType().getAs<AnyMetatypeType>()->getRepresentation());
-    MCI->replaceAllUsesWith(NewCast);
-    EraseInstAction(MCI);
-    return NewCast;
+  auto replaceCast = [&](SILValue newValue) -> SILValue {
+    assert(ty.getAs<AnyMetatypeType>()->getRepresentation() ==
+           newValue->getType().getAs<AnyMetatypeType>()->getRepresentation());
+    ReplaceValueUsesAction(mci, newValue);
+    EraseInstAction(mci);
+    return newValue;
   };

Notice how we use MCI->replaceAllUsesWith instead of one of our replace call
backs. SILCombine hooks these to know if it should re-run users.
@gottesmm gottesmm requested a review from atrick February 5, 2019 00:13
@gottesmm
Copy link
Contributor Author

gottesmm commented Feb 5, 2019

@swift-ci smoke test and merge

@swift-ci swift-ci merged commit ff5c406 into swiftlang:master Feb 5, 2019
@gottesmm gottesmm deleted the pr-23c9d327942745873a91662ca452bd1680948b52 branch February 5, 2019 04:14
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