Skip to content

[Stdlib] Make the comparison operatosr on concrete Int types transparent #18169

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 1 commit into from
Jul 26, 2018
Merged

[Stdlib] Make the comparison operatosr on concrete Int types transparent #18169

merged 1 commit into from
Jul 26, 2018

Conversation

ravikandhadai
Copy link
Contributor

This makes them symmetrical to (<) operators, and makes constant folding
and unreachable code analysis more precise and consistent.

rdar://39516135

@ravikandhadai
Copy link
Contributor Author

@swift-ci please smoke test

@ravikandhadai
Copy link
Contributor Author

@swift-ci please test Source Compatibility

@airspeedswift
Copy link
Member

Should we do the others as well?

@airspeedswift
Copy link
Member

@swift-ci please smoke test compiler performance

@ravikandhadai
Copy link
Contributor Author

@airspeedswift As far as I can see, expect for BinaryInteger.<(lhs: Self, rhs: Other) almost all comparison operators: <, >, <= and >= are tiny, non-recursive, functions which I think could be made transparent. I could try making all of them transparent.

@swift-ci
Copy link
Contributor

Build comment file:

Summary for master smoketest

Unexpected test results, excluded stats for Kingfisher, ReactiveCocoa

No regressions above thresholds

Debug

debug brief

Regressed (0)
name old new delta delta_pct
Improved (0)
name old new delta delta_pct
Unchanged (delta < 1.0% or delta < 100.0ms) (2)
name old new delta delta_pct
LLVM.NumLLVMBytesOutput 9,381,828 9,380,968 -860 -0.01%
time.swift-driver.wall 19.7s 19.7s -9.9ms -0.05%

debug detailed

Regressed (0)
name old new delta delta_pct
Improved (0)
name old new delta delta_pct
Unchanged (delta < 1.0% or delta < 100.0ms) (23)
name old new delta delta_pct
AST.NumImportedExternalDefinitions 5,870 5,870 0 0.0%
AST.NumLoadedModules 1,281 1,281 0 0.0%
AST.NumTotalClangImportedEntities 17,540 17,540 0 0.0%
AST.NumUsedConformances 1,283 1,283 0 0.0%
IRModule.NumIRBasicBlocks 31,258 31,258 0 0.0%
IRModule.NumIRFunctions 17,189 17,177 -12 -0.07%
IRModule.NumIRGlobals 14,002 14,002 0 0.0%
IRModule.NumIRInsts 437,761 437,761 0 0.0%
IRModule.NumIRValueSymbols 29,058 29,046 -12 -0.04%
LLVM.NumLLVMBytesOutput 9,381,828 9,380,968 -860 -0.01%
SILModule.NumSILGenFunctions 8,036 8,036 0 0.0%
SILModule.NumSILOptFunctions 10,969 10,969 0 0.0%
Sema.NumConformancesDeserialized 41,025 41,025 0 0.0%
Sema.NumConstraintScopes 73,605 73,605 0 0.0%
Sema.NumDeclsDeserialized 258,472 258,472 0 0.0%
Sema.NumDeclsValidated 27,541 27,541 0 0.0%
Sema.NumFunctionsTypechecked 4,974 4,974 0 0.0%
Sema.NumGenericSignatureBuilders 11,889 11,889 0 0.0%
Sema.NumLazyGenericEnvironments 50,511 50,511 0 0.0%
Sema.NumLazyGenericEnvironmentsLoaded 5,929 5,929 0 0.0%
Sema.NumLazyIterableDeclContexts 38,423 38,423 0 0.0%
Sema.NumTypesDeserialized 275,817 275,822 5 0.0%
Sema.NumTypesValidated 35,842 35,842 0 0.0%

Release

release brief

Regressed (0)
name old new delta delta_pct
Improved (0)
name old new delta delta_pct
Unchanged (delta < 1.0% or delta < 100.0ms) (2)
name old new delta delta_pct
LLVM.NumLLVMBytesOutput 10,552,536 10,551,348 -1,188 -0.01%
time.swift-driver.wall 32.6s 32.6s -15.3ms -0.05%

release detailed

