This is an archive of the discontinued LLVM Phabricator instance.

[JumpThreading] Re-enable JumpThreading for guards
ClosedPublic

Authored by mkazantsev on Feb 16 2017, 2:30 AM.

Details

Summary

JumpThreading for guards feature has been reverted at https://reviews.llvm.org/rL295200
due to the following problem: the feature used the following algorithm for detection of
diamond patters:

  1. Find a block with 2 predecessors;
  2. Check that these blocks have a common single parent;
  3. Check that the parent's terminator is a branch instruction.

The problem is that these checks are insufficient. They may pass for a non-diamond
construction in case if those two predecessors are actually the same block. This may
happen if parent's terminator is a br (either conditional or unconditional) to a block
that ends with "switch" instruction with exactly two branches going to one block.

This patch re-enables the JumpThreading for guards and fixes this issue by adding the
check that those found predecessors are actually different blocks. This guarantees that
parent's terminator is a conditional branch with exactly 2 different successors, which
is now ensured by assertions. It also adds two more tests for this situation (with parent's
terminator being a conditional and an unconditional branch).

Diff Detail

Repository
rL LLVM

Event Timeline

mkazantsev created this revision.Feb 16 2017, 2:30 AM
sanjoy accepted this revision.Feb 16 2017, 12:01 PM

LGTM! I'll check this in for you once you address the minor stylistic comment mentioned inline.

lib/Transforms/Scalar/JumpThreading.cpp
2076 ↗(On Diff #88705)

LLVM coding style is to use auto * for pointers.

This revision is now accepted and ready to land.Feb 16 2017, 12:01 PM
mkazantsev updated this revision to Diff 88845.Feb 16 2017, 7:48 PM
mkazantsev marked an inline comment as done.

Fixed minor code style issue.

This revision was automatically updated to reflect the committed changes.