[PATCH] D33543: Re-enable "[SCEV] Do not fold dominated SCEVUnknown into AddRecExpr start"

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy added a comment.

Hi Maxim,

I think there is one bit still unanswered -- in the case where we lack a proper `TargetMachine` (i.e. the case where we're running an ARM-only build on the `lsr-expand-quadratic.ll` test) we went from passing without your change to failing with your change.  What changed?  Was LSR relying on incorrect SCEV behavior (this won't surprise me)?



================
Comment at: lib/Analysis/ScalarEvolution.cpp:2209
+      switch (static_cast<SCEVTypes>(S->getSCEVType())) {
+      case scConstant:
+        return false;
----------------
In a later change, maybe you can change this code to just do:

```
if (auto *SU = dyn_cast<SCEVUnknown>(S))
  return checkUnknown(...);
return true;
```

(there should be no need to return false on constants -- they have no operands and thus nothing to follow)


================
Comment at: test/Transforms/LoopStrengthReduce/X86/lsr-expand-quadratic.ll:1
+; REQUIRES: x86
+; RUN: opt -loop-reduce -S < %s | FileCheck %s
----------------
Not sure if the `REQUIRES: x86` bit is needed -- this directory already has a lit.local.cfg that should have the same restriction.


https://reviews.llvm.org/D33543