SIL: Use a TypeExpansionContext in SIL lowering to expand opaque type archetypes as part of lowering #28044

aschwaighofer · 2019-11-03T23:00:29Z

As part of SIL type lowering opaque archetypes should be expanded to
their underlying types depending on the context -- the function they are
in. This means there is only ever one SIL type for an opaque archetype
AST type inside a function body: either the original opaque archetype or
an underlying type substitution but not both.

This fixes ambiguities in type substitution that would be introduced by
specializing opaque archetypes.

protocol Q {
  associatedtype Assoc
  func bind() -> Assoc
}
struct S : Q {
  func bind() -> some P {
    return 0
  }
}

apply %func<some P, S>(...) : $@convention(method) <t_0_0><t_0_1 where t_0_0 == t_1_0.Assoc, t_1_0 : Q>

  specialization (some P => Int)

apply %func<Int, S>(...) : $@convention(method) <t_0_0><t_0_1 where t_0_0 == t_1_0.Assoc, t_1_0 : Q>

  error: Int != S.Assoc (some P)

rdar://52706501

aschwaighofer · 2019-11-03T23:00:38Z

@swift-ci Please test

aschwaighofer · 2019-11-03T23:00:50Z

@swift-ci Please test source compat

swift-ci · 2019-11-03T23:46:22Z

Build failed
Swift Test Linux Platform
Git Sha - 47e15b4512dcb598022c8d4b4da62dd342956c50

swift-ci · 2019-11-03T23:58:12Z

Build failed
Swift Test OS X Platform
Git Sha - 47e15b4512dcb598022c8d4b4da62dd342956c50

aschwaighofer · 2019-11-04T15:43:02Z

test with swiftlang/llvm-project#144
@swift-ci please test

aschwaighofer · 2019-11-04T15:44:29Z

test with swiftlang/llvm-project#144
@swift-ci please test

aschwaighofer · 2019-11-04T15:56:23Z

test with swiftlang/llvm-project#144
@swift-ci please test

swift-ci · 2019-11-04T15:56:43Z

Build failed
Swift Test Linux Platform
Git Sha - 159f9886ba87126bf75f240bf16693106ee3a3b6

swift-ci · 2019-11-04T15:56:54Z

Build failed
Swift Test OS X Platform
Git Sha - 159f9886ba87126bf75f240bf16693106ee3a3b6

aschwaighofer · 2019-11-04T16:43:21Z

test with swiftlang/llvm-project#144
@swift-ci please test

aschwaighofer · 2019-11-04T20:24:49Z

test with swiftlang/llvm-project#144
@swift-ci please test

swift-ci · 2019-11-04T20:27:30Z

Build failed
Swift Test Linux Platform
Git Sha - 05db35fb55b1e433449e459a5cecdebd9428fec7

swift-ci · 2019-11-04T20:29:50Z

Build failed
Swift Test OS X Platform
Git Sha - 05db35fb55b1e433449e459a5cecdebd9428fec7

aschwaighofer · 2019-11-04T20:32:27Z

@swift-ci Please test source compatibility

aschwaighofer · 2019-11-04T20:37:42Z

@swift-ci Please test source compatibility

aschwaighofer · 2019-11-04T22:22:10Z

@swift-ci Please test os x

swift-ci · 2019-11-05T00:17:25Z

Build failed
Swift Test OS X Platform
Git Sha - 291b5ed2d268105db9a18d415b21cc5c7397cf6a

aschwaighofer · 2019-11-05T00:38:29Z

test with swiftlang/llvm-project#144
@swift-ci Please test os x

aschwaighofer · 2019-11-05T15:11:20Z

@swift-ci Please test source compat

jckarter · 2019-11-07T04:01:47Z

include/swift/SIL/SILInstruction.h

-  FunctionRefInst(SILDebugLocation DebugLoc, SILFunction *F);
+  /// \param context   The type expansion context of the function reference.
+  FunctionRefInst(SILDebugLocation DebugLoc, SILFunction *F,
+                  TypeExpansionContext context);


For instructions and basic block arguments, could they get their TypeExpansionContext from the containing SILFunction?

SILInstructions don't have access to their parent basic block (or SILFunction) on creation. The TypeExpansionContext is needed at creation to compute the SILType of the instruction.

Makes sense, I see that the SILBuilder APIs haven't changed and use the builder's current context too. Thanks!

jckarter · 2019-11-07T04:03:31Z

lib/AST/Type.cpp

