[WIP] Support or property delegates in DI #1

eeckstein · 2019-04-04T19:35:27Z

This is a first prototype implementation.

I'd especially like to get some feedback for the SILGen part. I'm not sure if what I did there is really the right way to do it.

to-dos:

Fix the problem with non-mutable setters for class-properties (see commented-out test case). I talked to Doug about it: we might want to make the setter mutable in this case. TBD.
Remove the dead partial-applies/applies for the not used init/setter function after assign_by_delegate is lowered. In -O they are removed by optimizations, but not in -Onone.
Don't generate assign_by_delegate at all if it's not in an initializer. In such a case we should generate the setter call directly.
Add test cases, especially SIL test cases.

Used for property delegates.

slavapestov · 2019-04-05T18:24:36Z

lib/SILGen/SILGenApply.cpp

+                                                          SILDeclRef ctorRef,
+                                                          Type delegateTy,
+                                                    CanAnyFunctionType funcTy) {
+  Callee callee = Callee::forDirect(*this, ctorRef, subs, loc);


Is this always a direct call?

As far as I understand, yes.

What if the initializer is @objc dynamic? Do we need to diagnose that case?

slavapestov · 2019-04-05T18:25:47Z

lib/SILGen/SILGenLValue.cpp

+        VarDecl *field = dyn_cast<VarDecl>(Storage);
+        VarDecl *backingVar = field->getPropertyDelegateBackingVar();
+        assert(backingVar);
+        CanType ValType = backingVar->getType()->getCanonicalType();


If the initializer is itself generic this will be written in terms of the wrong archetypes. I think you should use the interface type here and substitute the right substitutions.

There are restrictions on how the initializer can look like.
To get the substitutions, I basically did the same as Doug did in SILGenConstructor.cpp: emitImplicitValueConstructor (e674377#diff-0d8359a6c47d13683f989e2f711454c5R174)

I think both copies of the code are not going to work when the initializer has different archetypes from the type. That’s why I wanted you to try an init with it’s own generic parameters.

Good point about the different substitutions though. It’s worth an explanatory comment.

FWIW, the form of init(initialValue:) is locked down to have the same generic signature as the type. I am (separately) turning this into an expression so the type checker forms all of the calls for us.

slavapestov · 2019-04-05T18:26:05Z

lib/SILGen/SILGenLValue.cpp

+                                  typeData);
+          proj = std::move(REC).project(SGF, loc, base);
+        } else {
+          assert(BaseFormalType->getStructOrBoundGenericStruct());


Can a delegate ever be implemented by an enum?

I don't think so

Yes, it's allowed for a delegate to be implemented by an enum. The Lazy example from the proposal is one such example.

slavapestov · 2019-04-05T18:26:44Z

lib/SILGen/SILGenLValue.cpp

+        // Create the allocating initializer function. It captures the metadata.
+        auto delegateInfo = getAttachedPropertyDelegateInfo(field);
+        auto ctor = delegateInfo.initialValueInit;
+        SubstitutionMap subs = backingVar->getType()->getMemberSubstitutionMap(


I think this should be passed in or stored inside the SILGenLValue and not derived from scratch?

What do you mean?

In the AST when you reference a member of a generic type, Sema records a substitution map in the MemberRefExpr. I think it’s cleaner to use that if you can. Sorry for being vague here, I should read some of the surrounding code in more detail and github’s review interface isn’t so useful for looking at code that’s not part of the PR itself!

slavapestov · 2019-04-05T18:27:27Z

lib/SILGen/SILGenLValue.cpp

+        // Create the allocating setter function. It captures the base address.
+        auto setterInfo = SGF.getConstantInfo(setter);
+        SILValue setterFRef = SGF.emitGlobalFunctionRef(loc, setter, setterInfo);
+        auto setterSubs = SGF.getFunction().getForwardingSubstitutionMap();


Perhaps this will just be the same as subs above -- its better to use that instead

No, these are different substitutions, because the initializer is from the delegate struct/class, the setter is from the containing struct/class (the setter doesn't have any substitutions, except the forwarding ones from SGF.getFunction()),

slavapestov · 2019-04-05T18:28:44Z

test/SILOptimizer/di-property-delegates.swift

+testIntClass()
+testRefStruct()
+testGenericClass()
+//testIntStructWithClassWrapper()


Also try a type that has an init<T>

DougGregor · 2019-04-11T21:36:20Z

I cherry-picked this over to swiftlang#23701

@inline

To display a failure message in the debugger, create a function in the debug info which has the name of the failure message. The debug location of the trap/cond_fail is then wrapped into this function and the function is declared as "inlined". In case the debugger stops at the trap instruction, it displays the inline function, which looks like the failure message. For example: * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_INSTRUCTION (code=EXC_I386_INVOP, subcode=0x0) frame #0: 0x0000000100000cbf a.out`testit3(_:) [inlined] Unexpectedly found nil while unwrapping an Optional value at test.swift:14:11 [opt] 11 12 @inline(never) 13 func testit(_ a: Int?) -> Int { -> 14 return a! 15 } 16 This change is currently not enabled by default, but can be enabled with the option "-Xllvm -enable-trap-debug-info". Enabling this feature needs some changes in lldb. When the lldb part is done, this option can be removed and the feature enabled by default.

@inline

To display a failure message in the debugger, create a function in the debug info which has the name of the failure message. The debug location of the trap/cond_fail is then wrapped into this function and the function is declared as "inlined". In case the debugger stops at the trap instruction, it displays the inline function, which looks like the failure message. For example: * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_INSTRUCTION (code=EXC_I386_INVOP, subcode=0x0) frame #0: 0x0000000100000cbf a.out`testit3(_:) [inlined] Unexpectedly found nil while unwrapping an Optional value at test.swift:14:11 [opt] 11 12 @inline(never) 13 func testit(_ a: Int?) -> Int { -> 14 return a! 15 } 16 This change is currently not enabled by default, but can be enabled with the option "-Xllvm -enable-trap-debug-info". Enabling this feature needs some changes in lldb. When the lldb part is done, this option can be removed and the feature enabled by default.

@inline

The implementation was done quite a while ago. Now, that we have support in lldb (swiftlang/llvm-project#773), we can enable it by default in the compiler. LLDB now shows the runtime failure reason, for example: * thread #1, queue = 'com.apple.main-thread', stop reason = Swift runtime failure: arithmetic overflow frame #1: 0x0000000100000f0d a.out`testit(a=127) at trap_message.swift:4 1 2 @inline(never) 3 func testit(_ a: Int8) -> Int8 { -> 4 return a + 1 5 } 6 For details, see swiftlang#25978 rdar://problem/51278690

- Properly clone and use debug scopes for all instructions in pullback functions. - Emit `debug_value` instructions for adjoint values. - Add debug locations and variable info to adjoint buffer allocations. - Add `TangentBuilder` (a `SILBuilder` subclass) to unify and simplify special emitter utilities for tangent vector code generation. More simplifications to come. Pullback variable inspection example: ```console (lldb) n Process 50984 stopped * thread #1, queue = 'com.apple.main-thread', stop reason = step over frame #0: 0x0000000100003497 main`pullback of foo(x=0) at main.swift:12:11 9 import _Differentiation 10 11 func foo(_ x: Float) -> Float { -> 12 let y = sin(x) 13 let z = cos(y) 14 let k = tanh(z) + cos(y) 15 return k Target 0: (main) stopped. (lldb) fr v (Float) x = 0 (Float) k = 1 (Float) z = 0.495846391 (Float) y = -0.689988375 ``` Resolves rdar://68616528 / SR-13535.

…est1 [CodeCompletion] Migrate some tests to batch completion test #1

* Synchronize both versions of actor_counters.swift test * Synchronize on Job address Make sure to synchronize on Job address (AsyncTasks are Jobs, but not all Jobs are AsyncTasks). * Add fprintf debug output for TSan acquire/release * Add tsan_release edge on task creation without this, we are getting false data races between when a task is created and immediately scheduled on a different thread. False positive for `Sanitizers/tsan/actor_counters.swift` test: ``` WARNING: ThreadSanitizer: data race (pid=81452) Read of size 8 at 0x7b2000000560 by thread T5: #0 Counter.next() <null>:2 (a.out:x86_64+0x1000047f8) #1 (1) suspend resume partial function for worker(identity:counters:numIterations:) <null>:2 (a.out:x86_64+0x100005961) #2 swift::runJobInEstablishedExecutorContext(swift::Job*) <null>:2 (libswift_Concurrency.dylib:x86_64+0x280ef) Previous write of size 8 at 0x7b2000000560 by main thread: #0 Counter.init(maxCount:) <null>:2 (a.out:x86_64+0x1000046af) #1 Counter.__allocating_init(maxCount:) <null>:2 (a.out:x86_64+0x100004619) #2 runTest(numCounters:numWorkers:numIterations:) <null>:2 (a.out:x86_64+0x100006d2e) #3 swift::runJobInEstablishedExecutorContext(swift::Job*) <null>:2 (libswift_Concurrency.dylib:x86_64+0x280ef) #4 main <null>:2 (a.out:x86_64+0x10000a175) ``` New edge with this change: ``` [4357150208] allocate task 0x7b3800000000, parent = 0x0 [4357150208] creating task 0x7b3800000000 with parent 0x0 [4357150208] tsan_release on 0x7b3800000000 <<< new release edge [139088221442048] tsan_acquire on 0x7b3800000000 [139088221442048] trying to switch from executor 0x0 to 0x7ff85e2d9a00 [139088221442048] switch failed, task 0x7b3800000000 enqueued on executor 0x7ff85e2d9a00 [139088221442048] enqueue job 0x7b3800000000 on executor 0x7ff85e2d9a00 [139088221442048] tsan_release on 0x7b3800000000 [139088221442048] tsan_release on 0x7b3800000000 [4357150208] tsan_acquire on 0x7b3800000000 counters: 1, workers: 1, iterations: 1 [4357150208] allocate task 0x7b3c00000000, parent = 0x0 [4357150208] creating task 0x7b3c00000000 with parent 0x0 [4357150208] tsan_release on 0x7b3c00000000 <<< new release edge [139088221442048] tsan_acquire on 0x7b3c00000000 [4357150208] task 0x7b3800000000 waiting on task 0x7b3c00000000, going to sleep [4357150208] tsan_release on 0x7b3800000000 [4357150208] tsan_release on 0x7b3800000000 [139088221442048] getting current executor 0x0 [139088221442048] tsan_release on 0x7b3c00000000 ... ``` rdar://78932849 * Add static_cast<Job *>() * Move TSan release edge to swift_task_enqueueGlobal() Move the TSan release edge from `swift_task_create_commonImpl()` to `swift_task_enqueueGlobalImpl()`. Task creation itself is not an event that needs synchronization, but rather that task creation "happens before" execution of that task on another thread. This edge is usually added when the task is scheduled via `swift_task_enqueue()` (which then usually calls `swift_task_enqueueGlobal()`). However, not all task scheduling goes through the `swift_task_enqueue()` funnel as some places call the more specific `swift_task_enqueueGlobal()` directly. So let's annotate this function (duplicate edges aren't harmful) to ensure we cover all schedule events, including newly-created tasks (our original problem here). rdar://78932849 Co-authored-by: Julian Lettner <[email protected]>

@thin

Case #1. Literal zero = natural alignment %1 = integer_literal $Builtin.Int64, 0 %2 = builtin "uncheckedAssertAlignment" (%0 : $Builtin.RawPointer, %1 : $Builtin.Int64) : $Builtin.RawPointer %3 = pointer_to_address %2 : $Builtin.RawPointer to [align=1] $*Int Erases the `pointer_to_address` `[align=]` attribute: Case #2. Literal nonzero = forced alignment. %1 = integer_literal $Builtin.Int64, 16 %2 = builtin "uncheckedAssertAlignment" (%0 : $Builtin.RawPointer, %1 : $Builtin.Int64) : $Builtin.RawPointer %3 = pointer_to_address %2 : $Builtin.RawPointer to [align=1] $*Int Promotes the `pointer_to_address` `[align=]` attribute to a higher value. Case #3. Folded dynamic alignment %1 = builtin "alignof"<T>(%0 : $@thin T.Type) : $Builtin.Word %2 = builtin "uncheckedAssertAlignment" (%0 : $Builtin.RawPointer, %1 : $Builtin.Int64) : $Builtin.RawPointer %3 = pointer_to_address %2 : $Builtin.RawPointer to [align=1] $*T Erases the `pointer_to_address` `[align=]` attribute.

Update DifferentiableProgrammingImplementation.md

Update bridging

Co-authored-by: Karoy Lorentey <[email protected]>

[Inclusive-Language] change comment with "sanity" to "soundness"

Local symbols

Windows: add Swift installer custom action

Add a new demangler option which excludes a closure's type signature. This will be used in lldb. Closures are not subject to overloading, and so the signature will never be used to disambiguate. A demangled closure is uniquely identifiable by its index(s) and parent. Where opaque types are involved, the concrete type signature can be quite complex. This demangling option allows callers to avoid printing the underlying complex nested concrete types. Example: before: `closure #1 (Swift.Int) -> () in closure #1 (Swift.Int) -> () in main` after: `closure #1 in closure #1 in main`

…wiftlang#73353) Add a new demangler option which excludes a closure's type signature. This will be used in lldb. Closures are not subject to overloading, and so the signature will never be used to disambiguate. A demangled closure is uniquely identifiable by its index(s) and parent. Where opaque types are involved, the concrete type signature can be quite complex. This demangling option allows callers to avoid printing the underlying complex nested concrete types. Example: before: `closure #1 (Swift.Int) -> () in closure #1 (Swift.Int) -> () in main` after: `closure #1 in closure #1 in main`

This inserts a suitably named function into the stack trace whenever a dynamic cast failure involves a NULL source or target type. Very often, crash logs include backtraces with function names but no log output; with this change, such a backtrace might look like the following -- note `TARGET_TYPE_NULL` in the function name here to mark the missing type information: ``` frame #0: __pthread_kill + 8 frame #1: pthread_kill + 288 frame #2: abort + 128 frame #3: swift::fatalErrorv() frame #4: swift::fatalError() frame swiftlang#5: swift_dynamicCastFailure_TARGET_TYPE_NULL() frame swiftlang#6: swift::swift_dynamicCastFailure() frame swiftlang#7: ::swift_dynamicCast() ``` Resolves rdar://130630157

…n's demangled name when processing it in RegionAnalysis. Just trying to improve logging to speed up triaging further. This is useful so that I can quickly find specific closures we process by using the closure numbering (e.x.: closure #1 in XXXX).

Two are fixes needed in most of the `RawSpan` and `Span` initializers. For example: ``` let baseAddress = buffer.baseAddress let span = RawSpan(_unchecked: baseAddress, byteCount: buffer.count) // As a trivial value, 'baseAddress' does not formally depend on the // lifetime of 'buffer'. Make the dependence explicit. self = _overrideLifetime(span, borrowing: buffer) ``` Fix #1. baseAddress needs to be a variable `span` has a lifetime dependence on `baseAddress` via its initializer. Therefore, the lifetime of `baseAddress` needs to include the call to `_overrideLifetime`. The override sets the lifetime dependency of its result, not its argument. It's argument still needs to be non-escaping when it is passed in. Alternatives: - Make the RawSpan initializer `@_unsafeNonescapableResult`. Any occurrence of `@_unsafeNonescapableResult` actually signals a bug. We never want to expose this annotation. In addition to being gross, it would totally disable enforcement of the initialized span. But we really don't want to side-step `_overrideLifetime` where it makes sense. We want the library author to explicitly indicate that they understand exactly which dependence is unsafe. And we do want to eventually expose the `_overrideLifetime` API, which needs to be well understood, supported, and tested. - Add lifetime annotations to a bunch of `UnsafePointer`-family APIs so the compiler can see that the resulting pointer is derived from self, where self is an incoming `Unsafe[Buffer]Pointer`. This would create a massive lifetime annotation burden on the `UnsafePointer`-family APIs, which don't really have anything to do with lifetime dependence. It makes more sense for the author of `Span`-like APIs to reason about pointer lifetimes. Fix #2. `_overrideLifetime` changes the lifetime dependency of span to be on an incoming argument rather than a local variable. This makes it legal to escape the function (by assigning it to self). Remember that self is implicitly returned, so the `@lifetime(borrow buffer)` tells the compiler that `self` is valid within `buffer`'s borrow scope.

This is the missing check for "rule #1" in the isolated conformances proposal, which states that an isolated conformance can only be referenced within the same isolation domain as the conformance. For example, a main-actor-isolated conformance can only be used within main-actor code.

…g#81643) which generates IR without a llvm.trap function All the normal CI generated this: ``` ret i32 %1 } ; Function Attrs: cold noreturn nounwind memory(inaccessiblemem: write) declare void @llvm.trap() #1 attributes #0 = { "frame-pointer"= ``` But the test-simulator CI doesn't for some reason, so just check for the closing brace instead.

eeckstein added 5 commits April 4, 2019 11:24

SIL: add assign_by_delegate instruction

62050d5

Used for property delegates.

DI: support assign_by_delegate instruction

a00f8b1

Fix an assert in AccessEnformentSelections to support assign_by_delegate

582035e

RawSILInstLowering: lower assign_by_delegate

bf2a80f

SILGen: generate assign_by_delegate for property delegates

3a10238

slavapestov reviewed Apr 5, 2019

View reviewed changes

eeckstein closed this Aug 20, 2019

DougGregor pushed a commit that referenced this pull request Apr 6, 2021

Merge pull request swiftlang#36714 from rintaro/ide-completion-batcht…

876a0ee

…est1 [CodeCompletion] Migrate some tests to batch completion test #1

eeckstein deleted the property-delegates2 branch April 17, 2021 15:04

DougGregor pushed a commit that referenced this pull request Dec 21, 2021

Merge pull request #1 from philipturner/philipturner-patch-4-pt-1

b5e151c

Update DifferentiableProgrammingImplementation.md

DougGregor pushed a commit that referenced this pull request Nov 12, 2023

Merge pull request #1 from Genaro-Chris/Genaro-Chris-patch-1

49ea89d

Update bridging

DougGregor pushed a commit that referenced this pull request Jan 19, 2024

Take doc suggestion from Karoy #1

f28ebc9

Co-authored-by: Karoy Lorentey <[email protected]>

DougGregor pushed a commit that referenced this pull request Feb 7, 2024

Merge pull request #1 from Serena748/Serena748/inclusive-language

141b4d0

[Inclusive-Language] change comment with "sanity" to "soundness"

DougGregor pushed a commit that referenced this pull request Feb 26, 2024

Merge pull request #1 from skrtks/local-symbols

b97c636

Local symbols

DougGregor pushed a commit that referenced this pull request Apr 28, 2024

Merge pull request #1 from compnerd/Windows-CustomActions

9e2c19f

Windows: add Swift installer custom action

[WIP] Support or property delegates in DI #1

[WIP] Support or property delegates in DI #1

Uh oh!

Conversation

eeckstein commented Apr 4, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DougGregor commented Apr 11, 2019

Uh oh!

Uh oh!