Commit Graph

22 Commits

Author SHA1 Message Date
Meghana Gupta
984f9f6cd2 Remove unreachable blocks after inlining 2024-12-10 17:01:11 -08:00
Erik Eckstein
7cceaff5f3 SIL: don't print operand types in textual SIL
Type annotations for instruction operands are omitted, e.g.

```
  %3 = struct $S(%1, %2)
```

Operand types are redundant anyway and were only used for sanity checking in the SIL parser.

But: operand types _are_ printed if the definition of the operand value was not printed yet.
This happens:

* if the block with the definition appears after the block where the operand's instruction is located

* if a block or instruction is printed in isolation, e.g. in a debugger

The old behavior can be restored with `-Xllvm -sil-print-types`.
This option is added to many existing test files which check for operand types in their check-lines.
2024-11-21 18:49:52 +01:00
Nate Chandler
71239d6357 [CoroutineAccessors] SIL represents callee alloc.
When its operand has coroutine kind `yield_once_2`, a `begin_apply`
instruction produces an additional value representing the storage
allocated by the callee.  This storage must be deallocated by a
`dealloc_stack` on every path out of the function.  Like any other stack
allocation, it must obey stack discipline.
2024-10-11 08:25:03 -07:00
Nate Chandler
5307ac666e [SIL] Permit end_borrow(begin_apply). 2024-04-25 17:03:41 -07:00
Anton Korobeynikov
d84847ac9d Reland Allow normal function results of @yield_once coroutines (#71645)
* Allow normal function results of @yield_once coroutines

* Address review comments

* Workaround LLVM coroutine codegen problem: it assumes that unwind path never returns.
This is not true to Swift coroutines as unwind path should end with error result.
2024-03-27 13:09:02 -07:00
Nate Cook
e317febc9d Revert "Allow normal function results of @yield_once coroutines (#69843)"
This reverts commit aa5b505014.
2024-02-07 14:57:31 -06:00
Anton Korobeynikov
aa5b505014 Allow normal function results of @yield_once coroutines (#69843)
This adds SIL-level support and LLVM codegen for normal results of a coroutine.

The main user of this will be autodiff as VJP of a coroutine must be a coroutine itself (in order to produce the yielded result) and return a pullback closure as a normal result.

For now only direct results are supported, but this seems to be enough for autodiff purposes.
2024-02-06 22:13:15 -08:00
Nate Chandler
182bd34427 [SILInliner] Borrow guaranteed yields.
When inlining a begin_apply, if one of the values is yielded by
guaranteed convention, if the yielded value is itself owned, borrow it
during inlining.

Doing so is necessary because users of the yielded value are expecting a
value with guaranteed ownership.  For example, it's valid to
store_borrow such a value but not an owned value.
2022-11-28 16:53:03 -08:00
Meghana Gupta
2658cbec9b Fix begin_apply inlining when there are no yields
In this PR, preFixUp function in SILCloner is added which can be
overidden by implementations so that the SIL is cleaned for `commonFixup` processing.
For begin_apply inlining, blocks split due to end_apply and abort_apply
are fixed when no yields are found.
2021-10-08 14:45:08 -07:00
Michael Gottesman
f854547c55 [ownership] Enable ownership verification by default.
I also removed the -verify-sil-ownership flag in favor of a disable flag
-disable-sil-ownership-verifier. I used this on only two tests that still need
work to get them to pass with ownership, but whose problems are well understood,
small corner cases. I am going to fix them in follow on commits. I detail them
below:

1. SILOptimizer/definite_init_inout_super_init.swift. This is a test case where
DI is supposed to error. The only problem is that we crash before we error since
the code emitting by SILGen to trigger this error does not pass ownership
invariants. I have spoken with JoeG about this and he suggested that I fix this
earlier in the compiler. Since we do not run the ownership verifier without
asserts enabled, this should not affect compiler users. Given that it has
triggered DI errors previously I think it is safe to disable ownership here.

2. PrintAsObjC/extensions.swift. In this case, the signature generated by type
lowering for one of the thunks here uses an unsafe +0 return value instead of
doing an autorelease return. The ownership checker rightly flags this leak. This
is going to require either an AST level change or a change to TypeLowering. I
think it is safe to turn this off since it is such a corner case that it was
found by a test that has nothing to do with it.

rdar://43398898
2019-03-25 00:11:52 -07:00
Michael Gottesman
2a9fbe159a [ownership] Enable ownership verification on inline_begin_apply and fix some code. 2019-03-24 19:34:34 -07:00
Erik Eckstein
e433759e73 SILOptimizer: fix a stupid bug in StackNesting which can cause a miscompile in functions with unreachable blocks.
rdar://problem/47973577
2019-02-27 10:17:46 -08:00
Erik Eckstein
767ad5e70a Inliner: fix a stack nesting problem when inlining coroutines
Beside fixing the compiler crash, this change also improves the stack-nesting correction mechanisms in the inliners:

* Instead of trying to correct the nesting after each inlining of a callee, correct the nesting once when inlining is finished for a caller function.
This fixes a potential compile time problem, because StackNesting iterates over the whole function.
In worst case this can lead to quadratic behavior in case many begin_apply instructions with overlapping stack locations are inlined.

* Because we are doing it only once for a caller, we can remove the complex logic for checking if it is necessary.
We can just do it unconditionally in case any coroutine gets inlined.
The inliners iterate over all instruction of a function anyway, so this does not increase the computational complexity (StackNesting is roughly linear with the number of instructions).

rdar://problem/47615442
2019-02-01 08:32:19 -08:00
Michael Gottesman
40a09c9c21 Fixup tests for -assume-parsing-unqualified-ownership-sil => [ossa] transition. 2018-12-18 00:49:32 -08:00
Michael Gottesman
0af0d5fddc [ownership] Replace ValueOwnershipKind::Trivial with ValueOwnershipKind::Any.
In a previous commit, I banned in the verifier any SILValue from producing
ValueOwnershipKind::Any in preparation for this.

This change arises out of discussions in between John, Andy, and I around
ValueOwnershipKind::Trivial. The specific realization was that this ownership
kind was an unnecessary conflation of the a type system idea (triviality) with
an ownership idea (@any, an ownership kind that is compatible with any other
ownership kind at value merge points and can only create). This caused the
ownership model to have to contort to handle the non-payloaded or trivial cases
of non-trivial enums. This is unnecessary if we just eliminate the any case and
in the verifier separately verify that trivial => @any (notice that we do not
verify that @any => trivial).

NOTE: This is technically an NFC intended change since I am just replacing
Trivial with Any. That is why if you look at the tests you will see that I
actually did not need to update anything except removing some @trivial ownership
since @any ownership is represented without writing @any in the parsed sil.

rdar://46294760
2018-12-04 23:01:43 -08:00
Andrew Trick
9c46b2a053 Fix block merging after inlining to help avoid quadratic inlining.
A recent SILCloner rewrite removed a special case hack for single
basic block callee functions:

commit c6865c0dff
Merge: 76e6c4157e 9e440d13a6
Author: Andrew Trick <atrick@apple.com>
Date:   Thu Oct 11 14:23:32 2018

    Merge pull request #19786 from atrick/silcloner-cleanup

    SILCloner and SILInliner rewrite.

Instead, the new inliner simply merges trivial unconditional branches
after inlining the return block. This way, the CFG is always in
canonical state after inlining. This is more robust, and avoids
interfering with subsequent SIL passes when non-single-block callees
are inlined.

The problem is that inlining a series of calls within a large block
could result in interleaved block splitting and merging operations,
which is quadratic in the block size. This showed up when inlining the
tens of thousands of array subscript calls emitted for a large array
initialization.

The first half of the fix is to simply defer block merging until all
calls are inlined. We can't expect SimplifyCFG to run immediately
after inlining, nor would we want to do that, *especially* for
mandatory inlining. This fix instead exposes block merging as a
trivial utility.

Note: by eliminating some unconditional branches, this change could
reduce the number of debug locations emitted. This does not
fundamentally change any debug information guarantee, and I was unable
to observe any behavior difference in the debugger.
2018-11-26 09:41:28 -08:00
John McCall
18a8ed6090 Fix a stack-discipline bug with inlining coroutines. 2018-11-05 17:14:29 -05:00
Andrew Trick
bd28b0ea1b SILCloner and SILInliner rewrite.
Mostly functionally neutral:
- may fix latent bugs.
- may reduce useless basic blocks after inlining.

This rewrite encapsulates the cloner's internal state, providing a
clean API for the CRTP subclasses. The subclasses are rewritten to use
the exposed API and extension points. This makes it much easier to
understand, work with, and extend SIL cloners, which are central to
many optimization passes. Basic SIL invariants are now clearly
expressed and enforced. There is no longer a intricate dance between
multiple levels of subclasses operating on underlying low-level data
structures. All of the logic needed to keep the original SIL in a
consistent state is contained within the SILCloner itself. Subclasses
only need to be responsible for their own modifications.

The immediate motiviation is to make CFG updates self-contained so
that SIL remains in a valid state. This will allow the removal of
critical edge splitting hacks and will allow general SIL utilities to
take advantage of the fact that we don't allow critical edges.

This rewrite establishes a simple principal that should be followed
everywhere: aside from the primitive mutation APIs on SIL data types,
each SIL utility is responsibile for leaving SIL in a valid state and
the logic for doing so should exist in one central location.

This includes, for example:
- Generating a valid CFG, splitting edges if needed.
- Returning a valid instruction iterator if any instructions are removed.
- Updating dominance.
- Updating SSA (block arguments).

(Dominance info and SSA properties are fundamental to SIL verification).

LoopInfo is also somewhat fundamental to SIL, and should generally be
updated, but it isn't required.

This also fixes some latent bugs related to iterator invalidation in
recursivelyDeleteTriviallyDeadInstructions and SILInliner. Note that
the SILModule deletion callback should be avoided. It can be useful as
a simple cache invalidation mechanism, but it is otherwise bug prone,
too limited to be very useful, and basically bad design. Utilities
that mutate should return a valid instruction iterator and provide
their own deletion callbacks.
2018-10-08 19:30:09 -07:00
John McCall
8613761487 Fix some edge cases when inlining coroutines.
The current inlining strategy doesn't support inlining coroutines
when there are multiple end_apply or abort_apply instructions in
the caller, so refuse to inline such cases.  Also, handle the case
where there are no yield instructions in the callee, which can
happen if e.g. the callee calls a no-return function.

I also simplified the code somewhat by removing the vestiges of the
code that tried to unify control flow with switches.

As an unrelated fix, suppress function signature optimization for
coroutines for now.
2018-08-20 19:23:11 -04:00
Arnold Schwaighofer
f40a13a925 SILInliner: Disable inlining mulitple yields and don't create a temporary for yielded address values
Just use the yielded address from the callee directly. This also allows
handling yielded 'inout'/'out' values.
2018-08-02 14:57:03 -07:00
Michael Gottesman
be568902f2 [ownership] Always print out ownership argument annotations whether or not -enable-sil-ownership is passed in.
This is how we originally controlled whether or not we printed out ownership
annotations when we printed SIL. Since then, I have changed (a few months ago I
believe) the ownership model eliminator to know how to eliminate these
annotations from the SIL itself. So this hack can be removed.

As an additional benefit, this will let me rename -enable-sil-ownership to
-enable-sil-ownership-verifier. This will I hope eliminate confusion around this
option in the short term while I am preparing to work on semantic sil again.

rdar://42509812
2018-07-24 13:18:37 -07:00
Arnold Schwaighofer
dfec52b332 SILInliner: Initial support for begin_apply
rdar://35399839
2018-06-06 13:40:16 -07:00