correctly handle union-store-splitting in "is" and "object_id"

Author: vtjnash

src/builtins.c

@@ -77,30 +77,40 @@ static int NOINLINE compare_svec(jl_svec_t *a, jl_svec_t *b)
 // See comment above for an explanation of NOINLINE.
 static int NOINLINE compare_fields(jl_value_t *a, jl_value_t *b, jl_datatype_t *dt)
 {
-    size_t nf = jl_datatype_nfields(dt);
-    for (size_t f=0; f < nf; f++) {
+    size_t f, nf = jl_datatype_nfields(dt);
+    for (f = 0; f < nf; f++) {
         size_t offs = jl_field_offset(dt, f);
         char *ao = (char*)jl_data_ptr(a) + offs;
         char *bo = (char*)jl_data_ptr(b) + offs;
-        int eq;
         if (jl_field_isptr(dt, f)) {
             jl_value_t *af = *(jl_value_t**)ao;
             jl_value_t *bf = *(jl_value_t**)bo;
-            if (af == bf) eq = 1;
-            else if (af==NULL || bf==NULL) eq = 0;
-            else eq = jl_egal(af, bf);
+            if (af != bf) {
+                if (af == NULL || bf == NULL)
+                    return 0;
+                if (!jl_egal(af, bf))
+                    return 0;
+            }
         }
         else {
             jl_datatype_t *ft = (jl_datatype_t*)jl_field_type(dt, f);
+            if (jl_is_uniontype(ft)) {
+                uint8_t asel = ((uint8_t*)ao)[jl_field_size(dt, f) - 1];
+                uint8_t bsel = ((uint8_t*)bo)[jl_field_size(dt, f) - 1];
+                if (asel != bsel)
+                    return 0;
+                ft = (jl_datatype_t*)jl_nth_union_component((jl_value_t*)ft, asel);
+            }
             if (!ft->layout->haspadding) {
-                eq = bits_equal(ao, bo, jl_field_size(dt, f));
+                if (!bits_equal(ao, bo, jl_field_size(dt, f)))
+                    return 0;
             }
             else {
                 assert(jl_datatype_nfields(ft) > 0);
-                eq = compare_fields((jl_value_t*)ao, (jl_value_t*)bo, ft);
+                if (!compare_fields((jl_value_t*)ao, (jl_value_t*)bo, ft))
+                    return 0;
             }
         }
-        if (!eq) return 0;
     }
     return 1;
 }
@@ -127,9 +137,11 @@ JL_DLLEXPORT int jl_egal(jl_value_t *a, jl_value_t *b)
             return 0;
         return !memcmp(jl_string_data(a), jl_string_data(b), l);
     }
-    if (dt->mutabl) return 0;
+    if (dt->mutabl)
+        return 0;
     size_t sz = jl_datatype_size(dt);
-    if (sz == 0) return 1;
+    if (sz == 0)
+        return 1;
     size_t nf = jl_datatype_nfields(dt);
     if (nf == 0)
         return bits_equal(jl_data_ptr(a), jl_data_ptr(b), sz);
