Although I don't plan to bring over new assertions wholesale
into the current qualification branch, it's entirely possible
that various minor changes in main will use the new assertions;
having this basic support in the release branch will simplify that.
(This is why I'm adding the includes as a separate pass from
rewriting the individual assertions)
This cleans up 90 instances of this warning and reduces the build spew
when building on Linux. This helps identify actual issues when
building which can get lost in the stream of warning messages. It also
helps restore the ability to build the compiler with gcc.
Fix innumerable latent bugs with iterator invalidation and callback invocation.
Removes dead code earlier and chips away at all the redundant copies the compiler generates.
This pass makes assumptions about the visibility of a type's memory
based on the visibility of its properties. This is the wrong way to
think about memory visibility.
Fixes <rdar://57564377> Struct component with 'let', unexpected
behavior
This pass wants assume that the contents of a property is known based
on whether the property is declared as a 'let' and the visibility of
the initializers that access the property. For example:
```
public struct X<T> {
public let hidden: T
init(t: T) { self.hidden = t }
}
```
The pass currently assumes that `X` only takes on values that are
assigned by the invocations of `X.init`, which is only visible in `X`s
module. This is wrong if the layout of `Impl` is exposed to other
modules. A struct's memory may be initialized by any module with
access to the struct's layout.
In fact, this assumption is wrong even if the struct, and it's let
property cannot be accessed externally by name. In this next example,
external modules cannot access `Impl` or `Impl.hidden` by name, but
can still access the memory.
```
internal struct Impl<T> {
let hidden: T
init(t: T) { self.hidden = t }
}
public struct Wrapper<T> {
var impl: Impl<T>
public var property: T {
get {
return impl.hidden
}
}
}
```
As long as `Wrapper`s layout is exposed to other modules, the contents
of `Wrapper`, `Impl`, and `hidden' can all be initialized in another
module
```
// Legal as long Wrapper's home module is *not* built with library evolution
// (or if Wrapper is declared `@frozen`).
func inExternalModule(buffer: UnsafeRawPointer) -> Wrapper<Int64> {
return buffer.load(as: Wrapper<Int64>.self)
}
```
If library evolution is enabled and a `public` struct is not declared
`@frozen` then external modules cannot assume its layout, and therefore
cannot initialize the struct memory. In that case, it is possible to optimize
`X.hidden` and `Impl.hidden` as if the properties are only initialized inside
their home module.
The right way to view a type's memory visibility is to consider whether
external modules have access to the layout of the type. If not, then the
property can still be optimized As long as a struct is never enclosed in a
public effectively-`@frozen` type. However, finding all places where a struct
is explicitly created is still insufficient. Instead, the optimization needs
to find all uses of enclosing types and determine if every use has a known
constant initialization, or is simply copied from another value. If an
escaping unsafe pointer to any enclosing type is created, then the
optimization is not valid.
When viewed this way, the fact that a property is declared 'let' is mostly
irrelevant to this optimization--it can be expanded to handle non-'let'
properties. The more salient feature is whether the propery has a public
setter.
For now, this optimization only recognizes class properties because class
properties are only accessibly via a ref_element_addr instruction. This is a
side effect of the fact that accessing a class property requires a "formal
access". This means that begin_access marker must be emitted directly on the
address produced by a ref_element_addr. Struct properties are not handled, as
explained above, because they can be indirectly accessed via addresses of
outer types.
This is in prepration for other bug fixes.
Clarify the SIL utilities that return canonical address values for
formal access given the address used by some memory operation:
- stripAccessMarkers
- getAddressAccess
- getAccessedAddress
These are closely related to the code in MemAccessUtils.
Make sure passes use these utilities consistently so that
optimizations aren't defeated by normal variations in SIL patterns.
Create an isLetAddress() utility alongside these basic utilities to
make sure it is used consistently with the address corresponding to
formal access. When this query is used inconsistently, it defeats
optimization. It can also cause correctness bugs because some
optimizations assume that 'let' initialization is only performed on a
unique address value.
Functional changes to Memory Behavior:
- An instruction with side effects now conservatively still has side
effects even when the queried value is a 'let'. Let values are
certainly sensitive to side effects, such as the parent object being
deallocated.
- Return the correct MemBehavior for begin/end_access markers.
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 utility is generally a horrible idea but even worse the
callers were not doing anything to ensure the required
invariants actually held.
Add a new canReplaceLoadSequence() method and chek it in the
right places.
This disables a bunch of passes when ownership is enabled. This will allow me to
keep transparent functions in ossa and skip most of the performance pipeline without
being touched by passes that have not been updated for ownership.
This is important so that we can in -Onone code import transparent functions and
inline them into other ossa functions (you can't inline from ossa => non-ossa).
Don't exponentially recurse through the use-def graph.
Fixes <rdar://problem/45691574> [SR-9146]
SILCloner crash in IBAnimatable: LetPropertiesOpt duplicates the property initializer sequence.
When the initializer of a property is analyzed we now don't bail if such a property is copied from another instance of a struct/class.
This fixes a strange performance regression with constant let properties in structs which conform to an unrelated protocol.
SR-7713
rdar://problem/40332203
* SILModule::isVisibleExternally utility for VarDecls.
* Fix the SIL parser so it doesn't drop global variable decls.
This information was getting lost in SIL printing/parsing.
Some passes rely on it. Regardless of whether passes should rely on it,
it is totally unacceptable for the SIL passes to have subtle differences
in behavior depending on the frontend mode. So, if we don't want passes
to rely on global variable decls, that needs to be enforced by the API
independent of how the frontend is invoked or how SIL is serialized.
* Use custom DemangleOptions to lookup global variable identifiers.
introduce a common superclass, SILNode.
This is in preparation for allowing instructions to have multiple
results. It is also a somewhat more elegant representation for
instructions that have zero results. Instructions that are known
to have exactly one result inherit from a class, SingleValueInstruction,
that subclasses both ValueBase and SILInstruction. Some care must be
taken when working with SILNode pointers and testing for equality;
please see the comment on SILNode for more information.
A number of SIL passes needed to be updated in order to handle this
new distinction between SIL values and SIL instructions.
Note that the SIL parser is now stricter about not trying to assign
a result value from an instruction (like 'return' or 'strong_retain')
that does not produce any.
"Accessibility" has a different meaning for app developers, so we've
already deliberately excised it from our diagnostics in favor of terms
like "access control" and "access level". Do the same in the compiler
now that we aren't constantly pulling things into the release branch.
Rename AccessibilityAttr to AccessControlAttr and
SetterAccessibilityAttr to SetterAccessAttr, then track down the last
few uses of "accessibility" that don't have to do with
NSAccessibility. (I left the SourceKit XPC API alone because that's
supposed to be more stable.)
"Accessibility" has a different meaning for app developers, so we've
already deliberately excised it from our diagnostics in favor of terms
like "access control" and "access level". Do the same in the compiler
now that we aren't constantly pulling things into the release branch.
This commit changes the 'Accessibility' enum to be named 'AccessLevel'.
At some point, pass definitions were heavily macro-ized. Pass
descriptive names were added in two places. This is not only redundant
but a source of confusion. You could waste a lot of time grepping for
the wrong string. I removed all the getName() overrides which, at
around 90 passes, was a fairly significant amount of code bloat.
Any pass that we want to be able to invoke by name from a tool
(sil-opt) or pipeline plan *should* have unique type name, enum value,
commend-line string, and name string. I removed a comment about the
various inliner passes that contradicted that.
Side note: We should be consistent with the policy that a pass is
identified by its type. We have a couple passes, LICM and CSE, which
currently violate that convention.
There are now separate functions for function addition and deletion instead of InvalidationKind::Function.
Also, there is a new function for witness/vtable invalidations.
rdar://problem/29311657
Changes:
* Terminate all namespaces with the correct closing comment.
* Make sure argument names in comments match the corresponding parameter name.
* Remove redundant get() calls on smart pointers.
* Prefer using "override" or "final" instead of "virtual". Remove "virtual" where appropriate.
One minor revision: this lifts the proposed restriction against
overriding a non-open method with an open one. On reflection,
that was inconsistent with the existing rule permitting non-public
methods to be overridden with public ones. The restriction on
subclassing a non-open class with an open class remains, and is
in fact consistent with the existing access rule.
'fileprivate' is considered a broader level of access than 'private',
but for now both of them are still available to the entire file. This
is intended as a migration aid.
One interesting fallout of the "access scope" model described in
758cf64 is that something declared 'private' at file scope is actually
treated as 'fileprivate' for diagnostic purposes. This is something
we can fix later, once the full model is in place. (It's not really
/wrong/ in that they have identical behavior, but diagnostics still
shouldn't refer to a type explicitly declared 'private' as
'fileprivate'.)
As a note, ValueDecl::getEffectiveAccess will always return 'FilePrivate'
rather than 'Private'; for purposes of optimization and code generation,
we should never try to distinguish these two cases.
This should have essentially no effect on code that's /not/ using
'fileprivate' other than altered diagnostics.
Progress on SE-0025 ('fileprivate' and 'private')
...in code that I wrote. The integrated REPL, deprecated though it may
be, does not have an associated DeclContext because its SourceFile is
not considered complete. (The proper LLDB REPL does not suffer from
this problem because they use a new SourceFile for every block of
input.)
Elsewhere, tighten up code that may have hit similar bugs, though we
haven't seen anything hit these yet.
rdar://problem/26476281
Based on the review of my previous commit, I did some re-factorings.
The code is now simpler and handles more cases. More tests were added.
The overall idea of this rewrite is that the pass basically tries to check if it can
see all possible writes (i.e. initializations) into a given let property. Only if it can be proven
that the pass sees all possible writes and all those initializations are producing
the same constant, statically known value, the pass propagates this constant value
into uses of a property.
SR-1026 and rdar://25303106
The optimization pass was inspecting only init methods to determine if a given let property is defined
in the same way by all initializers. But this is not enough in certain cases, e.g. when some of the
initializers were inlined into the application code and the body of the inlined SIL function representing
such an initializer was removed afterwards by the dead function elimination pass. In such situations,
the Let Properties Optimization pass was assuming that there is only one initializer and considered the
constant let property value defined there as the only possible value of this let property. Therefore it
propagated it into let-property uses, which resulted in an incorrect code.
The right thing to do is to analyze all assignments to a given let property whether they are inside initializer
SIL functions or not. This makes sure that all possible values of a let property are analyzed and compared.
The propagation of a constant let property value can only happen if all found possible values are all the same.
Fixes SR-1026 and rdar://25303106
The optimization should not proceed if there is more than one assignment to a let property inside an initializer.
In this case, the value of the let property is considered unknown.
As there are no instructions left which produce multiple result values, this is a NFC regarding the generated SIL and generated code.
Although this commit is large, most changes are straightforward adoptions to the changes in the ValueBase and SILValue classes.
