[PATCH] D116766: [SCEV] Sequential/in-order `UMin` expression

[Nikita Popov via Phabricator via llvm-commits]

nikic accepted this revision.
nikic added a comment.
This revision is now accepted and ready to land.

I'm on board with the "sequential" name, LGTM from my side. But please wait for @reames to approve as well.

There are some pretty obvious folds we can do (in particular, we should be trying to convert umin_seq to umin with known non-zero/non-poison values), but those are best left for later.



================
Comment at: llvm/include/llvm/Analysis/ScalarEvolution.h:632
                             SmallVectorImpl<const SCEV *> &Operands);
+  const SCEV *getSaturatingMinMaxExpr(SCEVTypes Kind,
+                                      SmallVectorImpl<const SCEV *> &Operands);
----------------
Still using the old name here.


================
Comment at: llvm/include/llvm/Analysis/ScalarEvolution.h:736
+  const SCEV *getUMinFromMismatchedTypes(const SCEV *LHS, const SCEV *RHS,
+                                         bool Saturating = false);
 
----------------
Rename to Sequential here as well for consistency?


================
Comment at: llvm/include/llvm/Analysis/ScalarEvolutionExpressions.h:56
+  scUnknown,
+  scCouldNotCompute
+};
----------------
Precommit reformat of this file?


================
Comment at: llvm/include/llvm/Analysis/ScalarEvolutionExpressions.h:552
+    /// Set flags for a non-recurrence without clearing previously set flags.
+    void setNoWrapFlags(NoWrapFlags Flags) { SubclassData |= Flags; }
+
----------------
Is this needed? Doesn't look relevant for minmax.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116766