[llvm-commits] [llvm] r145563 - in /llvm/trunk: lib/Transforms/InstCombine/InstCombineCompares.cpp test/Transforms/InstCombine/compare-abs-nonzero.ll

[Peter Cooper]

Hi Duncan

Thanks for the comments.  I'd thought the call to getComplexity would have sorted the inputs so i only had to check the one case but i was wrong.

Fixes to all the issues are r145618.  {} have been left in on the if statement, but only because of the nested if and the following else.

Thanks,
Pete

On Dec 1, 2011, at 1:03 AM, Duncan Sands wrote:

> Hi Peter,
> 
>> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp (original)
>> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp Wed Nov 30 21:58:40 2011
>> @@ -1795,6 +1795,20 @@
>>    if (Value *V = SimplifyICmpInst(I.getPredicate(), Op0, Op1, TD))
>>      return ReplaceInstUsesWith(I, V);
>> 
>> +  // comparing -val or val with non-zero is the same as just comparing val
>> +  // ie, (val != 0) == (-val != 0)
> 
> this comment made me think you were going to replace "(-val) != 0" with
> "val != 0" as a general optimization.  It makes no mention of "abs" or select.
> Can you please change it to explain what you are really up to.
> 
>> +  if (I.getPredicate() == ICmpInst::ICMP_NE&&  match(Op1, m_Zero()))
>> +  {
>> +    Value *Cond, *SubSrc, *SelectFalse;
>> +    if (match(Op0, m_Select(m_Value(Cond), m_Sub(m_Zero(), m_Value(SubSrc)),
> 
> Instcombine has a special dyn_castNegVal for checking whether something is
> "-val". Also, this logic seems too special: why should the "-val" be on the
> false branch?  If it is the true branch your transform will fail even though
> it would still be correct.
> 
>> +                            m_Value(SelectFalse)))) {
>> +      if (SubSrc == SelectFalse) {
>> +        return CmpInst::Create(Instruction::ICmp, I.getPredicate(),
>> +                               SubSrc, Op1);
>> +      }
> 
> Style: no curly brackets when there is only one statement.
> 
>> +    }
>> +  }
>> +
> 
> Ciao, Duncan.
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits