[ownership] Enable ownership lowering /after/ the diagnostic passes. #28042

gottesmm · 2019-11-03T18:30:26Z

I also updated the last group of straggling tests.

gottesmm · 2019-11-03T18:30:32Z

@swift-ci test

gottesmm · 2019-11-03T18:30:36Z

@swift-ci asan test

gottesmm · 2019-11-03T18:30:52Z

@swift-ci test source compatibility

swift-ci · 2019-11-03T19:35:38Z

Build failed
Swift Test Linux Platform
Git Sha - 7250f26eba25ad883a5036fcb53cf8adab6a3ecf

gottesmm · 2019-11-03T19:41:16Z

Passed macOS testing. Lets see what happens with iphonesimulator. I am looking into the swiftpm and linux failure.

gottesmm · 2019-11-03T20:23:52Z

I have a solution for the swiftpm issue.

swift-ci · 2019-11-03T20:31:06Z

Build failed
Swift Test OS X Platform
Git Sha - 7250f26eba25ad883a5036fcb53cf8adab6a3ecf

gottesmm · 2019-11-03T21:16:13Z

Hit some lldb issues. LLDB fix is here: swiftlang/llvm-project#114

Also the macOS tests passed the full test.

gottesmm · 2019-11-03T23:30:03Z

Landed the lldb change (just needed to lower ownership). Curious if I get significantly further this time on the macOS test (ignoring the swiftpm failure that I am going to hit).

gottesmm · 2019-11-03T23:30:10Z

@swift-ci test os x platform

swift-ci · 2019-11-03T23:59:03Z

Build failed
Swift Test OS X Platform
Git Sha - 7250f26eba25ad883a5036fcb53cf8adab6a3ecf

gottesmm · 2019-11-04T00:28:26Z

OS X failure was this:

15:55:44 :0: note: Cannot read '/Users/buildnode/jenkins/workspace/swift-PR-osx/branch-master/buildbot_incremental/swift-macosx-x86_64/./lib/swift/iphoneos/layouts-arm64.yaml'?

I think that might mean a bad node? Going to try again.

gottesmm · 2019-11-04T00:28:33Z

@swift-ci test os x platform

gottesmm · 2019-11-04T00:30:15Z

Looked at the ASAN failure. Was a source kit error. Definitely not mine:

