Skip to content

Improve optimizations on NULL pointer check #37114

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

Merged
merged 1 commit into from
Aug 21, 2020
Merged

Improve optimizations on NULL pointer check #37114

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
4 changes: 2 additions & 2 deletions base/pointer.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ function unsafe_convert end

unsafe_convert(::Type{Ptr{UInt8}}, x::Symbol) = ccall(:jl_symbol_name, Ptr{UInt8}, (Any,), x)
unsafe_convert(::Type{Ptr{Int8}}, x::Symbol) = ccall(:jl_symbol_name, Ptr{Int8}, (Any,), x)
unsafe_convert(::Type{Ptr{UInt8}}, s::String) = convert(Ptr{UInt8}, pointer_from_objref(s)+sizeof(Int))
unsafe_convert(::Type{Ptr{Int8}}, s::String) = convert(Ptr{Int8}, pointer_from_objref(s)+sizeof(Int))
unsafe_convert(::Type{Ptr{UInt8}}, s::String) = ccall(:jl_string_ptr, Ptr{UInt8}, (Any,), s)
unsafe_convert(::Type{Ptr{Int8}}, s::String) = ccall(:jl_string_ptr, Ptr{Int8}, (Any,), s)
# convert strings to String etc. to pass as pointers
cconvert(::Type{Ptr{UInt8}}, s::AbstractString) = String(s)
cconvert(::Type{Ptr{Int8}}, s::AbstractString) = String(s)
Expand Down
25 changes: 23 additions & 2 deletions src/ccall.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1227,6 +1227,10 @@ std::string generate_func_sig(const char *fname)
continue;
attributes = attributes.addAttributes(jl_LLVMContext, i + 1, as);
}
// If return value is boxed it must be non-null.
if (retboxed)
attributes = attributes.addAttribute(jl_LLVMContext, AttributeList::ReturnIndex,
Attribute::NonNull);
if (rt == jl_bottom_type) {
attributes = attributes.addAttribute(jl_LLVMContext,
AttributeList::FunctionIndex,
Expand Down Expand Up @@ -1692,8 +1696,25 @@ static jl_cgval_t emit_ccall(jl_codectx_t &ctx, jl_value_t **args, size_t nargs)
else if (is_libjulia_func(jl_string_ptr)) {
assert(lrt == T_size);
assert(!isVa && !llvmcall && nccallargs == 1);
Value *obj = ctx.builder.CreatePtrToInt(emit_pointer_from_objref(ctx, boxed(ctx, argv[0])), T_size);
Value *strp = ctx.builder.CreateAdd(obj, ConstantInt::get(T_size, sizeof(void*)));
auto obj = emit_bitcast(ctx, emit_pointer_from_objref(ctx, boxed(ctx, argv[0])),
T_pprjlvalue);
// The inbounds gep makes it more clear to LLVM that the resulting value is not
// a null pointer.
auto strp = ctx.builder.CreateConstInBoundsGEP1_32(T_prjlvalue, obj, 1);
strp = ctx.builder.CreatePtrToInt(strp, T_size);
JL_GC_POP();
return mark_or_box_ccall_result(ctx, strp, retboxed, rt, unionall, static_rt);
}
else if (is_libjulia_func(jl_symbol_name)) {
assert(lrt == T_size);
assert(!isVa && !llvmcall && nccallargs == 1);
auto obj = emit_bitcast(ctx, emit_pointer_from_objref(ctx, boxed(ctx, argv[0])),
T_pprjlvalue);
// The inbounds gep makes it more clear to LLVM that the resulting value is not
// a null pointer.
auto strp = ctx.builder.CreateConstInBoundsGEP1_32(
T_prjlvalue, obj, (sizeof(jl_sym_t) + sizeof(void*) - 1) / sizeof(void*));
strp = ctx.builder.CreatePtrToInt(strp, T_size);
JL_GC_POP();
return mark_or_box_ccall_result(ctx, strp, retboxed, rt, unionall, static_rt);
}
Expand Down
14 changes: 7 additions & 7 deletions src/codegen.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -361,15 +361,15 @@ static AttributeList get_attrs_sext(LLVMContext &C)
{
return AttributeList::get(C,
AttributeSet(),
AttributeSet(),
Attributes(C, {Attribute::NonNull}),
{Attributes(C, {Attribute::SExt})});
}

static AttributeList get_attrs_zext(LLVMContext &C)
{
return AttributeList::get(C,
AttributeSet(),
AttributeSet(),
Attributes(C, {Attribute::NonNull}),
{Attributes(C, {Attribute::ZExt})});
}

Expand Down Expand Up @@ -614,7 +614,7 @@ static const auto jl_newbits_func = new JuliaFunction{
{T_prjlvalue, T_pint8}, false); },
[](LLVMContext &C) { return AttributeList::get(C,
AttributeSet(),
Attributes(C, {Attribute::NoAlias, Attribute::NonNull}),
Attributes(C, {Attribute::NonNull}),
Copy link
Member

@vtjnash vtjnash Aug 19, 2020

Choose a reason for hiding this comment

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

backport?

Copy link
Contributor Author

@yuyichao yuyichao Aug 19, 2020

Choose a reason for hiding this comment

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

Oh yeah, this line of change is in principle a bug fix..... (commit was from a week ago and I've forgot about that...............)

Copy link
Member

@vtjnash vtjnash Aug 19, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, I think I missed the second half of the langref documentation for this attribute. (for arguments, it means that it doesn't alias any mutable memory, but for return values, it also means the pointer itself is unique à la malloc)

None); },
};
static const auto jl_typeof_func = new JuliaFunction{
Expand Down Expand Up @@ -769,9 +769,9 @@ BOX_FUNC(uint32, T_prjlvalue, T_int32, get_attrs_zext);
BOX_FUNC(int64, T_prjlvalue, T_int64, get_attrs_sext);
BOX_FUNC(uint64, T_prjlvalue, T_int64, get_attrs_zext);
BOX_FUNC(char, T_prjlvalue, T_char, get_attrs_zext);
BOX_FUNC(float32, T_prjlvalue, T_float32, nullptr);
BOX_FUNC(float64, T_prjlvalue, T_float64, nullptr);
BOX_FUNC(ssavalue, T_prjlvalue, T_size, nullptr);
BOX_FUNC(float32, T_prjlvalue, T_float32, get_func_attrs);
BOX_FUNC(float64, T_prjlvalue, T_float64, get_func_attrs);
BOX_FUNC(ssavalue, T_prjlvalue, T_size, get_func_attrs);
#undef BOX_FUNC


Expand Down Expand Up @@ -805,7 +805,7 @@ static const auto pointer_from_objref_func = new JuliaFunction{
{PointerType::get(T_jlvalue, AddressSpace::Derived)}, false); },
[](LLVMContext &C) { return AttributeList::get(C,
AttributeSet::get(C, makeArrayRef({Attribute::get(C, Attribute::ReadNone), Attribute::get(C, Attribute::NoUnwind)})),
AttributeSet(),
Attributes(C, {Attribute::NonNull}),
None); },
};

Expand Down
1 change: 1 addition & 0 deletions src/julia.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1012,6 +1012,7 @@ STATIC_INLINE jl_value_t *jl_field_type_concrete(jl_datatype_t *st JL_PROPAGATES
#define jl_datatype_nfields(t) (((jl_datatype_t*)(t))->layout->nfields)
#define jl_datatype_isinlinealloc(t) (((jl_datatype_t *)(t))->isinlinealloc)

JL_DLLEXPORT void *jl_symbol_name(jl_sym_t *s);
// inline version with strong type check to detect typos in a `->name` chain
STATIC_INLINE char *jl_symbol_name_(jl_sym_t *s) JL_NOTSAFEPOINT
{
Expand Down