From 2cce1bd106686da17e03f6ddb3884363485475ea Mon Sep 17 00:00:00 2001 From: Gabriel Baraldi Date: Wed, 25 Sep 2024 16:19:52 -0300 Subject: [PATCH 1/4] Replace store of freeze in allocop and in emit new struct with memset since aggregate stores are bad --- src/cgutils.cpp | 6 +++--- src/llvm-alloc-opt.cpp | 11 +++-------- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/src/cgutils.cpp b/src/cgutils.cpp index 9124638ce7446..cc2caa4eedeef 100644 --- a/src/cgutils.cpp +++ b/src/cgutils.cpp @@ -4199,7 +4199,7 @@ static jl_cgval_t emit_new_struct(jl_codectx_t &ctx, jl_value_t *ty, size_t narg else { strct = UndefValue::get(lt); if (nargs < nf) - strct = ctx.builder.CreateFreeze(strct); + strct = ctx.builder.CreateFreeze(strct); // Change this to zero initialize instead? } } else if (tracked.second) { @@ -4393,9 +4393,9 @@ static jl_cgval_t emit_new_struct(jl_codectx_t &ctx, jl_value_t *ty, size_t narg if (promotion_point) ctx.builder.SetInsertPoint(promotion_point); if (strct) { - promotion_point = cast(ctx.builder.CreateFreeze(UndefValue::get(lt))); jl_aliasinfo_t ai = jl_aliasinfo_t::fromTBAA(ctx, ctx.tbaa().tbaa_stack); - ai.decorateInst(ctx.builder.CreateStore(promotion_point, strct)); + ai.decorateInst(ctx.builder.CreateMemSet(strct, ConstantInt::get(getInt8Ty(ctx.builder.getContext()), 0), + jl_datatype_size(ty), MaybeAlign(jl_datatype_align(ty)))); } ctx.builder.restoreIP(savedIP); } diff --git a/src/llvm-alloc-opt.cpp b/src/llvm-alloc-opt.cpp index 188955fd50972..0dc07cbd1ae4c 100644 --- a/src/llvm-alloc-opt.cpp +++ b/src/llvm-alloc-opt.cpp @@ -622,14 +622,9 @@ void Optimizer::initializeAlloca(IRBuilder<> &prolog_builder, AllocaInst *buff, return; assert(!buff->isArrayAllocation()); Type *T = buff->getAllocatedType(); - Value *Init = UndefValue::get(T); - if ((allockind & AllocFnKind::Zeroed) != AllocFnKind::Unknown) - Init = Constant::getNullValue(T); // zero, as described - else if (allockind == AllocFnKind::Unknown) - Init = Constant::getNullValue(T); // assume zeroed since we didn't find the attribute - else - Init = prolog_builder.CreateFreeze(UndefValue::get(T)); // assume freeze, since LLVM does not natively support this case - prolog_builder.CreateStore(Init, buff); + const DataLayout &DL = F.getParent()->getDataLayout(); + prolog_builder.CreateMemSet(buff, ConstantInt::get(Type::getInt8Ty(prolog_builder.getContext()), 0), DL.getTypeAllocSize(T), buff->getAlign()); + } // This function should not erase any safepoint so that the lifetime marker can find and cache From a891be26de2607fea994bc37025f2ef4f21c2f80 Mon Sep 17 00:00:00 2001 From: gbaraldi Date: Thu, 26 Sep 2024 14:30:54 -0300 Subject: [PATCH 2/4] Apply suggestions from code review --- src/cgutils.cpp | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/src/cgutils.cpp b/src/cgutils.cpp index cc2caa4eedeef..993ccd9ccc05a 100644 --- a/src/cgutils.cpp +++ b/src/cgutils.cpp @@ -4366,25 +4366,18 @@ static jl_cgval_t emit_new_struct(jl_codectx_t &ctx, jl_value_t *ty, size_t narg ctx.builder.restoreIP(savedIP); } } - for (size_t i = nargs; i < nf; i++) { - if (!jl_field_isptr(sty, i) && jl_is_uniontype(jl_field_type(sty, i))) { - ssize_t offs = jl_field_offset(sty, i); - ssize_t ptrsoffs = -1; - if (!inline_roots.empty()) - std::tie(offs, ptrsoffs) = split_value_field(sty, i); - assert(ptrsoffs < 0 && offs >= 0); - int fsz = jl_field_size(sty, i) - 1; - if (init_as_value) { + if (init_as_value) { + for (size_t i = nargs; i < nf; i++) { + if (!jl_field_isptr(sty, i) && jl_is_uniontype(jl_field_type(sty, i))) { + ssize_t offs = jl_field_offset(sty, i); + ssize_t ptrsoffs = -1; + if (!inline_roots.empty()) + std::tie(offs, ptrsoffs) = split_value_field(sty, i); + assert(ptrsoffs < 0 && offs >= 0); + int fsz = jl_field_size(sty, i) - 1; unsigned llvm_idx = convert_struct_offset(ctx, cast(lt), offs + fsz); strct = ctx.builder.CreateInsertValue(strct, ConstantInt::get(getInt8Ty(ctx.builder.getContext()), 0), ArrayRef(llvm_idx)); } - else { - jl_aliasinfo_t ai = jl_aliasinfo_t::fromTBAA(ctx, ctx.tbaa().tbaa_unionselbyte); - Instruction *dest = cast(emit_ptrgep(ctx, strct, offs + fsz)); - if (promotion_point == nullptr) - promotion_point = dest; - ai.decorateInst(ctx.builder.CreateAlignedStore(ctx.builder.getInt8(0), dest, Align(1))); - } } } if (nargs < nf) { From dd89975db37f9d1febaa1479f6a73e3a4ad2ee54 Mon Sep 17 00:00:00 2001 From: Gabriel Baraldi Date: Tue, 15 Oct 2024 17:40:55 -0300 Subject: [PATCH 3/4] Fix promotion_point mishap --- src/cgutils.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cgutils.cpp b/src/cgutils.cpp index f37db5c93aa5c..a166b0a2c4800 100644 --- a/src/cgutils.cpp +++ b/src/cgutils.cpp @@ -4401,7 +4401,7 @@ static jl_cgval_t emit_new_struct(jl_codectx_t &ctx, jl_value_t *ty, size_t narg ctx.builder.SetInsertPoint(promotion_point); if (strct) { jl_aliasinfo_t ai = jl_aliasinfo_t::fromTBAA(ctx, ctx.tbaa().tbaa_stack); - ai.decorateInst(ctx.builder.CreateMemSet(strct, ConstantInt::get(getInt8Ty(ctx.builder.getContext()), 0), + promotion_point = ai.decorateInst(ctx.builder.CreateMemSet(strct, ConstantInt::get(getInt8Ty(ctx.builder.getContext()), 0), jl_datatype_size(ty), MaybeAlign(jl_datatype_align(ty)))); } ctx.builder.restoreIP(savedIP); From 9c6875c270d3c72a8e890ddf2832806e97ce8320 Mon Sep 17 00:00:00 2001 From: Gabriel Baraldi Date: Wed, 16 Oct 2024 10:57:40 -0300 Subject: [PATCH 4/4] Fix llvmpasses tests --- test/llvmpasses/alloc-opt-pass.ll | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/test/llvmpasses/alloc-opt-pass.ll b/test/llvmpasses/alloc-opt-pass.ll index b962157120456..665687e86835d 100644 --- a/test/llvmpasses/alloc-opt-pass.ll +++ b/test/llvmpasses/alloc-opt-pass.ll @@ -76,7 +76,7 @@ L3: ; preds = %L2, %L1, %0 ; CHECK-LABEL: @legal_int_types ; CHECK: alloca [12 x i8] ; CHECK-NOT: alloca i96 -; CHECK: store [12 x i8] zeroinitializer, +; CHECK: call void @llvm.memset.p0.i64(ptr align 16 %var1, ; CHECK: ret void define void @legal_int_types() { %pgcstack = call ptr @julia.get_pgcstack() @@ -140,7 +140,7 @@ L2: ; preds = %0 ; CHECK: alloca ; CHECK-NOT: call token(...) @llvm.julia.gc_preserve_begin ; CHECK: call void @llvm.lifetime.start -; CHECK: store [8 x i8] zeroinitializer, +; CHECK: call void @llvm.memset.p0.i64(ptr align 16 %v, ; CHECK-NOT: call void @llvm.lifetime.end define void @lifetime_no_preserve_end(ptr noalias nocapture noundef nonnull sret({}) %0) { %pgcstack = call ptr @julia.get_pgcstack() @@ -164,11 +164,8 @@ define void @lifetime_no_preserve_end(ptr noalias nocapture noundef nonnull sret ; CHECK: alloca [1 x i8] ; CHECK-DAG: alloca [2 x i8] ; CHECK-DAG: alloca [3 x i8] -; CHECK-DAG: freeze [1 x i8] undef -; CHECK-DAG: store [1 x i8] % -; CHECK-DAG: store [3 x i8] zeroinitializer, -; CHECK-NOT: store -; CHECK-NOT: zeroinitializer +; CHECK-DAG: call void @llvm.memset.p0.i64(ptr align 1 %var1, +; CHECK-DAG: call void @llvm.memset.p0.i64(ptr align 4 %var7, ; CHECK: ret void define void @initializers() { %pgcstack = call ptr @julia.get_pgcstack()