Skip to content

[CanonicalizeOSSALifetime] Run on lexical lifetimes. #63606

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 6 commits into from
Mar 27, 2023
Merged

[CanonicalizeOSSALifetime] Run on lexical lifetimes. #63606

merged 6 commits into from
Mar 27, 2023

Conversation

nate-chandler
Copy link
Contributor

@nate-chandler nate-chandler commented Feb 12, 2023 •
edited
Loading

Previously, the utility bailed out on lexical lifetimes because it didn't respect deinit barriers. Here, deinit barriers are found and added to liveness if the value is lexical. This enables copies to be propagated without hoisting destroys over deinit barriers.

rdar://104630103

@nate-chandler nate-chandler force-pushed the canonicalize_ossa_lifetime/lexical_values branch from 3ca1472 to 1034d65 Compare February 12, 2023 23:36
atrick
atrick reviewed Feb 23, 2023
View reviewed changes
@nate-chandler nate-chandler force-pushed the canonicalize_ossa_lifetime/lexical_values branch 3 times, most recently from 46970df to a6bd234 Compare February 24, 2023 23:31
@nate-chandler nate-chandler force-pushed the canonicalize_ossa_lifetime/lexical_values branch 6 times, most recently from 027dd25 to 989faa0 Compare March 22, 2023 01:01
@nate-chandler
Copy link
Contributor Author

@swift-ci please test

@nate-chandler
Copy link
Contributor Author

@swift-ci please benchmark

@nate-chandler nate-chandler marked this pull request as ready for review March 22, 2023 04:01
@nate-chandler
Copy link
Contributor Author

@swift-ci please test macos platform

1 similar comment
@nate-chandler
Copy link
Contributor Author

@swift-ci please test macos platform

@nate-chandler nate-chandler requested a review from atrick March 22, 2023 18:07
@nate-chandler
Copy link
Contributor Author 

------- Performance (x86_64): -O -------

REGRESSION                                OLD        NEW        DELTA     RATIO
StringWithCString2                        0.0        0.002      +200.0%   **0.33x**
Calculator                                139.765    157.692    +12.8%    **0.89x (?)**
Data.hash.Empty                           52.261     56.632     +8.4%     **0.92x (?)**
Chars2                                    3147.727   3408.333   +8.3%     **0.92x (?)**
StringDistance.characters.ascii           3100.0     3352.0     +8.1%     **0.92x (?)**

IMPROVEMENT                               OLD        NEW        DELTA     RATIO
FlattenListLoop                           1624.0     1051.0     -35.3%    **1.55x (?)**
FlattenListFlatMap                        4290.0     3097.0     -27.8%    **1.39x (?)**
EqualSubstringSubstringGenericEquatable   30.269     23.15      -23.5%    **1.31x**
LessSubstringSubstring                    29.829     22.857     -23.4%    **1.31x (?)**
EqualSubstringString                      29.938     22.969     -23.3%    **1.30x (?)**
EqualSubstringSubstring                   30.25      23.294     -23.0%    **1.30x (?)**
LessSubstringSubstringGenericComparable   29.625     22.97      -22.5%    **1.29x (?)**
EqualStringSubstring                      30.154     23.407     -22.4%    **1.29x**
ObjectiveCBridgeStubDateAccess            152.5      130.727    -14.3%    **1.17x (?)**
StringComparison_longSharedPrefix         241.0      207.778    -13.8%    **1.16x (?)**
String.replaceSubrange.Substring.Small    44.913     40.583     -9.6%     **1.11x (?)**
Set.isDisjoint.Box.Empty                  55.55      51.795     -6.8%     **1.07x (?)**

------- Code size: -O -------

------- Performance (x86_64): -Osize -------

REGRESSION                                OLD        NEW       DELTA    RATIO
Dictionary4                               157.5      198.125   +25.8%   **0.79x (?)**
Dictionary4OfObjects                      202.0      240.571   +19.1%   **0.84x (?)**
Chars2                                    3077.273   3564.0    +15.8%   **0.86x (?)**
ArrayInClass                              174.412    196.406   +12.6%   **0.89x (?)**
NormalizedIterator_ascii                  88.783     99.292    +11.8%   **0.89x (?)**
Calculator                                146.667    162.0     +10.5%   **0.91x (?)**
Breadcrumbs.UTF16ToIdxRange.longASCII     13.786     15.067    +9.3%    **0.91x (?)**
Breadcrumbs.UTF16ToIdx.longASCII          39.258     42.491    +8.2%    **0.92x (?)**
Breadcrumbs.IdxToUTF16Range.longASCII     8.101      8.755     +8.1%    **0.93x (?)**
StringDistance.characters.ascii           3106.0     3347.0    +7.8%    **0.93x (?)**

