[PATCH] D3127: [ScalarEvolution]Fix PR18607 resulting in long compilation time and memory usage

[Sanjoy Das via llvm-commits]

sanjoy added a subscriber: sanjoy.
sanjoy requested changes to this revision.
sanjoy added a reviewer: sanjoy.
This revision now requires changes to proceed.

================
Comment at: lib/Analysis/ScalarEvolution.cpp:545
@@ -544,3 +544,3 @@
           return (int)LBitWidth - (int)RBitWidth;
-        return LA.ult(RA) ? -1 : 1;
+        return LA.ult(RA) ? -1 : (LA.eq(RA) ? 0 : 1);
       }
----------------
mehdi_amini wrote:
> Is it related to the threshold defined in this patch?
> Otherwise it should be a separate patch.
We should not be seeing this, since we unique `SCEVConstant` s by their content, so two equal `SCEVConstant` s should have been caught by the pointer equality check in the beginning of the method.

================
Comment at: test/Transforms/IndVarSimplify/pr18607.ll:1
@@ +1,2 @@
+; RUN: opt < %s -indvars -disable-output -S  
+
----------------
mehdi_amini wrote:
> A test without output is not great.
> With a cl::opt threshold you should be able to have a simpler case and run it with two threshold and check how the SCEV that is printed changes
If possible, this test should directly test `-analyze -scalar-evolution`.

================
Comment at: test/Transforms/IndVarSimplify/pr18607.ll:89
@@ +88,3 @@
+  %tmp199 = mul i32 %tmp196, %tmp193
+  %tmp202 = mul i32 %tmp199, %tmp196
+  %tmp205 = mul i32 %tmp202, %tmp199
----------------
Not sure that you even need a loop -- just a sequence of multiplications should do the trick.


https://reviews.llvm.org/D3127