-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[Constraint solver] De-prioritize SIMD operators. #21231
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1457,6 +1457,40 @@ static void tryOptimizeGenericDisjunction(ConstraintSystem &cs, | |
} | ||
} | ||
|
||
/// If there are any SIMD operators in the overload set, partition the set so | ||
/// that the SIMD operators come at the end. | ||
static ArrayRef<OverloadChoice> partitionSIMDOperators( | ||
ArrayRef<OverloadChoice> choices, | ||
SmallVectorImpl<OverloadChoice> &scratch) { | ||
// If the first element isn't an operator, none of them are. | ||
if (!choices[0].isDecl() || | ||
!isa<FuncDecl>(choices[0].getDecl()) || | ||
!cast<FuncDecl>(choices[0].getDecl())->isOperator() || | ||
choices[0].getDecl()->getASTContext().LangOpts | ||
.SolverEnableOperatorDesignatedTypes) | ||
return choices; | ||
|
||
// Check whether we have any SIMD operators. | ||
bool foundSIMDOperator = false; | ||
for (const auto &choice : choices) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure if it's actually worse to check if we have any simd operators and then sort, might be faster to simplify sort since there are not going to be thousands of operators anyway... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm trying to avoid the malloc + copy for the common case. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for clarification! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can also use |
||
if (isSIMDOperator(choice.getDecl())) { | ||
foundSIMDOperator = true; | ||
break; | ||
} | ||
} | ||
|
||
if (!foundSIMDOperator) | ||
return choices; | ||
|
||
scratch.assign(choices.begin(), choices.end()); | ||
std::stable_partition(scratch.begin(), scratch.end(), | ||
[](const OverloadChoice &choice) { | ||
return !isSIMDOperator(choice.getDecl()); | ||
}); | ||
|
||
return scratch; | ||
} | ||
|
||
void ConstraintSystem::addOverloadSet(Type boundType, | ||
ArrayRef<OverloadChoice> choices, | ||
DeclContext *useDC, | ||
|
@@ -1473,6 +1507,9 @@ void ConstraintSystem::addOverloadSet(Type boundType, | |
|
||
tryOptimizeGenericDisjunction(*this, choices, favoredChoice); | ||
|
||
SmallVector<OverloadChoice, 4> scratchChoices; | ||
choices = partitionSIMDOperators(choices, scratchChoices); | ||
|
||
SmallVector<Constraint *, 4> overloads; | ||
|
||
// As we do for other favored constraints, if a favored overload has been | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
// RUN: %target-typecheck-verify-swift | ||
// RUN: %target-typecheck-verify-swift -solver-enable-operator-designated-types | ||
// REQUIRES: OS=macosx | ||
|
||
import simd | ||
import CoreGraphics | ||
import Foundation | ||
|
||
class SomeView { | ||
func layoutSubviews() { | ||
let descriptionTextViewFrame = CGRect.zero | ||
let availableBounds = CGRect() | ||
let descriptionLabelProperties = SomeView.descriptionTextViewLabelProperties() | ||
let textSize = descriptionTextView.sizeThatFits(availableBounds.size) | ||
let textInset = descriptionTextView.textInset(forBounds: CGRect(origin: .zero, size: textSize)) | ||
let descriptionTextBaselineOffset: CGFloat = CGFloat() | ||
let displayScale: CGFloat = CGFloat() | ||
let _ = (descriptionTextViewFrame.height | ||
+ (-descriptionTextView.lastBaselineOffsetFromBottom - textInset.bottom + descriptionLabelProperties.lastBaselineOffsetFromBottom) | ||
+ (-descriptionTextView.firstBaselineOffsetFromTop - textInset.top + descriptionTextBaselineOffset).ceilingValue(scale: displayScale) | ||
) | ||
} | ||
|
||
static func descriptionTextViewLabelProperties() -> FontDescriptorBaselineProperties { | ||
fatalError() | ||
} | ||
|
||
lazy var descriptionTextView: SomeOtherView = SomeOtherView() | ||
} | ||
|
||
class SomeOtherView { | ||
init() { } | ||
func sizeThatFits(_ size: CGSize) -> CGSize { return size } | ||
} | ||
|
||
|
||
struct FontDescriptorBaselineProperties { | ||
// let fontDescriptor: MPUFontDescriptor | ||
let defaultFirstBaselineOffsetFromTop: CGFloat | ||
let defaultLastBaselineOffsetFromBottom: CGFloat | ||
var firstBaselineOffsetFromTop: CGFloat { fatalError() } | ||
var lastBaselineOffsetFromBottom: CGFloat { | ||
fatalError() | ||
} | ||
} | ||
|
||
struct EdgeInsets { | ||
var top: CGFloat | ||
var bottom: CGFloat | ||
} | ||
|
||
extension SomeOtherView { | ||
func textInset(forBounds bounds: CGRect) -> EdgeInsets { fatalError() } | ||
var firstBaselineOffsetFromTop: CGFloat { fatalError() } | ||
var lastBaselineOffsetFromBottom: CGFloat { | ||
fatalError() | ||
} | ||
} | ||
|
||
extension CGFloat { | ||
public func ceilingValue(scale: CGFloat) -> CGFloat { fatalError() } | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: can combine two ifs into one
!(func && function->isOperator())
, and same could be done for nominal using De Morgan's Law :)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like breaking out the conditions here.