Page MenuHomePhabricator

[SimplifyCFG] Update FoldBranchToCommonDest to be poison-safe
Needs ReviewPublic

Authored by aqjune on Jan 20 2021, 12:16 AM.

Details

Summary

This patch makes FoldBranchToCommonDest merge branch conditions into select i1 rather than and/or i1 when it is called by SimplifyCFG.
It is known that merging conditions into and/or is poison-unsafe, and this is towards making things *more* correct by removing possible miscompilations.
Currently, InstCombine simply consumes these selects into and/or of i1 (which is also unsafe), so the visible effect would be very small. The unsafe select -> and/or transformation will be removed in the future.
There has been efforts for updating optimizations to support the select form as well, and they are linked to D93065.

The safe transformation is fired when it is called by SimplifyCFG only. This is done by setting the new PoisonSafe argument as true.
Another place that calls FoldBranchToCommonDest is LoopSimplify. I think this will have a nontrivial impact since SCEV is more conservative in this case.

Diff Detail

Event Timeline

aqjune created this revision.Jan 20 2021, 12:16 AM
aqjune requested review of this revision.Jan 20 2021, 12:16 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 20 2021, 12:16 AM
aqjune added inline comments.Jan 20 2021, 12:22 AM
llvm/lib/Target/BPF/BPFAdjustOpt.cpp
118

This update was needed to support llvm/test/CodeGen/BPF/adjust-opt-icmp1.ll .

Another place that calls FoldBranchToCommonDest is LoopSimplify. I think this will have a nontrivial impact since SCEV is more conservative in this case.

Enabling PoisonSafe when called by LoopSimplify requires performance evaluation.
For any patches that require experiment, I'd like to write on Feb; right now I don't have enough bandwidth.

Ping
I'd like to leave the select i1 -> and/or folding in InstCombine as a single point where such transformation happens. This patch is a step to it.

lebedev.ri added inline comments.Jan 24 2021, 11:33 PM
llvm/lib/Transforms/Utils/SimplifyCFG.cpp
2982–2993

This needs changing, too

aqjune added inline comments.Jan 25 2021, 2:44 AM
llvm/lib/Transforms/Utils/SimplifyCFG.cpp
2982–2993

The created selects are semantically equivalent to and/or - but would it be more correct to count them as having select costs? (I'm not familiar with the instruction cost things)