This is an archive of the discontinued LLVM Phabricator instance.

[JumpThreading] Allow duplicating a basic block into preds when its branch condition is freeze(phi)
ClosedPublic

Authored by aqjune on Jul 31 2020, 7:40 AM.

Details

Summary

This is the last JumpThreading patch for getting the performance numbers shown at
https://reviews.llvm.org/D84940#2184653 .

This patch makes ProcessBlock call ProcessBranchOnPHI when the branch condition
is freeze(phi) as well (originally it calls the function when the condition is
phi only).

Since what ProcessBranchOnPHI does is to duplicate the basic block into
predecessors if profitable, it is still valid when the condition is freeze(phi)
too.

    p = phi [a, pred1] [b, pred2]
    p.fr = freeze p
    br p.fr, ...
=>
  pred1:
    p.fr = freeze a
    br p.fr, ...
  pred2:
    p.fr2 = freeze b
    br p.fr2, ...

Diff Detail

Event Timeline

aqjune created this revision.Jul 31 2020, 7:40 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 31 2020, 7:40 AM
aqjune requested review of this revision.Jul 31 2020, 7:40 AM
efriedma added inline comments.Aug 3 2020, 1:57 PM
llvm/lib/Transforms/Scalar/JumpThreading.cpp
1207

Would it make sense to use SimplifyValue here? (That sort of interacts with your other patch, I think?)

aqjune updated this revision to Diff 282831.Aug 4 2020, 1:56 AM

Rebase

llvm/lib/Transforms/Scalar/JumpThreading.cpp
1207

I think it is tricky, since SimplifyValue may be extracted from icmp as well (line 1179)

efriedma added inline comments.Aug 4 2020, 1:12 PM
llvm/lib/Transforms/Scalar/JumpThreading.cpp
1207

By the same logic that makes it okay to look through freeze, it would be legal to look through icmps, I think? But maybe not profitable, and out of scope for this patch, I guess.

1778

This comment is a bit misleading. Really, this transform is valid on any PHI node, whether or not it feeds into the branch. It's just a heuristic that we only do this for i1 PHIs that feed directly into a branch.

aqjune updated this revision to Diff 283142.Aug 5 2020, 12:30 AM

Update the comment

aqjune marked an inline comment as done.Aug 5 2020, 12:32 AM
aqjune added inline comments.
llvm/lib/Transforms/Scalar/JumpThreading.cpp
1778

Rephrased the comment a bit

efriedma accepted this revision.Aug 5 2020, 2:58 PM

LGTM

This revision is now accepted and ready to land.Aug 5 2020, 2:58 PM
aqjune edited the summary of this revision. (Show Details)Aug 5 2020, 5:51 PM
This revision was landed with ongoing or failed builds.Aug 5 2020, 5:51 PM
This revision was automatically updated to reflect the committed changes.
aqjune marked an inline comment as done.