[llvm-commits] [llvm] r60042 - in /llvm/trunk: lib/Support/APFloat.cpp test/Transforms/IndVarsSimplify/2008-11-25-APFloatAssert.ll

[Dale Johannesen]

On Nov 25, 2008, at 5:14 PMPST, Evan Cheng wrote:

>
> On Nov 25, 2008, at 5:08 PM, Dale Johannesen wrote:
>
>> On Nov 25, 2008, at 4:55 PMPST, Evan Cheng wrote:
>>> On Nov 25, 2008, at 11:14 AM, Chris Lattner wrote:
>>>> On Nov 25, 2008, at 11:00 AM, Evan Cheng wrote:
>>>>
>>>>> Author: evancheng
>>>>> Date: Tue Nov 25 13:00:29 2008
>>>>> New Revision: 60042
>>>>>
>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=60042&view=rev
>>>>> Log:
>>>>> convertToSignExtendedInteger should return opInvalidOp instead of
>>>>> asserting if sematics of float does not allow arithmetics.
>>>>
>>>> Are you sure?  This behavior was consistent with all the other
>>>> methods.  Usually the client has to check to see if it is hacking  
>>>> on
>>>> ppc long double and not do the optimization in question.
>>>
>>> The client is calling convertToInteger(). How would it know it's not
>>> possible to convert ppc long double? Seems like only APFloat has  
>>> this
>>> information.
>>
>> Chris is right, it's currently caller's responsibility to check this.
>> Search for PPCDoubleDouble, you'll see examples.
>> isArithmeticOK is a good idea though, that's better than what it does
>> now.  But many places you've only got an MVT to look at.
>
> I've reverted this and added code to check for PPCDoubleDouble. It's
> important to keep all methods consistent. However, since these methods
> return a APFloat::opStatus, I think we should rely on that instead of
> the explicit semantics check.

opStatus is intended to mimic IEEE754 exception codes.  We could add  
an auxiliary return value like losesInfo in convert.

However, even I recognize that what you suggest is much more appealing  
aesthetically, and ISTR I tried to do this and it was not as easy as  
you would expect, although I've forgotten why.

> Evan
>
>>
>>
>>> Evan
>>>
>>>>
>>>>
>>>> -Chris
>>>>
>>>>>
>>>>>
>>>>> Added:
>>>>> llvm/trunk/test/Transforms/IndVarsSimplify/2008-11-25-
>>>>> APFloatAssert.ll
>>>>> Modified:
>>>>> llvm/trunk/lib/Support/APFloat.cpp
>>>>>
>>>>> Modified: llvm/trunk/lib/Support/APFloat.cpp
>>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/APFloat.cpp?rev=60042&r1=60041&r2=60042&view=diff
>>>>>
>>>>> =
>>>>> =
>>>>> =
>>>>> =
>>>>> =
>>>>> =
>>>>> =
>>>>> =
>>>>> =
>>>>> =
>>>>> =
>>>>> = 
>>>>> ==================================================================
>>>>> --- llvm/trunk/lib/Support/APFloat.cpp (original)
>>>>> +++ llvm/trunk/lib/Support/APFloat.cpp Tue Nov 25 13:00:29 2008
>>>>> @@ -117,6 +117,11 @@
>>>>>        && "Compile-time arithmetic does not support these
>>>>> semantics");
>>>>> }
>>>>>
>>>>> +  static inline bool
>>>>> +  isArithmeticOk(const llvm::fltSemantics &semantics) {
>>>>> +    return semantics.arithmeticOK;
>>>>> +  }
>>>>> +
>>>>> /* Return the value of a decimal exponent of the form
>>>>>  [+-]ddddddd.
>>>>>
>>>>> @@ -1787,7 +1792,8 @@
>>>>> const integerPart *src;
>>>>> unsigned int dstPartsCount, truncatedBits;
>>>>>
>>>>> -  assertArithmeticOK(*semantics);
>>>>> +  if (!isArithmeticOk(*semantics))
>>>>> +    return opInvalidOp;
>>>>>
>>>>> *isExact = false;
>>>>>
>>>>>
>>>>> Added: llvm/trunk/test/Transforms/IndVarsSimplify/2008-11-25-
>>>>> APFloatAssert.ll
>>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/IndVarsSimplify/2008-11-25-APFloatAssert.ll?rev=60042&view=auto
>>>>>
>>>>> =
>>>>> =
>>>>> =
>>>>> =
>>>>> =
>>>>> =
>>>>> =
>>>>> =
>>>>> =
>>>>> =
>>>>> =
>>>>> = 
>>>>> ==================================================================
>>>>> --- llvm/trunk/test/Transforms/IndVarsSimplify/2008-11-25-
>>>>> APFloatAssert.ll (added)
>>>>> +++ llvm/trunk/test/Transforms/IndVarsSimplify/2008-11-25-
>>>>> APFloatAssert.ll Tue Nov 25 13:00:29 2008
>>>>> @@ -0,0 +1,11 @@
>>>>> +; RUN: llvm-as < %s | opt -indvars
>>>>> +
>>>>> +define void @t() nounwind {
>>>>> +entry:
>>>>> +	br label %bb23.i91
>>>>> +
>>>>> +bb23.i91:		; preds = %bb23.i91, %entry
>>>>> +	%result.0.i89 = phi ppc_fp128
>>>>> [ 0xM00000000000000000000000000000000, %entry ], [ %0,
>>>>> %bb23.i91 ]		; <ppc_fp128> [#uses=2]
>>>>> +	%0 = mul ppc_fp128 %result.0.i89, %result.0.i89		; <ppc_fp128>
>>>>> [#uses=1]
>>>>> +	br label %bb23.i91
>>>>> +}
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> llvm-commits mailing list
>>>>> llvm-commits at cs.uiuc.edu
>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits