[llvm-commits] [PATCH] PR970: isFloatingPoint audit 5 of 5

[Reid Spencer]

Hi Gordon,

Thanks for the patches. I've committed all 5 in your patch series. There
were only minor issues which I corrected:

     1. I just remove m_Neg pattern matcher since you commented it out
        and it appears to be dead in llvm.
     2. Your changes to CmpInst::CmpInst were not correct. We currently
        don't allow comparison of packed types in the assembler or the
        verifier. I changed the assertions to match the verifier. This
        wasn't your fault, it was left over cruft from a time when we
        though comparison of packed types was going to be implemented.
     3. Minor nit in ConstantExpr::getZeroValueForNegationExpr: the PTy
        variable should be moved into the if statement.

The rest looked good. Applied. Thanks, Gordon!

Reid.

P.S. If you're done with your review of FP use in LLVM, please close
PR970. Thanks.

On Sat, 2007-01-20 at 14:56 -0500, Gordon Henriksen wrote:
> This final patch simplifies the logic in SelectionDAGISel.cpp with no
> functional change. This construct was in use:
> 
> 
>     if (I.getType()->isFloatingPoint())
>       visitFPBinary(I, ISD::FADD, ISD::VADD); 
>     else
>       visitIntBinary(I, ISD::ADD, ISD::VADD); 
> 
> 
> Which confusingly sent even floating-point vector operations through
> the visitIntBinary routine. However, this was not a bug because
> visitFPBinary and visitIntBinary were identical (as is the vector
> opcode parameter).
> 
> 
> This patch streamlines the logic and eliminates redundant methods.
> 
> 
> — Gordon
> 
> 
> 
> 
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits