This is an archive of the discontinued LLVM Phabricator instance.

[SimpleLoopUnswitch] Drop make.implicit metadata in case of non-trivial unswitching
ClosedPublic

Authored by mkazantsev on Jul 30 2020, 1:36 AM.

Details

Summary

Non-trivial unswitching simply moves terminator being unswitch from the loop
up to the switch block. It also preserves all metadata that was there. It might not
be a correct thing to do for make.implicit metadata. Consider case:

for (...) {
  cond = // computed in loop
  if (cond) return X;
  if (p == null) throw_npe(); !make implicit
}

Before the unswitching, if p is null and we reach this check, we are guaranteed
to go to throw_npe() block. Now we unswitch on p == null condition:

if (p == null) !make implicit {
  for (...) {
    if (cond) return X;
    throw_npe()
  }
} else {
  for (...) {
    if (cond) return X;
  }
}

Now, following true branch of p == null does not always lead us to
throw_npe() because the loop has side exit. Now, if we run ImplicitNullCheck
pass on this code, it may end up making the unswitch condition implicit. This may
lead us to turning normal path to return X into signal-throwing path, which is
not efficient.

Note that this does not happen during trivial unswitch: it guarantees that we do not
have side exits before condition being unswitched.

This patch fixes this situation by unconditional dropping of make.implicit metadata
when we perform non-trivial unswitch. We could preserve it if we could prove that the
condition always executes. This can be done as a follow-up.

Diff Detail