[PATCH] D116766: [SCEV] Saturating `UMin` expression

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri updated this revision to Diff 398166.
lebedev.ri retitled this revision from "[SCEV] Poison-safe `UMin` expression" to "[SCEV] Saturating `UMin` expression".
lebedev.ri added a comment.

Thanks for taking a look!

In D116766#3226340 <https://reviews.llvm.org/D116766#3226340>, @reames wrote:

> Before you go any further, can you explain what you mean by "poison safe"?  What IR are you hoping to generate in the end, and why is that more correct than what we have previously?

I believe others have already explained it well before i could.
I've adjusted the differential's description with a bit more blurb and alive2 reasoning.
Let me know if that is still not sufficient.

In D116766 <https://reviews.llvm.org/D116766>, @nikic wrote:

> getMinMaxExpr() currently assumes that the operands are commutative, e.g. in GroupByComplexity. Some of the folds would have to be skipped or done differently for "safe umin".

Right. I've now made the expression non-commutative.

In D116766#3226805 <https://reviews.llvm.org/D116766#3226805>, @nikic wrote:

> Worth noting that we should be using "safe umin" not just for the logical or case, but also when combining exit counts from multiple exits, which is the more common case. It's probably best to do that separately, but that would give a clearer picture of how bad this is in terms of practical impact, because we have a lot more tests for that.

Yep, i really don't want to deal with everything at once :)

Using "safe" really bothered me, i've gone ahead and used "saturating" instead,
since that is what happens in reality: https://alive2.llvm.org/ce/z/abvHQS (+ non-commutativity)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116766

Files:
  llvm/include/llvm/Analysis/ScalarEvolution.h
  llvm/include/llvm/Analysis/ScalarEvolutionDivision.h
  llvm/include/llvm/Analysis/ScalarEvolutionExpressions.h
  llvm/include/llvm/IR/IRBuilder.h
  llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h
  llvm/lib/Analysis/ScalarEvolution.cpp
  llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
  llvm/test/Analysis/ScalarEvolution/exit-count-select-safe.ll
  llvm/test/Transforms/IndVarSimplify/exit-count-select.ll
  polly/include/polly/Support/SCEVAffinator.h
  polly/lib/Support/SCEVAffinator.cpp
  polly/lib/Support/SCEVValidator.cpp
  polly/lib/Support/ScopHelper.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D116766.398166.patch
Type: text/x-patch
Size: 40671 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220107/b71ed3a0/attachment-0001.bin>