[llvm-commits] [llvm] r168711 - in /llvm/trunk: lib/Analysis/ScalarEvolution.cpp test/Transforms/IndVarSimplify/eliminate-comparison.ll

[Andrew Trick]

On Nov 29, 2012, at 8:00 AM, Benjamin Kramer <benny.kra at gmail.com> wrote:
>>> 
>>> Chris, can you approve this for 3.2? It fixes a miscompilation and is apparently a regression from 3.1.
>>> 
>>> - Ben
>>> 
>>>> 
>>>> Modified:
>>>>  llvm/trunk/lib/Analysis/ScalarEvolution.cpp
>>>>  llvm/trunk/test/Transforms/IndVarSimplify/eliminate-comparison.ll
>>>> 
>>>> Modified: llvm/trunk/lib/Analysis/ScalarEvolution.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ScalarEvolution.cpp?rev=168711&r1=168710&r2=168711&view=diff
>>>> ==============================================================================
>>>> --- llvm/trunk/lib/Analysis/ScalarEvolution.cpp (original)
>>>> +++ llvm/trunk/lib/Analysis/ScalarEvolution.cpp Tue Nov 27 12:16:32 2012
>>>> @@ -6149,9 +6149,10 @@
>>>> if (SimplifyICmpOperands(Pred, LHS, RHS))
>>>>   if (LHS == RHS)
>>>>     return CmpInst::isTrueWhenEqual(Pred);
>>>> -  if (SimplifyICmpOperands(FoundPred, FoundLHS, FoundRHS))
>>>> -    if (FoundLHS == FoundRHS)
>>>> -      return CmpInst::isFalseWhenEqual(Pred);
>>>> +
>>>> +  // Canonicalize the found cond too.  We can't conclude a result from the
>>>> +  // simplified values.
>>>> +  SimplifyICmpOperands(FoundPred, FoundLHS, FoundRHS);

Hi Ben,

Thanks for the fix. But why didn't you go for the more obvious fix?

-      return CmpInst::isFalseWhenEqual(Pred);
+      return CmpInst::isFalseWhenEqual(FoundPred);

As an example, invert the predicate in your test case:

 %tmp76 = icmp ne i32 %sext34, %sext21

In this case, I think the following compare should be eliminated, but won't be with your fix:

 %tmp46 = icmp slt i32 %__key8.0, 10

-Andy