Page MenuHomePhabricator

[SimplifyCFG] Use a (trivially) dominanting widenable branch to remove later slow path blocks

Authored by reames on Oct 31 2019, 4:28 PM.



This transformation is a variation on the GuardWidening transformation we have checked in as it's own pass. Instead of focusing on merge (i.e. hoisting and simplifying) two widenable branches, this transform makes the observation that simply removing a second slowpath block (by reusing an existing one) is often a very useful canonicalization. This may lead to later merging, or may not. This is a useful generalization when the intermediate block has loads whose dereferenceability is hard to establish.

As noted in the patch, this can be generalized further. I'd like to get this in, and then iterate on top if that's okay with reviewers.

Diff Detail

Event Timeline

reames created this revision.Oct 31 2019, 4:28 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 31 2019, 4:28 PM
apilipenko added inline comments.Oct 31 2019, 5:36 PM

This can be structured better. First, you need NoSideEffects(*BI->getParent()) for both of the cases, so it can be turned into an early return check. And second, these cases seem to differ only by successor index. Factor out a lambda and call it for 0 and 1?

reames marked an inline comment as done.Nov 1 2019, 3:18 PM
reames added inline comments.

The use of the lambda here is important. I only want to evaluate the expensive predicate if both of the cheap ones have passed.

As for the second point, possible. I played with a couple and nothing really seemed cleaner, but I can give it another attempt.

apilipenko accepted this revision.Nov 1 2019, 7:49 PM
apilipenko added inline comments.

Ah, makes sense. May be a comment explaining the use of the lambda then?

This revision is now accepted and ready to land.Nov 1 2019, 7:49 PM
This revision was automatically updated to reflect the committed changes.