Regressed (0)
name old new delta delta_pct
Improved (0)
name old new delta delta_pct
Unchanged (delta < 1.0% or delta < 100.0ms) (23)
name old new delta delta_pct
AST.NumImportedExternalDefinitions 1,411 1,411 0 0.0%
AST.NumLoadedModules 100 100 0 0.0%
AST.NumTotalClangImportedEntities 4,936 4,936 0 0.0%
AST.NumUsedConformances 1,286 1,286 0 0.0%
IRModule.NumIRBasicBlocks 34,314 34,314 0 0.0%
IRModule.NumIRFunctions 15,340 15,340 0 0.0%
IRModule.NumIRGlobals 13,664 13,664 0 0.0%
IRModule.NumIRInsts 338,863 338,863 0 0.0%
IRModule.NumIRValueSymbols 27,261 27,261 0 0.0%
LLVM.NumLLVMBytesOutput 10,552,536 10,551,348 -1,188 -0.01%
SILModule.NumSILGenFunctions 6,179 6,179 0 0.0%
SILModule.NumSILOptFunctions 9,346 9,346 0 0.0%
Sema.NumConformancesDeserialized 19,641 19,641 0 0.0%
Sema.NumConstraintScopes 72,201 72,201 0 0.0%
Sema.NumDeclsDeserialized 55,728 55,728 0 0.0%
Sema.NumDeclsValidated 20,786 20,786 0 0.0%
Sema.NumFunctionsTypechecked 3,079 3,079 0 0.0%
Sema.NumGenericSignatureBuilders 2,841 2,841 0 0.0%
Sema.NumLazyGenericEnvironments 9,786 9,786 0 0.0%
Sema.NumLazyGenericEnvironmentsLoaded 1,362 1,362 0 0.0%
Sema.NumLazyIterableDeclContexts 5,654 5,654 0 0.0%
Sema.NumTypesDeserialized 71,316 71,316 0 0.0%
Sema.NumTypesValidated 17,086 17,086 0 0.0%

@moiseev
Copy link
Contributor

moiseev commented Jul 24, 2018

Exercise caution, though... One of the metrics we don't gather is the memory consumption while compiling the standard library itself. And it can grow significantly with more @_transparent functions.

@ravikandhadai
Copy link
Contributor Author

@moiseev I see. Thanks for the heads up Max.

@ravikandhadai
[Stdlib] Make the comparison operators (<, >, <= and >=) on concrete …
81fad3c 
…Int types transparent.

This makes all operators symmetric, and makes constant folding
and unreachable code analysis more precise and consistent.

<rdar://39516135>
@ravikandhadai ravikandhadai force-pushed the InconsistentUnreachWarning branch from 0a43150 to 81fad3c Compare July 25, 2018 20:32
@ravikandhadai ravikandhadai changed the title [Stdlib] Make the greater than (>) operator on concrete Int types transparent [Stdlib] Make the comparison operatosr on concrete Int types transparent Jul 25, 2018
@ravikandhadai
Copy link
Contributor Author

@swift-ci please smoke test

@ravikandhadai
Copy link
Contributor Author

@swift-ci please test Source Compatibility

@ravikandhadai
Copy link
Contributor Author

@swift-ci please smoke test compiler performance

@ravikandhadai
Copy link
Contributor Author

@moiseev @airspeedswift I have marked two more methods on concrete integer types as transparent. Now all comparison operators on concrete integer types are transparent. I checked the optimized SIL of these functions and they have about 5 to 10 lines of SIL instructions. Also, there do not seem to be any noticeable increase in the memory usage while compiling stdlib.

@swift-ci
Copy link
Contributor

Build comment file:

Summary for master smoketest

Unexpected test results, excluded stats for Kingfisher, ReactiveCocoa

No regressions above thresholds

Debug

debug brief

Regressed (0)
name old new delta delta_pct
Improved (0)
name old new delta delta_pct
Unchanged (delta < 1.0% or delta < 100.0ms) (2)
name old new delta delta_pct
LLVM.NumLLVMBytesOutput 9,385,968 9,383,452 -2,516 -0.03%
time.swift-driver.wall 19.8s 19.6s -166.8ms -0.84%

debug detailed

