some small tweaks to the layout optimization PR

- replace inbounds with inline where intended
- rename jl_justbits
- fix compiler warnings about std::move
- guard jl_is_datatype_singleton query correctly
- add missing undef reference checks to arrayref and fieldref (and fix array isassigned)

Author: JeffBezanson

base/array.jl

@@ -177,7 +177,8 @@ false
 isbitsunion(u::Union) = allocatedinline(u)
 isbitsunion(x) = false
 
-@inbounds function _unsetindex!(A::Array{T}, i::Int) where {T}
+function _unsetindex!(A::Array{T}, i::Int) where {T}
+    @_inline_meta
     @boundscheck checkbounds(A, i)
     t = @_gc_preserve_begin A
     p = Ptr{Ptr{Cvoid}}(pointer(A, i))

src/array.c

@@ -563,7 +563,7 @@ JL_DLLEXPORT jl_value_t *jl_arrayref(jl_array_t *a, size_t i)
         if (jl_is_datatype_singleton((jl_datatype_t*)eltype))
             return ((jl_datatype_t*)eltype)->instance;
     }
-    return jl_new_bits(eltype, &((char*)a->data)[i * a->elsize]);
+    return undefref_check((jl_datatype_t*)eltype, jl_new_bits(eltype, &((char*)a->data)[i * a->elsize]));
 }
 
 JL_DLLEXPORT int jl_array_isassigned(jl_array_t *a, size_t i)
@@ -574,9 +574,8 @@ JL_DLLEXPORT int jl_array_isassigned(jl_array_t *a, size_t i)
     else if (a->flags.hasptr) {
          jl_datatype_t *eltype = (jl_datatype_t*)jl_tparam0(jl_typeof(a));
          assert(eltype->layout->first_ptr >= 0);
-         jl_value_t **slot =
-             (jl_value_t**)(&((char*)a->data)[i*a->elsize] + eltype->layout->first_ptr);
-         return *slot != NULL;
+         jl_value_t **elem = (jl_value_t**)((char*)a->data + i * a->elsize);
+         return elem[eltype->layout->first_ptr] != NULL;
     }
     return 1;
 }

src/builtins.c

@@ -760,10 +760,7 @@ JL_CALLABLE(jl_f_getfield)
         jl_sym_t *fld = (jl_sym_t*)args[1];
         idx = jl_field_index(st, fld, 1);
     }
-    jl_value_t *fval = jl_get_nth_field(v, idx);
-    if (fval == NULL)
-        jl_throw(jl_undefref_exception);
-    return fval;
+    return jl_get_nth_field_checked(v, idx);
 }
 
 JL_CALLABLE(jl_f_setfield)

src/ccall.cpp