15:04:12 --
15:04:12 Exit Code: 1
15:04:12
15:04:12 Command Output (stderr):
15:04:12 --
15:04:12
/Users/buildnode/jenkins/workspace/swift-PR-osx-ASAN-test/swift/test/SourceKit/CursorInfo/invalid_offset.swift:15:11: error: CHECK: expected string not found in input
15:04:12 // CHECK: (Request Failed): Unable to resolve the start of the token
15:04:12 ^
15:04:12 :1:1: note: scanning from here
15:04:12 {
15:04:12 ^
15:04:12 :34:30: note: possible intended match here
15:04:12 <empty cursor info; internal diagnostic: "Unable to resolve cursor info.">
15:04:12 ^
15:04:12
15:04:12 --
15:04:12
15:04:12 ******.

Everything else passed though.

swift-ci · 2019-11-04T02:04:22Z

Build failed
Swift Test OS X Platform
Git Sha - 7250f26eba25ad883a5036fcb53cf8adab6a3ecf

gottesmm · 2019-11-04T03:11:59Z

Ran the lldb test again with my failure. Failed again, but only in the ref count test:

18:04:16 lldb-Suite :: lang/swift/swift_reference_counting/TestSwiftReferenceCount.py

The failure is since lldb is testing exact reference counts and after my changes the ref count is 4 instead of 3.

gottesmm · 2019-11-12T19:59:51Z

#28208 should fix the swiftpm issue. I discovered some similar fixes I want to do on the load [copy] side, but the load_borrow fix there should take care of that.

gottesmm · 2019-11-12T20:52:10Z

@swift-ci test

gottesmm · 2019-11-12T20:52:30Z

I think I am going to hit an lldb test failure, but the swiftpm issue is fixed.

airspeedswift · 2019-11-12T21:38:39Z

@swift-ci please benchmark

swift-ci · 2019-11-12T21:51:43Z

Build failed
Swift Test Linux Platform
Git Sha - 7250f26eba25ad883a5036fcb53cf8adab6a3ecf

gottesmm · 2019-11-12T22:08:18Z

Linux hit the option set failure. I think that just needs a small test update.

gottesmm · 2019-11-12T22:10:37Z

I am expecting an lldb test failure as well.

gottesmm · 2019-11-12T22:10:54Z

But on the macOS side. I need to provide a separate PR to test with this that updates that lldb test.

swift-ci · 2019-11-12T22:12:48Z

Performance: -O

Regression	OLD	NEW	DELTA	RATIO
SubstringComparable	11	15	+36.4%	0.73x
MapReduceShort	2090	2330	+11.5%	0.90x (?)
Dictionary	520	565	+8.7%	0.92x (?)

Improvement	OLD	NEW	DELTA	RATIO
CharacterLiteralsLarge	108	97	-10.2%	1.11x
FlattenListLoop	5305	4779	-9.9%	1.11x (?)
FlattenListFlatMap	10418	9638	-7.5%	1.08x (?)
ObjectiveCBridgeStubNSDateRefAccess	400	371	-7.2%	1.08x (?)

Code size: -O

Regression	OLD	NEW	DELTA	RATIO
BinaryFloatingPointConversionFromBinaryInteger.o	24993	27172	+8.7%	0.92x
Phonebook.o	11309	11681	+3.3%	0.97x
Combos.o	6684	6828	+2.2%	0.98x
NibbleSort.o	12270	12430	+1.3%	0.99x

Improvement	OLD	NEW	DELTA	RATIO
Substring.o	18093	17389	-3.9%	1.04x
OpenClose.o	3200	3136	-2.0%	1.02x
Codable.o	25279	25007	-1.1%	1.01x

Performance: -Osize

Regression	OLD	NEW	DELTA	RATIO
SubstringComparable	12	16	+33.3%	0.75x
DropWhileAnySeqCRangeIter	163	198	+21.5%	0.82x
DropFirstAnyCollection	164	182	+11.0%	0.90x (?)
Dictionary	575	635	+10.4%	0.91x
MapReduceShort	2220	2420	+9.0%	0.92x (?)
ParseFloat.Double.Exp	25	27	+8.0%	0.93x (?)
ObjectiveCBridgeStubNSDateRefAccess	371	400	+7.8%	0.93x (?)

Improvement	OLD	NEW	DELTA	RATIO
MapReduceLazyCollectionShort	85	41	-51.8%	2.07x
DropWhileAnySeqCntRangeLazy	282	252	-10.6%	1.12x
PrefixAnyCollection	182	164	-9.9%	1.11x
PrefixAnySeqCntRangeLazy	176	159	-9.7%	1.11x

Code size: -Osize

Regression	OLD	NEW	DELTA	RATIO
BinaryFloatingPointConversionFromBinaryInteger.o	25121	27204	+8.3%	0.92x
Combos.o	7230	7326	+1.3%	0.99x
SortIntPyramids.o	11749	11893	+1.2%	0.99x
DictOfArraysToArrayOfDicts.o	23900	24156	+1.1%	0.99x

Improvement	OLD	NEW	DELTA	RATIO
NibbleSort.o	13500	12148	-10.0%	1.11x
OpenClose.o	3544	3240	-8.6%	1.09x
Substring.o	17028	16365	-3.9%	1.04x
Phonebook.o	11596	11188	-3.5%	1.04x
CSVParsing.o	56584	54840	-3.1%	1.03x
StringEnum.o	13125	12893	-1.8%	1.02x
DiffingMyers.o	8281	8145	-1.6%	1.02x

Performance: -Onone

Regression	OLD	NEW	DELTA	RATIO
LinkedList	31800	40720	+28.1%	0.78x
Calculator	1134	1419	+25.1%	0.80x

Code size: -swiftlibs

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: 12-Core Intel Xeon E5
  Processor Speed: 2.7 GHz
  Number of Processors: 1
  Total Number of Cores: 12
  L2 Cache (per Core): 256 KB
  L3 Cache: 30 MB
  Memory: 64 GB

swift-ci · 2019-11-12T22:25:56Z

Build failed
Swift Test OS X Platform
Git Sha - 7250f26eba25ad883a5036fcb53cf8adab6a3ecf

gottesmm · 2019-11-12T22:42:29Z

@swift-ci test source compatibility

gottesmm · 2019-11-12T22:43:05Z

I am fixing another bug right now, but I am going to loop back on this in a bit with the lldb patch/linux fix.

gottesmm · 2019-11-12T22:43:50Z

OS X failed on the lldb test as expected, but all of the normal tests passed.

gottesmm · 2019-11-12T23:38:07Z

I sat down with Davide and un-hardcoded the check for the specific ref count of the object in the test and instead just made sure we can properly get output. The fix is here: swiftlang/llvm-project#294. I am able to land it before landing this so I don't need to do cross repo testing.

gottesmm · 2019-11-17T12:19:47Z

@swift-ci test

gottesmm · 2019-11-17T12:19:57Z

@swift-ci asan test

gottesmm · 2019-11-17T12:20:00Z

@swift-ci asan test

gottesmm · 2019-11-17T12:20:25Z

@swift-ci test source compatibility

gottesmm · 2019-11-17T12:20:27Z

@swift-ci test source compatibility

swift-ci · 2019-11-17T12:20:32Z

Build failed
Swift Test OS X Platform
Git Sha - 7250f26eba25ad883a5036fcb53cf8adab6a3ecf

swift-ci · 2019-11-17T12:22:16Z

Build failed
Swift Test Linux Platform
Git Sha - 7250f26eba25ad883a5036fcb53cf8adab6a3ecf

gottesmm · 2019-11-17T12:22:21Z

@swift-ci test windows platform

gottesmm · 2019-11-17T12:22:24Z

@swift-ci test windows platform

gottesmm · 2019-11-17T12:26:18Z

@swift-ci test windows platform

gottesmm · 2019-11-17T12:27:09Z

@swift-ci test windows platform

gottesmm · 2019-11-17T12:28:46Z

@swift-ci asan test

gottesmm · 2019-11-17T16:53:47Z

Got full greens using full tests.. Going to grab a little zzzs, then see if I can quickly fix the os log test and then smoke and merge.

gottesmm · 2019-11-17T19:29:22Z

I talked with Ravi. I am going to commit that test disabled so I can land this and I am going to post a patch with the post-world fix that Ravi can review.

… complete. I talked with Ravi (the maintainer) about this and he is cool. I am going to provide a patch that he can review to the oslog specific pass that also undisables the test. This test was passing last week when I originally tested it so that suggests the special optimization pass is pattern matching too specific of a pattern.

I also updated the last group of straggling tests.

gottesmm · 2019-11-17T19:55:19Z

Asan, source compat, full Mac/Linux testing, and windows too! I’m going to clean up the git history a little and then smoke test and merge.

gottesmm · 2019-11-17T20:37:36Z

@swift-ci smoke test

gottesmm · 2019-11-17T20:37:41Z

@swift-ci smoke test

gottesmm · 2019-11-17T20:37:48Z

@swift-ci smoke test

drodriguez · 2019-11-18T21:24:30Z

Good morning/evening/night,

I was looking into the failures that appeared in https://ci-external.swift.org/job/oss-swift-RA-linux-ubuntu-16.04-android/4509/ (Android ARMv7) after the introduction of this patch. It seems that the change bool StripOwnershipAfterSerialization = false/true; makes the test test/Frontend/skip-non-inlinable-function-bodies.swift introduced in #20420 fail. It seems, but I am not sure, that the output after this change is introduced avoids the setter to be inlined. Does that makes sense?

I can XFAIL this test, but I will not mind a ELI5 of what's going on to understand the problem better, and maybe find an actual solution.

The two outputs (before and after the patch) are in https://gist.github.com/drodriguez/658c0e0818e1d89c03633150d9d858b1

/cc @harlanhaskins

harlanhaskins · 2019-11-18T21:32:50Z

Text of the failure:

Command Output (stderr):
--
/home/ubuntu/jenkins/workspace/oss-swift-RA-linux-ubuntu-16.04-android/swift/test/Frontend/skip-non-inlinable-function-bodies.swift:160:49: error: CHECK: expected string not found in input
 _blackHole("@inlinable deinit body") // CHECK: @inlinable deinit body
                                                ^
<stdin>:1790:51: note: scanning from here
 %36 = string_literal utf8 "@inlinable setter body" // user: %39
                                                  ^
<stdin>:1831:39: note: possible intended match here
} // end sil function '$s4main6StructV15inlinableSetterSivsTf4dn_n'
                                      ^

