Fix ExistentialSpecializer bugs for swiftpm and enable existential-specializer for sil-opt #20617

rajbarik · 2018-11-16T02:27:00Z

No description provided.

rajbarik · 2018-11-16T02:27:27Z

@slavapestov fix for swiftpm bugs..

rajbarik · 2018-11-20T20:48:06Z

@slavapestov test cases added and made into a single commit. Can you check this one?

rajbarik · 2018-11-26T18:21:03Z

@slavapestov can you check this one now?

rajbarik · 2018-11-26T21:14:35Z

lib/SILOptimizer/FunctionSignatureTransforms/ExistentialSpecializer.cpp

@@ -101,7 +101,8 @@ static bool findConcreteType(ApplySite AI, int ArgIdx, CanType &ConcreteType) {
    SILValue InitExistential =
        findInitExistentialFromGlobalAddrAndApply(GAI, AI, ArgIdx);
    /// If the Arg is already init_existential, return the concrete type.
-    if (findConcreteTypeFromInitExistential(InitExistential, ConcreteType)) {
+    if (InitExistential &&


This fixes the case where global_addr analysis can not find an init_existential. See "global_addr_init" test case below.

rajbarik · 2018-11-26T21:15:06Z

lib/SILOptimizer/FunctionSignatureTransforms/ExistentialSpecializer.cpp

@@ -257,6 +258,12 @@ bool ExistentialSpecializer::canSpecializeCalleeFunction(ApplySite &Apply) {
  if (Callee->getInlineStrategy() == Inline_t::AlwaysInline)
    return false;

+  /// Ignore externally linked functions with public_external or higher
+  /// linkage.
+  if (isAvailableExternally(Callee->getLinkage())) {


This fixes the swiftpm bug related to external linkage functions causing name mangling issues. See test case: s7dealloc20wrap_foo_ncp_another1aSiAA1P_pz_tF below.

rajbarik · 2018-11-26T21:16:58Z

lib/SILOptimizer/FunctionSignatureTransforms/ExistentialTransform.cpp

-        ReturnInsts.insert(RI);
-      }
+  /// Find the set of return instructions in a function.
+  for (auto &BB : NewF) {


This fixes the old code that was inserting dealloc_stack after destroy_addr which is wrong. dealloc_stack are now inserted just before return instructions. See test case: $s7dealloc12wrap_foo_ncp1a1bSiAA1P_pz_AaE_pztFTf4ee_n below.

rajbarik · 2018-11-26T21:17:46Z

include/swift/AST/SILOptions.h

@@ -51,7 +51,7 @@ class SILOptions {
  bool RemoveRuntimeAsserts = false;

  /// Enable existential specializer optimization.
-  bool ExistentialSpecializer = false;
+  bool ExistentialSpecializer = true;


default enable of ExistentialSpecializer.

slavapestov · 2018-11-27T05:20:18Z

Can you file a separate PR with the bug fixes before we enable this by default?

slavapestov · 2018-11-27T05:20:22Z

@swift-ci Please test

slavapestov · 2018-11-27T05:20:28Z

@swift-ci Please test source compatibility

slavapestov · 2018-11-27T05:20:34Z

@swift-ci Please benchmark

swift-ci · 2018-11-27T05:22:17Z

Build failed
Swift Test Linux Platform
Git Sha - e9e15b27f6daf92e7c6c5ba81fa3c3c91a4a2c3c

swift-ci · 2018-11-27T05:22:17Z

Build failed
Swift Test OS X Platform
Git Sha - e9e15b27f6daf92e7c6c5ba81fa3c3c91a4a2c3c

swift-ci · 2018-11-27T06:06:36Z

Build comment file:

Performance: -O

TEST	OLD	NEW	DELTA	RATIO
Regression
StringEqualPointerComparison	538	589	+9.5%	0.91x
Improvement
ObserverUnappliedMethod	2227	1848	-17.0%	1.21x
ObserverForwarderStruct	843	749	-11.2%	1.13x (?)
EqualStringSubstring	11	10	-9.1%	1.10x (?)

Code size: -O

TEST	OLD	NEW	DELTA	RATIO
Improvement
ProtocolDispatch2.o	1888	1766	-6.5%	1.07x
ObserverForwarderStruct.o	3594	3546	-1.3%	1.01x
ObserverUnappliedMethod.o	5266	5202	-1.2%	1.01x

Performance: -Osize

TEST	OLD	NEW	DELTA	RATIO
Regression
PrefixAnyCollection	148	14383	+9618.2%	0.01x
SuffixAnyCollection	53	4802	+8960.2%	0.01x
DropLastAnyCollection	53	4801	+8958.3%	0.01x
DropFirstAnyCollection	164	14371	+8662.8%	0.01x
ReversedBidirectional	15486	28483	+83.9%	0.54x
CStringLongAscii	393	463	+17.8%	0.85x
StringEqualPointerComparison	512	589	+15.0%	0.87x
DropWhileAnySeqCntRangeLazy	221	253	+14.5%	0.87x
CharacterLiteralsLarge	89	100	+12.4%	0.89x
Improvement
ObserverForwarderStruct	889	773	-13.0%	1.15x (?)
ObserverUnappliedMethod	2123	1887	-11.1%	1.13x (?)
PrefixWhileAnyCollectionLazy	158	143	-9.5%	1.10x
PrefixAnySeqCntRangeLazy	158	143	-9.5%	1.10x
ReversedDictionary2	307	283	-7.8%	1.08x

Code size: -Osize

TEST	OLD	NEW	DELTA	RATIO
Regression
Prefix.o	22305	22601	+1.3%	0.99x
DropFirst.o	22428	22724	+1.3%	0.99x
Improvement
ReversedCollections.o	11564	10670	-7.7%	1.08x
ProtocolDispatch2.o	1932	1827	-5.4%	1.06x
ObserverForwarderStruct.o	3838	3638	-5.2%	1.05x
ObserverUnappliedMethod.o	5571	5362	-3.8%	1.04x
StackPromo.o	2373	2349	-1.0%	1.01x

Performance: -Onone

TEST	OLD	NEW	DELTA	RATIO
Improvement
ArrayOfPOD	766	689	-10.1%	1.11x (?)

How to read the data

The tables contain differences in performance which are larger than 8% and differences in code size which are larger than 1%.

If you see any unexpected regressions, you should consider fixing the
regressions before you merge the PR.

Noise: Sometimes the performance results (not code size!) contain false
alarms. Unexpected regressions which are marked with '(?)' are probably noise.
If you see regressions which you cannot explain you can try to run the
benchmarks again. If regressions still show up, please consult with the
performance team (@eeckstein).

Hardware Overview

  Model Name: Mac Pro
  Model Identifier: MacPro6,1
  Processor Name: 8-Core Intel Xeon E5
  Processor Speed: 3 GHz
  Number of Processors: 1
  Total Number of Cores: 8
  L2 Cache (per Core): 256 KB
  L3 Cache: 25 MB
  Memory: 64 GB

--------------

rajbarik · 2018-11-27T18:12:57Z

I notice that you ran tests. Do you still suggest I need to separate them into two PRs -- one with just ExistentialSpecializer set to true in SILOptions (this exists here #19820) and another with all bug fixes (I can modify this one to remove the set to ExistentialSpecializer=true)?

A question: if I do not set "ExistentialSpecializer=true" in SILOptions.h, ExistentialSpecializer does not get invoked with sil-opt using the "-existential-specializer" (aka, "// RUN: %target-sil-opt -assume-parsing-unqualified-ownership-sil -enable-sil-verify-all %s -existential-specializer | %FileCheck %s") used in my test case existential_transform_extras.sil. How do I enable this at sil-opt level?

rajbarik · 2018-11-29T23:22:19Z

@slavapestov anything for me here?

slavapestov · 2018-11-30T18:43:13Z

Since we have performance regressions here, I’d like you to merge the bug fixes separately. Did you figure out how to get the tests to pass even when the flag is off?

rajbarik · 2018-11-30T19:17:14Z

Looking at Performance regression with -O for StringEqualPointerComparison:
This test has nothing to do with Protocols.

rajbarik · 2018-11-30T20:22:08Z

Similarly, none of the regressed one have anything to do with protocols.

What am I missing?

…ecializer in sil-opt

rajbarik · 2018-11-30T22:33:32Z

tools/sil-opt/SILOpt.cpp

@@ -167,6 +167,11 @@ static llvm::cl::opt<int>
 SILInlineThreshold("sil-inline-threshold", llvm::cl::Hidden,
                   llvm::cl::init(-1));

+static llvm::cl::opt<bool>
+SILExistentialSpecializer("enable-sil-existential-specializer", 


Add a flag to enable and disable existential-specializer from sil-opt.

rajbarik · 2018-11-30T22:33:49Z

tools/sil-opt/SILOpt.cpp

@@ -328,6 +333,7 @@ int main(int argc, char **argv) {
  // Setup the SIL Options.
  SILOptions &SILOpts = Invocation.getSILOptions();
  SILOpts.InlineThreshold = SILInlineThreshold;
+  SILOpts.ExistentialSpecializer = SILExistentialSpecializer;


Store it in the module's Options.

rajbarik · 2018-11-30T22:35:54Z

OK. I committed some more changes to make it independent:

It does not default enable ExistentialSpecializer option from the compiler. We should use the old PR Enable ExistentialSpecializer by default #19820 for turning on by default.
Introduced a enable flag in sil-opt to be able to test it using sil-opt.

rajbarik · 2018-11-30T22:40:48Z

@slavapestov can we run tests on this one and accept these changes quickly?

The default enabler of ExistentialSpecializer is a separate PR at #19820

slavapestov · 2018-12-01T00:06:33Z

@swift-ci Please test

slavapestov · 2018-12-01T00:06:49Z

@swift-ci Please test

swift-ci · 2018-12-01T00:11:38Z

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

swift-ci · 2018-12-01T01:22:09Z

Build failed
Swift Test Linux Platform
Git Sha - c4df82b

slavapestov · 2018-12-03T18:19:45Z

@swift-ci Please smoke test Linux

rajbarik mentioned this pull request Nov 16, 2018

Enable ExistentialSpecializer by default #19820

Merged

rajbarik force-pushed the raj-es-fix-default-enable branch 3 times, most recently from df27d8f to 29cbac7 Compare November 20, 2018 20:46

rajbarik changed the title ~~ExistentialTransform fixes for swiftpm bugs~~ Enable ExistentialSpecializer by default and fix swiftpm bugs Nov 20, 2018

rajbarik force-pushed the raj-es-fix-default-enable branch 2 times, most recently from ffd2d6f to 6e32c8d Compare November 26, 2018 21:12

rajbarik commented Nov 26, 2018

View reviewed changes

slavapestov self-assigned this Nov 27, 2018

rajbarik mentioned this pull request Nov 27, 2018

Fix GlobalAddr in ExistentialSpecializer #20541

Closed

ExistentialTransform fixes for swiftpm bugs and enable existential-sp…

c4df82b

…ecializer in sil-opt

rajbarik force-pushed the raj-es-fix-default-enable branch from 6e32c8d to c4df82b Compare November 30, 2018 22:31

rajbarik changed the title ~~Enable ExistentialSpecializer by default and fix swiftpm bugs~~ Fix ExistentialSpecializer bugs for swiftpm and enable existential-specializer for sil-opt Nov 30, 2018

rajbarik commented Nov 30, 2018

View reviewed changes

rajbarik merged commit be0bdd1 into swiftlang:master Dec 3, 2018

Fix ExistentialSpecializer bugs for swiftpm and enable existential-specializer for sil-opt #20617

Fix ExistentialSpecializer bugs for swiftpm and enable existential-specializer for sil-opt #20617

Uh oh!

Conversation

rajbarik commented Nov 16, 2018

Uh oh!

rajbarik commented Nov 16, 2018

Uh oh!

rajbarik commented Nov 20, 2018

Uh oh!

rajbarik commented Nov 26, 2018

Uh oh!

rajbarik Nov 26, 2018

Choose a reason for hiding this comment

Uh oh!

rajbarik Nov 26, 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!

rajbarik Nov 26, 2018

Choose a reason for hiding this comment

Uh oh!

rajbarik Nov 26, 2018

Choose a reason for hiding this comment

Uh oh!

slavapestov commented Nov 27, 2018

Uh oh!

slavapestov commented Nov 27, 2018

Uh oh!

slavapestov commented Nov 27, 2018

Uh oh!

slavapestov commented Nov 27, 2018

Uh oh!

swift-ci commented Nov 27, 2018

Uh oh!

swift-ci commented Nov 27, 2018

Uh oh!

swift-ci commented Nov 27, 2018

Build comment file:

Performance: -O

Code size: -O

Performance: -Osize

Code size: -Osize

Performance: -Onone

Uh oh!

rajbarik commented Nov 27, 2018

Uh oh!

rajbarik commented Nov 29, 2018

Uh oh!

slavapestov commented Nov 30, 2018

Uh oh!

rajbarik commented Nov 30, 2018

Uh oh!

rajbarik commented Nov 30, 2018

Uh oh!

rajbarik Nov 30, 2018

Choose a reason for hiding this comment

Uh oh!

rajbarik Nov 30, 2018

Choose a reason for hiding this comment

Uh oh!

rajbarik commented Nov 30, 2018

Uh oh!

rajbarik commented Nov 30, 2018

Uh oh!

slavapestov commented Dec 1, 2018

Uh oh!

slavapestov commented Dec 1, 2018

Uh oh!

swift-ci commented Dec 1, 2018

Uh oh!

swift-ci commented Dec 1, 2018

Uh oh!

slavapestov commented Dec 3, 2018

Uh oh!

Uh oh!

rajbarik Nov 26, 2018 •

edited

Loading