A SIL argument index was being passed as an AST function type
parameter index. Don't do this.
I also cleaned up the peephole to avoid processing indirect results as
parameter indices, but that part is NFC.
Fixes rdar://45415719 Assertion failed: (Index < Length && "Invalid index!")
To do so this commit does a few different things:
1. I changed SILOptFunctionBuilder to notify the pass manager's logging
functionality when new functions are added to the module and to notify analyses
as well. NOTE: This on purpose does not put the new function on the pass manager
worklist since we do not want to by mistake introduce a large amount of
re-optimizations. Such a thing should be explicit.
2. I eliminated SILModuleTransform::notifyAddFunction. This just performed the
operations from 1. Now that SILOptFunctionBuilder performs this operation for
us, it is not needed.
3. I changed SILFunctionTransform::notifyAddFunction to just add the function to
the passmanager worklist. It does not need to notify the pass manager's logging
or analyses that a new function was added to the module since
SILOptFunctionBuilder now performs that operation. Given its reduced
functionality, I changed the name to addFunctionToPassManagerWorklist(...). The
name is a little long/verbose, but this is a feature since one should think
before getting the pass manager to rerun transforms on a function. Also, giving
it a longer name calls out the operation in the code visually, giving this
operation more prominance when reading code. NOTE: I did the rename using
Xcode's refactoring functionality!
rdar://42301529
I am going to add the code in a bit that does the notifications. I tried to pass
down the builder instead of the pass manager. I also tried not to change the
formatting.
rdar://42301529
All this does is automate the creation of the ${DIRNAME}_SOURCES variables that we already create and allows for the author to avoid having to prefix with the directory name, i.e.:
set(FOOBAR_SOURCES
FooBar/Source.cpp
PARENT_SCOPE)
=>
silopt_register_sources(
Source.cpp)
Much easier and cleaner to read. I put the code that implements this in the
CMakeLists.txt file just for the SILOptimizer.
Fixes <rdar://40555427> [SR-7773]:
SILCombiner::propagateConcreteTypeOfInitExistential fails to full propagate type
substitutions.
Fixes <rdar://problem/40923849>
SILCombiner::propagateConcreteTypeOfInitExistential crashes on protocol
compositions.
This rewrite fixes several fundamental bugs in the SILCombiner optimization that
propagates concrete types. In particular, the pass needs to handle:
- Arguments of callee Self type in non-self position.
- Indirect and direct return values of Self type.
- Types that indirectly depend on Self within callee function signature.
- Protocol composition existentials.
- All of the above need to work for protocol extensions as well as witness methods.
- For protocol extensions, conformance lookup should be based on the existential's conformance list.
Additionally, the optimization should not depend on a SILFunction's DeclContext,
which is not serialized. (In fact, we should prevent SIL passes from using
DeclContext). Furthermore, the code needs to be expressed in a way that one can
reason about correctness and invariants.
The root cause of these bugs is that SIL passes are written based on untested
assumptions of Swift type system. A SIL pass needs to handle all verifiable SIL
input because passes need to be composable. Bail-out logic can be added to
simplify the design; however, _the bail-out logic itself cannot make any
assumptions about the language or type system_ that aren't clearly and
explicitly enforced in the SIL verifier. This is a common mistake and major
source of bugs.
I created as many unit tests as I reasonably could to prevent this code from
regressing. Creating enough unit tests to cover all corner cases that were
broken in the original code would be intractable. But the code has been
simplified such that many corner cases disappear.
This opens up some oportunity for generalizing the optimization and eliminating
special cases. However, I want this PR to be limited to fixing correctness
issues only. In the long term, it would be preferable to replace this
optimization entirely with a much more powerful general type propagation pass.
For example:
%0 = string_literal "abc"
%1 = integer_literal 0x8000000000000000
%2 = builtin "stringObjectOr_Int64" (%0, %1)
%3 = integer_literal 0x4000000000000000
%4 = builtin "and_Int64" (%2, %3)
In this case we know that %4 is 0.
We can always eliminate ARC operations on the result of a value_to_bridge_object instruction.
There is no need to restrict the operand to a specific SIL pattern.
Introduced during the bring-up of the generics system in July, 2012,
Substitution (and SubstitutionList) has been completely superseded by
SubstitutionMap. R.I.P.
This optimization inserted bit casts based on structural properties of types.
It caused a miscompile in case of imported C structs. Also, by inspection, I found that checks for resilient types are missing.
As this optimization does not have any noticeable impact on the benchmarks it's better to remove it at all, together with the complexity for checking the types.
rdar://problem/40074362
This is a property of an instruction and should be a member
function of `SILInstruction` and not a free function in
`DebugUtils`. Discussed with Adrian.
Certain patterns of directly applied partial_apply's were not being
inlined. This can happen when a closure is defined in the
implementation of a generic witness method. I noticed this issue while
debugging SR-6254: "Fix (or explain) strange ways to make code >3
times faster/slower".
After the witness method is devirtualization and specialized, the
closure application looks like this:
%pa = partial_apply [callee_guaranteed] %fn() : $@convention(thin) (@in Int) -> @out Int
%cvt = convert_escape_to_noescape %pa
apply %cvt(...)
SILCombine already removes the partial_apply and convert_escape_to_noescape generating:
%thick = thin_to_thick %fn
apply %thick(...)
However, surprisingly, neither the inliner nor SILCombine can handle this.
I cleaned up the code in SILCombine's apply visitor that handles
thin_to_thick and generalized it to handle this and other patterns.
I also added a restriction to this code so that it doesn't break SIL
ownership in the future, as I discussed with Arnold.
Certain patterns of directly applied partial_apply's were not being
inlined. This can happen when a closure is defined in the
implementation of a generic witness method. I noticed this issue while
debugging SR-6254: "Fix (or explain) strange ways to make code >3
times faster/slower".
After the witness method is devirtualization and specialized, the
closure application looks like this:
%pa = partial_apply [callee_guaranteed] %fn() : $@convention(thin) (@in Int) -> @out Int
%cvt = convert_escape_to_noescape %pa
apply %cvt(...)
SILCombine already removes the partial_apply and convert_escape_to_noescape generating:
%thick = thin_to_thick %fn
apply %thick(...)
However, surprisingly, neither the inliner nor SILCombine can handle this.
I cleaned up the code in SILCombine's apply visitor that handles
thin_to_thick and generalized it to handle this and other patterns.
I also added a restriction to this code so that it doesn't break SIL
ownership in the future, as I discussed with Arnold.
This patch both makes debug variable information it optional on
alloc_stack and alloc_box instructions, and forced variable
information on debug_value and debug_value_addr instructions. The
change of the interface uncovered a plethora of bugs in SILGen,
SILTransform, and IRGen's LoadableByAddress pass.
Most importantly this fixes the previously commented part of the
DebugInfo/local-vars.swift.gyb testcase.
rdar://problem/37720555
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.
