[llvm-commits] [PATCH][FAST-MATH] recognize fp negation

[Shuxin Yang]

I personally think it is more natural to define isFNeg() as a member 
function of class BinaryOperator.
It is in many ways similar to BinaryOperator::isCompoundAssignmentOp(), 
.., BinaryOperator::isShiftAssignOp() etc.
How come it sounds odd to you.

The only thing that sounds bit odd to me is that the negation, by its 
very nature, is unary operator.
Therefore, isFNeg() dose not fit the name "BinaryOperator" very well. 
But it is only a matter of name,
not a big deal.


On 1/3/13 8:41 PM, Duncan Sands wrote:
> Hi Shuxin,
>
>>      Currently the compiler only considers "-0.0 - x" as a negation 
>> of quantity
>> x regardless the
>>   fast-math flags associated with the expr or the fast-math flags 
>> derived from
>> the supper-expr.
>>   This patch is to fix this problem.
>>
>>     With this change, "0.0 - x" will be considered as negation of x 
>> as well so
>> long as we don't care
>> "signed-zero".
>>
>>     It is about two weeks old. Unfortunately, as of I write this 
>> mail, I'm not
>> able to update the latest revision
>> and re-test it.
>
> how about getting rid of isFNeg from InstrTypes, and move users over 
> to the
> version in llvm/Support/PatternMatch.h instead?  I'm not sure it makes 
> much
> sense to have these pattern matching predicates in InstrTypes.h any 
> more -
> I suspect they are something of a historical relic nowadays.
>
> Ciao, Duncan.
>
> PS: Otherwise your patch looks fine, except for:
>
>> --- lib/VMCore/Instructions.cpp (revision 170838)
>> +++ lib/VMCore/Instructions.cpp    (working copy)
>> @@ -1928,11 +1928,11 @@
>>    return false;
>>  }
>>
>> -bool BinaryOperator::isFNeg(const Value *V) {
>> +bool BinaryOperator::isFNeg(const Value *V, bool IgnoreZeroSign) {
>>    if (const BinaryOperator *Bop = dyn_cast<BinaryOperator>(V))
>>      if (Bop->getOpcode() == Instruction::FSub)
>>        if (Constant* C = dyn_cast<Constant>(Bop->getOperand(0)))
>> -        return C->isNegativeZeroValue();
>> +        return !IgnoreZeroSign ? C->isNegativeZeroValue() : 
>> C->isZeroValue();
>
> If IgnoreZeroSign is true, don't you want to match both ordinary and 
> negative
> zero?  Right now it only matches ordinary zero.
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits