Skip to content

codegen: propagate at-pure macro to llvm #32368

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

Closed
wants to merge 2 commits into from
Closed

codegen: propagate at-pure macro to llvm #32368

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
508f5bf
codegen: propagate at-pure macro to llvm
vtjnash Jun 19, 2019
a23af5e
pure: remove incorrect or unnecessary annotations and functions
vtjnash Jun 19, 2019
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
6 changes: 1 addition & 5 deletions base/bool.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,7 @@ julia> .![true false true]
0 1 0
```
"""
function !(x::Bool)
## We need a better heuristic to detect this automatically
@_pure_meta
return not_int(x)
end
!(x::Bool) = not_int(x)

(~)(x::Bool) = !x
(&)(x::Bool, y::Bool) = and_int(x, y)
Expand Down
19 changes: 8 additions & 11 deletions base/compiler/optimize.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,23 +171,20 @@ function optimize(opt::OptimizationState, @nospecialize(result))

# compute inlining and other related optimizations
if (isa(result, Const) || isconstType(result))
proven_pure = false
# must be proven pure to use const_api; otherwise we might skip throwing errors
# (issue #20704)
# TODO: Improve this analysis; if a function is marked @pure we should really
# only care about certain errors (e.g. method errors and type errors).
if length(ir.stmts) < 10
proven_pure = def isa Method && def.pure
# if it can be proven pure, we can use const_api; otherwise we might skip side-effects (like throwing errors)
if !proven_pure
proven_pure = true
for i in 1:length(ir.stmts)
stmt = ir.stmts[i]
if stmt_affects_purity(stmt, ir) && !stmt_effect_free(stmt, ir.types[i], ir, ir.sptypes)
for fl in opt.src.slotflags
if (fl & SLOT_USEDUNDEF) != 0
proven_pure = false
break
end
end
if proven_pure
for fl in opt.src.slotflags
if (fl & SLOT_USEDUNDEF) != 0
for i in 1:length(ir.stmts)
stmt = ir.stmts[i]
if stmt_affects_purity(stmt, ir) && !stmt_effect_free(stmt, ir.types[i], ir, ir.sptypes)
proven_pure = false
break
end
Expand Down
4 changes: 2 additions & 2 deletions base/essentials.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ tail(::Tuple{}) = throw(ArgumentError("Cannot call tail on an empty tuple."))
tuple_type_head(T::Type) = (@_pure_meta; fieldtype(T::Type{<:Tuple}, 1))

function tuple_type_tail(T::Type)
@_pure_meta
@_pure_meta # TODO: this method is wrong (and not @pure)
Copy link
Member

Choose a reason for hiding this comment

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

not pure because it throws? or because it's defined on T::Type?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, it would be nice if you could expand on how this is wrong and impure in the comment. That would make it easier for others to revisit and attempt to fix later.

if isa(T, UnionAll)
return UnionAll(T.var, tuple_type_tail(T.body))
elseif isa(T, Union)
Expand Down Expand Up @@ -698,7 +698,7 @@ julia> f(Val(true))
struct Val{x}
end

Val(x) = (@_pure_meta; Val{x}())
Val(x) = Val{x}()

"""
invokelatest(f, args...; kwargs...)
Expand Down
4 changes: 2 additions & 2 deletions base/promotion.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
Return the closest common ancestor of `T` and `S`, i.e. the narrowest type from which
they both inherit.
"""
typejoin() = (@_pure_meta; Bottom)
typejoin(@nospecialize(t)) = (@_pure_meta; t)
typejoin() = Bottom
typejoin(@nospecialize(t)) = t
typejoin(@nospecialize(t), ts...) = (@_pure_meta; typejoin(t, typejoin(ts...)))
function typejoin(@nospecialize(a), @nospecialize(b))
@_pure_meta
Expand Down
32 changes: 3 additions & 29 deletions base/reflection.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -538,31 +538,6 @@ struct type with no fields.
"""
issingletontype(@nospecialize(t)) = (@_pure_meta; isa(t, DataType) && isdefined(t, :instance))

"""
Copy link
Member

Choose a reason for hiding this comment

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

Why does this change necessitate removing this function entirely?

Copy link
Member Author

Choose a reason for hiding this comment

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

The function's only here so that it could be exported from Compat in the past. As the docstring said, don't use this.

Base.parameter_upper_bound(t::UnionAll, idx)

Determine the upper bound of a type parameter in the underlying datatype.
This method should generally not be relied upon:
code instead should usually use static parameters in dispatch to extract these values.

# Examples
```jldoctest
julia> struct Foo{T<:AbstractFloat, N}
x::Tuple{T, N}
end

