Skip to content

Commit

Permalink
correct the codegen of floating point subtypes
Browse files Browse the repository at this point in the history
ensures that alignment is consistent in the LLVM struct representation as in the Julia struct representation computation

also helps ensure that the floating point NaN payload bits won't accidentally get discarded for non-IEEE-754 subtypes of AbstractFloat

fix #21216

(cherry picked from commit ece60d0)
ref #21425

resolve an ambiguity from the AbstractFloat codegen test type
  • Loading branch information
vtjnash authored and tkelman committed May 3, 2017
1 parent 2a5dd2a commit 3cdb0b3
Show file tree
Hide file tree
Showing 7 changed files with 54 additions and 40 deletions.
14 changes: 4 additions & 10 deletions examples/embedding.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

{
Expand All @@ -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);
}

{
Expand Down
1 change: 1 addition & 0 deletions src/abi_aarch64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ static Type *get_llvm_vectype(jl_datatype_t *dt)
return lltype;
}

#define jl_is_floattype(v) jl_subtype(v,(jl_value_t*)jl_floatingpoint_type)
static Type *get_llvm_fptype(jl_datatype_t *dt)
{
// Assume jl_is_datatype(dt) && !jl_is_abstracttype(dt)
Expand Down
2 changes: 2 additions & 0 deletions src/abi_arm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ bool need_private_copy(jl_value_t *ty, bool byRef)
return false;
}

#define jl_is_floattype(v) jl_subtype(v,(jl_value_t*)jl_floatingpoint_type)

static Type *get_llvm_fptype(jl_datatype_t *dt)
{
// Assume jl_is_datatype(dt) && !jl_is_abstracttype(dt)
Expand Down
17 changes: 4 additions & 13 deletions src/cgutils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -331,20 +331,11 @@ JL_DLLEXPORT Type *julia_type_to_llvm(jl_value_t *jt, bool *isboxed)
if (jl_is_bitstype(jt)) {
if (jt == (jl_value_t*)jl_long_type)
return T_size;
if (jt == (jl_value_t*)jl_float32_type)
return T_float32;
if (jt == (jl_value_t*)jl_float64_type)
return T_float64;
int nb = jl_datatype_size(jt);
if (jl_is_floattype(jt)) {
#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);
}
if (jl_isbits(jt)) {
Expand Down
24 changes: 11 additions & 13 deletions src/intrinsics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,10 @@ static Value *uint_cnvt(Type *to, Value *x)
#define LLVM_FP(a,b) APFloat(a,b)
static Constant *julia_const_to_llvm(void *ptr, jl_value_t *bt)
{
// assume `jl_isbits(bt)`.
// assumes `jl_isbits(bt)`.
// `ptr` can point to a inline field, do not read the tag from it.
// make sure to return exactly the type specified by
// julia_type_to_llvm as this will be assumed by the callee.
if (bt == (jl_value_t*)jl_bool_type)
return ConstantInt::get(T_int8, (*(uint8_t*)ptr) ? 1 : 0);

Expand All @@ -180,22 +182,22 @@ 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)));
if (bt == (jl_value_t*)jl_float32_type)
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)));
if (bt == (jl_value_t*)jl_float64_type)
return ConstantFP::get(jl_LLVMContext,
LLVM_FP(APFloat::IEEEdouble,
APInt(64, data64)));
return ConstantInt::get(T_int64, data64);
}
default:
Expand All @@ -215,10 +217,6 @@ static Constant *julia_const_to_llvm(void *ptr, jl_value_t *bt)
else
#endif
val = APInt(8*nb, ArrayRef<uint64_t>(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);
}
}
Expand Down
4 changes: 0 additions & 4 deletions src/julia.h
Original file line number Diff line number Diff line change
Expand Up @@ -854,10 +854,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_subtype(v,(jl_value_t*)jl_floatingpoint_type,1)
#define jl_is_floattype(v) jl_subtype(v,(jl_value_t*)jl_floatingpoint_type,0)
#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)
Expand Down
32 changes: 32 additions & 0 deletions test/core.jl
Original file line number Diff line number Diff line change
Expand Up @@ -4628,3 +4628,35 @@ ccall(:jl_free, Void, (Ptr{Void}, ), p15240)

# issue #19963
ccall(:jl_free, Void, (Ptr{Void}, ), C_NULL)

# issue 21216
bitstype 128 FP128test <: AbstractFloat
immutable FP128align <: AbstractFloat
i::Int # cause forced misalignment
fp::FP128test
end
FP128align(x, r::RoundingMode) = nothing # for ambiguity test

This comment has been minimized.

Copy link
@tkelman

tkelman May 3, 2017

Contributor

reminder to self, this is missing x::Real to actually fix the ambiguity

let ni128 = sizeof(FP128test) ÷ sizeof(Int),
ns128 = sizeof(FP128align) ÷ sizeof(Int),
nbit = sizeof(Int) * 8,
arr = reinterpret(FP128align, collect(Int, 1:(2 * ns128))),
offset = Base.datatype_alignment(FP128test) ÷ sizeof(Int),
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 += offset
expected = (expected << nbit) + little
end
@test arr[1].fp == reinterpret(FP128test, expected)
expected = UInt128(0)
for little in ni128:-1:1
little += offset + ns128
expected = (expected << nbit) + little
end
@test reinterpret(UInt128, arr[2].fp) == expected
end

0 comments on commit 3cdb0b3

Please sign in to comment.