In canEscapeToUsePoint only check the content node if it's a reference (see comment why this is needed).
For all other node types, especially addresses, handle defer edges by propagating use-point infomation backward in the graph.
This makes escape analysis more precise with address types, e.g. don't consider an inout address to escape to an apply if just the loaded value is passed to an apply argument.
The XXOptUtils.h convention is already established and parallels
the SIL/XXUtils convention.
New:
- InstOptUtils.h
- CFGOptUtils.h
- BasicBlockOptUtils.h
- ValueLifetime.h
Removed:
- Local.h
- Two conflicting CFG.h files
This reorganization is helpful before I introduce more
utilities for block cloning similar to SinkAddressProjections.
Move the control flow utilies out of Local.h, which was an
unreadable, unprincipled mess. Rename it to InstOptUtils.h, and
confine it to small APIs for working with individual instructions.
These are the optimizer's additions to /SIL/InstUtils.h.
Rename CFG.h to CFGOptUtils.h and remove the one in /Analysis. Now
there is only SIL/CFG.h, resolving the naming conflict within the
swift project (this has always been a problem for source tools). Limit
this header to low-level APIs for working with branches and CFG edges.
Add BasicBlockOptUtils.h for block level transforms (it makes me sad
that I can't use BBOptUtils.h, but SIL already has
BasicBlockUtils.h). These are larger APIs for cloning or removing
whole blocks.
This removes it from the AST and largely replaces it with AnyObject
at the SIL and IRGen layers. Some notes:
- Reflection still uses the notion of "unknown object" to mean an
object with unknown refcounting. There's no real reason to make
this different from AnyObject (an existential containing a
single object with unknown refcounting), but this way nothing
changes for clients of Reflection, and it's consistent with how
native objects are represented.
- The value witness table and reflection descriptor for AnyObject
use the mangling "BO" instead of "yXl".
- The demangler and remangler continue to support "BO" because it's
still in use as a type encoding, even if it's not an AST-level
Type anymore.
- Type-based alias analysis for Builtin.UnknownObject was incorrect,
so it's a good thing we weren't using it.
- Same with enum layout. (This one assumed UnknownObject never
referred to an Objective-C tagged pointer. That certainly wasn't how
we were using it!)
Previously, CallerAnalysis::FunctionInfo.foundAllCallers(), which is
documented to return true only when specialization of a function will
not require a thunk, returned true for functions which are possibly used
externally. Now, that member function only returns false for functions
which may be used externally since dead code elimination will not be
able to remove them.
This provides a singular instruction for convert an unmanaged value to a ref,
then strong_retain it. I expanded the definition of UNCHECKED_REF_STORAGE to
include these copy like instructions. This instruction is valid in all SIL.
The reason why I am adding this instruction is that currently when we emit an
access to an unowned (unsafe) ivar, we use an unmanaged_to_ref and a strong
retain. This can look to the optimizer like a strong retain that can potentially
be optimized. By combining the two together into a new instruction, we can avoid
this potential problem since the pattern matching will break.
Now that we've moved to C++14, we no longer need the llvm::make_unique
implementation from STLExtras.h. This patch is a mechanical replacement
of (hopefully) all the llvm::make_unique instances in the swift repo.
Describe the algorithm in the file-level doc comment. The basic
algorithm in the referenced paper is similar, but the most
interesting/important information is how it is adapted to SIL.
This improves on the previous situation:
- The request ensures that the backing storage for lazy properties
and property wrappers gets synthesized first; previously it was
only somewhat guaranteed by callers.
- Instead of returning a range this just returns an ArrayRef,
which simplifies clients.
- Indexing into the ArrayRef is O(1), which addresses some FIXMEs
in the SIL optimizer.
The SIL generation for this builtin also changes: instead of generating the cond_fail instructions upfront, let the optimizer generate it, if the operand is a static string literal.
In worst case, if the second operand is not a static string literal, the Builtin.condfail is lowered at the end of the optimization pipeline with a default message: "unknown program error".
The SIL generation for this builtin also changes: instead of generating the cond_fail instructions upfront, let the optimizer generate it, if the operand is a static string literal.
In worst case, if the second operand is not a static string literal, the Builtin.condfail is lowered at the end of the optimization pipeline with a default message: "unknown program error".
Handle is_escaping_closure like an RC-checking instructions. It looks
like it "may-decrement" RC because: if the object must be live after
the instruction and the subsequent local release, then it expects the
refcount to be greater than one. In other words, do not remove a
retain/release pair that spans an is_escaping_closure
instruction.
This would otherwise fail to catch closures that are returned
from the function.
The following retain/release pair cannot be removed:
%me = partial_apply
retain %me
is_escaping_closure %me
release %me
return %me
Fixes <rdar://problem/52803634>
This analysis helper was inverting the result for builtins. Builtins
such as "copyMemory" were treated as never using a value.
This manifested in a crash in TestFoundation. NSDictionary's
initializer released the incoming array before copying it. This
crashed later during dictionary destruction.
The crash was hidden by a secondary bug in mayHaveSymmetricInterference
that effectively ignored the result from canNeverUseValue.
Rename the helper to canUseObject, invert the result for builtins, and
fix mayHaveSymmetricInterference to respect the result of
canUseObject.
Note that instructions that cannot access a referenced object
obviously cannot not "interfere" with a release.
Fixing these bugs now allows ARC optimization around dealloc_stack and
other operations that don't care about the reference count.
With the advent of dynamic_function_ref the actual callee of such a ref
my vary. Optimizations should not assume to know the content of a
function referenced by dynamic_function_ref. Introduce
getReferencedFunctionOrNull which will return null for such function
refs. And getInitialReferencedFunction to return the referenced
function.
Use as appropriate.
rdar://50959798
- code simplification critical for comprehension
- substantially improves the overhead of AccessedStorage comparison
- as a side effect improves precision of analysis in some cases
AccessedStorage is meant to be an immutable value type that identifies
a storage location with minimal representation. It is used in many global
interprocedural data structures.
The RefElementAddress instruction that it was derived from does not
contribute to the uniqueness of the storage location. It doesn't
belong here. It was being used to create a ProjectionPath, which is an
extremely inneficient way to compare access paths.
Just delete all the code related to that extra field.
This will make the forthcoming CanonicalizeInstruction interface more
clear.
This is generally the better approach to utilities that mutate the
instruction stream. It avoids the temptation to assume that only a
single instruction will be deleted or that only instructions before
the current iterator will be deleted. This often happens to work but
eventually fails in the presense of debug and end-of-scope
instructions.
A function returning an iterator has a more clear contract than one
accepting some iterator reference of unknown
providence. Unfortunately, it doesn't work at the lowest level of
utilities, such as recursivelyDeleteTriviallyDeadInstructions, where
we want to handle instruction batches.
This is a large patch; I couldn't split it up further while still
keeping things working. There are four things being changed at
once here:
- Places that call SILType::isAddressOnly()/isLoadable() now call
the SILFunction overload and not the SILModule one.
- SILFunction's overloads of getTypeLowering() and getLoweredType()
now pass the function's resilience expansion down, instead of
hardcoding ResilienceExpansion::Minimal.
- Various other places with '// FIXME: Expansion' now use a better
resilience expansion.
- A few tests were updated to reflect SILGen's improved code
generation, and some new tests are added to cover more code paths
that previously were uncovered and only manifested themselves as
standard library build failures while I was working on this change.
Just treat begin_apply conservatively.
It's probably not worth adding much complexity for begin_apply to the analysis as co-routines are often inlined anyway.
Fixes a miscompile.
https://bugs.swift.org/browse/SR-10444
rdar://problem/49755264
Reuse AccessStorageAnalysis to summarize accesses.
Don't ignore call sites in loops.
Don't consider a read access in a loop to conflict with a read
outside.
Use the unidentified access flag from the analysis.
Remove extraneous code, some of which was unreachable.
General cleanup.
The previous design was customized to perfoming IPO with
GenericSideEffectAnalysis. Expose the underlying logic as a utility so
that AccessEnforcementOpts can use it to summarize loops (in addition
to call sites).
LLVM r355981 changed various intrinsic functions, including expect,
to require immediate arguments. Swift's _branchHint function has an
expected value that is passed in as an argument, so that it cannot
use LLVM's expect intrinsic. The good news is that _branchHint is only
ever used with immediate arguments, so we can just move the intrinsic
into _fastPath and _slowPath and use those instead of _branchHint.
As was noted in the documentation, the _fastPath and _slowPath names are
confusing but we have passed the point where we can simply rename them.
We could add new names but would still need to keep the old ones around
for binary compatibility, and it is not clear that it is worth the
trouble. I have removed that note from the documentation.
