The reason why I am doing this is that in certain cases the AST captures indices
will never actually line up with partial apply capture indices since we seem to
"smush" together closures and locally defined functions.
NOTE: The reason for the really small amount of test changes is that this change
does not change the actual output by design. The only cases I had to change were
a case where we began to emit a better diagnostic and also where I added code
coverage around _ and let _ since those require ignored_use to be implemented so
that they would be diagnosed (previously we just did not emit anything so we
couldn't emit the diagnostic at the SIL level).
rdar://142661388
In Embedded Swift, witness method lookup is done from specialized witness tables.
For this to work, the type of witness_method must be specialized as well.
Otherwise the method call would be done with wrong parameter conventions (indirect instead of direct).
This commit just introduces the instruction. In a subsequent commit, I am going
to add support to SILGen to emit this. This ensures that when we assign into a
tuple var we initialize it with one instruction instead of doing it in pieces.
The problem with doing it in pieces is that when one is emitting diagnostics it
looks semantically like SILGen actually is emitting code for initializing in
pieces which could be an error.
SelectEnumInstBase will be templated in the next commit.
Instead of using templated SelectEnumInstBase everywhere, introduce
a new wrapper type SelectEnumOperation.
This is phase-1 of switching from llvm::Optional to std::optional in the
next rebranch. llvm::Optional was removed from upstream LLVM, so we need
to migrate off rather soon. On Darwin, std::optional, and llvm::Optional
have the same layout, so we don't need to be as concerned about ABI
beyond the name mangling. `llvm::Optional` is only returned from one
function in
```
getStandardTypeSubst(StringRef TypeName,
bool allowConcurrencyManglings);
```
It's the return value, so it should not impact the mangling of the
function, and the layout is the same as `std::optional`, so it should be
mostly okay. This function doesn't appear to have users, and the ABI was
already broken 2 years ago for concurrency and no one seemed to notice
so this should be "okay".
I'm doing the migration incrementally so that folks working on main can
cherry-pick back to the release/5.9 branch. Once 5.9 is done and locked
away, then we can go through and finish the replacement. Since `None`
and `Optional` show up in contexts where they are not `llvm::None` and
`llvm::Optional`, I'm preparing the work now by going through and
removing the namespace unwrapping and making the `llvm` namespace
explicit. This should make it fairly mechanical to go through and
replace llvm::Optional with std::optional, and llvm::None with
std::nullopt. It's also a change that can be brought onto the
release/5.9 with minimal impact. This should be an NFC change.
drop_deinit only exists in ownership SIL. Remove IRGen support.
A drop_deinit can only ever be destroyed or destructured.
A destructure of a struct-with-deinit requires a drop_deinit operand.
This is a fundamental abstraction over loads. It was in
OwnershipOptUtils because load_borrow happens to be restricted by
OSSA. Passes should not include OwnershipOptUtils just because they
work with loads.
Rerun RLE with cutting off the base address of loads at `ref_element/tail_addr [immutable]`. This increases the chance of catching loads of immutable COW class properties or elements.
Define the possible runtime effects of an instruction in an enum `RuntimeEffect`.
Add a new utility `swift:getRuntimeEffect` to estimate the runtime effects of an instruction.
Also, add a mechanism to validate the correctness of the analysis in IRGen: annotate all runtime functions in RuntimeFunctions.def with the actual effect what the runtime function has or can have. Then check if the effects of emitted runtime functions for an instruction match what `getRuntimeEffect` predicts.
This check is only enabled on demand by defining the CHECK_RUNTIME_EFFECT_ANALYSIS macro in RuntimeEffect.h
Determines whether an address might be inside a borrowed scope. If so,
then any address substitution needs to find that scope boundary to
avoid violating its basic guarantee that all uses are within scope.
The reason why is that addresses from pointer_to_address never have transitive
interior pointer constraints from where ever the pointer originally came
from. This is the issue that was causing a CSE test to fail, so I added a test
to ossa_rauw_test that works this code path.
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.
This will ensure that if an expert user is using this feature and makes a
mistake as a result of tweaking their code, they get an error. This will ensure
they do not ship and look into why this is happening.
This is not intended to be used by anyone except for expert stdlib users.
Specifically, this transforms:
builtin "generic_add"<Builtin.Vec4xInt32>(
->
builtin "add_Vec4xInt32"(
If we do not have a static overload for the type, we just leave the generic call
alone. If the generic builtin takes addresses as its arguments (i.e. 2x
in_guaranteed + 1x out), we load the arguments, evaluate the static overloaded
builtin and then store the result into the out parameter.
Now we just always mark functions as having or not having ownership. So we know
when parsing/deserializing what state we are in already. There are also
assertions in SILBuilder around most of the instructions that can exist in only
a single mode, meaning even the error was sort of redundant.
I discovered this due to the mandatory inliner doing devirtualization. I ported
all of the relevant SIL tests to increase code coverage of this code when
ownership is enabled.
In the included, test case, the optimization was sinking
releases past is_escaping_closure.
Rewrite the isBarrier logic to be conservative and define the
mayCheckRefCount property in SIL/InstructionUtils. Properties that may
need to be updated when SIL changes belong there.
Note that it is particularly bad behavior if the presence of access
markers in the code cause miscompiles unrelated to access enforcement.
Fixes <rdar://problem/45846920> TestFoundation, TestProcess, closure
argument passed as @noescape to Objective-C has escaped.
With removing of pinning and with addressors, the pattern matching did not work anymore.
The good thing is that the SIL is now much simpler and we can handle the 2D case without pattern matching at all.
This removes a lot of code from COWArrayOpts.
rdar://problem/43863081
DiagnoseStaticExclusivity no longer asserts as follows when
non-escaping closures are passed to withoutActuallyEscaping:
Applied argument must be @noescape function type: ...
A partial_apply with @inout_aliasable may only be used as a @noescape
function type argument.
Subsequent commits will improve diagnostics to detect actual conflicts
in these situations.
Fixes <rdar://problem/43059088> Assertion in DiagnoseStaticExclusivity.
Add support to static diagnostics for tracking noescape closures through block
arguments.
Improve the noescape closure SIL verification logic to match. Cleanup noescape
closure handling in both diagnostics and SIL verification to be more
robust--they must perfectly match each other.
Fixes <rdar://problem/42560459> [Exclusivity] Failure to statically diagnose a
conflict when passing conditional noescape closures.
Initially reported in [SR-8266] Compiler crash when checking exclusivity of
inout alias.
Example:
struct S {
var x: Int
mutating func takeNoescapeClosure(_ f: ()->()) { f() }
mutating func testNoescapePartialApplyPhiUse(z : Bool) {
func f1() {
x = 1 // expected-note {{conflicting access is here}}
}
func f2() {
x = 1 // expected-note {{conflicting access is here}}
}
takeNoescapeClosure(z ? f1 : f2)
// expected-error@-1 2 {{overlapping accesses to 'self', but modification requires exclusive access; consider copying to a local variable}}
}
}
Added new files, MemAccessUtils.h and MemAccessUtils.cpp to house the
new utility. Three functions previously in InstructionUtils.h move
here:
- findAccessedAddressBase
- isPossibleFormalAccessBase
- visitAccessedAddress
Rather than working with SILValues, these routines now work with the
AccessedStorage abstraction. This allows enforcement logic and SIL
pattern recognition to be shared across diagnostics and
optimization. (It's very important for this to be consistent).
The new AccessedStorage utility is a superset of the class that was
local to DiagnoseStaticExclusivity. It exposes the full set of all
recognized kinds of storage. It also represents function arguments as
an index rather that a SILValue. This allows an analysis pass to
compare/merge AccessedStorage results from multiple callee functions,
or more naturally propagate from callee to caller context.
Create helpers in InstructionUtils.h wherever we need a guarantee that the diagnostics cover the same patterns as the verifier. Eventually this will be called from both SILVerifier and the diagnostic pass:
- findAccessedAddressBase
- isPossibleFormalAccessBase
- isPartialApplyOfReabstractionThunk
- findClosureForAppliedArg
- visitAccessedAddress
Add partial_apply verification assert.
This applies the normal "find a closure" logic inside the "find all partial_apply uses" verification. Making the verifier round-trip ensures that we don't have holes in exclusivity enforcement related to this logic.
SubstitutionList is going to be a more compact representation of
a SubstitutionMap, suitable for inline allocation inside another
object.
For now, it's just a typedef for ArrayRef<Substitution>.
Over the past day or so I have been thinking about how we are going to need to
manage verification of semantic ARC semantics in the pass pipeline. Specifically
the Eliminator pass really needs to be a function pass to ensure that we can
transparently put it at any stage of the optimization pipeline. This means that
just having a flag on the SILVerifier that states whether or not ownership is
enabled is not sufficient for our purposes. Instead, while staging in the SIL
ownership model, we need a bit on all SILFunctions to state whether the function
has been run through the ownership model eliminator so that the verifier can
ensure that we are in a world with "SIL ownership" or in a world without "SIL
ownership", never in a world with only some "SIL ownership" instructions. We
embed this distinction in SIL by creating the concept of a function with
"qualified ownership" and a function with "unqualified ownership".
Define a function with "qualified ownership" as a function that contains no
instructions with "unqualified ownership" (i.e. unqualified load) and a function
with "unqualified ownership" as a function containing such no "ownership
qualified" instructions (i.e. load [copy]) and at least 1 unqualified ownership
instruction.
This commit embeds this distinction into SILFunction in a manner that is
transparently ignored when compiling with SIL ownership disabled. This is done
by representing qualified or unqualified ownership via an optional Boolean on
SILFunction. If the Boolean is None, then SILOwnership is not enabled and the
verifier/passes can work as appropriate. If the Boolean is not None, then it
states whether or not the function has been run through the Ownership Model
Eliminator and thus what invariants the verifier should enforce.
How does this concept flow through the compilation pipeline for functions in a
given module? When SIL Ownership is enabled, all SILFunctions that are produced
in a given module start with "qualified ownership" allowing them to contain SIL
ownership instructions. After the Ownership Model eliminator has run, the
Ownership Model sets the "unqualified" ownership flag on the SILFunction stating
that no more ownership qualified instructions are allowed to be seen in the
given function.
But what about functions that are parsed or are deserialized from another
module? Luckily, given the manner in which we have categories our functions, we
can categorize functions directly without needing to add anything to the parser
or to the deserializer. This is done by enforcing that it is illegal to have a
function with qualified ownership and unqualified ownership instructions and
asserting that functions without either are considered qualified.
rdar://28685236
