Perform ExistentialSpecializer when SoleConformingType is known #20977

rajbarik · 2018-12-03T22:20:13Z

The criteria for ExistentialSpecializer is now based on an argument having "findInitExistential or SoleConformingType".

rajbarik · 2018-12-03T22:22:03Z

lib/SILOptimizer/FunctionSignatureTransforms/ExistentialSpecializer.cpp

    /// Perform specialization.
    specializeExistentialArgsInAppliesWithinFunction(*F);
  }
 };
 } // namespace

+/// Find concrete type from Sole Conforming Type.
+bool ExistentialSpecializer::findConcreteTypeFromSoleConformingType(


This method checks if a SoleConformingType can be determined for the argument. Some parts of code (5-6 lines) overlap with what was done in SILCombiner, but there are lot of disparities to make it a new APi in Existential.h altogether.

@atrick You had a similar concern from SILCombiner, but the overlap is minor to warrant a new API?

rajbarik · 2018-12-03T22:22:47Z

lib/SILOptimizer/FunctionSignatureTransforms/ExistentialSpecializer.cpp

@@ -158,6 +208,14 @@ findIfCalleeUsesArgInDestroyUse(SILValue Arg,
  }
 }

+/// Helper function to ensure that the argument is not InOut or InOut_Aliasable


Added to filter out inout args, which otherwise would be fed to openExistentialRef as non-objects!

rajbarik · 2018-12-03T22:23:23Z

lib/SILOptimizer/FunctionSignatureTransforms/ExistentialSpecializer.cpp

    CanType ConcreteType;
