[PATCH] D112573: [IndVarSimplify] Reduce nondeterministic behaviour in visitIVCast.

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added a comment.

Hi @mkazantsev, thanks for having a look at this patch!

In D112573#3099600 <https://reviews.llvm.org/D112573#3099600>, @mkazantsev wrote:

> If I understand your proposal correctly, you propose to replace scheme "take signedness from 1st user which is non-deterministically selected" with scheme "if at least one user of widest type is signed cast, widen as signed; otherwize widen as unsigned"?

Yes, that's right, I figured that consistently choosing one over the other at least removes the non-determinism.

> In general, it makes sense to me and it also seems to solve the non-determinism you are describing. The only concern I have is, you use signed widening as default (and unsigned is possible under stricter conditions). On the other hand, IV tries hard to turn all `icmp`'s to unsigned predicate form, preferring unsigned as default. This creates some discord between different parts of it.

I guess it's possible to favour `unsigned` as the default, but the specific example in the test doesn't optimise as well when choosing `unsigned`, because we end up with 2 induction variables in the loop instead of 1. For one critical loop that I investigated, this led to multiple redundant sign-extends.
This code is not very familiar to me though, so if you can give me some hints on how to make IndVarSimplify handle the unsigned case, I'd be happy to look into that!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112573/new/

https://reviews.llvm.org/D112573