+
+bool TypeBase::hasOpaqueArchetypePropertiesOrCases() {
+  if (hasOpaqueArchetype())
+    return true;


A type could be parameterized by an opaque archetype, but not use that type parameter in any of its fields or cases. Was this intentional?

Yes. But I admit it is confusing. I will remove this hasOpaqueArchetype check in TypeBase and move it to the users of this API.

The place in TypeLowering that uses this predicate wants to know about either. It is used to map different type expansion context type lowerings to one map entry if we know that a type does not contain any opaque archetypes and is not resilient in any resilience domain.

jckarter · 2019-11-07T04:09:02Z

lib/IRGen/GenClass.h

-                                                 VarDecl *field);
+
+  OwnedAddress projectPhysicalClassMemberAddress(
+      TypeExpansionContext context, IRGenFunction &IGF, llvm::Value *base,


If a SILFunction always has a particular TypeExpansionContext, then similarly, maybe IRGenFunction could as well. For IRGenSILFunction it could use the context from the function being lowered, and for other IGFs, maybe we could add it as a constructor argument?

…though by IRGen time, maybe we can also always use a maximal TypeExpansionContext, since we don't have to worry about cross-module SIL inlining anymore?

You are right.
I have IRGenModule::getMaximalTypeExpansionContext() which I should have used here instead.

aschwaighofer · 2019-11-08T16:13:17Z

@swift-ci Please test

swift-ci · 2019-11-08T16:15:38Z

Build failed
Swift Test Linux Platform
Git Sha - 291b5ed2d268105db9a18d415b21cc5c7397cf6a

swift-ci · 2019-11-08T16:17:25Z

Build failed
Swift Test OS X Platform
Git Sha - 291b5ed2d268105db9a18d415b21cc5c7397cf6a

aschwaighofer · 2019-11-08T18:21:01Z

test with swiftlang/llvm-project#144
@swift-ci test

aschwaighofer · 2019-11-11T19:33:59Z

@swift-ci Please test source compat

Also add hasOpaqueArchetypePropertiesOrCases() this function will be needed by SIL type lowering to determine whether types containing opaque archetype stored properties or cases should be considered as potentially address types.

…by the type expansion context

aschwaighofer · 2019-11-12T13:36:38Z

test with swiftlang/llvm-project#144
@swift-ci test

swift-ci · 2019-11-12T13:38:53Z

Build failed
Swift Test OS X Platform
Git Sha - 07224f2b923cf99c2d74e0867598bf59363a0e35

swift-ci · 2019-11-12T13:39:02Z

Build failed
Swift Test Linux Platform
Git Sha - 07224f2b923cf99c2d74e0867598bf59363a0e35

aschwaighofer · 2019-11-12T15:44:44Z

test with swiftlang/llvm-project#144
@swift-ci test os x

swift-ci · 2019-11-12T17:39:44Z

Build failed
Swift Test OS X Platform
Git Sha - f3a5d69

aschwaighofer · 2019-11-12T18:13:26Z

test with swiftlang/llvm-project#144
@swift-ci test os x

Back in 33f4f57 of swiftlang#28044 fame, non-`CaptureKind::Constant:` uses of `Entry.val` in `SILGenFunction::emitCaptures` were replaced with a use of the newly added lambda `getAddressValue` applied at `Entry.val`. The replacement of `Entry.value` with `getAddressValue(Entry.value)` in the case of `CaptureKind::Box` had no effect. Back then, the reason was that the condition SGM.Types .getTypeLowering( valueType, TypeExpansionContext::noOpaqueTypeArchetypesSubstitution( expansion.getResilienceExpansion())) .isAddressOnly() && !entryValue->getType().isAddress() under which something other than the input was returned would never hold: CaptureKind::Box is used for LValues and those never satisfy `!entryValue->getType().isAddress()`. Since that PR, the getAddressValue lambda has grown. There are two additional aspects to consider: (1) forceCopy, (2) isPack. (1) is not relevant because `false` was being passed for the `CaptureKind::Box` case. (2) can not currently happen because pack lvalues haven't been implemented. But even if they were implemented, the argument passed to the lambda would still need to be an address. The same all holds for the `CaptureKind::ImmutableBox` case which only differs from the `CaptureKind::Box` by a couple of lines.

aschwaighofer force-pushed the sil_type_expansion_context branch from 47e15b4 to 159f988 Compare November 4, 2019 15:42

aschwaighofer mentioned this pull request Nov 4, 2019

Adjust to TypeExpansionContext change swiftlang/llvm-project#144

Merged

aschwaighofer force-pushed the sil_type_expansion_context branch from 05db35f to 291b5ed Compare November 4, 2019 20:20

aschwaighofer requested review from slavapestov and jckarter and removed request for slavapestov November 5, 2019 15:40

jckarter mentioned this pull request Nov 5, 2019

SIL: Type lowering for function values with substituted SIL function types. #28065

Merged

jckarter reviewed Nov 7, 2019

View reviewed changes

aschwaighofer added 15 commits November 11, 2019 14:21

Add TypeExpansionContext

d9a575c

Add an API to map substitution map into a type expansion context.

f5fe7c1

Add TypeExpansionContext to AST substitution functions

8ec3a77

Also add hasOpaqueArchetypePropertiesOrCases() this function will be needed by SIL type lowering to determine whether types containing opaque archetype stored properties or cases should be considered as potentially address types.

SIL: Add TypeExpansion to SIL type lowering

6b3e1b3

SIL: Add getTypeExpansionContext to SILFunction

e67b961

SIL: Plumb TypeExpansionContext through SIL

9ecda0c

Add TypeExpansionContext to SIL serialization and parsing

e997110

SILGen: Add TypeExpansionContext to SILGen

33f4f57

SILOptimizer: Pipe through TypeExpansionContext

8aaa7b4

IRGen: Add TypeExpansionContext to IRGen

4cba763

Update tests

4f300bb

Address review comment TypeBase::hasOpaqueArchetypePropertiesOrCases

74d1478

Use IGM.getMaximalTypeExpansionContext() in more places

2ea1c5c

Remove FIXME in TypeExpansionAnalysis: It's cache needed to be keyed …

8a4d61f

…by the type expansion context

Adjust to recent change

f3a5d69

aschwaighofer force-pushed the sil_type_expansion_context branch from 07224f2 to f3a5d69 Compare November 12, 2019 13:36

aschwaighofer merged commit a30f804 into swiftlang:master Nov 12, 2019

nate-chandler mentioned this pull request Aug 9, 2023

[SILGen] Remove subtle identity function call. #67846

Merged

SIL: Use a TypeExpansionContext in SIL lowering to expand opaque type archetypes as part of lowering #28044

SIL: Use a TypeExpansionContext in SIL lowering to expand opaque type archetypes as part of lowering #28044

Uh oh!

Conversation

aschwaighofer commented Nov 3, 2019

Uh oh!

aschwaighofer commented Nov 3, 2019

Uh oh!

aschwaighofer commented Nov 3, 2019

Uh oh!

swift-ci commented Nov 3, 2019

Uh oh!

swift-ci commented Nov 3, 2019

Uh oh!

aschwaighofer commented Nov 4, 2019

Uh oh!

aschwaighofer commented Nov 4, 2019

Uh oh!

aschwaighofer commented Nov 4, 2019

Uh oh!

swift-ci commented Nov 4, 2019

Uh oh!

swift-ci commented Nov 4, 2019

Uh oh!

aschwaighofer commented Nov 4, 2019

Uh oh!

aschwaighofer commented Nov 4, 2019

Uh oh!

swift-ci commented Nov 4, 2019

Uh oh!

swift-ci commented Nov 4, 2019

Uh oh!

aschwaighofer commented Nov 4, 2019

Uh oh!

aschwaighofer commented Nov 4, 2019

Uh oh!

aschwaighofer commented Nov 4, 2019

Uh oh!

swift-ci commented Nov 5, 2019

Uh oh!

aschwaighofer commented Nov 5, 2019

Uh oh!

aschwaighofer commented Nov 5, 2019

Uh oh!

jckarter Nov 7, 2019

Choose a reason for hiding this comment

Uh oh!

aschwaighofer Nov 7, 2019

Choose a reason for hiding this comment

Uh oh!

jckarter Nov 7, 2019

Choose a reason for hiding this comment

Uh oh!

jckarter Nov 7, 2019

Choose a reason for hiding this comment

Uh oh!

aschwaighofer Nov 7, 2019

Choose a reason for hiding this comment

Uh oh!

jckarter Nov 7, 2019

Choose a reason for hiding this comment

Uh oh!

jckarter Nov 7, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aschwaighofer Nov 7, 2019

Choose a reason for hiding this comment

Uh oh!

aschwaighofer commented Nov 8, 2019

Uh oh!

swift-ci commented Nov 8, 2019

Uh oh!

swift-ci commented Nov 8, 2019

Uh oh!

aschwaighofer commented Nov 8, 2019

Uh oh!

aschwaighofer commented Nov 11, 2019

Uh oh!

aschwaighofer commented Nov 12, 2019

jckarter Nov 7, 2019 •

edited

Loading