--

********************

It looks like the deinit wasn't emitted, maybe that's a side effect of ownership visiting slightly differently?

harlanhaskins · 2019-11-18T21:33:09Z

Anything marked @inlinable needs to be SILGen'd.

drodriguez · 2019-11-18T21:35:46Z

The deinit was emitted, but it is emitted before the setter, but because the setter is checked before, the deinit is not found below the setter, and the test fails. But I imagine is something like you say, that the ownership being enabled or disabled changes the visiting order slightly… and only in ARMv7? (32 bits vs 64 bits?)

(Also, my pbcopy truncated the output… sigh, I will try to recover the full thing)

harlanhaskins · 2019-11-18T21:39:06Z

Ah, I see. That's...not very great. I should probably be passing -emit-sorted-sil...or using CHECK-DAG

drodriguez · 2019-11-18T21:44:49Z

Let me check if that solves the problem. Thanks for the clue!

gottesmm added 2 commits November 17, 2019 11:33

[ownership] Enable ownership lowering /after/ the diagnostic passes.

269762e

I also updated the last group of straggling tests.

gottesmm force-pushed the pr-d5ceb87c25bcf343e5f92bd726f205aea30e3ff5 branch from 3178dc3 to 269762e Compare November 17, 2019 20:37

gottesmm merged commit 8c8bbf1 into swiftlang:master Nov 17, 2019