@@ -161,10 +173,10 @@ static uintptr_t bits_hash(void *b, size_t sz)
 static uintptr_t NOINLINE hash_svec(jl_svec_t *v)
 {
     uintptr_t h = 0;
-    size_t l = jl_svec_len(v);
-    for(size_t i = 0; i < l; i++) {
-        jl_value_t *x = jl_svecref(v,i);
-        uintptr_t u = x==NULL ? 0 : jl_object_id(x);
+    size_t i, l = jl_svec_len(v);
+    for (i = 0; i < l; i++) {
+        jl_value_t *x = jl_svecref(v, i);
+        uintptr_t u = (x == NULL) ? 0 : jl_object_id(x);
         h = bitmix(h, u);
     }
     return h;
@@ -188,9 +200,11 @@ static uintptr_t jl_object_id_(jl_value_t *tv, jl_value_t *v)
     if (dt == jl_typename_type)
         return ((jl_typename_t*)v)->hash;
 #ifdef _P64
-    if (v == jl_ANY_flag) return 0x31c472f68ee30bddULL;
+    if (v == jl_ANY_flag)
+        return 0x31c472f68ee30bddULL;
 #else
-    if (v == jl_ANY_flag) return 0x8ee30bdd;
+    if (v == jl_ANY_flag)
+        return 0x8ee30bdd;
 #endif
     if (dt == jl_string_type) {
 #ifdef _P64
@@ -199,24 +213,29 @@ static uintptr_t jl_object_id_(jl_value_t *tv, jl_value_t *v)
         return memhash32_seed(jl_string_data(v), jl_string_len(v), 0xedc3b677);
 #endif
     }
-    if (dt->mutabl) return inthash((uintptr_t)v);
+    if (dt->mutabl)
+        return inthash((uintptr_t)v);
     size_t sz = jl_datatype_size(tv);
     uintptr_t h = jl_object_id(tv);
-    if (sz == 0) return ~h;
-    size_t nf = jl_datatype_nfields(dt);
-    if (nf == 0) {
+    if (sz == 0)
+        return ~h;
+    size_t f, nf = jl_datatype_nfields(dt);
+    if (nf == 0)
         return bits_hash(jl_data_ptr(v), sz) ^ h;
-    }
-    for (size_t f=0; f < nf; f++) {
+    for (f = 0; f < nf; f++) {
         size_t offs = jl_field_offset(dt, f);
         char *vo = (char*)jl_data_ptr(v) + offs;
         uintptr_t u;
         if (jl_field_isptr(dt, f)) {
             jl_value_t *f = *(jl_value_t**)vo;
-            u = f==NULL ? 0 : jl_object_id(f);
+            u = (f == NULL) ? 0 : jl_object_id(f);
         }
         else {
             jl_datatype_t *fieldtype = (jl_datatype_t*)jl_field_type(dt, f);
+            if (jl_is_uniontype(fieldtype)) {
+                uint8_t sel = ((uint8_t*)vo)[jl_field_size(dt, f) - 1];
+                fieldtype = (jl_datatype_t*)jl_nth_union_component((jl_value_t*)fieldtype, sel);
+            }
             assert(jl_is_datatype(fieldtype) && !fieldtype->abstract && !fieldtype->mutabl);
             if (fieldtype->layout->haspadding)
                 u = jl_object_id_((jl_value_t*)fieldtype, (jl_value_t*)vo);
@@ -244,7 +263,7 @@ JL_CALLABLE(jl_f_is)
     JL_NARGS(===, 2, 2);
     if (args[0] == args[1])
         return jl_true;
-    return jl_egal(args[0],args[1]) ? jl_true : jl_false;
+    return jl_egal(args[0], args[1]) ? jl_true : jl_false;
 }
 
 JL_CALLABLE(jl_f_typeof)

src/cgutils.cpp

@@ -2245,28 +2245,26 @@ static void emit_setfield(jl_codectx_t &ctx,
             if (wb && strct.isboxed)
                 emit_checked_write_barrier(ctx, boxed(ctx, strct), r);
         }
-        else {
-            if (jl_is_uniontype(jfty)) {
-                int fsz = jl_field_size(sty, idx0);
-                // compute tindex from rhs
-                jl_cgval_t rhs_union = convert_julia_type(ctx, rhs, jfty);
-                if (rhs_union.typ == jl_bottom_type)
-                    return;
-                Value *tindex = compute_tindex_unboxed(ctx, rhs_union, jfty);
-                tindex = ctx.builder.CreateNUWSub(tindex, ConstantInt::get(T_int8, 1));
-                Value *ptindex = ctx.builder.CreateGEP(T_int8, emit_bitcast(ctx, addr, T_pint8), ConstantInt::get(T_size, fsz - 1));
-                ctx.builder.CreateStore(tindex, ptindex);
-                // copy data
-                if (!rhs.isghost) {
-                    emit_unionmove(ctx, addr, rhs, NULL, false, NULL);
-                }
-            }
-            else {
-                int align = jl_field_align(sty, idx0);
-                typed_store(ctx, addr, ConstantInt::get(T_size, 0), rhs, jfty,
-                    strct.tbaa, data_pointer(ctx, strct, T_pjlvalue), align);
+        else if (jl_is_uniontype(jfty)) {
+            int fsz = jl_field_size(sty, idx0);
+            // compute tindex from rhs
+            jl_cgval_t rhs_union = convert_julia_type(ctx, rhs, jfty);
+            if (rhs_union.typ == jl_bottom_type)
+                return;
+            Value *tindex = compute_tindex_unboxed(ctx, rhs_union, jfty);
+            tindex = ctx.builder.CreateNUWSub(tindex, ConstantInt::get(T_int8, 1));
+            Value *ptindex = ctx.builder.CreateGEP(T_int8, emit_bitcast(ctx, addr, T_pint8), ConstantInt::get(T_size, fsz - 1));
+            ctx.builder.CreateStore(tindex, ptindex);
+            // copy data
+            if (!rhs.isghost) {
+                emit_unionmove(ctx, addr, rhs, NULL, false, NULL);
             }
         }
+        else {
+            int align = jl_field_align(sty, idx0);
+            typed_store(ctx, addr, ConstantInt::get(T_size, 0), rhs, jfty,
+                strct.tbaa, data_pointer(ctx, strct, T_pjlvalue), align);
+        }
     }
     else {
         // TODO: better error

src/codegen.cpp

@@ -2077,11 +2077,48 @@ static jl_cgval_t emit_getfield(jl_codectx_t &ctx, const jl_cgval_t &strct, jl_s
     return mark_julia_type(ctx, result, true, jl_any_type, needsgcroot);
 }
 
+static Value *emit_bits_compare(jl_codectx_t &ctx, const jl_cgval_t &arg1, const jl_cgval_t &arg2);
+
+static Value *emit_bitsunion_compare(jl_codectx_t &ctx, const jl_cgval_t &arg1, const jl_cgval_t &arg2)
+{
+    assert(arg1.typ == arg2.typ && arg1.TIndex && arg2.TIndex && jl_is_uniontype(arg1.typ) && "unimplemented");
+    Value *tindex = arg1.TIndex;
+    BasicBlock *defaultBB = BasicBlock::Create(jl_LLVMContext, "unionbits_is_boxed", ctx.f);
+    SwitchInst *switchInst = ctx.builder.CreateSwitch(tindex, defaultBB);
+    BasicBlock *postBB = BasicBlock::Create(jl_LLVMContext, "post_unionbits_is", ctx.f);
+    ctx.builder.SetInsertPoint(postBB);
+    PHINode *phi = ctx.builder.CreatePHI(T_int1, 2);
+    unsigned counter = 0;
+    for_each_uniontype_small(
+        [&](unsigned idx, jl_datatype_t *jt) {
+            BasicBlock *tempBB = BasicBlock::Create(jl_LLVMContext, "unionbits_is", ctx.f);
+            ctx.builder.SetInsertPoint(tempBB);
+            switchInst->addCase(ConstantInt::get(T_int8, idx), tempBB);
+            jl_cgval_t sel_arg1(arg1, (jl_value_t*)jt, NULL);
+            jl_cgval_t sel_arg2(arg2, (jl_value_t*)jt, NULL);
+            phi->addIncoming(emit_bits_compare(ctx, sel_arg1, sel_arg2), tempBB);
+            ctx.builder.CreateBr(postBB);
+        },
+        arg1.typ,
+        counter);
+    ctx.builder.SetInsertPoint(defaultBB);
+    Function *trap_func = Intrinsic::getDeclaration(
+        ctx.f->getParent(),
+        Intrinsic::trap);
+    ctx.builder.CreateCall(trap_func);
+    ctx.builder.CreateUnreachable();
+    ctx.builder.SetInsertPoint(postBB);
+    return ctx.builder.CreateAnd(phi, ctx.builder.CreateICmpEQ(arg1.TIndex, arg2.TIndex));
+}
+
 static Value *emit_bits_compare(jl_codectx_t &ctx, const jl_cgval_t &arg1, const jl_cgval_t &arg2)
 {
     assert(jl_is_datatype(arg1.typ) && arg1.typ == arg2.typ);
     Type *at = julia_type_to_llvm(arg1.typ);
 
+    if (type_is_ghost(at))
+        return ConstantInt::get(T_int1, 1);
+
     if (at->isIntegerTy() || at->isPointerTy() || at->isFloatingPointTy()) {
         Type *at_int = INTT(at);
         Value *varg1 = emit_unbox(ctx, at_int, arg1, arg1.typ);
@@ -2130,11 +2167,29 @@ static Value *emit_bits_compare(jl_codectx_t &ctx, const jl_cgval_t &arg1, const
                 Value *subAns, *fld1, *fld2;
                 fld1 = ctx.builder.CreateConstGEP2_32(at, varg1, 0, i);
                 fld2 = ctx.builder.CreateConstGEP2_32(at, varg2, 0, i);
-                if (type_is_ghost(fld1->getType()->getPointerElementType()))
+                Type *at_i = cast<GetElementPtrInst>(fld1)->getResultElementType();
+                if (type_is_ghost(at_i))
                     continue;
-                subAns = emit_bits_compare(ctx,
-                        mark_julia_slot(fld1, fldty, NULL, arg1.tbaa),
-                        mark_julia_slot(fld2, fldty, NULL, arg2.tbaa));
+                if (jl_is_uniontype(fldty)) {
+                    unsigned tindex_offset = cast<StructType>(at_i)->getNumElements() - 1;
+                    Value *ptindex1 = ctx.builder.CreateConstInBoundsGEP2_32(
+                            at_i, fld1, 0, tindex_offset);
+                    Value *ptindex2 = ctx.builder.CreateConstInBoundsGEP2_32(
+                            at_i, fld2, 0, tindex_offset);
+                    Value *tindex1 = ctx.builder.CreateNUWAdd(ConstantInt::get(T_int8, 1),
+                            ctx.builder.CreateLoad(T_int8, ptindex1));
+                    Value *tindex2 = ctx.builder.CreateNUWAdd(ConstantInt::get(T_int8, 1),
+                            ctx.builder.CreateLoad(T_int8, ptindex2));
+                    subAns = emit_bitsunion_compare(ctx,
+                            mark_julia_slot(fld1, fldty, tindex1, arg1.tbaa),
+                            mark_julia_slot(fld2, fldty, tindex2, arg2.tbaa));
+                }
+                else {
+                    assert(jl_is_leaf_type(fldty));
+                    subAns = emit_bits_compare(ctx,
+                            mark_julia_slot(fld1, fldty, NULL, arg1.tbaa),
+                            mark_julia_slot(fld2, fldty, NULL, arg2.tbaa));
+                }
                 answer = ctx.builder.CreateAnd(answer, subAns);
             }
             return answer;
@@ -2198,6 +2253,9 @@ static Value *emit_f_is(jl_codectx_t &ctx, const jl_cgval_t &arg1, const jl_cgva
         return cmp;
     }
 
+    // if (arg1.tindex || arg2.tindex)
+    //   TODO: handle with emit_bitsunion_compare
+
     int ptr_comparable = 0; // whether this type is unique'd by pointer
     if (rt1 == (jl_value_t*)jl_sym_type || rt2 == (jl_value_t*)jl_sym_type)
         ptr_comparable = 1;
@@ -2413,7 +2471,8 @@ static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f,
                     Value *selidx = ctx.builder.CreateMul(emit_arraylen_prim(ctx, ary), nbytes);
                     selidx = ctx.builder.CreateAdd(selidx, idx);
                     Value *ptindex = ctx.builder.CreateGEP(T_int8, data, selidx);
-                    Value *tindex = ctx.builder.CreateNUWAdd(ConstantInt::get(T_int8, 1), tbaa_decorate(tbaa_arrayselbyte, ctx.builder.CreateLoad(T_int8, ptindex)));
+                    Value *tindex = ctx.builder.CreateNUWAdd(ConstantInt::get(T_int8, 1),
+                            tbaa_decorate(tbaa_arrayselbyte, ctx.builder.CreateLoad(T_int8, ptindex)));
                     Type *AT = ArrayType::get(IntegerType::get(jl_LLVMContext, 8 * al), (elsz + al - 1) / al);
                     AllocaInst *lv = emit_static_alloca(ctx, AT);
                     if (al > 1)

src/rtutils.c

@@ -883,9 +883,12 @@ static size_t jl_static_show_x_(JL_STREAM *out, jl_value_t *v, jl_datatype_t *vt
                     n += jl_static_show_x(out, *(jl_value_t**)fld_ptr, depth);
                 }
                 else {
-                    n += jl_static_show_x_(out, (jl_value_t*)fld_ptr,
-                                           (jl_datatype_t*)jl_field_type(vt, i),
-                                           depth);
+                    jl_datatype_t *ft = (jl_datatype_t*)jl_field_type(vt, i);
+                    if (jl_is_uniontype(ft)) {
+                        uint8_t sel = ((uint8_t*)fld_ptr)[jl_field_size(vt, i) - 1];
+                        ft = (jl_datatype_t*)jl_nth_union_component((jl_value_t*)ft, sel);
+                    }
+                    n += jl_static_show_x_(out, (jl_value_t*)fld_ptr, ft, depth);
                 }
                 if (istuple && tlen == 1)
                     n += jl_printf(out, ",");

test/core.jl

@@ -5326,17 +5326,30 @@ struct B
     y::AA
     z::Int8
 end
-b = B(91, AA(ntuple(i -> Int8(i), Val(7))), 23)
-
-@test b.x === Int8(91)
-@test b.z === Int8(23)
-@test b.y === AA(ntuple(i -> Int8(i), Val(7)))
-@test sizeof(b) == 12
-@test AA(Int8(1)).x === Int8(1)
-@test AA(Int8(0)).x === Int8(0)
-@test AA(Int16(1)).x === Int16(1)
-@test AA(nothing).x === nothing
-@test sizeof(b.y) == 8
+@noinline compare(a, b) = (a === b) # test code-generation of `is`
+let
+    b = B(91, AA(ntuple(i -> Int8(i), Val(7))), 23)
+    b2 = Ref(b)[] # copy b via field assignment
+    b3 = B[b][1] # copy b via array assignment
+    @test pointer_from_objref(b) == pointer_from_objref(b)
+    @test pointer_from_objref(b) != pointer_from_objref(b2)
+    @test pointer_from_objref(b) != pointer_from_objref(b3)
+    @test pointer_from_objref(b2) != pointer_from_objref(b3)
+
+    @test b === b2 === b3
+    @test compare(b, b2)
+    @test compare(b, b3)
+    @test object_id(b) === object_id(b2) == object_id(b3)
+    @test b.x === Int8(91)
+    @test b.z === Int8(23)
+    @test b.y === AA((Int8(1), Int8(2), Int8(3), Int8(4), Int8(5), Int8(6), Int8(7)))
+    @test sizeof(b) == 12
+    @test AA(Int8(1)).x === Int8(1)
+    @test AA(Int8(0)).x === Int8(0)
+    @test AA(Int16(1)).x === Int16(1)
+    @test AA(nothing).x === nothing
+    @test sizeof(b.y) == 8
+end
 
 for U in boxedunions
     local U