[llvm] [LV] Fix random behaviour in LoopVectorizationLegality::isUniform (PR #170463)

[David Sherwood via llvm-commits]

================
@@ -517,7 +517,18 @@ class SCEVAddRecForUniformityRewriter
         SE.getMulExpr(Step, SE.getConstant(Ty, StepMultiplier));
     const SCEV *ScaledOffset = SE.getMulExpr(Step, SE.getConstant(Ty, Offset));
     const SCEV *NewStart = SE.getAddExpr(Expr->getStart(), ScaledOffset);
-    return SE.getAddRecExpr(NewStart, NewStep, TheLoop, SCEV::FlagAnyWrap);
+    // We have to be careful when creating new SCEVAddRec expressions because
+    // we may pick up a cached SCEV object with wrap flags already set. This
+    // then leads to random behaviour depending upon which combinations of
+    // offset, StepMultiplier and TheLoop are used. The safest thing we can do
+    // here is to reuse existing wrap flags on the scalar SCEV, since if the
+    // scalar version of the SCEV cannot wrap then the vector version also
+    // cannot. There are situations where the lane of the vector may exceed the
+    // trip count, such as tail-folding. In those cases we shouldn't even be
+    // asking if something is uniform anyway.
+    const SCEV *Res =
+        SE.getAddRecExpr(NewStart, NewStep, TheLoop, Expr->getNoWrapFlags());
----------------
david-arm wrote:

OK, the alternatives seem to me to be:

1. Disable this optimisation entirely, because it seems like bad behaviour to insert rogue SCEV nodes into the cache that shouldn't be there. Any later analysis that picks up these cached nodes may be surprised that there are no wrapping flags set when the analysis says that they shouldn't in fact wrap (perhaps because the IR nodes themselves have nsw/nuw on them). Or,
2. Have a way of preventing ScalarEvolution from inserting nodes into the cache while performing these rewrites.



https://github.com/llvm/llvm-project/pull/170463