[compiler-rt] r250365 - builtins: Expand out floating point exponents for MSVC

[Stephen Canon via llvm-commits]

This is a bit ridiculous, and dangerous too.

Hexadecimal FP constants are an essential tool for numerical library work.  This change obfuscates the code.  The comments help somewhat, but there’s no way for a casual reader to see that the values are the same. They need to either take your word for it, or check each one.  The hex constants are clear and unambiguous.

Decimal-binary conversions are hard to get right, so this is also prone to introducing bugs (MSVC itself has been documented to have bugs in decimal-to-binary conversion routines, the very routines you’re depending on here: http://www.exploringbinary.com/incorrectly-rounded-conversions-in-visual-c-plus-plus/).  A bug introduced in that way would likely give people slightly wrong answers for a long time before it was noticed.

MSVC support is great, but at some point obfuscating code and risking bugs to support a compiler that so willfully refuses to support 25-year-old basic engineering tools is ridiculous.  If we really need to avoid hexadecimal FP to support MSVC (ugh), *please* specify their representation as hexadecimal integer literals rather than their value as decimal literals, to avoid any risk of decimal-to-binary conversion producing the wrong value with MSVC or some other compiler.

– Steve

> On Oct 15, 2015, at 12:26 AM, Saleem Abdulrasool via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> Author: compnerd
> Date: Wed Oct 14 23:26:19 2015
> New Revision: 250365
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=250365&view=rev
> Log:
> builtins: Expand out floating point exponents for MSVC
> 
> MSVC 2013 doesnt support C99 fully, including the hexidecimal floating point
> representation.  Use the expanded value to permit building with it.
> 
> Patch by Tee Hao Wei!
> 
> Modified:
>    compiler-rt/trunk/lib/builtins/floatdidf.c
>    compiler-rt/trunk/lib/builtins/floatundidf.c
> 
> Modified: compiler-rt/trunk/lib/builtins/floatdidf.c
> URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/builtins/floatdidf.c?rev=250365&r1=250364&r2=250365&view=diff
> ==============================================================================
> --- compiler-rt/trunk/lib/builtins/floatdidf.c (original)
> +++ compiler-rt/trunk/lib/builtins/floatdidf.c Wed Oct 14 23:26:19 2015
> @@ -32,8 +32,8 @@ ARM_EABI_FNALIAS(l2d, floatdidf)
> COMPILER_RT_ABI double
> __floatdidf(di_int a)
> {
> -	static const double twop52 = 0x1.0p52;
> -	static const double twop32 = 0x1.0p32;
> +	static const double twop52 = 4503599627370496.0; // 0x1.0p52
> +	static const double twop32 = 4294967296.0; // 0x1.0p32
> 	
> 	union { int64_t x; double d; } low = { .d = twop52 };
> 	
> 
> Modified: compiler-rt/trunk/lib/builtins/floatundidf.c
> URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/builtins/floatundidf.c?rev=250365&r1=250364&r2=250365&view=diff
> ==============================================================================
> --- compiler-rt/trunk/lib/builtins/floatundidf.c (original)
> +++ compiler-rt/trunk/lib/builtins/floatundidf.c Wed Oct 14 23:26:19 2015
> @@ -32,9 +32,9 @@ ARM_EABI_FNALIAS(ul2d, floatundidf)
> COMPILER_RT_ABI double
> __floatundidf(du_int a)
> {
> -	static const double twop52 = 0x1.0p52;
> -	static const double twop84 = 0x1.0p84;
> -	static const double twop84_plus_twop52 = 0x1.00000001p84;
> +	static const double twop52 = 4503599627370496.0; // 0x1.0p52
> +	static const double twop84 = 19342813113834066795298816.0; // 0x1.0p84
> +	static const double twop84_plus_twop52 = 19342813118337666422669312.0; // 0x1.00000001p84
> 	
> 	union { uint64_t x; double d; } high = { .d = twop84 };
> 	union { uint64_t x; double d; } low = { .d = twop52 };
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits