This is an archive of the discontinued LLVM Phabricator instance.

[InstCombine] Remove unneeded isa<PHINode> check in foldOpIntoPhi
ClosedPublic

Authored by 0xdc03 on Jul 19 2023, 8:42 AM.

Details

Summary

This check is redundant as it is covered by the call to
isPotentiallyReachable.

Depends on D155726.

Diff Detail

Event Timeline

0xdc03 created this revision.Jul 19 2023, 8:42 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 19 2023, 8:42 AM
0xdc03 requested review of this revision.Jul 19 2023, 8:42 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 19 2023, 8:42 AM
0xdc03 updated this revision to Diff 542099.Jul 19 2023, 10:23 AM
  • Add test case
0xdc03 edited the summary of this revision. (Show Details)Jul 19 2023, 10:24 AM
0xdc03 updated this revision to Diff 542297.Jul 19 2023, 9:25 PM
  • Rebase on updated test
  • Fix broken test

Looks like the build timed out on windows...

nikic accepted this revision.Jul 20 2023, 12:49 AM

The change LGTM, but this patch should not be based on D154064. We should land this first and D154064 later.

This revision is now accepted and ready to land.Jul 20 2023, 12:49 AM
0xdc03 updated this revision to Diff 542364.Jul 20 2023, 1:24 AM
  • Invert the patch stack

The change LGTM, but this patch should not be based on D154064. We should land this first and D154064 later.

Okay, I have done this and changed the patch order but the issue I have now is that no changes are visible in the tests introduced in D155726.

0xdc03 edited the summary of this revision. (Show Details)Jul 20 2023, 1:31 AM
nikic added a comment.Jul 20 2023, 1:37 AM

The change LGTM, but this patch should not be based on D154064. We should land this first and D154064 later.

Okay, I have done this and changed the patch order but the issue I have now is that no changes are visible in the tests introduced in D155726.

The patch order should be D155726, then D155718, then D154064. If you order the patches that way you should see a test change on SwitchTest. It's expected that you don't see a change on BranchTest, which only makes sure that D154064 doesn't regress it.

0xdc03 updated this revision to Diff 542375.Jul 20 2023, 1:49 AM
  • Change the patch order
0xdc03 updated this revision to Diff 550711.Aug 16 2023, 5:51 AM
  • Rebase on main
0xdc03 edited the summary of this revision. (Show Details)Aug 16 2023, 5:53 AM