julia> Base.parameter_upper_bound(Foo, 1)
AbstractFloat

julia> Base.parameter_upper_bound(Foo, 2)
Any
```
"""
function parameter_upper_bound(t::UnionAll, idx)
@_pure_meta
return rewrap_unionall((unwrap_unionall(t)::DataType).parameters[idx], t)
end

"""
typeintersect(T, S)

Expand Down Expand Up @@ -727,13 +702,12 @@ julia> instances(Color)
function instances end

function to_tuple_type(@nospecialize(t))
@_pure_meta
if isa(t,Tuple) || isa(t,AbstractArray) || isa(t,SimpleVector)
if isa(t, Tuple) || isa(t, AbstractArray) || isa(t, SimpleVector)
t = Tuple{t...}
end
if isa(t,Type) && t<:Tuple
if isa(t, Type) && t <: Tuple
for p in unwrap_unionall(t).parameters
if !(isa(p,Type) || isa(p,TypeVar))
if !(isa(p, Type) || isa(p, TypeVar))
error("argument tuple type must contain only types")
end
end
Expand Down
1 change: 1 addition & 0 deletions base/tuple.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ function _compute_eltype(t::Type{<:Tuple})
@_pure_meta
t isa Union && return promote_typejoin(eltype(t.a), eltype(t.b))
t´ = unwrap_unionall(t)
# TODO: handle Union/UnionAll correctly here
r = Union{}
for ti in t´.parameters
r = promote_typejoin(r, rewrap_unionall(unwrapva(ti), t))
Expand Down
3 changes: 2 additions & 1 deletion doc/src/devdocs/ast.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -621,7 +621,8 @@ A (usually temporary) container for holding lowered source code.
* 0 = inbounds
* 1,2 = <reserved> inlinehint,always-inline,noinline
* 3 = <reserved> strict-ieee (strictfp)
* 4-6 = <unused>
* 4 = <reserved> inferred-pure
* 5-6 = <unused>
* 7 = <reserved> has out-of-band info

* `linetable`
Expand Down
68 changes: 46 additions & 22 deletions src/codegen.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,13 @@ static Value *emit_condition(jl_codectx_t &ctx, const jl_cgval_t &condV, const s
static void allocate_gc_frame(jl_codectx_t &ctx, BasicBlock *b0);
static void CreateTrap(IRBuilder<> &irbuilder);
static CallInst *emit_jlcall(jl_codectx_t &ctx, Value *theFptr, Value *theF,
jl_cgval_t *args, size_t nargs, CallingConv::ID cc);
jl_cgval_t *args, size_t nargs,
AttributeList Attrs, CallingConv::ID cc);
static CallInst *emit_jlcall(jl_codectx_t &ctx, Value *theFptr, Value *theF,
jl_cgval_t *args, size_t nargs) {
AttributeList Attrs;
return emit_jlcall(ctx, theFptr, theF, args, nargs, Attrs, JLCALL_F_CC);
}

static Value *literal_pointer_val(jl_codectx_t &ctx, jl_value_t *p);
static GlobalVariable *prepare_global_in(Module *M, GlobalVariable *G);
Expand Down Expand Up @@ -2182,7 +2188,7 @@ static jl_cgval_t emit_getfield(jl_codectx_t &ctx, const jl_cgval_t &strct, jl_s
strct,
mark_julia_const((jl_value_t*)name)
};
Value *result = emit_jlcall(ctx, jlgetfield_func, maybe_decay_untracked(V_null), myargs_array, 2, JLCALL_F_CC);
Value *result = emit_jlcall(ctx, jlgetfield_func, maybe_decay_untracked(V_null), myargs_array, 2);
return mark_julia_type(ctx, result, true, jl_any_type);
}

Expand Down Expand Up @@ -3022,7 +3028,8 @@ static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f,
}

static CallInst *emit_jlcall(jl_codectx_t &ctx, Value *theFptr, Value *theF,
jl_cgval_t *argv, size_t nargs, CallingConv::ID cc)
jl_cgval_t *argv, size_t nargs,
AttributeList Attrs, CallingConv::ID cc)
{
// emit arguments
SmallVector<Value*, 3> theArgs;
Expand All @@ -3040,7 +3047,8 @@ static CallInst *emit_jlcall(jl_codectx_t &ctx, Value *theFptr, Value *theF,
CallInst *result = ctx.builder.CreateCall(FTy,
ctx.builder.CreateBitCast(prepare_call(theFptr), FTy->getPointerTo()),
theArgs);
add_return_attr(result, Attribute::NonNull);
Attrs = Attrs.addAttribute(jl_LLVMContext, AttributeList::ReturnIndex, Attribute::NonNull);
result->setAttributes(Attrs);
result->setCallingConv(cc);
return result;
}
Expand All @@ -3052,7 +3060,14 @@ static jl_cgval_t emit_call_specfun_other(jl_codectx_t &ctx, jl_code_instance_t
// emit specialized call site
jl_value_t *jlretty = codeinst->rettype;
jl_returninfo_t returninfo = get_specsig_function(jl_Module, specFunctionObject, codeinst->def->specTypes, jlretty);
FunctionType *cft = returninfo.decl->getFunctionType();
Function *f = returninfo.decl;
FunctionType *cft = f->getFunctionType();
if (codeinst->def->def.method->pure) {
// pure marked functions don't have side-effects, nor observe them
f->addFnAttr(Thunk);
f->addFnAttr(Attribute::ReadNone);
Copy link
Member

Choose a reason for hiding this comment

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

Might be good to have a lengthy comment here why ReadNone is legal.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd be more worried about the other one, if I were you. This one is simple: it doesn't read any memory which may be written to, aka ReadNone.

Also, from reading the LICM code, it looks like we should have Attribute::Speculatable also.

Copy link
Member

Choose a reason for hiding this comment

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

I still don't think ReadNone is correct. Any access to the PTLS is a violation of that and that means the code can't allocate (which is hard to control). While the rules for @pure are tricky it took me a while to realize that returning a mutable or a immutable containing an allocated object might not be valid. Even code that allocates, but doesn't return the allocation should then no longer be considered pure, but that code might be of interest to be LICM.

I would like a more fine-grained differentiation between the different kinds of side-effects.

f->addFnAttr(Attribute::NoUnwind);
}

size_t nfargs = cft->getNumParams();
Value **argvals = (Value**)alloca(nfargs * sizeof(Value*));
Expand Down Expand Up @@ -3116,8 +3131,8 @@ static jl_cgval_t emit_call_specfun_other(jl_codectx_t &ctx, jl_code_instance_t
idx++;
}
assert(idx == nfargs);
CallInst *call = ctx.builder.CreateCall(returninfo.decl, ArrayRef<Value*>(&argvals[0], nfargs));
call->setAttributes(returninfo.decl->getAttributes());
CallInst *call = ctx.builder.CreateCall(f, ArrayRef<Value*>(&argvals[0], nfargs));
call->setAttributes(f->getAttributes());

jl_cgval_t retval;
switch (returninfo.cc) {
Expand Down Expand Up @@ -3157,7 +3172,7 @@ static jl_cgval_t emit_call_specfun_other(jl_codectx_t &ctx, jl_code_instance_t
return retval;
}

static jl_cgval_t emit_call_specfun_boxed(jl_codectx_t &ctx, StringRef specFunctionObject,
static jl_cgval_t emit_call_specfun_boxed(jl_codectx_t &ctx, jl_code_instance_t *codeinst, StringRef specFunctionObject,
jl_cgval_t *argv, size_t nargs, jl_value_t *inferred_retty)
{
auto theFptr = jl_Module->getOrInsertFunction(specFunctionObject, jl_func_sig)
Expand All @@ -3166,11 +3181,20 @@ static jl_cgval_t emit_call_specfun_boxed(jl_codectx_t &ctx, StringRef specFunct
#else
;
#endif
if (auto F = dyn_cast<Function>(theFptr->stripPointerCasts())) {
add_return_attr(F, Attribute::NonNull);
F->addFnAttr(Thunk);
}
Value *ret = emit_jlcall(ctx, theFptr, nullptr, argv, nargs, JLCALL_F_CC);
AttributeList Attrs;
auto F = dyn_cast<Function>(theFptr->stripPointerCasts());
if (F)
Attrs = F->getAttributes();
Attrs = Attrs.addAttribute(jl_LLVMContext, AttributeList::ReturnIndex, Attribute::NonNull)
.addAttribute(jl_LLVMContext, AttributeList::FunctionIndex, Thunk);
if (codeinst->def->def.method->pure) {
// pure marked functions don't have side-effects, nor observe them
Attrs = Attrs.addAttribute(jl_LLVMContext, AttributeList::FunctionIndex, Attribute::ReadNone)
.addAttribute(jl_LLVMContext, AttributeList::FunctionIndex, Attribute::NoUnwind);
}
if (F)
F->setAttributes(Attrs);
Value *ret = emit_jlcall(ctx, theFptr, nullptr, argv, nargs, Attrs, JLCALL_F_CC);
return mark_julia_type(ctx, ret, true, inferred_retty);
}

Expand Down Expand Up @@ -3204,7 +3228,7 @@ static jl_cgval_t emit_invoke(jl_codectx_t &ctx, jl_expr_t *ex, jl_value_t *rt)
}
if (decls.functionObject) {
if (!strcmp(decls.functionObject, "jl_fptr_args")) {
result = emit_call_specfun_boxed(ctx, decls.specFunctionObject, argv, nargs, rt);
result = emit_call_specfun_boxed(ctx, codeinst, decls.specFunctionObject, argv, nargs, rt);
handled = true;
}
else if (!!strcmp(decls.functionObject, "jl_fptr_sparam")) {
Expand All @@ -3216,7 +3240,7 @@ static jl_cgval_t emit_invoke(jl_codectx_t &ctx, jl_expr_t *ex, jl_value_t *rt)
}
}
if (!handled) {
Value *r = emit_jlcall(ctx, prepare_call(jlinvoke_func), boxed(ctx, lival), argv, nargs, JLCALL_F2_CC);
Value *r = emit_jlcall(ctx, prepare_call(jlinvoke_func), boxed(ctx, lival), argv, nargs, jlinvoke_func->getAttributes(), JLCALL_F2_CC);
result = mark_julia_type(ctx, r, true, rt);
}
if (result.typ == jl_bottom_type)
Expand Down Expand Up @@ -3257,13 +3281,13 @@ static jl_cgval_t emit_call(jl_codectx_t &ctx, jl_expr_t *ex, jl_value_t *rt)
std::map<jl_fptr_args_t, Function*>::iterator it = builtin_func_map.find(jl_get_builtin_fptr(f.constant));
if (it != builtin_func_map.end()) {
Value *theFptr = it->second;
Value *ret = emit_jlcall(ctx, theFptr, maybe_decay_untracked(V_null), &argv[1], nargs - 1, JLCALL_F_CC);
Value *ret = emit_jlcall(ctx, theFptr, maybe_decay_untracked(V_null), &argv[1], nargs - 1);
return mark_julia_type(ctx, ret, true, rt);
}
}

// emit function and arguments
Value *callval = emit_jlcall(ctx, jlapplygeneric_func, nullptr, argv, nargs, JLCALL_F_CC);
Value *callval = emit_jlcall(ctx, jlapplygeneric_func, nullptr, argv, nargs);
return mark_julia_type(ctx, callval, true, rt);
}

Expand Down Expand Up @@ -4128,7 +4152,7 @@ static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr, ssize_t ssaval)
assert(nargs <= jl_datatype_nfields(jl_tparam0(ty)) + 1);
return emit_new_struct(ctx, jl_tparam0(ty), nargs - 1, &argv[1]);
}
Value *val = emit_jlcall(ctx, jlnew_func, nullptr, argv, nargs, JLCALL_F_CC);
Value *val = emit_jlcall(ctx, jlnew_func, nullptr, argv, nargs);
// temporarily mark as `Any`, expecting `emit_ssaval_assign` to update
// it to the inferred type.
return mark_julia_type(ctx, val, true, (jl_value_t*)jl_any_type);
Expand Down Expand Up @@ -4325,7 +4349,7 @@ static void emit_cfunc_invalidate(
}
}
assert(AI == gf_thunk->arg_end());
Value *gf_ret = emit_jlcall(ctx, jlapplygeneric_func, nullptr, myargs, nargs, JLCALL_F_CC);
Value *gf_ret = emit_jlcall(ctx, jlapplygeneric_func, nullptr, myargs, nargs);
jl_cgval_t gf_retbox = mark_julia_type(ctx, gf_ret, true, jl_any_type);
jl_value_t *astrt = codeinst->rettype;
if (cc != jl_returninfo_t::Boxed) {
Expand Down Expand Up @@ -4697,11 +4721,11 @@ static Function* gen_cfun_wrapper(
// for jlcall, we need to pass the function object even if it is a ghost.
Value *theF = boxed(ctx, inputargs[0]);
assert(theF);
ret_jlcall = emit_jlcall(ctx, theFptr, theF, &inputargs[1], nargs, JLCALL_F_CC);
ret_jlcall = emit_jlcall(ctx, theFptr, theF, &inputargs[1], nargs);
ctx.builder.CreateBr(b_after);
ctx.builder.SetInsertPoint(b_generic);
}
Value *ret = emit_jlcall(ctx, prepare_call(jlapplygeneric_func), NULL, inputargs, nargs + 1, JLCALL_F_CC);
Value *ret = emit_jlcall(ctx, prepare_call(jlapplygeneric_func), NULL, inputargs, nargs + 1);
if (age_ok) {
ctx.builder.CreateBr(b_after);
ctx.builder.SetInsertPoint(b_after);
Expand Down Expand Up @@ -5998,7 +6022,7 @@ static std::unique_ptr<Module> emit_function(
emit_varinfo_assign(ctx, vi, tuple);
} else {
Value *vtpl = emit_jlcall(ctx, prepare_call(jltuple_func), maybe_decay_untracked(V_null),
vargs, ctx.nvargs, JLCALL_F_CC);
vargs, ctx.nvargs);
jl_cgval_t tuple = mark_julia_type(ctx, vtpl, true, vi.value.typ);
emit_varinfo_assign(ctx, vi, tuple);
}
Expand Down
5 changes: 3 additions & 2 deletions src/julia.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,8 +243,9 @@ typedef struct _jl_code_info_t {
// 0 = inbounds
// 1,2 = <reserved> inlinehint,always-inline,noinline
// 3 = <reserved> strict-ieee (strictfp)
// 4-6 = <unused>
// 7 = has out-of-band info
// 4 = <reserved> inferred-pure
// 5-6 = <unused>
// 7 = <reserved> has out-of-band info
// miscellaneous data:
jl_value_t *method_for_inference_limit_heuristics; // optional method used during inference
jl_value_t *linetable; // Table of locations [TODO: make this volatile like slotnames]
Expand Down
20 changes: 12 additions & 8 deletions src/llvm-late-gc-lowering.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1160,16 +1160,20 @@ State LateLowerGCFrame::LocalScan(Function &F) {
callee == write_barrier_func || callee->getName() == "memcmp") {
continue;
}
if (callee->hasFnAttribute(Attribute::ReadNone) ||
callee->hasFnAttribute(Attribute::ReadOnly) ||
callee->hasFnAttribute(Attribute::ArgMemOnly)) {
continue;
if (!callee->hasFnAttribute("thunk")) {
if (callee->hasFnAttribute(Attribute::ReadNone) ||
callee->hasFnAttribute(Attribute::ReadOnly) ||
callee->hasFnAttribute(Attribute::ArgMemOnly)) {
continue;
}
}
}
if (isa<IntrinsicInst>(CI) || CI->hasFnAttr(Attribute::ArgMemOnly) ||
CI->hasFnAttr(Attribute::ReadNone) || CI->hasFnAttr(Attribute::ReadOnly)) {
// Intrinsics are never safepoints.
continue;
if (!CI->hasFnAttr("thunk")) {
if (isa<IntrinsicInst>(CI) || CI->hasFnAttr(Attribute::ArgMemOnly) ||
CI->hasFnAttr(Attribute::ReadNone) || CI->hasFnAttr(Attribute::ReadOnly)) {
// Intrinsics are never safepoints.
continue;
}
}
int SafepointNumber = NoteSafepoint(S, BBS, CI);
BBS.HasSafepoint = true;
Expand Down
Loading