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

[Michael Ilseman]

On Jan 4, 2013, at 11:05 AM, Shuxin Yang <shuxin.llvm at gmail.com> wrote:

> 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.

I think Duncan was saying that all those methods feel a bit antiquated. In a separate patch, it might be good to remove these, but I'll let someone who has better historical context make that call. I don't know what's considered modern LLVM style for those checks, but I think it might be to use PatternMatch.h.

> 
> 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
>