Skip to content

Commit

Permalink
fix #37872, avoid cycles in codegen for === (#37896)
Browse files Browse the repository at this point in the history
  • Loading branch information
JeffBezanson authored Oct 9, 2020
1 parent 85d1cba commit 7f7ab69
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 2 deletions.
12 changes: 10 additions & 2 deletions src/codegen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2500,8 +2500,16 @@ static Value *emit_bits_compare(jl_codectx_t &ctx, jl_cgval_t arg1, jl_cgval_t a
Value *nullcheck2 = nullptr;
auto fld1 = emit_getfield_knownidx(ctx, arg1, i, sty, &nullcheck1);
auto fld2 = emit_getfield_knownidx(ctx, arg2, i, sty, &nullcheck2);
answer = ctx.builder.CreateAnd(answer, emit_f_is(ctx, fld1, fld2,
nullcheck1, nullcheck2));
Value *fld_answer;
if (jl_field_isptr(sty, i) && jl_is_concrete_immutable(fldty)) {
// concrete immutables that are !isinlinealloc might be reference cycles
// issue #37872
fld_answer = emit_box_compare(ctx, fld1, fld2, nullcheck1, nullcheck2);
}
else {
fld_answer = emit_f_is(ctx, fld1, fld2, nullcheck1, nullcheck2);
}
answer = ctx.builder.CreateAnd(answer, fld_answer);
}
return answer;
}
Expand Down
5 changes: 5 additions & 0 deletions test/compiler/codegen.jl
Original file line number Diff line number Diff line change
Expand Up @@ -495,3 +495,8 @@ let x = reinterpret(Has256Bits, [0xfcdac822cac89d82de4f9b3326da8294, 0x6ebac4d59
@test reinterpret(UInt128, [h(x)]) == lshifted
@test reinterpret(UInt128, [Base.shl_int(x, 0x8)]) == lshifted
end

# issue #37872
let f(@nospecialize(x)) = x===Base.ImmutableDict(Int128=>:big)
@test !f(Dict(Int=>Int))
end

6 comments on commit 7f7ab69

@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)

@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 package evaluation, I will reply here when finished:

@nanosoldier runtests(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

Something went wrong when running your job:

NanosoldierError: failed to run benchmarks against primary commit: failed process: Process(`sudo cset shield -c 1,2,3,4 -k on`, ProcessExited(2)) [2]

Logs and partial data can be found here
cc @christopher-dG

@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 package evaluation, I will reply here when finished:

@nanosoldier runtests(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

Your package evaluation job has completed - possible new issues were detected. A full report can be found here. cc @maleadt

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

Your package evaluation job has completed - possible issues were detected. A full report can be found here. cc @maleadt

Please sign in to comment.