Skip to content

Commit a5b993c

Browse files
vtjnashvtjnash
authored
Merge pull request #21276 from JuliaLang/jn/21271
fix jl_isa optimization
2 parents cab83c5 + d6c5c67 commit a5b993c

File tree

4 files changed

+54
-57
lines changed

4 files changed

+54
-57
lines changed

‎src/cgutils.cpp

Lines changed: 36 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -941,21 +941,24 @@ static void emit_type_error(const jl_cgval_t &x, Value *type, const std::string
941941
#endif
942942
}
943943

944-
static Value *emit_isa(const jl_cgval_t &x, jl_value_t *type, const std::string *msg, jl_codectx_t *ctx)
944+
static std::pair<Value*, bool> emit_isa(const jl_cgval_t &x, jl_value_t *type, const std::string *msg, jl_codectx_t *ctx)
945945
{
946-
bool maybe_isa = true;
946+
Optional<bool> known_isa;
947947
if (x.constant)
948-
maybe_isa = jl_isa(x.constant, type);
948+
known_isa = jl_isa(x.constant, type);
949+
else if (jl_subtype(x.typ, type))
950+
known_isa = true;
949951
else if (jl_type_intersection(x.typ, type) == (jl_value_t*)jl_bottom_type)
950-
maybe_isa = false;
951-
if (!maybe_isa && msg) {
952-
emit_type_error(x, literal_pointer_val(type), *msg, ctx);
953-
builder.CreateUnreachable();
954-
BasicBlock *failBB = BasicBlock::Create(jl_LLVMContext, "fail", ctx->f);
955-
builder.SetInsertPoint(failBB);
952+
known_isa = false;
953+
if (known_isa) {
954+
if (!*known_isa && msg) {
955+
emit_type_error(x, literal_pointer_val(type), *msg, ctx);
956+
builder.CreateUnreachable();
957+
BasicBlock *failBB = BasicBlock::Create(jl_LLVMContext, "fail", ctx->f);
958+
builder.SetInsertPoint(failBB);
959+
}
960+
return std::make_pair(ConstantInt::get(T_int1, *known_isa), true);
956961
}
957-
if (!maybe_isa || x.constant)
958-
return ConstantInt::get(T_int1, maybe_isa);
959962

960963
// intersection with Type needs to be handled specially
961964
if (jl_has_intersect_type_not_kind(type)) {
@@ -966,15 +969,15 @@ static Value *emit_isa(const jl_cgval_t &x, jl_value_t *type, const std::string
966969
#else
967970
builder.CreateCall2(prepare_call(jltypeassert_func), vx, literal_pointer_val(type));
968971
#endif
969-
return ConstantInt::get(T_int1, 1);
972+
return std::make_pair(ConstantInt::get(T_int1, 1), true);
970973
}
971-
return builder.CreateICmpNE(
974+
return std::make_pair(builder.CreateICmpNE(
972975
#if JL_LLVM_VERSION >= 30700
973976
builder.CreateCall(prepare_call(jlisa_func), { vx, literal_pointer_val(type) }),
974977
#else
975978
builder.CreateCall2(prepare_call(jlisa_func), vx, literal_pointer_val(type)),
976979
#endif
977-
ConstantInt::get(T_int32, 0));
980+
ConstantInt::get(T_int32, 0)), false);
978981
}
979982
// tests for isa leaftype can be handled with pointer comparisons
980983
if (jl_is_leaf_type(type)) {
@@ -983,7 +986,7 @@ static Value *emit_isa(const jl_cgval_t &x, jl_value_t *type, const std::string
983986
if (tindex > 0) {
984987
// optimize more when we know that this is a split union-type where tindex = 0 is invalid
985988
Value *xtindex = builder.CreateAnd(x.TIndex, ConstantInt::get(T_int8, 0x7f));
986-
return builder.CreateICmpEQ(xtindex, ConstantInt::get(T_int8, tindex));
989+
return std::make_pair(builder.CreateICmpEQ(xtindex, ConstantInt::get(T_int8, tindex)), false);
987990
}
988991
else {
989992
// test for (x.TIndex == 0x80 && typeof(x.V) == type)
@@ -999,31 +1002,29 @@ static Value *emit_isa(const jl_cgval_t &x, jl_value_t *type, const std::string
9991002
PHINode *istype = builder.CreatePHI(T_int1, 2);
10001003
istype->addIncoming(ConstantInt::get(T_int1, 0), currBB);
10011004
istype->addIncoming(istype_boxed, isaBB);
1002-
return istype;
1005+
return std::make_pair(istype, false);
10031006
}
10041007
}
1005-
return builder.CreateICmpEQ(emit_typeof_boxed(x, ctx), literal_pointer_val(type));
1008+
return std::make_pair(builder.CreateICmpEQ(emit_typeof_boxed(x, ctx), literal_pointer_val(type)), false);
10061009
}
10071010
// everything else can be handled via subtype tests
10081011
Value *vxt = emit_typeof_boxed(x, ctx);
1009-
return builder.CreateICmpNE(
1012+
return std::make_pair(builder.CreateICmpNE(
10101013
#if JL_LLVM_VERSION >= 30700
10111014
builder.CreateCall(prepare_call(jlsubtype_func), { vxt, literal_pointer_val(type) }),
10121015
#else
10131016
builder.CreateCall2(prepare_call(jlsubtype_func), vxt, literal_pointer_val(type)),
10141017
#endif
1015-
ConstantInt::get(T_int32, 0));
1018+
ConstantInt::get(T_int32, 0)), false);
10161019
}
10171020

10181021
static void emit_typecheck(const jl_cgval_t &x, jl_value_t *type, const std::string &msg,
10191022
jl_codectx_t *ctx)
10201023
{
1021-
// if (jl_subtype(x.typ, type)) {
1022-
// // This case should already be handled by the caller
1023-
// return;
1024-
// }
1025-
Value *istype = emit_isa(x, type, &msg, ctx);
1026-
if (!isa<Constant>(istype)) {
1024+
Value *istype;
1025+
bool handled_msg;
1026+
std::tie(istype, handled_msg) = emit_isa(x, type, &msg, ctx);
1027+
if (!handled_msg) {
10271028
BasicBlock *failBB = BasicBlock::Create(jl_LLVMContext, "fail", ctx->f);
10281029
BasicBlock *passBB = BasicBlock::Create(jl_LLVMContext, "pass");
10291030
builder.CreateCondBr(istype, passBB, failBB);
@@ -2176,12 +2177,11 @@ static jl_cgval_t emit_new_struct(jl_value_t *ty, size_t nargs, jl_value_t **arg
21762177
strct = emit_static_alloca(lt);
21772178

21782179
unsigned idx = 0;
2179-
for (size_t i=0; i < na; i++) {
2180-
jl_value_t *jtype = jl_svecref(sty->types,i);
2180+
for (size_t i = 0; i < na; i++) {
2181+
jl_value_t *jtype = jl_svecref(sty->types, i);
21812182
Type *fty = julia_type_to_llvm(jtype);
2182-
jl_cgval_t fval_info = emit_expr(args[i+1], ctx);
2183-
if (!jl_subtype(fval_info.typ, jtype))
2184-
emit_typecheck(fval_info, jtype, "new", ctx);
2183+
jl_cgval_t fval_info = emit_expr(args[i + 1], ctx);
2184+
emit_typecheck(fval_info, jtype, "new", ctx);
21852185
if (!type_is_ghost(fty)) {
21862186
Value *fval = NULL, *dest = NULL;
21872187
if (!init_as_value) {
@@ -2225,34 +2225,32 @@ static jl_cgval_t emit_new_struct(jl_value_t *ty, size_t nargs, jl_value_t **arg
22252225
jl_cgval_t strctinfo = mark_julia_type(strct, true, ty, ctx);
22262226
if (f1) {
22272227
jl_cgval_t f1info = mark_julia_type(f1, true, jl_any_type, ctx);
2228-
if (!jl_subtype(expr_type(args[1],ctx), jl_field_type(sty,0)))
2229-
emit_typecheck(f1info, jl_field_type(sty,0), "new", ctx);
2228+
emit_typecheck(f1info, jl_field_type(sty, 0), "new", ctx);
22302229
emit_setfield(sty, strctinfo, 0, f1info, ctx, false, false);
22312230
}
2232-
for(size_t i=j; i < nf; i++) {
2231+
for (size_t i = j; i < nf; i++) {
22332232
if (jl_field_isptr(sty, i)) {
22342233
tbaa_decorate(strctinfo.tbaa, builder.CreateStore(
22352234
V_null,
22362235
builder.CreatePointerCast(
22372236
builder.CreateGEP(emit_bitcast(strct, T_pint8),
2238-
ConstantInt::get(T_size, jl_field_offset(sty,i))),
2237+
ConstantInt::get(T_size, jl_field_offset(sty, i))),
22392238
T_ppjlvalue)));
22402239
}
22412240
}
22422241
bool need_wb = false;
22432242
// TODO: verify that nargs <= nf (currently handled by front-end)
2244-
for(size_t i=j+1; i < nargs; i++) {
2243+
for (size_t i = j + 1; i < nargs; i++) {
22452244
jl_cgval_t rhs = emit_expr(args[i], ctx);
22462245
if (jl_field_isptr(sty, i - 1) && !rhs.isboxed) {
22472246
need_wb = true;
22482247
}
22492248
if (rhs.isboxed) {
2250-
if (!jl_subtype(expr_type(args[i],ctx), jl_svecref(sty->types,i-1)))
2251-
emit_typecheck(rhs, jl_svecref(sty->types,i-1), "new", ctx);
2249+
emit_typecheck(rhs, jl_svecref(sty->types, i - 1), "new", ctx);
22522250
}
22532251
if (might_need_root(args[i])) // TODO: how to remove this?
22542252
need_wb = true;
2255-
emit_setfield(sty, strctinfo, i-1, rhs, ctx, false, need_wb);
2253+
emit_setfield(sty, strctinfo, i - 1, rhs, ctx, false, need_wb);
22562254
}
22572255
return strctinfo;
22582256
}

‎src/codegen.cpp

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@
7474

7575
// support
7676
#include <llvm/ADT/SmallBitVector.h>
77+
#include <llvm/ADT/Optional.h>
7778
#include <llvm/Support/raw_ostream.h>
7879
#include <llvm/Support/FormattedStream.h>
7980
#include <llvm/Support/SourceMgr.h> // for llvmcall
@@ -2525,9 +2526,9 @@ static Value *emit_f_is(const jl_cgval_t &arg1, const jl_cgval_t &arg2, jl_codec
25252526
if (arg1.isghost || arg2.isghost) {
25262527
// comparing to a singleton object
25272528
if (arg1.TIndex)
2528-
return emit_isa(arg1, rt2, NULL, ctx); // rt2 is a singleton type
2529+
return emit_isa(arg1, rt2, NULL, ctx).first; // rt2 is a singleton type
25292530
if (arg2.TIndex)
2530-
return emit_isa(arg2, rt1, NULL, ctx); // rt1 is a singleton type
2531+
return emit_isa(arg2, rt1, NULL, ctx).first; // rt1 is a singleton type
25312532
// mark_gc_use isn't needed since we won't load this pointer
25322533
// and we know at least one of them is a unique Singleton
25332534
// which is already enough to ensure pointer uniqueness for this test
@@ -2543,7 +2544,7 @@ static Value *emit_f_is(const jl_cgval_t &arg1, const jl_cgval_t &arg2, jl_codec
25432544
jl_value_t *typ = jl_isbits(rt1) ? rt1 : rt2;
25442545
if (rt1 == rt2)
25452546
return emit_bits_compare(arg1, arg2, ctx);
2546-
Value *same_type = (typ == rt2) ? emit_isa(arg1, typ, NULL, ctx) : emit_isa(arg2, typ, NULL, ctx);
2547+
Value *same_type = (typ == rt2) ? emit_isa(arg1, typ, NULL, ctx).first : emit_isa(arg2, typ, NULL, ctx).first;
25472548
BasicBlock *currBB = builder.GetInsertBlock();
25482549
BasicBlock *isaBB = BasicBlock::Create(jl_LLVMContext, "is", ctx->f);
25492550
BasicBlock *postBB = BasicBlock::Create(jl_LLVMContext, "post_is", ctx->f);
@@ -2637,8 +2638,7 @@ static bool emit_builtin_call(jl_cgval_t *ret, jl_value_t *f, jl_value_t **args,
26372638
jl_value_t *tp0 = jl_tparam0(ty);
26382639
*ret = emit_expr(args[1], ctx);
26392640
emit_expr(args[2], ctx);
2640-
if (!jl_subtype(arg, tp0))
2641-
emit_typecheck(*ret, tp0, "typeassert", ctx);
2641+
emit_typecheck(*ret, tp0, "typeassert", ctx);
26422642
ty = expr_type(expr, ctx); rt2 = ty;
26432643
*ret = update_julia_type(*ret, ty, ctx);
26442644
JL_GC_POP();
@@ -2672,13 +2672,10 @@ static bool emit_builtin_call(jl_cgval_t *ret, jl_value_t *f, jl_value_t **args,
26722672
jl_value_t *tp0 = jl_tparam0(ty);
26732673
jl_cgval_t rt_arg = emit_expr(args[1], ctx);
26742674
emit_expr(args[2], ctx);
2675-
if (jl_subtype(arg, tp0)) {
2676-
*ret = mark_julia_type(ConstantInt::get(T_int8, 1), false, jl_bool_type, ctx);
2677-
}
2678-
else {
2679-
Value *isa = emit_isa(rt_arg, tp0, NULL, ctx);
2680-
*ret = mark_julia_type(builder.CreateZExt(isa, T_int8), false, jl_bool_type, ctx);
2681-
}
2675+
Value *isa_result = emit_isa(rt_arg, tp0, NULL, ctx).first;
2676+
if (isa_result->getType() == T_int1)
2677+
isa_result = builder.CreateZExt(isa_result, T_int8);
2678+
*ret = mark_julia_type(isa_result, false, jl_bool_type, ctx);
26822679
JL_GC_POP();
26832680
return true;
26842681
}
@@ -4587,10 +4584,8 @@ static Function *gen_cfun_wrapper(jl_function_t *ff, jl_value_t *jlrettype, jl_t
45874584
retval = mark_julia_type(ret, true, astrt, &ctx);
45884585
}
45894586

4590-
if (!jl_subtype(astrt, declrt)) {
4591-
// inline a call to typeassert here
4592-
emit_typecheck(retval, declrt, "cfunction", &ctx);
4593-
}
4587+
// inline a call to typeassert here
4588+
emit_typecheck(retval, declrt, "cfunction", &ctx);
45944589

45954590
// Prepare the return value
45964591
Value *r;

‎src/intrinsics.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -661,9 +661,7 @@ static jl_cgval_t emit_pointerset(jl_cgval_t *argv, jl_codectx_t *ctx)
661661
return emit_runtime_pointerset(argv, ctx);
662662
if (!jl_is_datatype(ety))
663663
ety = (jl_value_t*)jl_any_type;
664-
jl_value_t *xty = x.typ;
665-
if (!jl_subtype(xty, ety))
666-
emit_typecheck(x, ety, "pointerset: type mismatch in assign", ctx);
664+
emit_typecheck(x, ety, "pointerset: type mismatch in assign", ctx);
667665

668666
Value *idx = emit_unbox(T_size, i, (jl_value_t*)jl_long_type);
669667
Value *im1 = builder.CreateSub(idx, ConstantInt::get(T_size, 1));

‎test/core.jl

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4794,3 +4794,9 @@ b21178(::F1,::F2) where {B1,B2,F1<:F21178{B1,<:Any},F2<:F21178{B2}} = F1,F2,B1,B
47944794
a21172 = f21172(x) = 2x
47954795
@test f21172(8) == 16
47964796
@test a21172 === f21172
4797+
4798+
# issue #21271
4799+
f21271() = convert(Tuple{Type{Int}, Type{Float64}}, (Int, Float64))::Tuple{Type{Int}, Type{Float64}}
4800+
f21271(x) = x::Tuple{Type{Int}, Type{Float64}}
4801+
@test_throws TypeError f21271()
4802+
@test_throws TypeError f21271((Int, Float64))

0 commit comments

Comments
 (0)