Page MenuHomePhabricator

[JumpThreading] Don't restrict cast-traversal to i1
ClosedPublic

Authored by loladiro on Jan 18 2018, 1:09 PM.

Details

Summary

In D17663, JumpThreading learned to look trough simple cast instructions,
but only if the source of those cast instructions was a phi/cmp i1
(in an effort to limit compile time effects). I think this condition
is too restrictive. For switches with limited value range, InstCombine
will readily introduce an extra trunc instruction to a smaller
integer type (e.g. from i8 to i2), leaving us in the somewhat perverse
situation that jump-threading would work before running instcombine,
but not after. Since instcombine produces this pattern, I think we
need to consider it canonical and support it in JumpThreading.
In general, for limiting recursion, I think the existing restriction
to phi and cmp nodes should be sufficient to avoid looking through
unprofitable chains of instructions.

Diff Detail

Repository
rL LLVM

Event Timeline

loladiro created this revision.Jan 18 2018, 1:09 PM

Hi Keno,

Sorry for the late response. What you said totally makes sense to me. Do you have any performance and/or compilation data to share?

Haicheng

This happens fairly frequently for us in julia, because we use i8 to represent tags for tagged unions and then switch on them to discover which of the union components is active.
Without this patch I saw ~3x performance regressions in the worst benchmark (because it prevented other optimizations from seeing optimization opportunities based on the
active union component). I did not see compilation performance impact in my testing.

This revision is now accepted and ready to land.Jan 31 2018, 1:30 PM
This revision was automatically updated to reflect the committed changes.