gottesmm deleted the pr-d5ceb87c25bcf343e5f92bd726f205aea30e3ff5 branch November 17, 2019 23:32

drodriguez mentioned this pull request Nov 18, 2019

[test] Use -emit-sorted-sil and reorder CHECKs to match output. #28339

Merged

[ownership] Enable ownership lowering /after/ the diagnostic passes. #28042

[ownership] Enable ownership lowering /after/ the diagnostic passes. #28042

Uh oh!

Conversation

gottesmm commented Nov 3, 2019

Uh oh!

gottesmm commented Nov 3, 2019

Uh oh!

gottesmm commented Nov 3, 2019

Uh oh!

gottesmm commented Nov 3, 2019

Uh oh!

swift-ci commented Nov 3, 2019

Uh oh!

gottesmm commented Nov 3, 2019

Uh oh!

gottesmm commented Nov 3, 2019

Uh oh!

swift-ci commented Nov 3, 2019

Uh oh!

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

Uh oh!

Uh oh!

gottesmm commented Nov 3, 2019

Uh oh!

gottesmm commented Nov 3, 2019

Uh oh!

swift-ci commented Nov 3, 2019

Uh oh!

gottesmm commented Nov 4, 2019

Uh oh!

gottesmm commented Nov 4, 2019

Uh oh!

gottesmm commented Nov 4, 2019

Uh oh!

swift-ci commented Nov 4, 2019

Uh oh!

gottesmm commented Nov 4, 2019

Uh oh!

gottesmm commented Nov 12, 2019

Uh oh!

gottesmm commented Nov 12, 2019

Uh oh!

gottesmm commented Nov 12, 2019

Uh oh!

airspeedswift commented Nov 12, 2019

Uh oh!

swift-ci commented Nov 12, 2019

Uh oh!

gottesmm commented Nov 12, 2019

Uh oh!

gottesmm commented Nov 12, 2019

Uh oh!

gottesmm commented Nov 12, 2019

Uh oh!

swift-ci commented Nov 12, 2019

Performance: -O

Code size: -O

Performance: -Osize

Code size: -Osize

Performance: -Onone

Code size: -swiftlibs

Uh oh!

swift-ci commented Nov 12, 2019

Uh oh!

gottesmm commented Nov 12, 2019

Uh oh!

gottesmm commented Nov 12, 2019

Uh oh!

gottesmm commented Nov 12, 2019

Uh oh!

gottesmm commented Nov 12, 2019

Uh oh!

gottesmm commented Nov 17, 2019

Uh oh!

gottesmm commented Nov 17, 2019

Uh oh!

gottesmm commented Nov 17, 2019

Uh oh!

gottesmm commented Nov 17, 2019

gottesmm commented Nov 3, 2019 •

edited

Loading

drodriguez commented Nov 18, 2019 •

edited

Loading

harlanhaskins commented Nov 18, 2019 •

edited

Loading