Page MenuHomePhabricator

[JumpThreading] Add a basic support for freeze instruction
ClosedPublic

Authored by aqjune on Sun, Jul 26, 5:36 AM.

Details

Summary

This patch adds a basic support for freeze instruction to JumpThreading
by making ComputeValueKnownInPredecessorsImpl look into its operand.

Diff Detail

Event Timeline

aqjune created this revision.Sun, Jul 26, 5:36 AM
Herald added a project: Restricted Project. · View Herald TranscriptSun, Jul 26, 5:36 AM
nikic added a comment.Sun, Jul 26, 6:00 AM

Just as a side note, you may want to add freeze support to foldOpIntoPhi() in InstCombine, which I believe will also catch the two cases being tested here.

llvm/test/Transforms/JumpThreading/freeze.ll
30

We should also have a negative test, say for undef as phi operand.

aqjune updated this revision to Diff 280724.Sun, Jul 26, 6:04 AM

Leave diff of the test file only

aqjune updated this revision to Diff 280725.Sun, Jul 26, 6:08 AM

Add a negative test

aqjune marked an inline comment as done.Sun, Jul 26, 6:08 AM

Just as a side note, you may want to add freeze support to foldOpIntoPhi() in InstCombine, which I believe will also catch the two cases being tested here.

Okay, I'll have a look at it.

aqjune updated this revision to Diff 280726.Sun, Jul 26, 6:13 AM

Temporarily hide the negative test because its output was a bit strange

nikic added inline comments.Sun, Jul 26, 6:22 AM
llvm/lib/Transforms/Scalar/JumpThreading.cpp
709

You can probably write this loop a bit more elegantly using something along these lines:

erase_if(Result, [](std::pair<Constant *, BasicBlock *> &Pair) {
  return !isGuaranteedNotToBeUndefOrPoison(Pair.first);
});
aqjune updated this revision to Diff 280727.Sun, Jul 26, 6:30 AM

Revive test, use erase_if

aqjune marked an inline comment as done.Sun, Jul 26, 6:32 AM

erase_if seems great.

In the test, freeze undef can be folded into something. Maybe GetBestDestForJumpOnUndef can be used. I'll make this as a follow-up patch.

nikic accepted this revision.Sun, Jul 26, 7:30 AM

LGTM. I'm not particularly familiar with JumpThreading though, so please wait a day in case there are more comments.

This revision is now accepted and ready to land.Sun, Jul 26, 7:30 AM
efriedma added inline comments.Mon, Jul 27, 12:33 PM
llvm/lib/Transforms/Scalar/JumpThreading.cpp
699

Do we want to handle a freeze of a cast, or vice versa?

aqjune added inline comments.Tue, Jul 28, 9:48 AM
llvm/lib/Transforms/Scalar/JumpThreading.cpp
699

Yes, it sounds good.

aqjune updated this revision to Diff 281279.Tue, Jul 28, 10:23 AM

Handle a freeze of a cast and vice versa

This revision was landed with ongoing or failed builds.Tue, Jul 28, 11:12 AM
This revision was automatically updated to reflect the committed changes.