Skip to content

Commit

Permalink
Make has_concrete_subtype a memoized property (#31064)
Browse files Browse the repository at this point in the history 
Fixes #31062
  • Loading branch information
@Keno
Keno authored Jun 8, 2019
1 parent 7038210 commit c053cf2
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 18 deletions.
7 changes: 7 additions & 0 deletions src/datatype.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ jl_datatype_t *jl_new_uninitialized_datatype(void)
t->isbitstype = 0;
t->zeroinit = 0;
t->isinlinealloc = 0;
t->has_concrete_subtype = 1;
t->layout = NULL;
t->names = NULL;
return t;
Expand Down Expand Up @@ -309,6 +310,12 @@ void jl_compute_field_offsets(jl_datatype_t *st)
st->isbitstype = jl_is_datatype(fld) && ((jl_datatype_t*)fld)->isbitstype;
if (!st->zeroinit)
st->zeroinit = (jl_is_datatype(fld) && ((jl_datatype_t*)fld)->isinlinealloc) ? ((jl_datatype_t*)fld)->zeroinit : 1;
if (i < st->ninitialized) {
if (fld == jl_bottom_type)
st->has_concrete_subtype = 0;
else
st->has_concrete_subtype &= !jl_is_datatype(fld) || ((jl_datatype_t *)fld)->has_concrete_subtype;
}
}
if (st->isbitstype) {
st->isinlinealloc = 1;
Expand Down
4 changes: 3 additions & 1 deletion src/dump.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,8 @@ static void jl_serialize_datatype(jl_serializer_state *s, jl_datatype_t *dt) JL_
| (dt->isdispatchtuple << 2)
| (dt->isbitstype << 3)
| (dt->zeroinit << 4)
| (dt->isinlinealloc << 5));
| (dt->isinlinealloc << 5)
| (dt->has_concrete_subtype << 6));
if (!dt->abstract) {
write_uint16(s->s, dt->ninitialized);
}
Expand Down Expand Up @@ -1403,6 +1404,7 @@ static jl_value_t *jl_deserialize_datatype(jl_serializer_state *s, int pos, jl_v
dt->isbitstype = (memflags >> 3) & 1;
dt->zeroinit = (memflags >> 4) & 1;
dt->isinlinealloc = (memflags >> 5) & 1;
dt->has_concrete_subtype = (memflags >> 6) & 1;
dt->types = NULL;
dt->parameters = NULL;
dt->name = NULL;
Expand Down
14 changes: 1 addition & 13 deletions src/gf.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2582,19 +2582,7 @@ int jl_has_concrete_subtype(jl_value_t *typ)
typ = jl_unwrap_vararg(typ);
if (!jl_is_datatype(typ))
return 1;
if (((jl_datatype_t*)typ)->name == jl_namedtuple_typename)
return jl_has_concrete_subtype(jl_tparam1(typ));
jl_svec_t *fields = jl_get_fieldtypes((jl_datatype_t*)typ);
size_t i, l = jl_svec_len(fields);
if (l != ((jl_datatype_t*)typ)->ninitialized)
if (((jl_datatype_t*)typ)->name != jl_tuple_typename)
return 1;
for (i = 0; i < l; i++) {
jl_value_t *ft = jl_svecref(fields, i);
if (!jl_has_concrete_subtype(ft))
return 0;
}
return 1;
return ((jl_datatype_t*)typ)->has_concrete_subtype;
}

// TODO: separate the codegen and typeinf locks
Expand Down
10 changes: 6 additions & 4 deletions src/jltypes.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1707,7 +1707,7 @@ void jl_init_types(void) JL_GC_DISABLED
jl_datatype_type->name->wrapper = (jl_value_t*)jl_datatype_type;
jl_datatype_type->super = (jl_datatype_t*)jl_type_type;
jl_datatype_type->parameters = jl_emptysvec;
jl_datatype_type->name->names = jl_perm_symsvec(20,
jl_datatype_type->name->names = jl_perm_symsvec(21,
"name",
"super",
"parameters",
Expand All @@ -1726,9 +1726,10 @@ void jl_init_types(void) JL_GC_DISABLED
"isbitstype",
"zeroinit",
"isinlinealloc",
"has_concrete_subtype",
"llvm::StructType",
"llvm::DIType");
jl_datatype_type->types = jl_svec(20,
jl_datatype_type->types = jl_svec(21,
jl_typename_type,
jl_datatype_type,
jl_simplevector_type,
Expand All @@ -1737,7 +1738,7 @@ void jl_init_types(void) JL_GC_DISABLED
jl_any_type, jl_any_type, jl_any_type, jl_any_type,
jl_any_type, jl_any_type, jl_any_type, jl_any_type,
jl_any_type, jl_any_type, jl_any_type, jl_any_type,
jl_any_type, jl_any_type);
jl_any_type, jl_any_type, jl_any_type);
jl_datatype_type->instance = NULL;
jl_datatype_type->uid = jl_assign_type_uid();
jl_datatype_type->struct_decl = NULL;
Expand Down Expand Up @@ -2293,8 +2294,9 @@ void jl_init_types(void) JL_GC_DISABLED
jl_svecset(jl_datatype_type->types, 15, jl_bool_type);
jl_svecset(jl_datatype_type->types, 16, jl_bool_type);
jl_svecset(jl_datatype_type->types, 17, jl_bool_type);
jl_svecset(jl_datatype_type->types, 18, jl_voidpointer_type);
jl_svecset(jl_datatype_type->types, 18, jl_bool_type);
jl_svecset(jl_datatype_type->types, 19, jl_voidpointer_type);
jl_svecset(jl_datatype_type->types, 20, jl_voidpointer_type);
jl_svecset(jl_typename_type->types, 1, jl_module_type);
jl_svecset(jl_typename_type->types, 6, jl_long_type);
jl_svecset(jl_typename_type->types, 3, jl_type_type);
Expand Down
1 change: 1 addition & 0 deletions src/julia.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,7 @@ typedef struct _jl_datatype_t {
uint8_t isbitstype; // relevant query for C-api and type-parameters
uint8_t zeroinit; // if one or more fields requires zero-initialization
uint8_t isinlinealloc; // if this is allocated inline
uint8_t has_concrete_subtype; // If clear, no value will have this datatype
void *struct_decl; //llvm::Type*
void *ditype; // llvm::MDNode* to be used as llvm::DIType(ditype)
} jl_datatype_t;
Expand Down
12 changes: 12 additions & 0 deletions test/core.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6976,3 +6976,15 @@ let spvec = sparse_t31649(zeros(Float64,5), Vector{Int64}())
@test convert(Any, nothing) === nothing
@test_throws MethodError repr(spvec)
end

# Issue #31062 - Accidental recursion in jl_has_concrete_subtype
struct Bar31062
x::NTuple{N, Bar31062} where N
end
struct Foo31062
x::Foo31062
end
# Use eval to make sure that this actually gets executed and not
# just constant folded by (future) over-eager compiler optimizations
@test isa(Core.eval(@__MODULE__, :(Bar31062(()))), Bar31062)
@test precompile(identity, (Foo31062,))

1 comment on commit c053cf2

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Executing the daily benchmark build, I will reply here when finished:

@nanosoldier runbenchmarks(ALL, isdaily = true)

Please sign in to comment.