@@ -1791,7 +1791,7 @@ jl_cgval_t function_sig_t::emit_a_ccall(
     bool sretboxed = false;
     if (sret) {
         assert(!retboxed && jl_is_datatype(rt) && "sret return type invalid");
-        if (jl_justbits(rt, true)) {
+        if (jl_is_pointerfree(rt)) {
             result = emit_static_alloca(ctx, lrt);
             argvals[0] = ctx.builder.CreateBitCast(result, fargt_sig.at(0));
         }

src/cgutils.cpp

@@ -533,7 +533,7 @@ JL_DLLEXPORT Type *julia_type_to_llvm(jl_value_t *jt, bool *isboxed)
     if (isboxed) *isboxed = false;
     if (jt == (jl_value_t*)jl_bottom_type)
         return T_void;
-    if (jl_justbits(jt)) {
+    if (jl_is_concrete_immutable(jt)) {
         if (jl_datatype_nbits(jt) == 0)
             return T_void;
         Type *t = julia_struct_to_llvm(jt, NULL, isboxed);
@@ -768,7 +768,7 @@ static bool for_each_uniontype_small(
         allunbox &= for_each_uniontype_small(f, ((jl_uniontype_t*)ty)->b, counter);
         return allunbox;
     }
-    else if (jl_justbits(ty, true)) {
+    else if (jl_is_pointerfree(ty)) {
         f(++counter, (jl_datatype_t*)ty);
         return true;
     }
@@ -1292,12 +1292,12 @@ std::vector<unsigned> first_ptr(Type *T)
         unsigned i = 0;
         for (Type *ElTy : T->subtypes()) {
             if (isa<PointerType>(ElTy) && ElTy->getPointerAddressSpace() == AddressSpace::Tracked) {
-                return std::move(std::vector<unsigned>{i});
+                return std::vector<unsigned>{i};
             }
             auto path = first_ptr(ElTy);
             if (!path.empty()) {
                 path.push_back(i);
-                return std::move(path);
+                return path;
             }
             i++;
         }
@@ -1529,7 +1529,7 @@ static bool emit_getfield_unknownidx(jl_codectx_t &ctx,
     assert(!jl_is_vecelement_type((jl_value_t*)stt));
 
     if (!strct.ispointer()) { // unboxed
-        assert(jl_justbits((jl_value_t*)stt));
+        assert(jl_is_concrete_immutable((jl_value_t*)stt));
         bool isboxed = is_datatype_all_pointers(stt);
         bool issame = is_tupletype_homogeneous(stt->types);
         if (issame) {
@@ -2415,7 +2415,7 @@ static Value *boxed(jl_codectx_t &ctx, const jl_cgval_t &vinfo)
     }
     else {
         assert(vinfo.V && "Missing data for unboxed value.");
-        assert(jl_justbits(jt) && "This type shouldn't have been unboxed.");
+        assert(jl_is_concrete_immutable(jt) && "This type shouldn't have been unboxed.");
         Type *t = julia_type_to_llvm(jt);
         assert(!type_is_ghost(t)); // ghost values should have been handled by vinfo.constant above!
         box = _boxed_special(ctx, vinfo, t);
@@ -2438,8 +2438,8 @@ static void emit_unionmove(jl_codectx_t &ctx, Value *dest, MDNode *tbaa_dst, con
     if (jl_is_concrete_type(src.typ) || src.constant) {
         jl_value_t *typ = src.constant ? jl_typeof(src.constant) : src.typ;
         Type *store_ty = julia_type_to_llvm(typ);
-        assert(skip || jl_justbits(typ,  true));
-        if (jl_justbits(typ, true)) {
+        assert(skip || jl_is_pointerfree(typ));
+        if (jl_is_pointerfree(typ)) {
             if (!src.ispointer() || src.constant) {
                 emit_unbox(ctx, store_ty, src, typ, dest, tbaa_dst, isVolatile);
             }

src/codegen.cpp

@@ -366,23 +366,24 @@ static MDNode *best_tbaa(jl_value_t *jt) {
 
 // tracks whether codegen is currently able to simply stack-allocate this type
 // note that this includes jl_isbits, although codegen should work regardless
-static bool jl_justbits(jl_value_t* t, bool pointerfree=false)
+static bool jl_is_concrete_immutable(jl_value_t* t)
+{
+    return jl_is_immutable_datatype(t) && ((jl_datatype_t*)t)->layout;
+}
+
+static bool jl_is_pointerfree(jl_value_t* t)
 {
     if (!jl_is_immutable_datatype(t))
         return 0;
     const jl_datatype_layout_t *layout = ((jl_datatype_t*)t)->layout;
-    if (!layout)
-        return 0;
-    if (pointerfree && layout->npointers != 0)
-        return 0;
-    return 1;
+    return layout && layout->npointers == 0;
 }
 
 // these queries are usually related, but we split them out here
 // for convenience and clarity (and because it changes the calling convention)
 static bool deserves_stack(jl_value_t* t, bool pointerfree=false)
 {
-    if (!jl_justbits(t))
+    if (!jl_is_concrete_immutable(t))
         return false;
     return ((jl_datatype_t*)t)->isinlinealloc;
 }
@@ -753,7 +754,7 @@ static inline jl_cgval_t update_julia_type(jl_codectx_t &ctx, const jl_cgval_t &
         if (jl_is_datatype(utyp)) {
             bool alwaysboxed;
             if (jl_is_concrete_type(utyp))
-                alwaysboxed = !jl_justbits(utyp, true);
+                alwaysboxed = !jl_is_pointerfree(utyp);
             else
                 alwaysboxed = !((jl_datatype_t*)utyp)->abstract && ((jl_datatype_t*)utyp)->mutabl;
             if (alwaysboxed) {
@@ -1012,7 +1013,7 @@ static jl_cgval_t convert_julia_type(jl_codectx_t &ctx, const jl_cgval_t &v, jl_
         return ghostValue(typ);
     Value *new_tindex = NULL;
     if (jl_is_concrete_type(typ)) {
-        if (v.TIndex && !jl_justbits(typ, true)) {
+        if (v.TIndex && !jl_is_pointerfree(typ)) {
             // discovered that this union-split type must actually be isboxed
             if (v.Vboxed) {
                 return jl_cgval_t(v.Vboxed, nullptr, true, typ, NULL);
@@ -2480,8 +2481,8 @@ static Value *emit_f_is(jl_codectx_t &ctx, const jl_cgval_t &arg1, const jl_cgva
     if (jl_type_intersection(rt1, rt2) == (jl_value_t*)jl_bottom_type) // types are disjoint (exhaustive test)
         return ConstantInt::get(T_int1, 0);
 
-    bool justbits1 = jl_justbits(rt1);
-    bool justbits2 = jl_justbits(rt2);
+    bool justbits1 = jl_is_concrete_immutable(rt1);
+    bool justbits2 = jl_is_concrete_immutable(rt2);
     if (justbits1 || justbits2) { // whether this type is unique'd by value
         jl_value_t *typ = justbits1 ? rt1 : rt2;
         if (rt1 == rt2)
@@ -4110,7 +4111,8 @@ static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr, ssize_t ssaval)
         if (var == getfield_undefref_sym) {
             raise_exception_unless(ctx, cond,
                 literal_pointer_val(ctx, jl_undefref_exception));
-        } else {
+        }
+        else {
             undef_var_error_ifnot(ctx, cond, var);
         }
         return ghostValue(jl_void_type);
@@ -4661,7 +4663,7 @@ static Function* gen_cfun_wrapper(
                         ctx.builder.CreateLoad(emit_bitcast(ctx, val, T_pprjlvalue)),
                         true, jl_any_type);
             }
-            else if (static_at && jl_justbits(jargty)) { // anything that could be stored unboxed
+            else if (static_at && jl_is_concrete_immutable(jargty)) { // anything that could be stored unboxed
                 bool isboxed;
                 Type *T = julia_type_to_llvm(jargty, &isboxed);
                 assert(!isboxed);
@@ -5398,7 +5400,7 @@ static bool uses_specsig(jl_value_t *sig, size_t nreq, jl_value_t *rettype, bool
     bool allSingleton = true;
     for (size_t i = 0; i < jl_nparams(sig); i++) {
         jl_value_t *sigt = jl_tparam(sig, i);
-        bool issing = jl_is_datatype_singleton((jl_datatype_t*)sigt);
+        bool issing = jl_is_datatype(sigt) && jl_is_datatype_singleton((jl_datatype_t*)sigt);
         allSingleton &= issing;
         if (!deserves_argbox(sigt) && !issing) {
             return true;
@@ -5948,7 +5950,7 @@ static std::unique_ptr<Module> emit_function(
         }
         else if (varinfo.isArgument && !(specsig && i == (size_t)ctx.vaSlot)) {
             // if we can unbox it, just use the input pointer
-            if (i != (size_t)ctx.vaSlot && jl_justbits(jt))
+            if (i != (size_t)ctx.vaSlot && jl_is_concrete_immutable(jt))
                 return;
         }
         else if (jl_is_uniontype(jt)) {

src/datatype.c

@@ -272,7 +272,7 @@ JL_DLLEXPORT int jl_stored_inline(jl_value_t *eltype) JL_NOTSAFEPOINT
     return jl_islayout_inline(eltype, &fsz, &al);
 }
 
-// whether this type is unique'd by pointer
+// whether instances of this type can use pointer comparison for `===`
 int jl_pointer_egal(jl_value_t *t)
 {
     if (t == (jl_value_t*)jl_any_type)
@@ -1059,7 +1059,7 @@ JL_DLLEXPORT jl_value_t *jl_get_nth_field_checked(jl_value_t *v, size_t i)
     size_t offs = jl_field_offset(st, i);
     if (jl_field_isptr(st, i)) {
         jl_value_t *fval = *(jl_value_t**)((char*)v + offs);
-        if (fval == NULL)
+        if (__unlikely(fval == NULL))
             jl_throw(jl_undefref_exception);
         return fval;
     }
@@ -1071,7 +1071,7 @@ JL_DLLEXPORT jl_value_t *jl_get_nth_field_checked(jl_value_t *v, size_t i)
         if (jl_is_datatype_singleton((jl_datatype_t*)ty))
             return ((jl_datatype_t*)ty)->instance;
     }
-    return jl_new_bits(ty, (char*)v + offs);
+    return undefref_check((jl_datatype_t*)ty, jl_new_bits(ty, (char*)v + offs));
 }
 
 JL_DLLEXPORT void jl_set_nth_field(jl_value_t *v, size_t i, jl_value_t *rhs) JL_NOTSAFEPOINT

src/gc.c

@@ -1713,7 +1713,7 @@ JL_DLLEXPORT int jl_gc_mark_queue_obj(jl_ptls_t ptls, jl_value_t *obj)
 JL_DLLEXPORT void jl_gc_mark_queue_objarray(jl_ptls_t ptls, jl_value_t *parent,
                                             jl_value_t **objs, size_t nobjs)
 {
-    gc_mark_objarray_t data = { parent, objs, objs + nobjs,
+    gc_mark_objarray_t data = { parent, objs, objs + nobjs, 1,
                                 jl_astaggedvalue(parent)->bits.gc & 2 };
     gc_mark_stack_push(&ptls->gc_cache, &ptls->gc_mark_sp,
                        gc_mark_label_addrs[GC_MARK_L_objarray],

src/intrinsics.cpp

@@ -149,7 +149,7 @@ static Value *uint_cnvt(jl_codectx_t &ctx, Type *to, Value *x)
 
 static Constant *julia_const_to_llvm(const void *ptr, jl_datatype_t *bt)
 {
-    // assumes `jl_justbits(bt, true)`.
+    // assumes `jl_is_pointerfree(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.
@@ -270,7 +270,7 @@ static Constant *julia_const_to_llvm(jl_value_t *e)
     if (e == jl_false)
         return ConstantInt::get(T_int8, 0);
     jl_value_t *bt = jl_typeof(e);
-    if (!jl_justbits(bt, true))
+    if (!jl_is_pointerfree(bt))
         return NULL;
     return julia_const_to_llvm(e, (jl_datatype_t*)bt);
 }

src/julia_internal.h

@@ -352,6 +352,17 @@ void jl_set_t_uid_ctr(int i);
 uint32_t jl_get_gs_ctr(void);
 void jl_set_gs_ctr(uint32_t ctr);
 
+STATIC_INLINE jl_value_t *undefref_check(jl_datatype_t *dt, jl_value_t *v) JL_NOTSAFEPOINT
+{
+     if (dt->layout->first_ptr >= 0) {
+        jl_value_t *nullp = ((jl_value_t**)v)[dt->layout->first_ptr];
+        if (__unlikely(nullp == NULL))
+            jl_throw(jl_undefref_exception);
+    }
+    return v;
+}
+
+
 // -- functions -- //
 
 jl_code_info_t *jl_type_infer(jl_method_instance_t *li, size_t world, int force);