IMPROVEMENT                               OLD        NEW       DELTA    RATIO
FlattenListLoop                           1619.0     984.0     -39.2%   **1.65x (?)**
EqualSubstringSubstringGenericEquatable   29.923     22.742    -24.0%   **1.32x (?)**
EqualStringSubstring                      29.63      22.733    -23.3%   **1.30x**
EqualSubstringSubstring                   29.628     22.742    -23.2%   **1.30x (?)**
EqualSubstringString                      29.615     22.75     -23.2%   **1.30x**
LessSubstringSubstring                    30.258     23.302    -23.0%   **1.30x (?)**
LessSubstringSubstringGenericComparable   30.375     23.406    -22.9%   **1.30x**
Set.filter.Int100.24k                     38.703     30.289    -21.7%   **1.28x (?)**
Set.filter.Int100.20k                     32.627     25.646    -21.4%   **1.27x**
Set.filter.Int100.16k                     26.58      21.0      -21.0%   **1.27x (?)**
Set.filter.Int100.28k                     47.038     37.431    -20.4%   **1.26x (?)**
Set.subtracting.Empty.Box                 22.647     19.072    -15.8%   **1.19x (?)**
StringComparison_longSharedPrefix         243.3      210.111   -13.6%   **1.16x (?)**
String.replaceSubrange.Substring.Small    44.48      39.923    -10.2%   **1.11x (?)**
String.replaceSubrange.ArrChar.Small      41.688     38.298    -8.1%    **1.09x (?)**

------- Performance (x86_64): -Onone -------

REGRESSION                                OLD        NEW        DELTA    RATIO
BitCount                                  955.5      1064.5     +11.4%   **0.90x (?)**
NopDeinit                                 141200.0   152000.0   +7.6%    **0.93x (?)**

IMPROVEMENT                               OLD        NEW        DELTA    RATIO
ArrayAppendGenericStructs                 1562.0     650.0      -58.4%   **2.40x (?)**
LessSubstringSubstring                    34.357     27.281     -20.6%   **1.26x (?)**
EqualSubstringSubstringGenericEquatable   32.29      25.692     -20.4%   **1.26x (?)**
EqualSubstringSubstring                   34.483     27.568     -20.1%   **1.25x**
LessSubstringSubstringGenericComparable   32.88      26.394     -19.7%   **1.25x (?)**
EqualStringSubstring                      34.167     27.619     -19.2%   **1.24x (?)**
EqualSubstringString                      34.3       27.758     -19.1%   **1.24x (?)**
CharacterLiteralsLarge                    451.0      389.0      -13.7%   **1.16x (?)**
ObjectiveCBridgeStubFromNSDateRef         3190.0     2800.0     -12.2%   **1.14x (?)**
String.replaceSubrange.Substring.Small    46.667     41.24      -11.6%   **1.13x (?)**
CxxStringConversion.swift.to.cxx          97.727     89.667     -8.2%    **1.09x (?)**
String.replaceSubrange.ArrChar.Small      42.641     39.667     -7.0%    **1.07x (?)**

------- Code size: -swiftlibs -------

@nate-chandler nate-chandler force-pushed the canonicalize_ossa_lifetime/lexical_values branch from 989faa0 to 5182539 Compare March 23, 2023 04:13
@nate-chandler
Copy link
Contributor Author

@swift-ci please test

2 similar comments
@nate-chandler
Copy link
Contributor Author

@swift-ci please test

@nate-chandler
Copy link
Contributor Author

@swift-ci please test

atrick
atrick approved these changes Mar 24, 2023
View reviewed changes
Copy link
Contributor

@atrick atrick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!

A couple of suggestions...

@nate-chandler nate-chandler force-pushed the canonicalize_ossa_lifetime/lexical_values branch 2 times, most recently from cb84bd4 to bc382f4 Compare March 25, 2023 01:09
@nate-chandler
Copy link
Contributor Author

@swift-ci please test

@nate-chandler
[Reachability] Removed restrictive assert.
3e9f112
@nate-chandler
[Reachability] NFC: Allow more initial blocks.
4d40429 
Allow clients to specify any number (an array) of blocks beyond which
dataflow won't propagate rather than 1 or 0 (a pointer).
@nate-chandler
[Reachability] Added findBarriersBackward.
5614b63
@nate-chandler
[Test] Unit test lexical destroy folding.
1e4086f 
Adapt a preexisting test to be a unit test that runs the utility.
@nate-chandler
Copy link
Contributor Author

@swift-ci please test

@nate-chandler
[CanonOSSALifetime] Fixed term boundary extension.
f88ca51 
If multiple terminators which branch to the same merge block are added
to the boundary, depending on whether a destroy_value can be found
within the block either (a) every terminator must be added to the
boundary or (b) the destroy_value must be added to the boundary exactly
once.
@nate-chandler nate-chandler force-pushed the canonicalize_ossa_lifetime/lexical_values branch from bc382f4 to b9960b7 Compare March 25, 2023 23:05
@nate-chandler
Copy link
Contributor Author

@swift-ci please test

@nate-chandler
[CanonOSSALifetime] Run on lexical lifetimes.
ec3f005 
Previously, the utility bailed out on lexical lifetimes because it
didn't respect deinit barriers.  Here, deinit barriers are found and
added to liveness if the value is lexical.  This enables copies to be
propagated without hoisting destroys over deinit barriers.

rdar://104630103
@nate-chandler nate-chandler force-pushed the canonicalize_ossa_lifetime/lexical_values branch from b9960b7 to ec3f005 Compare March 26, 2023 04:17
@nate-chandler
Copy link
Contributor Author

@swift-ci please test

@nate-chandler nate-chandler merged commit c0d7a82 into swiftlang:main Mar 27, 2023
@nate-chandler nate-chandler deleted the canonicalize_ossa_lifetime/lexical_values branch March 27, 2023 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@atrick atrick atrick approved these changes

@meg-gupta meg-gupta Awaiting requested review from meg-gupta

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@nate-chandler @atrick