Concrete type propagation using ProtocolConformanceAnalysis #17373

rajbarik · 2018-06-20T20:42:47Z

Part 2 of #16739

This peephole optimization propagates concrete types for self arguments and rewrites witness methods using ProtocolConformanceAnalysis!

rajbarik · 2018-06-20T20:44:10Z

@slavapestov Second PR for concrete type propagation of self arguments using ProtocolConformanceAnalysis, that was merged couple of days ago.

slavapestov · 2018-06-27T20:17:42Z

lib/SILOptimizer/SILCombiner/SILCombiner.h

@@ -298,6 +312,20 @@ class SILCombiner :
                                                         WitnessMethodInst *WMI);
  SILInstruction *propagateConcreteTypeOfInitExistential(FullApplySite AI);

+  /// Propagate concrete types from ProtocolConformanceAnalysis.
+  SILInstruction *propagateConcreteTypeFromPCA(FullApplySite AI);


"propagateSoleConformingType"

slavapestov · 2018-06-27T20:17:57Z

lib/SILOptimizer/SILCombiner/SILCombiner.h

+
+  /// Internal method to prepare and check code for concrete type propagation
+  /// using ProtocolConformanceAnalysis via unchecked cast instructions.
+  bool prepareNCheckConcreteTypePropagationFromPCA(


"prepareAndCheckConcreteTypePropagation"

Maybe define a class with arguments?

slavapestov · 2018-06-27T20:18:04Z

lib/SILOptimizer/SILCombiner/SILCombiner.h

+
+  /// Traverse ProtocolConformanceMapCache from ProtocolConformanceAnalysis
+  /// recursively to determine concrete type.
+  NominalTypeDecl *findSoleConformingTypeFromPCA(ProtocolDecl *Protocol);


"findSoleConformingType"

slavapestov · 2018-06-27T20:20:55Z

lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp

+/// class, struct or enum type.
+NominalTypeDecl *
+SILCombiner::findSoleConformingTypeFromPCA(ProtocolDecl *Protocol) {
+  SmallVector<ProtocolDecl *, 16> PDWorkList;


All of this except for the CHA part should go into the protocol conformance analysis

slavapestov · 2018-06-27T20:23:58Z

lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp

+    return false;
+
+  /// Ignore already visited Apply sites.
+  if (VisitedAIForPCA.find(Apply) != VisitedAIForPCA.end())


Does this introduce a use-after-free?

slavapestov · 2018-06-27T20:26:52Z

lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp

+    return false;
+
+  // Do not handle unbounded generics.
+  auto ElementType = NTD->getDeclaredType();


Better way to check: NTD->isGenericContext(). And move this into findSoleConformingTypeFromPCA()

slavapestov · 2018-06-27T20:28:44Z

lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp

+
+  SILValue Self = Apply.getSelfArgument();
+  auto *WMI = dyn_cast<WitnessMethodInst>(Apply.getCallee());
+  if (!WMI)


What if I'm calling a statically-dispatched protocol extension method?

extension P { func f() { ... } }

slavapestov · 2018-06-27T20:29:32Z

lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp

@@ -858,7 +1029,7 @@ SILInstruction *SILCombiner::propagateConcreteTypeOfInitExistential(
  SILInstruction *InitExistential = findInitExistential(
      Apply, Self, OpenedArchetype, OpenedArchetypeDef, isCopied);
  if (!InitExistential)
-    return nullptr;
+    return propagateConcreteTypeFromPCA(Apply);


Move this to the callers of propagateConcreteTypeOfInitExistential()

slavapestov · 2018-06-27T20:30:37Z

test/SILOptimizer/sil_combine_protocol_conf.swift

+// CHECK: integer_literal
+// CHECK: ref_element_addr
+// CHECK: open_existential_addr
+// CHECK: witness_method


Can you elaborate these witness_methods so its easier to read the test case

slavapestov · 2018-06-27T20:31:57Z

lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp

+    return false;
+
+  // Determine the sole conforming type.
+  auto *NTD = findSoleConformingTypeFromPCA(Protocol);


Do this first, to avoid doing everything else

rajbarik · 2018-07-03T21:43:24Z

@atrick @slavapestov can you review this PR now? I have taken care of all the comments.

slavapestov · 2018-07-11T20:13:36Z

include/swift/SILOptimizer/Utils/Existential.h

+  // Empty initilization of ConcreteExistentialInfo. This is used when the
+  // concrete type is determined via ProtocolConformanceAnalysis. The fields
+  // are populated during sil_combiner.
+  ConcreteExistentialInfo() {}


Instead of default initializing, can you pass around an Optional<ConcreteExistentialInfo>?

slavapestov · 2018-07-11T20:13:50Z

lib/SILOptimizer/Analysis/ProtocolConformanceAnalysis.cpp

@@ -30,7 +30,7 @@ class NominalTypeWalker : public ASTWalker {

 public:
  NominalTypeWalker(ProtocolConformanceAnalysis::ProtocolConformanceMap
-                    &ProtocolConformanceCache)
+                        &ProtocolConformanceCache)


Unnecessary change?

slavapestov · 2018-07-11T20:16:38Z

lib/SILOptimizer/Analysis/ProtocolConformanceAnalysis.cpp

+ProtocolConformanceAnalysis::findSoleConformingType(ProtocolDecl *Protocol) {
+  SmallVector<ProtocolDecl *, 16> PDWorkList;
+  SmallPtrSet<ProtocolDecl *, 8> VisitedPDs;
+  SmallPtrSet<NominalTypeDecl *, 8> ConformanceSet;


Could this just be a NominalTypeDecl *?

slavapestov · 2018-07-11T20:16:47Z

lib/SILOptimizer/Analysis/ProtocolConformanceAnalysis.cpp

+/// class, struct or enum type.
+NominalTypeDecl *
+ProtocolConformanceAnalysis::findSoleConformingType(ProtocolDecl *Protocol) {
+  SmallVector<ProtocolDecl *, 16> PDWorkList;


Use 2 for the default size

slavapestov · 2018-07-11T20:17:04Z

lib/SILOptimizer/Analysis/ProtocolConformanceAnalysis.cpp

+        if (!VisitedPDs.count(Proto))
+          PDWorkList.push_back(Proto);
+      } else { // Classes, Structs and Enums are added here.
+        ConformanceSet.insert(ConformingNTD);


Bail out here

slavapestov · 2018-07-11T20:34:14Z

lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp

+    if (WMI->use_empty())
+      eraseInstFromFunction(*WMI);
+  }
+  auto *NewAI = createApplyWithConcreteType(Apply, CEI, BuilderCtx);


Create a new overload of createApplyWithConcreteType() so you can share this block of code between here and SILCombiner::propagateConcreteTypeOfInitExistential()

slavapestov · 2018-07-11T20:35:26Z

lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp

@@ -711,7 +848,8 @@ SILCombiner::createApplyWithConcreteType(FullApplySite Apply,
  }
  // The apply can only be rewritten in terms of the concrete value if it is
  // legal to pass that value as the self argument.
-  if (CEI.isCopied && !canReplaceCopiedSelf(Apply, CEI.InitExistential, DA))
+  if (CEI.InitExistential && CEI.isCopied &&
+      !canReplaceCopiedSelf(Apply, CEI.InitExistential, DA))


Is this correct?

@atrick suggested this: if (isCopied && (!InitExistential || !canReplaceCopiedSelf(Apply, InitExistential)
which seem reasonable.

slavapestov · 2018-07-11T20:39:02Z

lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp

+    if (!propagateConcreteTypeOfInitExistential(AI, WMI) &&
+        (WMI = dyn_cast<WitnessMethodInst>(AI->getCallee()))) {
+      // Propagate concrete type from ProtocolConformanceAnalysis.
+      propagateSoleConformingType(AI, WMI);


I think if propagateConcreteTypeOfInitExistential() returned a boolean indicating if the WMI was replaced, you would only call propagateSoleConformingType() if it returned false, and not have to worry about getting the WMI out again

OK. Will create a new PR with propagateConcreteTypeOfInitExistential() returning true/false.

slavapestov · 2018-07-11T20:40:47Z

lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp

+    if (!propagateConcreteTypeOfInitExistential(AI, WMI) &&
+        (WMI = dyn_cast<WitnessMethodInst>(AI->getCallee()))) {
+      // Propagate concrete type from ProtocolConformanceAnalysis.
+      propagateSoleConformingType(AI, WMI);


slavapestov · 2018-07-11T20:41:07Z

test/SILOptimizer/sil_combine_protocol_conf.swift

+  }
+}
+
+// case 5: Generic protocol -- should not  optimize


Generic conforming type

rajbarik · 2018-07-27T20:52:31Z

@atrick @slavapestov Any possibility of getting this review done? Thanks.

atrick · 2018-07-27T23:19:50Z

lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp

+
+  SILValue Self = Apply.getSelfArgument();
+  auto *PD = WMI->getLookupProtocol();
+  ConcreteExistentialInfo CEI(Apply.getSelfArgumentOperand(), true);


I don't think it really makes sense to construct a fake, invalid ConcreteExistentialInfo, then fill in its fields here. It seems like this should be an alternate constructor.

rajbarik · 2018-08-01T21:42:48Z

@atrick Moved the code that separately populated ConcreteExistentialInfo as an alternative constructor for ConcreteExistentialInfo. Can you check and let me know if you have any other concerns?

atrick

I don't really understand what's happening yet. Can you clarify some of the points below?

atrick · 2018-08-02T21:20:48Z

lib/SILOptimizer/Utils/Existential.cpp

+  if ((CD = dyn_cast<ClassDecl>(NTD)) &&
+      (CD->getEffectiveAccess() == AccessLevel::Open ||
+       CHA->hasKnownDirectSubclasses(CD) ||
+       CHA->hasKnownIndirectSubclasses(CD))) {


Out of curiosity, how can a class have indirect subclasses without having direct subclasses?

My understanding was that they were disjoint sets. But looking at the code closely seems like Indirect Subclasses are derived from DirectSubClasses. So, check for hasKnownDirectSubClasses should suffice in this case.

atrick · 2018-08-02T21:31:27Z

include/swift/SILOptimizer/Utils/Existential.h

  ConcreteExistentialInfo(ConcreteExistentialInfo &) = delete;

+  /// We do not not need to check for InitExistential not being not null


I don't understand the comment. Aren't you allowing InitExistential to be null when the concrete type was derived through analysis instead?

Will rephrase the comment.

atrick · 2018-08-02T21:32:48Z

lib/SILOptimizer/Utils/Existential.cpp

+  // findInitExistential for the argument.
+  isCopied = false;
+  OpenedArchetypeDef = Arg;
+  InitExistential = findInitExistential(ArgOperand, OpenedArchetype,


Can the returned InitExistential be nullptr? Please comment.

Yes, InitExistential can be nullptr. Will update the comment. Thanks.

atrick · 2018-08-02T21:34:57Z

lib/SILOptimizer/Utils/Existential.cpp

+  InitExistential = findInitExistential(ArgOperand, OpenedArchetype,
+                                        OpenedArchetypeDef, isCopied);
+
+  // If no InitExistential is found, check if the argument is already an opened


Sorry, I'm really confused now. The code below runs regardless of whether InitExistential was found or not. How is it related to InitExstential?

Dumb question: if the caller already knows the ConcreteType and Protocol, why do we need to bother looking for the InitExistential?

I still need the OpenedArchetypeDef to be able to cast it to the concrete type. findInitExistential helps me determine this OpenedArchetypeDef. For cases where OpenedArchetypeDef can not be found (because there was no InitExistential), then the code below does extra effort in trying to find if Arg is itself OpenedArcheTypeDef or AllocStackInst, in which case it determines the OpenedArcheTypeDef. I will update the comment. Let me know if this makes sense.

That's the part I was missing. It almost looked like you could reuse the same constructor for both the current case and the CHA/PCA case but just pass in an optional ConcreteType in the later case.

atrick · 2018-08-02T21:57:25Z

lib/SILOptimizer/Utils/Existential.cpp

+ConcreteExistentialInfo::ConcreteExistentialInfo(
+    ProtocolConformanceAnalysis *PCA, ClassHierarchyAnalysis *CHA,
+    SILBuilder &Builder, ProtocolDecl *Protocol, FullApplySite Apply,
+    SILValue &Arg, Operand &ArgOperand) {


This doesn't seem like a clean encapsulation anymore. Why can't the caller just pass in ConcreteType and Protocol when it can obtain those from its analyses? Why do you need to pass in PCA and CHA?

Apply, Arg, and ArgOperand all seem redundant. Why don't we just pass in a single openedUse (the open_existential_ref's use) just like the original constructor?

Also, the top-level comment on struct ConcreteExistentialInfo should be amended to mention new SIL patterns that we handle. Or move the existing comments onto the original constructor.

The reason I wanted it this way is that I can create ConcreteExistentialInfo from another optimization #17366. I did not want code duplication later. May be for now, I will simplify the constructor as you suggested and worry about it later with a common interface for the other PR. Again, will update the comment.

rajbarik · 2018-08-03T23:01:28Z

@atrick @slavapestov Cleaned up the interfaces a bit. Please have a look now.

atrick

Great. Just a few more comments...

atrick · 2018-08-04T01:22:28Z

lib/SILOptimizer/Utils/Existential.cpp

+  isCopied = false;
+  OpenedArchetypeDef = Arg;
+  InitExistential = findInitExistential(ArgOperand, OpenedArchetype,
+                                        OpenedArchetypeDef, isCopied);


Thanks for the comments. If InitExistential is non-null, do you think it would be useful to assert that it's concrete type is the same as the passed in ConcreteType?

atrick · 2018-08-04T01:22:53Z

lib/SILOptimizer/Utils/Existential.cpp

+
+  // We have the open_existential; we still need the conformance.
+  auto ConformanceRef =
+      M.getSwiftModule()->lookupConformance(ConcreteType, Protocol);


We're generally not supposed to call SwiftModule::lookupConformance from the optimizer because the conformance hasn't necessarilly been type checked. In this case, I don't really see an alternative though. @slavapestov , do you have any suggestions? Are there certain conditions where we can say that it's safe to do this?

atrick · 2018-08-04T01:30:57Z

lib/SILOptimizer/Utils/Existential.cpp

+    auto *URCI =
+        Builder.createUncheckedRefCast(OER->getLoc(), OER, ConcreteSILType);
+    OpenedArchetype = OER->getType().getASTType()->castTo<ArchetypeType>();
+    ConcreteValue = URCI;


Ok, this works, but it's kind of hacky because ConcreteExistentialInfo is supposed to be a mini-analysis that just summarizes all the various SIL patterns we may encounter where the common thread is that we have an open_existential as the self argument of an apply. It's confusing if an analysis utility also mutates the SIL. Rather than passing in a Builder, propagateSoleConformingType should be responsible for generating the cast that it needs. I think it's fine if you allow a valid ConcreteExistentialInfo to have an invalid ConcreteValue when InitExistential == nullptr.

So maybe you could remove ConcreteValue from the isValid and add an assert instead: !InitExistential || ConcreteValue

Then, in your new CHA/PCA case, you could use just use the existing ConcreteValue if an InitExistential is found without doing the cast. Would that make sense? If there's no InitExistential, then propagateSoleConformingType could generate the cast.

rajbarik · 2018-08-06T20:40:11Z

@atrick @slavapestov Done. Anything else?

atrick

Looks great! Just one argument type to fix, and a side question for @slavapestov that doesn't need to hold up the merge.

atrick · 2018-08-07T00:46:03Z

include/swift/SILOptimizer/Utils/Existential.h

+  // known ConcreteType and ProtocolDecl pair. It determines the
+  // OpenedArchetypeDef for the ArgOperand that will be used by unchecked_cast
+  // instructions to cast OpenedArchetypeDef to ConcreteType.
+  ConcreteExistentialInfo(Operand &ArgOperand, const CanType &ConcreteType,


The convention is to pass CanType by value:

, CanType ConcreteType,

atrick · 2018-08-07T00:50:31Z

lib/SILOptimizer/Utils/Existential.cpp

+
+  // We have the open_existential; we still need the conformance.
+  auto ConformanceRef =
+      M.getSwiftModule()->lookupConformance(ConcreteType, Protocol);


I'm still curious what @slavapestov has to say about calling lookupConformance here. It seems like we should come up with some rules for when it's ok for the optimizer to do this and enforce it in the API.

In this case it's fine to call lookupConformance() -- there's no way for the type checker to anticipate a conformance is needed here and store it somewhere. However, the resulting conformance might be incomplete, which will cause problems later in IRGen if you try to use it.

So I would add a check for the conformance being complete.

Also please try to come up with a test case that would fail without this check. It will require the conforming type to be defined in another file, so that when you build in non-WMO mode, the type checker will not check the conformance.

... OK, but since this optimization only applies in WMO mode, you won't be able to get a test case. In that case, please just assert that the conformance is complete.

slavapestov

Looks good, feel free to merge once you've addressed the other comments.

slavapestov · 2018-08-13T05:46:18Z

lib/SILOptimizer/Analysis/ProtocolConformanceAnalysis.cpp

+/// class, struct or enum type.
+NominalTypeDecl *
+ProtocolConformanceAnalysis::findSoleConformingType(ProtocolDecl *Protocol) {
+  SmallVector<ProtocolDecl *, 8> PDWorkList;


Should the result of this be cached?

slavapestov · 2018-08-13T05:46:29Z

lib/SILOptimizer/Analysis/ProtocolConformanceAnalysis.cpp

+  PDWorkList.push_back(Protocol);
+  while (!PDWorkList.empty()) {
+    auto *PD = PDWorkList.pop_back_val();
+    // Protocols must have internal or lower access scopes.


Just access, not access scope

slavapestov · 2018-08-13T05:46:51Z

lib/SILOptimizer/Analysis/ProtocolConformanceAnalysis.cpp

+    }
+    VisitedPDs.insert(PD);
+    auto NTDList = getConformances(PD);
+    if (NTDList.empty())


I don't think this check is necessary -- a for loop over an empty collection won't do much work.

slavapestov · 2018-08-13T05:48:01Z

lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp

+      Op.set(NewWMI);
+  }
+  if (WMI->use_empty())
+    eraseInstFromFunction(*WMI);


If it has other uses, they could also be replaced with NewWMI. In fact you could avoid passing in the Apply here, and look at all uses of the WMI instead.

slavapestov · 2018-08-13T05:49:14Z

lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp

+//  TO
+//  %4 = open_existential_ref %3 : $P to $@opened P
+//  %7 = unchecked_ref_cast %4 : $@opened P to $C
+//  %8 = class_method %7 : $C, #C.foo!1 : (C) -> () -> Int,


The actual replacement of the witness_method with class_method is done in the devirtualization pass, not this function. Also the SIL is just kind of hard to read. Can you instead just write a comment that says it replaces the witness_method instruction with one that has a concrete type, allowing other optimizations to devirtualize it later?

slavapestov · 2018-08-13T05:51:01Z

lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp

+
+  assert(CEI.ConcreteValue);
+
+  /// Replace the old WitnessMehod with a new one that has concrete type and


Typo: WitnessMehod

slavapestov · 2018-08-13T05:52:14Z

lib/SILOptimizer/Utils/Existential.cpp

+
+  /// Determine the OpenedArchetype.
+  OpenedArchetype =
+      OpenedArchetypeDef->getType().getASTType()->castTo<ArchetypeType>();


OpenedArchetypeDef->getType().castTo<ArchetypeType>();

slavapestov · 2018-08-13T05:52:34Z

lib/SILOptimizer/Utils/Existential.cpp

+
+  // We have the open_existential; we still need the conformance.
+  auto ConformanceRef =
+      M.getSwiftModule()->lookupConformance(ConcreteType, Protocol);


... OK, but since this optimization only applies in WMO mode, you won't be able to get a test case. In that case, please just assert that the conformance is complete.

rajbarik · 2018-08-14T18:24:09Z

@atrick I believe you wrote the following test case in sil_combine_concrete_existential.swift. Turns out that my pass determines concrete type for p and makes the p.returnsOptionalIndirect() a direct call.

// The first apply has been devirtualized and inlined. The second remains unspecialized.
// CHECK-LABEL: sil @$S32sil_combine_concrete_existential37testWitnessReturnOptionalIndirectSelfyyF : $@convention(thin) () -> () {
// CHECK: switch_enum_addr %{{.*}} : $*Optional<@opened("{{.*}}") PPP>, case #Optional.some!enumelt.1: bb{{.*}}, case #Optional.none!enumelt: bb{{.*}}
// CHECK: [[O:%.*]] = open_existential_addr immutable_access %{{.*}} : $*PPP to $*@opened("{{.*}}") PPP
// CHECK: [[W:%.*]] = witness_method $@opened("{{.*}}") PPP, #PPP.returnsOptionalIndirect!1 : <Self where Self : PPP> (Self) -> () -> @dynamic_self Self?, [[O]] : $*@opened("{{.*}}") PPP : $@convention(witness_method: PPP) <τ_0_0 where τ_0_0 : PPP> (@in_guaranteed τ_0_0) -> @out Optional<τ_0_0>
// CHECK: apply [[W]]<@opened("{{.*}}") PPP>(%{{.*}}, [[O]]) : $@convention(witness_method: PPP) <τ_0_0 where τ_0_0 : PPP> (@in_guaranteed τ_0_0) -> @out Optional<τ_0_0>
// CHECK-LABEL: } // end sil function '$S32sil_combine_concrete_existential37testWitnessReturnOptionalIndirectSelfyyF'
public func testWitnessReturnOptionalIndirectSelf() {
  let p: PPP = S()
  p.returnsOptionalIndirect()?.returnsOptionalIndirect()
}

I have updated the test case by adding another unused struct so that S is not the SoleConformingType:

struct SS: PPP {
  func returnsOptionalIndirect() -> SS? {
    return self
  }
}

Let me know if you disagree.

atrick · 2018-08-14T19:06:42Z

@rajbarik thanks for fixing the test.

slavapestov · 2018-08-20T21:48:16Z

lib/SILOptimizer/Utils/Existential.cpp

+
+  /// Determine the ExistentialConformances and SubstitutionMap.
+  auto ExistentialConformances = ArrayRef<ProtocolConformanceRef>(
+      ProtocolConformanceRef(ConcreteConformance));


Isn't this a use-after-free? I would rely on the implicit conversion between T[] and ArrayRef<T> and just pass {ProtocolConformanceRef(ConcreteConformance)} as the parameter to SubstitutionMap::get below

I tried this before and even now. I get the following error:

../Existential.cpp:378:21: error: call to 'get' is ambiguous ExistentialSubs = SubstitutionMap::get(ExistentialSig, {ConcreteType}, ^~~~~~~~~~~~~~~~~~~~ .../swift/include/swift/AST/SubstitutionMap.h:105:26: note: candidate function static SubstitutionMap get(GenericSignature *genericSig, ^.../swift/include/swift/AST/SubstitutionMap.h:118:26: note: candidate function static SubstitutionMap get(GenericSignature *genericSig, ^ 1 error generated.

Code changed:

ExistentialSubs = SubstitutionMap::get(ExistentialSig, {ConcreteType}, /*ExistentialConformances*/ {ProtocolConformanceRef(ConcreteConformance)});

slavapestov · 2018-08-21T19:52:36Z

@swift-ci Please test

slavapestov · 2018-08-21T19:52:42Z

@swift-ci Please test source compatibility

swift-ci · 2018-08-21T19:53:10Z

Build failed
Swift Test OS X Platform
Git Sha - cc1638c2ef50a4f0ef357f5f9bd09430d1814799

slavapestov · 2018-08-21T19:53:11Z

@swift-ci Please test source compatibility

slavapestov · 2018-08-21T19:53:18Z

@swift-ci Please test

swift-ci · 2018-08-21T20:58:34Z

Build failed
Swift Test OS X Platform
Git Sha - a5aef01496d233e6ac135e597c0e2141d7c4b452

slavapestov · 2018-08-22T03:54:53Z

@rajbarik

/Users/buildnode/jenkins/workspace/swift-PR-osx/branch-master/swift/test/SILOptimizer/sil_combine_protocol_conf.swift:247:11: error: CHECK: expected string not found in input
15:46:57 // CHECK: load [[S11]] : $*Builtin.Int64
15:46:57           ^
15:46:57 <stdin>:1376:51: note: scanning from here
15:46:57  %8 = struct_element_addr %7 : $*Int, #Int._value // user: %9
15:46:57                                                   ^
15:46:57 <stdin>:1376:51: note: with variable "S11" equal to "%8"
15:46:57  %8 = struct_element_addr %7 : $*Int, #Int._value // user: %9
15:46:57                                                   ^
15:46:57 <stdin>:1377:7: note: possible intended match here
15:46:57  %9 = load %8 : $*Builtin.Int32 // user: %20
15:46:57       ^
15:46:57 
15:46:57 --
15:46:57

Looks like you need to tweak the regex for 32 bit. Please run check-swift-validation-iphonesimulator-i386 locally.

rajbarik · 2018-08-22T18:23:56Z

@slavapestov please run tests.

slavapestov · 2018-08-22T20:06:55Z

@swift-ci Please test

slavapestov · 2018-08-22T20:07:14Z

@swift-ci Please test

gottesmm · 2018-08-22T22:48:41Z

@rajbarik +1! Congrats!

rajbarik · 2018-08-22T22:56:56Z

@atrick @slavapestov Thanks a lot for all your help!

atrick · 2018-08-22T23:07:08Z

@rajbarik Thanks for keeping at it.

rajbarik force-pushed the raj-cta branch from 969a781 to a8248ec Compare June 20, 2018 22:11

slavapestov reviewed Jun 27, 2018

View reviewed changes

slavapestov self-assigned this Jun 27, 2018

rajbarik force-pushed the raj-cta branch 2 times, most recently from bb9fd48 to e0a80de Compare July 3, 2018 21:41

slavapestov reviewed Jul 11, 2018

View reviewed changes

rajbarik mentioned this pull request Jul 13, 2018

Refactor SILCombiner code for better reuse and extensibility #17934

Closed

rajbarik force-pushed the raj-cta branch from e0a80de to 8375a35 Compare July 25, 2018 18:19

atrick reviewed Jul 27, 2018

View reviewed changes

rajbarik force-pushed the raj-cta branch from 8375a35 to 905e43c Compare August 1, 2018 21:40

atrick reviewed Aug 2, 2018

View reviewed changes

rajbarik force-pushed the raj-cta branch from 905e43c to c9ec432 Compare August 3, 2018 23:00

atrick reviewed Aug 4, 2018

View reviewed changes

rajbarik force-pushed the raj-cta branch 2 times, most recently from 1b3ec63 to cc1638c Compare August 6, 2018 20:37

atrick approved these changes Aug 7, 2018

View reviewed changes

rajbarik force-pushed the raj-cta branch 2 times, most recently from bd61a1d to 9732bdf Compare August 7, 2018 21:25

slavapestov approved these changes Aug 13, 2018

View reviewed changes

rajbarik force-pushed the raj-cta branch from 9732bdf to b5f4944 Compare August 14, 2018 18:23

rajbarik force-pushed the raj-cta branch from b5f4944 to 8b3fa92 Compare August 14, 2018 18:29

slavapestov approved these changes Aug 20, 2018

View reviewed changes

rajbarik force-pushed the raj-cta branch from 8b3fa92 to a5aef01 Compare August 21, 2018 00:34

Concrete type propagation using ProtocolConformanceAnalysis

d9a051e

rajbarik force-pushed the raj-cta branch from a5aef01 to d9a051e Compare August 22, 2018 18:08

slavapestov merged commit 42785be into swiftlang:master Aug 22, 2018

		ConcreteExistentialInfo(ConcreteExistentialInfo &) = delete;

		/// We do not not need to check for InitExistential not being not null


		assert(CEI.ConcreteValue);

		/// Replace the old WitnessMehod with a new one that has concrete type and

Concrete type propagation using ProtocolConformanceAnalysis #17373

Concrete type propagation using ProtocolConformanceAnalysis #17373

Uh oh!

Conversation

rajbarik commented Jun 20, 2018

Uh oh!

rajbarik commented Jun 20, 2018

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!

rajbarik commented Jul 3, 2018

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!

rajbarik commented Jul 27, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rajbarik commented Aug 1, 2018

Uh oh!

atrick left a comment

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