-    if (!findConcreteType(Apply, Idx, ConcreteType)) {
+    if (!((F->getModule().isWholeModule() &&


First tries to check if there is a SoleConformingType, if not, then tries findConcreteTypeUsingInitExistential.

rajbarik · 2018-12-03T22:23:52Z

lib/SILOptimizer/FunctionSignatureTransforms/ExistentialTransform.cpp

@@ -417,6 +417,8 @@ void ExistentialTransform::populateThunkBody() {
        break;
      }
      case ExistentialRepresentation::Class: {
+        /// If the operand is not object type, we would need an explicit load.
+        assert(OrigOperand->getType().isObject());


Extra check for accidental non-object operand type being passed to OEI.

rajbarik · 2018-12-03T22:32:09Z

test/SILOptimizer/existential_transform_soletype.swift

+// CHECK:   return
+// CHECK-LABEL: } // end sil function '$s30existential_transform_soletype4opt11bs5Int32VAA3SPP_p_tF'
+
+// CHECK-LABEL: sil shared [noinline] @$s30existential_transform_soletype4opt21bs5Int32VAA3SPP_p_tFTf4e_n : $@convention(thin) <τ_0_0 where τ_0_0 : SPP> (@in_guaranteed τ_0_0) -> Int32 {


opt1 does not get optimized since the caller foo is under @_optimize(none).

It's generally better to use .sil test cases for individual passes, but not an absolute requirement. The source is definitely helpful, but that can be included as a comment.

rajbarik · 2018-12-03T22:33:38Z

@slavapestov @atrick Can you review this small PR? This one extends ExistentialSpecializer to also perform the optimization when SoleConformingType can be found.

rajbarik · 2018-12-14T17:13:57Z

lib/SILOptimizer/FunctionSignatureTransforms/ExistentialSpecializer.cpp

-    if (!findConcreteType(Apply, Idx, ConcreteType)) {
+    if (!((F->getModule().isWholeModule() &&
+           isNonInoutIndirectArgument(Arg, ArgConvention) &&
+           findConcreteTypeFromSoleConformingType(Arg, ConcreteType)) ||


@atrick fyi

atrick

Otherwise, LGTM.

atrick · 2018-12-14T17:40:21Z

lib/SILOptimizer/FunctionSignatureTransforms/ExistentialSpecializer.cpp

+      (CD->getEffectiveAccess() == AccessLevel::Open ||
+       CHA->hasKnownDirectSubclasses(CD))) {
+    return false;
+  }


This should be a standalone API because it has special logic to reason about possible conformances. That logic should only exist in one place in the compiler.

Originally I had this in findSoleConformingType function of ProtocolConformanceAnalysis, however since this also required ClassHierarchyAnalysis, we had to split. I can put this in Existential.cpp in Utils. This would be a wrapper for findSoleConformingType.

I'm just trying to avoid duplicating logic that reasons about the type system in multiple places in the SILOptimizer. As an alternative, we could just have a CHA->mayHaveSubclasses query.

atrick · 2018-12-14T17:44:01Z

test/SILOptimizer/existential_transform_soletype.swift

+// CHECK:   return
+// CHECK-LABEL: } // end sil function '$s30existential_transform_soletype4opt11bs5Int32VAA3SPP_p_tF'
+
+// CHECK-LABEL: sil shared [noinline] @$s30existential_transform_soletype4opt21bs5Int32VAA3SPP_p_tFTf4e_n : $@convention(thin) <τ_0_0 where τ_0_0 : SPP> (@in_guaranteed τ_0_0) -> Int32 {


It's generally better to use .sil test cases for individual passes, but not an absolute requirement. The source is definitely helpful, but that can be included as a comment.

rajbarik · 2018-12-20T18:21:07Z

@atrick after you merge your changes #21316, we should clean this up and get this merged with cleaner interface.

atrick · 2018-12-22T00:45:31Z

@rajbarik I finished rebasing
#21316 but just missed the branch lockdown. Of course, you could check this branch out yourself from my repo if you want to cleanup this PR. I'll merge my PR without further changes on Jan 2.

rajbarik · 2019-01-05T01:39:47Z

@atrick please check. I extracted out the common code between SILCombiner and ExistentialSpecializer to ProtocolConformanceAnalysis. Have a look.

rajbarik · 2019-01-07T18:30:53Z

@atrick, fyi, this needs your approval. thanks.

atrick · 2019-01-07T19:37:09Z

@rajbarik it looks like I already approved this, but I think you need to add a commit message.

slavapestov · 2019-01-07T21:48:50Z

@swift-ci Please test

slavapestov · 2019-01-07T21:48:56Z

@swift-ci Please test source compatibility

slavapestov · 2019-01-07T21:49:00Z

@swift-ci Please benchmark

swift-ci · 2019-01-07T21:49:21Z

Build failed
Swift Test OS X Platform
Git Sha - 6b0d1a109d22a2bcb614c10d91d9db2fed646cbf

swift-ci · 2019-01-07T21:50:51Z

Build failed
Swift Test Linux Platform
Git Sha - 6b0d1a109d22a2bcb614c10d91d9db2fed646cbf

swift-ci · 2019-01-07T22:33:41Z

Build comment file:

No performance and code size changes

atrick · 2019-01-07T23:23:01Z

@swift-ci test.

swift-ci · 2019-01-07T23:24:58Z

Build failed
Swift Test Linux Platform
Git Sha - 1f4c6cac47ab5bf3a54402d80eb1bdbdbbebcacb

swift-ci · 2019-01-07T23:27:03Z

Build failed
Swift Test OS X Platform
Git Sha - 1f4c6cac47ab5bf3a54402d80eb1bdbdbbebcacb

slavapestov · 2019-01-08T01:51:00Z

@rajbarik Are you able to build and test Swift on Linux?

18:39:35 The following tests FAILED:
18:39:35 	  1 - TestFoundation (SEGFAULT)

rajbarik · 2019-01-08T02:27:51Z

@slavapestov I do not have a Linux Box. Other means?

rajbarik · 2019-01-08T23:01:40Z

Update: Found a linux box and installed Swift on it. It built fine. When I run tests, I get a lot of precompiled header errors, nothing like Foundation SEGFAULT.
@atrick any clue how to go about handling the Linux error that Slava is pointing out?

rajbarik · 2019-01-14T18:58:17Z

@atrick //cc @airspeedswift @eeckstein could you please look at this one when you get time? I am unable to reproduce this on a linux box at my end. I am in the process of writing a benchmark for #19820 -- this one needs to be committed before I commit the benchmark.

atrick · 2019-01-14T19:11:40Z

@swift-ci test linux platform.

atrick · 2019-01-14T19:12:52Z

I don't think I'll have time to debug this week. Let's get the results again. Maybe it's unrelated to this PR.

rajbarik commented Dec 3, 2018

View reviewed changes

rajbarik force-pushed the raj-extend-existential2generic branch from 4974abc to 6b0d1a1 Compare December 3, 2018 22:31

rajbarik commented Dec 3, 2018

View reviewed changes

rajbarik mentioned this pull request Dec 10, 2018

Enable ExistentialSpecializer by default #19820

Merged

rajbarik commented Dec 14, 2018

View reviewed changes

atrick approved these changes Dec 14, 2018

View reviewed changes

rajbarik mentioned this pull request Dec 14, 2018

Generalize and cleanup code for existential specialization. #21316

Merged

rajbarik force-pushed the raj-extend-existential2generic branch from 6b0d1a1 to 1f4c6ca Compare January 5, 2019 01:36

Perform ExistentialSpecializer when SoleConformingType is known

0070730

rajbarik force-pushed the raj-extend-existential2generic branch from 1f4c6ca to 0070730 Compare January 7, 2019 22:48

atrick merged commit bc884d4 into swiftlang:master Jan 15, 2019

Perform ExistentialSpecializer when SoleConformingType is known #20977

Perform ExistentialSpecializer when SoleConformingType is known #20977

Uh oh!

Conversation

rajbarik commented Dec 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!

rajbarik commented Dec 3, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

atrick left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rajbarik Dec 14, 2018 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rajbarik commented Dec 20, 2018

Uh oh!

atrick commented Dec 22, 2018

Uh oh!

rajbarik commented Jan 5, 2019

Uh oh!

rajbarik commented Jan 7, 2019

Uh oh!

atrick commented Jan 7, 2019

Uh oh!

slavapestov commented Jan 7, 2019

Uh oh!

slavapestov commented Jan 7, 2019

Uh oh!

slavapestov commented Jan 7, 2019

Uh oh!

swift-ci commented Jan 7, 2019

Uh oh!

swift-ci commented Jan 7, 2019

Uh oh!

swift-ci commented Jan 7, 2019

Build comment file:

No performance and code size changes

Uh oh!

atrick commented Jan 7, 2019

Uh oh!

swift-ci commented Jan 7, 2019

Uh oh!

swift-ci commented Jan 7, 2019

Uh oh!

slavapestov commented Jan 8, 2019

Uh oh!

rajbarik commented Jan 8, 2019

Uh oh!

rajbarik commented Jan 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rajbarik commented Jan 14, 2019

Uh oh!

atrick commented Jan 14, 2019

Uh oh!

atrick commented Jan 14, 2019

Uh oh!

rajbarik Dec 14, 2018 •

edited

Loading

rajbarik commented Jan 8, 2019 •

edited

Loading