This is an archive of the discontinued LLVM Phabricator instance.

[LazyValueInfo] Let getEdgeValueLocal look into freeze instructions
ClosedPublic

Authored by aqjune on Jul 27 2020, 3:19 AM.

Details

Summary

This patch makes getEdgeValueLocal more precise when a freeze instruction is
given, by adding support for freeze into constantFoldUser

Diff Detail

Event Timeline

aqjune created this revision.Jul 27 2020, 3:19 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 27 2020, 3:19 AM

Which of the changes in the patch corresponds to each testcase?

nikic added inline comments.Jul 27 2020, 12:09 PM
llvm/lib/Analysis/LazyValueInfo.cpp
1290

Wouldn't we expect such a freeze to get dropped by InstSimplify/InstCombine anyway?

efriedma added inline comments.Jul 27 2020, 12:26 PM
llvm/lib/Analysis/LazyValueInfo.cpp
1290

You mean, we should rewrite uses of a freeze in blocks where we know the operand of the freeze can't be undef/poison? I guess that's something we should do. I don't think instsimplify has the context necessary to do that sort of rewrite.

nikic added inline comments.Jul 27 2020, 12:58 PM
llvm/lib/Analysis/LazyValueInfo.cpp
1290

You're right, if there are multiple uses and only some are dominated by the condition we currently don't handle it in InstCombine. The @simple case from the test would be handled though, because the freeze gets sunk.

getEdgeValueLocal's change at if (BranchInst *BI ...) { .. } corresponds to the simple example.
For the switch example, if (SwitchInst *SI ...) { ... } branch calls constantFoldUser, which is newly updated.

llvm/lib/Analysis/LazyValueInfo.cpp
1290

Would it be the right direction if I implement this optimization in InstCombine instead?

Since D84940 can introduce freezes, this patch can be effective now IMO.

nikic added inline comments.Aug 10 2020, 12:07 PM
llvm/lib/Analysis/LazyValueInfo.cpp
1290

Is this code actually needed? I'm thinking that the constantFoldUser code should already take care of it. The Freeze usesOperand Condition, and should then get folded.

aqjune updated this revision to Diff 284541.Aug 10 2020, 6:16 PM

Remove redundant check

aqjune marked an inline comment as done.Aug 10 2020, 6:17 PM
aqjune added inline comments.
llvm/lib/Analysis/LazyValueInfo.cpp
1290

The check seemed redundant to me as well, so removed it. The unit tests still work too.

aqjune marked an inline comment as done.Aug 10 2020, 6:18 PM
This revision is now accepted and ready to land.Aug 10 2020, 7:21 PM
aqjune edited the summary of this revision. (Show Details)Aug 11 2020, 12:39 AM