Skip to content

Commit

Permalink
Fix correctness issues in sizeof tfunc
Browse files Browse the repository at this point in the history
The `Core.sizeof` function can take either a value or a type,
which can be a bit confusing in the tfunc, because the tfunc
operates on the types of the values passed to the builtin.
In particular, we were incorrectly returning Const(16) (on 64bit)
for sizeof_tfunc(UnionAll), because it was treating it the asme
as `sizeof_tfunc(Const(UnionAll))`. Try to clean that up as well
as fixing a similar confusion in the nothrow version of this
function. Lastly, we had a similar fast path in codegen, which
would try to inline the actual size for a constant datatype argument.
For codegen, just rm that whole code path, since it should have no
more information than inference and figuring it out in inference
exposes strictly more optimization opportunities.

Fixes #36710
  • Loading branch information
Keno committed Jul 18, 2020
1 parent ad4b1ec commit bfea720
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 23 deletions.
36 changes: 31 additions & 5 deletions base/compiler/tfuncs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -302,16 +302,30 @@ function sizeof_nothrow(@nospecialize(x))
end
elseif isa(x, Conditional)
return true
else
x = widenconst(x)
end
if isa(x, Union)
return sizeof_nothrow(x.a) && sizeof_nothrow(x.b)
end
isconstType(x) && (x = x.parameters[1]) # since sizeof(typeof(x)) == sizeof(x)
x === DataType && return false
t, exact = instanceof_tfunc(x)
if !exact
# Could always be bottom at runtime, which throws
return false
end
if t !== Bottom
# The value corresponding to `x` at runtime could be a type.
# Normalize the query to ask about that type.
t === DataType && return true
x = t
if isa(x, Union)
return sizeof_nothrow(x.a) && sizeof_nothrow(x.b)
end
else
x = widenconst(x)
x === DataType && return false
end
return isconcretetype(x) || isprimitivetype(x)
end

function _const_sizeof(@nospecialize(x))
# Constant Vector does not have constant size
isa(x, Vector) && return Int
Expand All @@ -330,10 +344,22 @@ function sizeof_tfunc(@nospecialize(x),)
isa(x, Const) && return _const_sizeof(x.val)
isa(x, Conditional) && return _const_sizeof(Bool)
isconstType(x) && return _const_sizeof(x.parameters[1])
x = widenconst(x)
if isa(x, Union)
return tmerge(sizeof_tfunc(x.a), sizeof_tfunc(x.b))
end
# Core.sizeof operates on either a type or a value. First check which
# case we're in.
t, exact = instanceof_tfunc(x)
if t !== Bottom
# The value corresponding to `x` at runtime could be a type.
# Normalize the query to ask about that type.
x = t
if isa(x, Union)
return sizeof_nothrow(x.a) && sizeof_nothrow(x.b)
end
else
x = widenconst(x)
end
x !== DataType && isconcretetype(x) && return _const_sizeof(x)
isprimitivetype(x) && return _const_sizeof(x)
return Int
Expand Down
15 changes: 0 additions & 15 deletions src/codegen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3039,21 +3039,6 @@ static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f,
*ret = mark_julia_type(ctx, ctx.builder.CreateMul(len, elsize), false, jl_long_type);
return true;
}
if (jl_is_type_type((jl_value_t*)sty) && !jl_is_typevar(jl_tparam0(sty))) {
sty = (jl_datatype_t*)jl_tparam0(sty);
}
if (jl_is_datatype(sty) && sty != jl_symbol_type &&
sty->name != jl_array_typename &&
sty != jl_simplevector_type && sty != jl_string_type &&
// exclude DataType, since each DataType has its own size, not sizeof(DataType).
// this is issue #8798
sty != jl_datatype_type) {
if (jl_is_concrete_type((jl_value_t*)sty) ||
(jl_field_names(sty) == jl_emptysvec && jl_datatype_size(sty) > 0)) {
*ret = mark_julia_type(ctx, ConstantInt::get(T_size, jl_datatype_size(sty)), false, jl_long_type);
return true;
}
}
}

else if (f == jl_builtin_apply_type && nargs > 0) {
Expand Down
11 changes: 8 additions & 3 deletions test/compiler/inference.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1400,11 +1400,11 @@ egal_conditional_lattice3(x, y) = x === y + y ? "" : 1

using Core.Compiler: PartialStruct, nfields_tfunc, sizeof_tfunc, sizeof_nothrow
@test sizeof_tfunc(Const(Ptr)) === sizeof_tfunc(Union{Ptr, Int, Type{Ptr{Int8}}, Type{Int}}) === Const(Sys.WORD_SIZE ÷ 8)
@test sizeof_tfunc(Type{Ptr}) === Int
@test sizeof_tfunc(Type{Ptr}) === Const(sizeof(Ptr))
@test sizeof_nothrow(Union{Ptr, Int, Type{Ptr{Int8}}, Type{Int}})
@test sizeof_nothrow(Const(Ptr))
@test !sizeof_nothrow(Type{Ptr})
@test !sizeof_nothrow(Type{Union{Ptr{Int}, Int}})
@test sizeof_nothrow(Type{Ptr})
@test sizeof_nothrow(Type{Union{Ptr{Int}, Int}})
@test !sizeof_nothrow(Const(Tuple))
let PT = PartialStruct(Tuple{Int64,UInt64}, Any[Const(10, false), UInt64])
@test sizeof_tfunc(PT) === Const(16)
Expand Down Expand Up @@ -2734,3 +2734,8 @@ end

f_generator_splat(t::Tuple) = tuple((identity(l) for l in t)...)
@test Base.return_types(f_generator_splat, (Tuple{Symbol, Int64, Float64},)) == Any[Tuple{Symbol, Int64, Float64}]

# Issue #36710 - sizeof(::UnionAll) tfunc correctness
@test (sizeof(Ptr),) == sizeof.((Ptr,)) == sizeof.((Ptr{Cvoid},))
@test Core.Compiler.sizeof_tfunc(UnionAll) === Int
@test !Core.Compiler.sizeof_nothrow(UnionAll)

0 comments on commit bfea720

Please sign in to comment.