[all-commits] [llvm/llvm-project] 305816: [IndVarSimplify] Reduce nondeterministic behaviour...

[sdesmalen-arm via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 305816ff1e737d9589f34dab69a3f5abd884f7d5
      https://github.com/llvm/llvm-project/commit/305816ff1e737d9589f34dab69a3f5abd884f7d5
  Author: Sander.DeSmalen at arm.com <Sander.DeSmalen at arm.com>
  Date:   2021-11-16 (Tue, 16 Nov 2021)

  Changed paths:
    M llvm/lib/Transforms/Scalar/IndVarSimplify.cpp
    A llvm/test/Transforms/IndVarSimplify/deterministic-sign.ll

  Log Message:
  -----------
  [IndVarSimplify] Reduce nondeterministic behaviour in visitIVCast.

rGf39978b84f1d3a1da6c32db48f64c8daae64b3ad led to and/or exposed
an issue with IndVarSimplification for a loop where a i32 phi node is
no longer replaced by a widened (i64) phi node, because the SCEVs of a
sign-extend no longer folded the same way. I'm unsure how to properly
explain this because it's all rather complicated, but in short: SCEVs
don't fold as nicely as they used to and this caused a difference.

While investigating this, I found that IndVarSimplify can actually
optimise the case in the way we want to if it chooses the widened IV to
be 'signed' (the i32 IV is both sign and zero-extended). Oddly enough,
there is some level of indeterminism in the way the algorithm works,
it just picks the sign of the 'first' zext/sext user, where the order of
the users-iterator is not guaranteed to be the same on each invocation
of the pass (e.g. shown by first running loop-rotate, which puts the
users in a different order).

While I think the fix is valid in the sense that consistently picking
_any_ order is better than having an nondeterministic order, I can
use a bit of advice from people more familiar in this area of the
code-base.

For example, I'm not sure if this fix is hiding another issue where the
IndVarSimplify pass could actually draw the same conclusions (i.e. that
it only needs an i64 phi node) if it does a bit more work, regardless
of whether it chooses the induction variable to be signed or unsigned.

I'm also not sure if choosing signed is better than unsigned, or whether
that just happens to be beneficial only in this individual case.

Any feedback would be much appreciated!

Reviewed By: reames

Differential Revision: https://reviews.llvm.org/D112573