codegen: propagate at-pure macro to llvm #32368
Conversation
| if (codeinst->def->def.method->pure) { | ||
| // pure marked functions don't have side-effects, nor observe them | ||
| f->addFnAttr(Thunk); | ||
| f->addFnAttr(Attribute::ReadNone); |
There was a problem hiding this comment.
Might be good to have a lengthy comment here why ReadNone is legal.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
| @@ -538,31 +538,6 @@ struct type with no fields. | |||
| """ | |||
| issingletontype(@nospecialize(t)) = (@_pure_meta; isa(t, DataType) && isdefined(t, :instance)) | |||
|
|
|||
| """ | |||
There was a problem hiding this comment.
Why does this change necessitate removing this function entirely?
There was a problem hiding this comment.
The function's only here so that it could be exported from Compat in the past. As the docstring said, don't use this.
| @@ -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) | |||
There was a problem hiding this comment.
not pure because it throws? or because it's defined on T::Type?
There was a problem hiding this comment.
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.
|
deleting this, since it is quite stale, and we've landed the cleanup commit, but decided people use |
Adds ReadNone and NoUnwind (and thunk) to call sites marked
@purein the source code. This allows LLVM to perform LICM and hoisting on these functions, which can be highly profitable.