From 8be0e22e815d6839229b502019f4b0eebc46b667 Mon Sep 17 00:00:00 2001 From: Nathan Daly Date: Wed, 21 Jun 2023 02:03:50 -0600 Subject: [PATCH 1/2] Stack allocate `unsafe_load()` Ptr to immutable. Add test that this doesn't allocate. The test fails on master, and passes after this commit. --- src/intrinsics.cpp | 2 +- test/intrinsics.jl | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/intrinsics.cpp b/src/intrinsics.cpp index 09e04eb683af1..22495b5a7e204 100644 --- a/src/intrinsics.cpp +++ b/src/intrinsics.cpp @@ -695,7 +695,7 @@ static jl_cgval_t emit_pointerref(jl_codectx_t &ctx, jl_cgval_t *argv) ai.decorateInst(load); return mark_julia_type(ctx, load, true, ety); } - else if (!jl_isbits(ety)) { + else if (!jl_is_immutable(ety)) { assert(jl_is_datatype(ety)); uint64_t size = jl_datatype_size(ety); Value *strct = emit_allocobj(ctx, (jl_datatype_t*)ety); diff --git a/test/intrinsics.jl b/test/intrinsics.jl index 3c49afe2c4d7e..85dd1ecc8bd68 100644 --- a/test/intrinsics.jl +++ b/test/intrinsics.jl @@ -134,6 +134,17 @@ end struct GhostStruct end @test unsafe_load(Ptr{GhostStruct}(rand(Int))) === GhostStruct() +# issue #50243 +struct ImmutableNonIsBits + v::Vector{Int} +end +const ref50243 = Ref(ImmutableNonIsBits([1,2,3])) +f50243() = unsafe_load(Ptr{ImmutableNonIsBits}(pointer_from_objref(ref50243))) +sum50243() = sum(f50243().v) # (Do something with v to prevent the compiler from eliding) +@test f50243() === ref50243[] +@test sum50243() === 6 +@test @allocated(sum50243()) === 0 + # macro to verify and compare the compiled output of an intrinsic with its runtime version macro test_intrinsic(intr, args...) output = args[end] From 9dfc27db202367c55b438148d3e0ec0870adcd3c Mon Sep 17 00:00:00 2001 From: Nathan Daly Date: Wed, 21 Jun 2023 02:50:05 -0600 Subject: [PATCH 2/2] Also fix codegen for `unsafe_store!()` on non-isbits immutables. This also **fixes a crash** caused by illegal instruction. See: https://github.com/JuliaLang/julia/issues/50243#issuecomment-1600445786 --- src/intrinsics.cpp | 2 +- test/intrinsics.jl | 13 ++++++++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/intrinsics.cpp b/src/intrinsics.cpp index 22495b5a7e204..8b812619555d0 100644 --- a/src/intrinsics.cpp +++ b/src/intrinsics.cpp @@ -775,7 +775,7 @@ static jl_cgval_t emit_pointerset(jl_codectx_t &ctx, jl_cgval_t *argv) jl_aliasinfo_t ai = jl_aliasinfo_t::fromTBAA(ctx, ctx.tbaa().tbaa_data); ai.decorateInst(store); } - else if (!jl_isbits(ety)) { + else if (!jl_is_immutable(ety)) { thePtr = emit_unbox(ctx, getInt8PtrTy(ctx.builder.getContext()), e, e.typ); uint64_t size = jl_datatype_size(ety); im1 = ctx.builder.CreateMul(im1, ConstantInt::get(ctx.types().T_size, diff --git a/test/intrinsics.jl b/test/intrinsics.jl index 85dd1ecc8bd68..a5e7e99e664f5 100644 --- a/test/intrinsics.jl +++ b/test/intrinsics.jl @@ -139,12 +139,19 @@ struct ImmutableNonIsBits v::Vector{Int} end const ref50243 = Ref(ImmutableNonIsBits([1,2,3])) -f50243() = unsafe_load(Ptr{ImmutableNonIsBits}(pointer_from_objref(ref50243))) -sum50243() = sum(f50243().v) # (Do something with v to prevent the compiler from eliding) -@test f50243() === ref50243[] +load50243() = unsafe_load(Ptr{ImmutableNonIsBits}(pointer_from_objref(ref50243))) +sum50243() = sum(load50243().v) # (Do something with v to prevent the compiler from eliding) +@test load50243() === ref50243[] @test sum50243() === 6 @test @allocated(sum50243()) === 0 +const ref50243 = Ref(ImmutableNonIsBits([1,2,3])) +store50243!(v2) = unsafe_store!(Ptr{ImmutableNonIsBits}(pointer_from_objref(ref50243)), v2) +new50243 = ImmutableNonIsBits([1]) +store50243!(new50243) +@test ref50243[] === new50243 +@test @allocated(store50243!(ref50243[])) === 0 + # macro to verify and compare the compiled output of an intrinsic with its runtime version macro test_intrinsic(intr, args...) output = args[end]