JuliaLang / julia / #37747

77%

Build Type

push

local

Committed by web-flow

Commit Message

ir: Set refined when narrowing PhiNode in IncrementalCompact (#54061)

This is a bit of a tricky one. What happens here is that
IncrementalCompact finds that one branch of an if/else is dead, so one
of the incoming values of the PhiNode goes away. This may have the
effect of narrowing the type of the PhiNode (if the branches have
different types). In the latest iteration of our compiler, such
situations need to be annotated with IR_FLAG_REFINED or `Refined()`,
otherwise irinterp will skip them on revisit.

In theory, the fix is quite simple: Check whether the type is being
refined and if so, set the flag. However, this code sits inside
IncrementalCompcat, which currently neither performs lattice operations
nor sets any Refined flags by itself. The three possible options here
are:

1. Thread lattice through IncrementalCompact.
2. Use an egal check inside IncrementalCompact rather than the proper
lattice query. This is legal, but may overapproximate the need for
`Refined`, thus causing unnecessary work later.
3. Move the phi node narrowing outside of IncrementalCompact.

This PR takes a hybrid approach of 2 and 3. Approach 1 is undesirable,
because we do not want to recompile all of IncrementalCompact for every
lattice, just for this one query. I almost took approach 2, but it is
unsatisfying, because of the imprecision, which could cuase a lot of
extra work in pathological cases.

The primary downside of approach 3 is that now every pass that performs
IncrementalCompact with CFG manipulation enabled will need to call the
`reprocess_phi_node!` helper. We have two of these in base (adce_pass!
and `compact!(..., true)` although only the former as access to a
lattice.

The compromise I came up with here is to do the egality check, except
leaving the PhiNode in place when it fails rather than setting `Refined`
internally. I think this is ok. There's a bit of an issue with duplicate
work, because the first thing that the la... (continued)

Run Details

21 of 21 new or added lines in 2 files covered. (100.0%)

1019 existing lines in 43 files now uncovered.

74293 of 86202 relevant lines covered (86.18%)

14549645.91 hits per line