Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix #32843, codegen attempting typeof(NULL) for some union tags #40365

Merged
merged 1 commit into from
Apr 11, 2021
Merged

fix #32843, codegen attempting typeof(NULL) for some union tags #40365

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 27 additions & 3 deletions src/cgutils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2472,6 +2472,27 @@ static Value *compute_box_tindex(jl_codectx_t &ctx, Value *datatype, jl_value_t
return tindex;
}

// Returns typeof(v), or null if v is a null pointer at run time.
// This is used when the value might have come from an undefined variable,
// yet we try to read its type to compute a union index when moving the value.
static Value *emit_typeof_or_null(jl_codectx_t &ctx, Value *v)
{
BasicBlock *nonnull = BasicBlock::Create(jl_LLVMContext, "nonnull", ctx.f);
BasicBlock *postBB = BasicBlock::Create(jl_LLVMContext, "postnull", ctx.f);
Value *isnull = ctx.builder.CreateICmpEQ(v, Constant::getNullValue(v->getType()));
ctx.builder.CreateCondBr(isnull, postBB, nonnull);
BasicBlock *entry = ctx.builder.GetInsertBlock();
ctx.builder.SetInsertPoint(nonnull);
Value *typof = emit_typeof(ctx, v);
ctx.builder.CreateBr(postBB);
nonnull = ctx.builder.GetInsertBlock(); // could have changed
ctx.builder.SetInsertPoint(postBB);
PHINode *ti = ctx.builder.CreatePHI(typof->getType(), 2);
ti->addIncoming(Constant::getNullValue(typof->getType()), entry);
ti->addIncoming(typof, nonnull);
return ti;
}
Copy link
Member

Choose a reason for hiding this comment

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

FWIW, we have a concise form of this function's pattern now. No need to change this, just thought I'd let you know about the alternative:

    Value *isnull = ctx.builder.CreateIsNull(v);
    return emit_guarded_test(ctx, isnull, V_rnull, [&] {
          return emit_typeof(ctx, v);
    });


// get the runtime tindex value, assuming val is already converted to type typ if it has a TIndex
static Value *compute_tindex_unboxed(jl_codectx_t &ctx, const jl_cgval_t &val, jl_value_t *typ)
{
Expand All @@ -2482,9 +2503,12 @@ static Value *compute_tindex_unboxed(jl_codectx_t &ctx, const jl_cgval_t &val, j

if (val.TIndex)
return ctx.builder.CreateAnd(val.TIndex, ConstantInt::get(T_int8, 0x7f));
if (val.isboxed)
return compute_box_tindex(ctx, emit_typeof_boxed(ctx, val), val.typ, typ);
return compute_box_tindex(ctx, emit_typeof_boxed(ctx, val), val.typ, typ);
Value *typof;
if (val.isboxed && !jl_is_concrete_type(val.typ) && !jl_is_type_type(val.typ))
typof = emit_typeof_or_null(ctx, val.V);
else
typof = emit_typeof_boxed(ctx, val);
return compute_box_tindex(ctx, typof, val.typ, typ);
}

static void union_alloca_type(jl_uniontype_t *ut,
Expand Down
6 changes: 4 additions & 2 deletions src/codegen.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1504,11 +1504,13 @@ static jl_cgval_t convert_julia_type_union(jl_codectx_t &ctx, const jl_cgval_t &
// actually need it.
Value *union_box_dt = NULL;
BasicBlock *union_isaBB = NULL;
BasicBlock *post_union_isaBB = NULL;
auto maybe_setup_union_isa = [&]() {
if (!union_isaBB) {
union_isaBB = BasicBlock::Create(jl_LLVMContext, "union_isa", ctx.f);
ctx.builder.SetInsertPoint(union_isaBB);
union_box_dt = emit_typeof(ctx, v.Vboxed);
union_box_dt = emit_typeof_or_null(ctx, v.Vboxed);
post_union_isaBB = ctx.builder.GetInsertBlock();
}
};

Expand Down Expand Up @@ -1540,7 +1542,7 @@ static jl_cgval_t convert_julia_type_union(jl_codectx_t &ctx, const jl_cgval_t &
ctx.builder.SetInsertPoint(postBB);
PHINode *tindex_phi = ctx.builder.CreatePHI(T_int8, 2);
tindex_phi->addIncoming(new_tindex, currBB);
tindex_phi->addIncoming(union_box_tindex, union_isaBB);
tindex_phi->addIncoming(union_box_tindex, post_union_isaBB);
new_tindex = tindex_phi;
}
}
Expand Down
8 changes: 8 additions & 0 deletions test/compiler/codegen.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -553,3 +553,11 @@ end
end
@test occursin("llvm.julia.gc_preserve_begin", get_llvm(f4, Tuple{Bool}, true, false, false))
end

# issue #32843
function f32843(vals0, v)
(length(vals0) > 1) && (vals = v[1])
(length(vals0) == 1 && vals0[1]==1) && (vals = 1:2)
vals
end
@test_throws UndefVarError f32843([6], Vector[[1]])