[llvm-commits] [llvm] r41747 - in /llvm/trunk: include/llvm/ lib/Analysis/ lib/AsmParser/ lib/Bitcode/Reader/ lib/Bitcode/Writer/ lib/CodeGen/ lib/CodeGen/SelectionDAG/ lib/ExecutionEngine/ lib/ExecutionEngine/JIT/ lib/Target/CBackend/ lib/Target/MSIL/ lib/Target/X86/ lib/Transforms/IPO/ lib/Transforms/Scalar/ lib/VMCore/ tools/llvm-upgrade/ tools/llvm2cpp/

Dale Johannesen dalej at apple.com
Fri Sep 7 12:58:03 PDT 2007


On Sep 7, 2007, at 11:47 AM, Chris Lattner wrote:

>> Use APFloat in UpgradeParser and AsmParser.
>
> Cool.
>
>> +++ llvm/trunk/include/llvm/Constants.h Thu Sep  6 13:13:44 2007
>> @@ -217,29 +217,13 @@
>>    APFloat Val;
>>    ConstantFP(const ConstantFP &);      // DO NOT IMPLEMENT
>>  protected:
>>    ConstantFP(const Type *Ty, const APFloat& V);
>>  public:
>>    /// get() - Static factory methods - Return objects of the  
>> specified value
>>    static ConstantFP *get(const Type *Ty, const APFloat& V);
>
> It seems that this method could drop the type argument, infering  
> the type from the format of the APFloat.  This might simplify some  
> clients, and avoids problem with a type is passed in that doesn't  
> match the APFloat.

Eventually, yes, this is on my list (and there are other methods like  
this).  One of the things happening is that we're moving from double 
+type, passed around separately, to APFloat.   This means some of the  
float vs double handling is moved out of the basic FP classes into  
their callers (there are a repulsively large number of examples in  
the patch, which I hope to improve) and bugs creep in when you do  
that.  Short term it has been quite useful to pass the Ty separately  
as an error check.

This transition is way too big to do all at once and some of the  
intermediate phases are going to be ugly.

>> ===================================================================== 
>> =========
>> --- llvm/trunk/lib/AsmParser/llvmAsmParser.y (original)
>> +++ llvm/trunk/lib/AsmParser/llvmAsmParser.y Thu Sep  6 13:13:44 2007
>> @@ -1862,9 +1866,13 @@
>>      CHECK_FOR_ERROR
>>    }
>>    | FPType FPVAL {                   // Float & Double constants
>> -    if (!ConstantFP::isValueValidForType($1, $2))
>> +    if (!ConstantFP::isValueValidForType($1, *$2))
>>        GEN_ERROR("Floating point constant invalid for type");
>> -    $$ = ConstantFP::get($1, $2);
>> +    // Lexer has no type info, so builds all FP constants as double.
>> +    // Fix this here.
>> +    if ($1==Type::FloatTy)
>> +      $2->convert(APFloat::IEEEsingle,  
>> APFloat::rmNearestTiesToEven);
>> +    $$ = ConstantFP::get($1, *$2);
>>      CHECK_FOR_ERROR
>>    };
>
> Because the APFloats are dynamically allocated, every production  
> that takes an FPVal has to be very careful to delete the FPVal  
> after it converts it to something else to avoid a memory leak.   
> Here, "delete $2;" should be sufficient after the ConstantFP is  
> created.

OK.

> One other thing.  Right now there is this code in the CBE:
>
>   // On X86, set the FP control word to 64-bits of precision  
> instead of 80 bits.
>   Out << "#if defined(__GNUC__) && !defined(__llvm__)\n"
> ...
>
> and this code in the X86 backend:   
> X86DAGToDAGISel::EmitSpecialCodeForMain
>
> Both of them should just be zapped.  It was a poor band-aid to  
> support 64-bit doubles with fewer precision problems, but obviously  
> won't work when we support long double!  In any case, they should  
> just be nuked now.

OK.  I was aware of this, but it will probably produce some  
slowdown.  I guess now is as good a time as any.




More information about the llvm-commits mailing list