This is an archive of the discontinued LLVM Phabricator instance.

[JumpThreading] Add a pattern to TryToUnfoldSelectInCurrBB()
ClosedPublic

Authored by haicheng on Jun 28 2017, 9:55 AM.

Details

Summary

Add the following pattern to TryToUnfoldSelectInCurrBB()

bb:
   %p = phi [0, %bb1], [1, %bb2], [0, %bb3], [1, %bb4], ...
   %c = cmp %p, 0
   %s = select %c, trueval, falseval

The Select in the above pattern will be unfolded and then jump-threaded. The current implementation does not allow CMP in the middle of PHI and Select.

Diff Detail

Repository
rL LLVM

Event Timeline

haicheng created this revision.Jun 28 2017, 9:55 AM
junbuml added inline comments.Jun 28 2017, 10:15 AM
lib/Transforms/Scalar/JumpThreading.cpp
2213

Do you intentionally go through all users of the PHI by removing the check PN->hasOneUse() ?

2219

Don't you need to check if Cmp is used as the condition in SelectInst ?

haicheng added inline comments.Jun 28 2017, 10:39 AM
lib/Transforms/Scalar/JumpThreading.cpp
2213

Yes, I found out that it is useful when benchmarking.

2219

It is checked in isUnfoldCandidate().

junbuml added inline comments.Jul 3 2017, 1:09 PM
lib/Transforms/Scalar/JumpThreading.cpp
2213

Can you add a test case for this? Don't you think this change itself should be a separate patch?

haicheng updated this revision to Diff 105276.Jul 5 2017, 8:42 AM

Added a test case as requested by Jun.

haicheng marked an inline comment as done.Jul 5 2017, 8:49 AM
haicheng added inline comments.
lib/Transforms/Scalar/JumpThreading.cpp
2213

I added a test case. Basically, I find out that this pattern is common and it is beneficial to split.

bb:
   %p = phi [0, %bb1], [1, %bb2], [2, %bb3], [3, %bb4], ...
   %c = cmp sgt %p, 2
   %s = select %c, trueval, %p

PHI is used in both cmp and select. If it is okay, I prefer to including it in this patch.

junbuml accepted this revision.Jul 7 2017, 9:07 AM

LGTM, but it needs to be reviewed by someone else as well.

This revision is now accepted and ready to land.Jul 7 2017, 9:07 AM

Thank you, Jun.

I will wait for a week before I commit it in case people have different opinions.

haicheng marked 2 inline comments as done.Jul 7 2017, 9:19 AM
This revision was automatically updated to reflect the committed changes.