Regressed (0)
name old new delta delta_pct
Improved (0)
name old new delta delta_pct
Unchanged (delta < 1.0% or delta < 100.0ms) (23)
name old new delta delta_pct
AST.NumImportedExternalDefinitions 5,870 5,870 0 0.0%
AST.NumLoadedModules 1,281 1,281 0 0.0%
AST.NumTotalClangImportedEntities 17,540 17,540 0 0.0%
AST.NumUsedConformances 1,283 1,283 0 0.0%
IRModule.NumIRBasicBlocks 31,394 31,358 -36 -0.11%
IRModule.NumIRFunctions 17,193 17,173 -20 -0.12%
IRModule.NumIRGlobals 13,961 13,971 10 0.07%
IRModule.NumIRInsts 438,866 438,764 -102 -0.02%
IRModule.NumIRValueSymbols 29,032 29,022 -10 -0.03%
LLVM.NumLLVMBytesOutput 9,385,968 9,383,452 -2,516 -0.03%
SILModule.NumSILGenFunctions 8,036 8,036 0 0.0%
SILModule.NumSILOptFunctions 10,968 10,968 0 0.0%
Sema.NumConformancesDeserialized 40,644 40,644 0 0.0%
Sema.NumConstraintScopes 73,605 73,605 0 0.0%
Sema.NumDeclsDeserialized 258,483 258,483 0 0.0%
Sema.NumDeclsValidated 27,541 27,541 0 0.0%
Sema.NumFunctionsTypechecked 4,974 4,974 0 0.0%
Sema.NumGenericSignatureBuilders 11,890 11,890 0 0.0%
Sema.NumLazyGenericEnvironments 50,511 50,511 0 0.0%
Sema.NumLazyGenericEnvironmentsLoaded 5,929 5,929 0 0.0%
Sema.NumLazyIterableDeclContexts 38,424 38,424 0 0.0%
Sema.NumTypesDeserialized 275,712 275,717 5 0.0%
Sema.NumTypesValidated 35,842 35,842 0 0.0%

Release

release brief

Regressed (0)
name old new delta delta_pct
Improved (0)
name old new delta delta_pct
Unchanged (delta < 1.0% or delta < 100.0ms) (2)
name old new delta delta_pct
LLVM.NumLLVMBytesOutput 10,553,024 10,554,828 1,804 0.02%
time.swift-driver.wall 32.5s 32.6s 56.3ms 0.17%

release detailed

Regressed (0)
name old new delta delta_pct
Improved (0)
name old new delta delta_pct
Unchanged (delta < 1.0% or delta < 100.0ms) (23)
name old new delta delta_pct
AST.NumImportedExternalDefinitions 1,411 1,411 0 0.0%
AST.NumLoadedModules 100 100 0 0.0%
AST.NumTotalClangImportedEntities 4,936 4,936 0 0.0%
AST.NumUsedConformances 1,286 1,286 0 0.0%
IRModule.NumIRBasicBlocks 34,247 34,224 -23 -0.07%
IRModule.NumIRFunctions 15,321 15,315 -6 -0.04%
IRModule.NumIRGlobals 13,598 13,608 10 0.07%
IRModule.NumIRInsts 338,827 338,714 -113 -0.03%
IRModule.NumIRValueSymbols 27,187 27,191 4 0.01%
LLVM.NumLLVMBytesOutput 10,553,024 10,554,828 1,804 0.02%
SILModule.NumSILGenFunctions 6,179 6,179 0 0.0%
SILModule.NumSILOptFunctions 9,364 9,364 0 0.0%
Sema.NumConformancesDeserialized 19,579 19,579 0 0.0%
Sema.NumConstraintScopes 72,201 72,201 0 0.0%
Sema.NumDeclsDeserialized 55,766 55,766 0 0.0%
Sema.NumDeclsValidated 20,786 20,786 0 0.0%
Sema.NumFunctionsTypechecked 3,079 3,079 0 0.0%
Sema.NumGenericSignatureBuilders 2,841 2,841 0 0.0%
Sema.NumLazyGenericEnvironments 9,787 9,787 0 0.0%
Sema.NumLazyGenericEnvironmentsLoaded 1,360 1,360 0 0.0%
Sema.NumLazyIterableDeclContexts 5,654 5,654 0 0.0%
Sema.NumTypesDeserialized 71,366 71,366 0 0.0%
Sema.NumTypesValidated 17,086 17,086 0 0.0%

moiseev
moiseev approved these changes Jul 25, 2018
View reviewed changes
Copy link
Contributor

@moiseev moiseev left a comment

Choose a reason for hiding this comment

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

:shipit:

@ravikandhadai
Copy link
Contributor Author

@swift-ci please test Source Compatibility

@ravikandhadai
Copy link
Contributor Author

@swift-ci please test and merge

@ravikandhadai
Copy link
Contributor Author

@swift-ci Please Test and Merge

@swift-ci swift-ci merged commit 128d33c into swiftlang:master Jul 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@moiseev moiseev moiseev approved these changes

@devincoughlin devincoughlin Awaiting requested review from devincoughlin

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

Successfully merging this pull request may close these issues.
4 participants
@ravikandhadai @airspeedswift @swift-ci @moiseev