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

[Gordon Henriksen]

This patch fixes several routines which deal with negation but failed  
to use the proper floating point negation formula, f(x) = -0.0 - x,  
for vectors. I simply factored out the logic to get the properly- 
signed "zero" value and extended it to get the correct floating-point  
vector.

In most cases, these local logic errors were harmless because callers  
operated exclusively on scalar integers, but in four cases there look  
to be potential problems or missed optimizations.

This patch fixes the dead m_Neg matcher that patch 3 commented out.


at lib/Transforms/Scalar/InstructionCombining.cpp:6788:

   NegVal = InsertNewInstBefore(
     BinaryOperator::createNeg(SubOp->getOperand(1), "tmp"), SI);

This can pass vectors to createNeg and looks like it would previous  
miscompile code by inserting a sub <+0.0, +0.0, etc.>, %x as it  
simplifies an expression. This patch should fix this case.


at lib/Target/CBackend/CBackend.cpp:2129

This is now http://llvm.org/PR1126.


at lib/Transforms/Scalar/InstructionCombining.cpp:325:

Please review this call site in getComplexity. It is clearly  
reachable with FP vector operands, and isNeg will now return true for  
vector negation, which is not a single machine instruction as fneg or  
integer negation are. Therefore, existing behavior might be  
preferable? If so, exclude vectors:

       - if (BinaryOperator::isNeg(V) || BinaryOperator::isNot(V))
       + if ((!isa<PackedType>(V->getType()) && BinaryOperator::isNeg 
(V)) ||
       +     BinaryOperator::isNot(V))


at lib/Transforms/Scalar/InstructionCombining.cpp:473:

static inline Value *dyn_castNegVal(Value *V) {
   if (BinaryOperator::isNeg(V))
     return BinaryOperator::getNegArgument(V);

This could clearly be invoked with FP vector operands. With this  
patch, the following transformations would be extended to FP vectors:

     -A + B   -->  B - A
     A + -B   -->  A - B
     A - (-B) -->  A + B
     -X * -Y  -->  X * Y

These would already have applied to scalar FP operations, so I have  
to assume that they're IEEE-approved.


Just one more!

— Gordon



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20070120/318b13bb/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pr970-4.patch
Type: application/octet-stream
Size: 7028 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20070120/318b13bb/attachment.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20070120/318b13bb/attachment-0001.html>