From 91044595e26424c3dc2416ffad94b4c70848cdeb Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Mon, 17 Apr 2017 19:01:27 -0400 Subject: [PATCH] correct the codegen of floating point subtypes avoids casting away floating point bits until used in a floating-point operation also ensures that alignment is consistent in the LLVM struct representation as in the Julia struct representation computation fix #21216 --- doc/src/manual/embedding.md | 10 +++++++--- examples/embedding/embedding.c | 14 ++++---------- src/abi_aarch64.cpp | 1 + src/abi_arm.cpp | 2 ++ src/cgutils.cpp | 17 ++++------------- src/intrinsics.cpp | 12 ------------ src/julia.h | 4 ---- test/core.jl | 30 ++++++++++++++++++++++++++++++ 8 files changed, 48 insertions(+), 42 deletions(-) diff --git a/doc/src/manual/embedding.md b/doc/src/manual/embedding.md index 7aa1d755ed9d8..14e8d65e79a2f 100644 --- a/doc/src/manual/embedding.md +++ b/doc/src/manual/embedding.md @@ -146,13 +146,17 @@ square root of 2 in Julia and reads back the result in C looks as follows: ``` jl_value_t *ret = jl_eval_string("sqrt(2.0)"); -if (jl_is_float64(ret)) { +if (jl_typeis(ret, jl_float64_type)) { double ret_unboxed = jl_unbox_float64(ret); printf("sqrt(2.0) in C: %e \n", ret_unboxed); } +else { + printf("ERROR: unexpected return type from sqrt(::Float64)\n"); +} ``` -In order to check whether `ret` is of a specific Julia type, we can use the `jl_is_...` functions. +In order to check whether `ret` is of a specific Julia type, we can use the +`jl_isa`, `jl_typeis`, or `jl_is_...` functions. By typing `typeof(sqrt(2.0))` into the Julia shell we can see that the return type is `Float64` (`double` in C). To convert the boxed Julia value into a C double the `jl_unbox_float64` function is used in the above code snippet. @@ -387,7 +391,7 @@ When writing Julia callable functions, it might be necessary to validate argumen to indicate errors. A typical type check looks like: ``` -if (!jl_is_float64(val)) { +if (!jl_typeis(val, jl_float64_type)) { jl_type_error(function_name, (jl_value_t*)jl_float64_type, val); } ``` diff --git a/examples/embedding/embedding.c b/examples/embedding/embedding.c index 3fc8c80d673ec..d0018e2e743c0 100644 --- a/examples/embedding/embedding.c +++ b/examples/embedding/embedding.c @@ -26,11 +26,8 @@ int main() // Accessing the return value jl_value_t *ret = jl_eval_string("sqrt(2.0)"); - - if (jl_is_float64(ret)) { - double retDouble = jl_unbox_float64(ret); - printf("sqrt(2.0) in C: %e\n", retDouble); - } + double retDouble = jl_unbox_float64(ret); + printf("sqrt(2.0) in C: %e\n", retDouble); } { @@ -39,11 +36,8 @@ int main() jl_function_t *func = jl_get_function(jl_base_module, "sqrt"); jl_value_t* argument = jl_box_float64(2.0); jl_value_t* ret = jl_call1(func, argument); - - if (jl_is_float64(ret)) { - double retDouble = jl_unbox_float64(ret); - printf("sqrt(2.0) in C: %e\n", retDouble); - } + double retDouble = jl_unbox_float64(ret); + printf("sqrt(2.0) in C: %e\n", retDouble); } { diff --git a/src/abi_aarch64.cpp b/src/abi_aarch64.cpp index 03938491cf64b..affdb93202001 100644 --- a/src/abi_aarch64.cpp +++ b/src/abi_aarch64.cpp @@ -58,6 +58,7 @@ Type *get_llvm_vectype(jl_datatype_t *dt) const return lltype; } +#define jl_is_floattype(v) jl_subtype(v,(jl_value_t*)jl_floatingpoint_type) Type *get_llvm_fptype(jl_datatype_t *dt) const { // Assume jl_is_datatype(dt) && !jl_is_abstracttype(dt) diff --git a/src/abi_arm.cpp b/src/abi_arm.cpp index feb0bbfedfe11..f2eb01d46745a 100644 --- a/src/abi_arm.cpp +++ b/src/abi_arm.cpp @@ -28,6 +28,8 @@ bool needPassByRef(jl_datatype_t *dt, AttrBuilder &ab) override return false; } +#define jl_is_floattype(v) jl_subtype(v,(jl_value_t*)jl_floatingpoint_type) + Type *get_llvm_fptype(jl_datatype_t *dt) const { // Assume jl_is_datatype(dt) && !jl_is_abstracttype(dt) diff --git a/src/cgutils.cpp b/src/cgutils.cpp index e61e82f9f9fae..1561f7883f1eb 100644 --- a/src/cgutils.cpp +++ b/src/cgutils.cpp @@ -402,6 +402,10 @@ static Type *bitstype_to_llvm(jl_value_t *bt) return T_int8; if (bt == (jl_value_t*)jl_long_type) return T_size; + if (bt == (jl_value_t*)jl_float32_type) + return T_float32; + if (bt == (jl_value_t*)jl_float64_type) + return T_float64; if (jl_is_cpointer_type(bt)) { Type *lt = julia_type_to_llvm(jl_tparam0(bt)); if (lt == T_void) @@ -409,19 +413,6 @@ static Type *bitstype_to_llvm(jl_value_t *bt) return PointerType::get(lt, 0); } int nb = jl_datatype_size(bt); - if (jl_is_floattype(bt)) { -#ifndef DISABLE_FLOAT16 - if (nb == 2) - return T_float16; - else -#endif - if (nb == 4) - return T_float32; - else if (nb == 8) - return T_float64; - else if (nb == 16) - return T_float128; - } return Type::getIntNTy(jl_LLVMContext, nb * 8); } diff --git a/src/intrinsics.cpp b/src/intrinsics.cpp index 7dd108e7bb4bd..95c8ed82c5dce 100644 --- a/src/intrinsics.cpp +++ b/src/intrinsics.cpp @@ -177,22 +177,14 @@ static Constant *julia_const_to_llvm(void *ptr, jl_value_t *bt) } case 2: { uint16_t data16 = *(uint16_t*)ptr; -#ifndef DISABLE_FLOAT16 - if (jl_is_floattype(bt)) - return ConstantFP::get(jl_LLVMContext, LLVM_FP(APFloat::IEEEhalf,APInt(16,data16))); -#endif return ConstantInt::get(T_int16, data16); } case 4: { uint32_t data32 = *(uint32_t*)ptr; - if (jl_is_floattype(bt)) - return ConstantFP::get(jl_LLVMContext, LLVM_FP(APFloat::IEEEsingle,APInt(32,data32))); return ConstantInt::get(T_int32, data32); } case 8: { uint64_t data64 = *(uint64_t*)ptr; - if (jl_is_floattype(bt)) - return ConstantFP::get(jl_LLVMContext, LLVM_FP(APFloat::IEEEdouble,APInt(64,data64))); return ConstantInt::get(T_int64, data64); } default: @@ -212,10 +204,6 @@ static Constant *julia_const_to_llvm(void *ptr, jl_value_t *bt) else #endif val = APInt(8*nb, ArrayRef(data, nw)); - if (nb == 16 && jl_is_floattype(bt)) { - return ConstantFP::get(jl_LLVMContext,LLVM_FP(APFloat::IEEEquad,val)); - // If we have a floating point type that's not hardware supported, just treat it like an integer for LLVM purposes - } return ConstantInt::get(IntegerType::get(jl_LLVMContext,8*nb),val); } } diff --git a/src/julia.h b/src/julia.h index f5ec511f337d9..d5488e2a8601c 100644 --- a/src/julia.h +++ b/src/julia.h @@ -847,10 +847,6 @@ static inline uint32_t jl_fielddesc_size(int8_t fielddesc_type) #define jl_is_uint16(v) jl_typeis(v,jl_uint16_type) #define jl_is_uint32(v) jl_typeis(v,jl_uint32_type) #define jl_is_uint64(v) jl_typeis(v,jl_uint64_type) -#define jl_is_float(v) jl_isa(v,(jl_value_t*)jl_floatingpoint_type) -#define jl_is_floattype(v) jl_subtype(v,(jl_value_t*)jl_floatingpoint_type) -#define jl_is_float32(v) jl_typeis(v,jl_float32_type) -#define jl_is_float64(v) jl_typeis(v,jl_float64_type) #define jl_is_bool(v) jl_typeis(v,jl_bool_type) #define jl_is_symbol(v) jl_typeis(v,jl_sym_type) #define jl_is_ssavalue(v) jl_typeis(v,jl_ssavalue_type) diff --git a/test/core.jl b/test/core.jl index 1ab108579bb9b..d5a5cc02967a6 100644 --- a/test/core.jl +++ b/test/core.jl @@ -4825,3 +4825,33 @@ f21271(x) = x::Tuple{Type{Int}, Type{Float64}} bar21397(x::T) where {T} = T foo21397(x) = bar21397(x) @test foo21397(Tuple) == DataType + +# issue 21216 +primitive type FP128test <: AbstractFloat 128 end +struct FP128align <: AbstractFloat + i::Int # cause forced misalignment (to word-size) + fp::FP128test +end +let ni128 = sizeof(FP128test) ÷ sizeof(Int), + ns128 = sizeof(FP128align) ÷ sizeof(Int), + nbit = sizeof(Int) * 8, + arr = reinterpret(FP128align, collect(Int, 1:(2 * ns128))), + little, + expected + @test sizeof(FP128test) == 16 + @test length(arr) == 2 + @test arr[1].i == 1 + @test arr[2].i == 1 + ns128 + expected = UInt128(0) + for little in ni128:-1:1 + little += 1 + expected = (expected << nbit) + little + end + @test arr[1].fp == reinterpret(FP128test, expected) + expected = UInt128(0) + for little in ni128:-1:1 + little += 1 + ns128 + expected = (expected << nbit) + little + end + @test reinterpret(UInt128, arr[2].fp) == expected +end