From e0199f2d98c17d4d1dcab48f71961c607bbe216b Mon Sep 17 00:00:00 2001 From: gregomni Date: Sun, 14 Jul 2019 09:34:11 -0700 Subject: [PATCH 01/22] Add condition for optimizing series of binops that every possible overload is (T,T)->T, with no (T,T)->U --- lib/Sema/CSGen.cpp | 23 +++++++++++++++++++++++ test/Constraints/sr10324.swift | 19 +++++++++++++++++++ 2 files changed, 42 insertions(+) create mode 100644 test/Constraints/sr10324.swift diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp index 2c3e9bbbd5c53..5ed80cab7d340 100644 --- a/lib/Sema/CSGen.cpp +++ b/lib/Sema/CSGen.cpp @@ -46,6 +46,26 @@ static bool isArithmeticOperatorDecl(ValueDecl *vd) { vd->getBaseName() == "%"); } +static bool hasBinOpOverloadWithSameArgTypesDifferingResult( + OverloadedDeclRefExpr *overloads) { + for (auto decl : overloads->getDecls()) { + auto metaFuncType = decl->getInterfaceType()->getAs(); + auto funcType = metaFuncType->getResult()->getAs(); + if (!funcType) + continue; + + auto params = funcType->getParams(); + if (params.size() != 2) + continue; + + if (params[0].getPlainType().getPointer() != params[1].getPlainType().getPointer()) + continue; + if (funcType->getResult().getPointer() != params[0].getPlainType().getPointer()) + return true; + } + return false; +} + static bool mergeRepresentativeEquivalenceClasses(ConstraintSystem &CS, TypeVariableType* tyvar1, TypeVariableType* tyvar2) { @@ -340,6 +360,9 @@ namespace { ODR2->getDecls()[0]->getBaseName()) return; + if (hasBinOpOverloadWithSameArgTypesDifferingResult(ODR1)) + return; + // All things equal, we can merge the tyvars for the function // types. auto rep1 = CS.getRepresentative(fnTy1); diff --git a/test/Constraints/sr10324.swift b/test/Constraints/sr10324.swift new file mode 100644 index 0000000000000..5e38121153008 --- /dev/null +++ b/test/Constraints/sr10324.swift @@ -0,0 +1,19 @@ +// RUN: %target-swift-frontend -typecheck -verify %s + +struct A { + static func * (lhs: A, rhs: A) -> B { return B() } + static func * (lhs: B, rhs: A) -> B { return B() } + static func * (lhs: A, rhs: B) -> B { return B() } +} +struct B {} + +let (x, y, z) = (A(), A(), A()) + +let w = A() * A() * A() // works + +// Should all work +let a = x * y * z +let b = x * (y * z) +let c = (x * y) * z +let d = x * (y * z as B) +let e = (x * y as B) * z From 2edba9dfbd884b865a38274411c82d0fa97072cc Mon Sep 17 00:00:00 2001 From: gregomni Date: Tue, 16 Jul 2019 11:16:33 -0700 Subject: [PATCH 02/22] Instead of chaining binops, favor disjunctions with op overloads whose types match existing binding choices --- include/swift/Sema/Constraint.h | 9 ++++ lib/Sema/CSGen.cpp | 89 --------------------------------- lib/Sema/CSSolver.cpp | 62 +++++++++++++++++++++-- lib/Sema/CSStep.h | 38 -------------- 4 files changed, 68 insertions(+), 130 deletions(-) diff --git a/include/swift/Sema/Constraint.h b/include/swift/Sema/Constraint.h index 5eb4ab14a0f3a..c2a6a6a1c67cb 100644 --- a/include/swift/Sema/Constraint.h +++ b/include/swift/Sema/Constraint.h @@ -672,6 +672,15 @@ class Constraint final : public llvm::ilist_node, return Nested; } + unsigned countFavoredNestedConstraints() const { + unsigned count = 0; + for (auto *constraint : Nested) + if (constraint->isFavored() && !constraint->isDisabled()) + count++; + + return count; + } + unsigned countActiveNestedConstraints() const { unsigned count = 0; for (auto *constraint : Nested) diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp index 5ed80cab7d340..09e9ea5c0827a 100644 --- a/lib/Sema/CSGen.cpp +++ b/lib/Sema/CSGen.cpp @@ -46,26 +46,6 @@ static bool isArithmeticOperatorDecl(ValueDecl *vd) { vd->getBaseName() == "%"); } -static bool hasBinOpOverloadWithSameArgTypesDifferingResult( - OverloadedDeclRefExpr *overloads) { - for (auto decl : overloads->getDecls()) { - auto metaFuncType = decl->getInterfaceType()->getAs(); - auto funcType = metaFuncType->getResult()->getAs(); - if (!funcType) - continue; - - auto params = funcType->getParams(); - if (params.size() != 2) - continue; - - if (params[0].getPlainType().getPointer() != params[1].getPlainType().getPointer()) - continue; - if (funcType->getResult().getPointer() != params[0].getPlainType().getPointer()) - return true; - } - return false; -} - static bool mergeRepresentativeEquivalenceClasses(ConstraintSystem &CS, TypeVariableType* tyvar1, TypeVariableType* tyvar2) { @@ -323,75 +303,6 @@ namespace { // solver can still make progress. auto favoredTy = (*lti.collectedTypes.begin())->getWithoutSpecifierType(); CS.setFavoredType(expr, favoredTy.getPointer()); - - // If we have a chain of identical binop expressions with homogeneous - // argument types, we can directly simplify the associated constraint - // graph. - auto simplifyBinOpExprTyVars = [&]() { - // Don't attempt to do linking if there are - // literals intermingled with other inferred types. - if (lti.hasLiteral) - return; - - for (auto binExp1 : lti.binaryExprs) { - for (auto binExp2 : lti.binaryExprs) { - if (binExp1 == binExp2) - continue; - - auto fnTy1 = CS.getType(binExp1)->getAs(); - auto fnTy2 = CS.getType(binExp2)->getAs(); - - if (!(fnTy1 && fnTy2)) - return; - - auto ODR1 = dyn_cast(binExp1->getFn()); - auto ODR2 = dyn_cast(binExp2->getFn()); - - if (!(ODR1 && ODR2)) - return; - - // TODO: We currently limit this optimization to known arithmetic - // operators, but we should be able to broaden this out to - // logical operators as well. - if (!isArithmeticOperatorDecl(ODR1->getDecls()[0])) - return; - - if (ODR1->getDecls()[0]->getBaseName() != - ODR2->getDecls()[0]->getBaseName()) - return; - - if (hasBinOpOverloadWithSameArgTypesDifferingResult(ODR1)) - return; - - // All things equal, we can merge the tyvars for the function - // types. - auto rep1 = CS.getRepresentative(fnTy1); - auto rep2 = CS.getRepresentative(fnTy2); - - if (rep1 != rep2) { - CS.mergeEquivalenceClasses(rep1, rep2, - /*updateWorkList*/ false); - } - - auto odTy1 = CS.getType(ODR1)->getAs(); - auto odTy2 = CS.getType(ODR2)->getAs(); - - if (odTy1 && odTy2) { - auto odRep1 = CS.getRepresentative(odTy1); - auto odRep2 = CS.getRepresentative(odTy2); - - // Since we'll be choosing the same overload, we can merge - // the overload tyvar as well. - if (odRep1 != odRep2) - CS.mergeEquivalenceClasses(odRep1, odRep2, - /*updateWorkList*/ false); - } - } - } - }; - - simplifyBinOpExprTyVars(); - return true; } diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index bdc519dec4758..5049c479758a7 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -2013,9 +2013,47 @@ static Constraint *tryOptimizeGenericDisjunction( llvm_unreachable("covered switch"); } +// Performance hack: favor operator overloads with types we're already binding elsewhere in this expression. +static void existingOperatorBindingsForDisjunction(ConstraintSystem &CS, ArrayRef constraints, SmallVectorImpl &found) { + auto *choice = constraints.front(); + if (choice->getKind() != ConstraintKind::BindOverload) + return; + + auto overload = choice->getOverloadChoice(); + if (!overload.isDecl()) + return; + auto decl = overload.getDecl(); + if (!decl->isOperator()) + return; + + for (auto *resolved = CS.getResolvedOverloadSets(); resolved; + resolved = resolved->Previous) { + if (!resolved->Choice.isDecl()) + continue; + auto representativeDecl = resolved->Choice.getDecl(); + + if (!representativeDecl->isOperator()) + continue; + + for (auto *constraint : constraints) { + if (constraint->isFavored()) + continue; + auto choice = constraint->getOverloadChoice(); + if (choice.getDecl()->getInterfaceType()->isEqual(representativeDecl->getInterfaceType())) + found.push_back(constraint); + } + } +} + void ConstraintSystem::partitionDisjunction( ArrayRef Choices, SmallVectorImpl &Ordering, SmallVectorImpl &PartitionBeginning) { + + SmallVector existing; + existingOperatorBindingsForDisjunction(*this, Choices, existing); + for (auto constraint : existing) + favorConstraint(constraint); + // Apply a special-case rule for favoring one generic function over // another. if (auto favored = tryOptimizeGenericDisjunction(DC, Choices)) { @@ -2134,12 +2172,30 @@ Constraint *ConstraintSystem::selectDisjunction() { if (auto *disjunction = selectBestBindingDisjunction(*this, disjunctions)) return disjunction; - // Pick the disjunction with the smallest number of active choices. + // Pick the disjunction with the smallest number of favored, then active choices. + auto cs = this; auto minDisjunction = std::min_element(disjunctions.begin(), disjunctions.end(), [&](Constraint *first, Constraint *second) -> bool { - return first->countActiveNestedConstraints() < - second->countActiveNestedConstraints(); + unsigned firstFavored = first->countFavoredNestedConstraints(); + unsigned secondFavored = second->countFavoredNestedConstraints(); + + if (firstFavored == secondFavored) { + // Look for additional choices to favor + SmallVector firstExisting; + SmallVector secondExisting; + + existingOperatorBindingsForDisjunction(*cs, first->getNestedConstraints(), firstExisting); + firstFavored = firstExisting.size() ?: first->countActiveNestedConstraints(); + existingOperatorBindingsForDisjunction(*cs, second->getNestedConstraints(), secondExisting); + secondFavored = secondExisting.size() ?: second->countActiveNestedConstraints(); + + return firstFavored < secondFavored; + } else { + firstFavored = firstFavored ?: first->countActiveNestedConstraints(); + secondFavored = secondFavored ?: second->countActiveNestedConstraints(); + return firstFavored < secondFavored; + } }); if (minDisjunction != disjunctions.end()) diff --git a/lib/Sema/CSStep.h b/lib/Sema/CSStep.h index 89b812ee53511..f01dc7fa6e759 100644 --- a/lib/Sema/CSStep.h +++ b/lib/Sema/CSStep.h @@ -654,7 +654,6 @@ class DisjunctionStep final : public BindingStep { : BindingStep(cs, {cs, disjunction}, solutions), Disjunction(disjunction), AfterDisjunction(erase(disjunction)) { assert(Disjunction->getKind() == ConstraintKind::Disjunction); - pruneOverloadSet(Disjunction); ++cs.solverState->NumDisjunctions; } @@ -702,43 +701,6 @@ class DisjunctionStep final : public BindingStep { /// simplified further, false otherwise. bool attempt(const DisjunctionChoice &choice) override; - // Check if selected disjunction has a representative - // this might happen when there are multiple binary operators - // chained together. If so, disable choices which differ - // from currently selected representative. - void pruneOverloadSet(Constraint *disjunction) { - auto *choice = disjunction->getNestedConstraints().front(); - auto *typeVar = choice->getFirstType()->getAs(); - if (!typeVar) - return; - - auto *repr = typeVar->getImpl().getRepresentative(nullptr); - if (!repr || repr == typeVar) - return; - - for (auto elt : getResolvedOverloads()) { - auto resolved = elt.second; - if (!resolved.boundType->isEqual(repr)) - continue; - - auto &representative = resolved.choice; - if (!representative.isDecl()) - return; - - // Disable all of the overload choices which are different from - // the one which is currently picked for representative. - for (auto *constraint : disjunction->getNestedConstraints()) { - auto choice = constraint->getOverloadChoice(); - if (!choice.isDecl() || choice.getDecl() == representative.getDecl()) - continue; - - constraint->setDisabled(); - DisabledChoices.push_back(constraint); - } - break; - } - }; - // Figure out which of the solutions has the smallest score. static Optional getBestScore(SmallVectorImpl &solutions) { assert(!solutions.empty()); From c84bd00819f5f3900146df88690eb30a47b427c7 Mon Sep 17 00:00:00 2001 From: Holly Borla Date: Tue, 13 Oct 2020 17:59:38 -0700 Subject: [PATCH 03/22] [ConstraintSystem] Move getResolvedOverloads() from CSStep to ConstraintSystem. --- include/swift/Sema/ConstraintSystem.h | 6 ++++++ lib/Sema/CSStep.h | 5 ----- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 93da7c8bafa45..407c71e46d93c 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -5300,6 +5300,12 @@ class ConstraintSystem { SmallVectorImpl &Ordering, SmallVectorImpl &PartitionBeginning); + /// The overload sets that have already been resolved along the current path. + const llvm::MapVector & + getResolvedOverloads() const { + return ResolvedOverloads; + } + /// If we aren't certain that we've emitted a diagnostic, emit a fallback /// diagnostic. void maybeProduceFallbackDiagnostic(SolutionApplicationTarget target) const; diff --git a/lib/Sema/CSStep.h b/lib/Sema/CSStep.h index f01dc7fa6e759..2b59badf1a38a 100644 --- a/lib/Sema/CSStep.h +++ b/lib/Sema/CSStep.h @@ -213,11 +213,6 @@ class SolverStep { CS.CG.addConstraint(constraint); } - const llvm::MapVector & - getResolvedOverloads() const { - return CS.ResolvedOverloads; - } - void recordDisjunctionChoice(ConstraintLocator *disjunctionLocator, unsigned index) const { CS.recordDisjunctionChoice(disjunctionLocator, index); From b06f749f0ee53ba6fef2cd6ce1fda34f3b7a3539 Mon Sep 17 00:00:00 2001 From: Holly Borla Date: Tue, 13 Oct 2020 18:01:16 -0700 Subject: [PATCH 04/22] [ConstraintSystem] Fix build failures and formatting. --- lib/Sema/CSSolver.cpp | 53 +++++++++++++++++++++---------------------- 1 file changed, 26 insertions(+), 27 deletions(-) diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index 5049c479758a7..9f153ae2c8cc4 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -2026,11 +2026,11 @@ static void existingOperatorBindingsForDisjunction(ConstraintSystem &CS, ArrayRe if (!decl->isOperator()) return; - for (auto *resolved = CS.getResolvedOverloadSets(); resolved; - resolved = resolved->Previous) { - if (!resolved->Choice.isDecl()) + for (auto overload : CS.getResolvedOverloads()) { + auto resolved = overload.second; + if (!resolved.choice.isDecl()) continue; - auto representativeDecl = resolved->Choice.getDecl(); + auto representativeDecl = resolved.choice.getDecl(); if (!representativeDecl->isOperator()) continue; @@ -2174,29 +2174,28 @@ Constraint *ConstraintSystem::selectDisjunction() { // Pick the disjunction with the smallest number of favored, then active choices. auto cs = this; - auto minDisjunction = - std::min_element(disjunctions.begin(), disjunctions.end(), - [&](Constraint *first, Constraint *second) -> bool { - unsigned firstFavored = first->countFavoredNestedConstraints(); - unsigned secondFavored = second->countFavoredNestedConstraints(); - - if (firstFavored == secondFavored) { - // Look for additional choices to favor - SmallVector firstExisting; - SmallVector secondExisting; - - existingOperatorBindingsForDisjunction(*cs, first->getNestedConstraints(), firstExisting); - firstFavored = firstExisting.size() ?: first->countActiveNestedConstraints(); - existingOperatorBindingsForDisjunction(*cs, second->getNestedConstraints(), secondExisting); - secondFavored = secondExisting.size() ?: second->countActiveNestedConstraints(); - - return firstFavored < secondFavored; - } else { - firstFavored = firstFavored ?: first->countActiveNestedConstraints(); - secondFavored = secondFavored ?: second->countActiveNestedConstraints(); - return firstFavored < secondFavored; - } - }); + auto minDisjunction = std::min_element(disjunctions.begin(), disjunctions.end(), + [&](Constraint *first, Constraint *second) -> bool { + unsigned firstFavored = first->countFavoredNestedConstraints(); + unsigned secondFavored = second->countFavoredNestedConstraints(); + + if (firstFavored == secondFavored) { + // Look for additional choices to favor + SmallVector firstExisting; + SmallVector secondExisting; + + existingOperatorBindingsForDisjunction(*cs, first->getNestedConstraints(), firstExisting); + firstFavored = firstExisting.size() ? firstExisting.size() : first->countActiveNestedConstraints(); + existingOperatorBindingsForDisjunction(*cs, second->getNestedConstraints(), secondExisting); + secondFavored = secondExisting.size() ? secondExisting.size() : second->countActiveNestedConstraints(); + + return firstFavored < secondFavored; + } else { + firstFavored = firstFavored ? firstFavored : first->countActiveNestedConstraints(); + secondFavored = secondFavored ? secondFavored : second->countActiveNestedConstraints(); + return firstFavored < secondFavored; + } + }); if (minDisjunction != disjunctions.end()) return *minDisjunction; From ab6d92b1cd84fa85b684adae578278dc01aa15ad Mon Sep 17 00:00:00 2001 From: gregomni Date: Tue, 16 Jul 2019 21:07:04 -0700 Subject: [PATCH 05/22] Favor operators based on existing binds via both basename and type --- include/swift/Sema/ConstraintSystem.h | 10 ++++++++ lib/Sema/CSSolver.cpp | 37 ++++++++++++++++++++++----- lib/Sema/CSStep.cpp | 8 ++++-- 3 files changed, 47 insertions(+), 8 deletions(-) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 407c71e46d93c..767ec57abdc84 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -2467,6 +2467,16 @@ class ConstraintSystem { favoredConstraints.push_back(constraint); } + /// Whether or not the given constraint is only favored during this scope. + bool isTemporarilyFavored(Constraint *constraint) { + assert(constraint->isFavored()); + + for (auto test : favoredConstraints) + if (test == constraint) + return true; + return false; + } + private: /// The list of constraints that have been retired along the /// current path, this list is used in LIFO fashion when constraints diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index 9f153ae2c8cc4..7def6c1f0c9e0 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -2013,7 +2013,8 @@ static Constraint *tryOptimizeGenericDisjunction( llvm_unreachable("covered switch"); } -// Performance hack: favor operator overloads with types we're already binding elsewhere in this expression. +// Performance hack: favor operator overloads with decl or type we're already +// binding elsewhere in this expression. static void existingOperatorBindingsForDisjunction(ConstraintSystem &CS, ArrayRef constraints, SmallVectorImpl &found) { auto *choice = constraints.front(); if (choice->getKind() != ConstraintKind::BindOverload) @@ -2025,6 +2026,9 @@ static void existingOperatorBindingsForDisjunction(ConstraintSystem &CS, ArrayRe auto decl = overload.getDecl(); if (!decl->isOperator()) return; + auto baseName = decl->getBaseName(); + + SmallSet typesFound; for (auto overload : CS.getResolvedOverloads()) { auto resolved = overload.second; @@ -2035,12 +2039,33 @@ static void existingOperatorBindingsForDisjunction(ConstraintSystem &CS, ArrayRe if (!representativeDecl->isOperator()) continue; - for (auto *constraint : constraints) { - if (constraint->isFavored()) + if (representativeDecl->getBaseName() == baseName) { + // Favor exactly the same decl, if we have a binding to the same name. + for (auto *constraint : constraints) { + if (constraint->isFavored()) + continue; + auto choice = constraint->getOverloadChoice(); + if (choice.getDecl() == representativeDecl) { + found.push_back(constraint); + break; + } + } + } else { + // Favor the same type, if we have a binding to an operator of that type. + auto representativeType = representativeDecl->getInterfaceType(); + if (typesFound.count(representativeType.getPointer())) continue; - auto choice = constraint->getOverloadChoice(); - if (choice.getDecl()->getInterfaceType()->isEqual(representativeDecl->getInterfaceType())) - found.push_back(constraint); + typesFound.insert(representativeType.getPointer()); + + for (auto *constraint : constraints) { + if (constraint->isFavored()) + continue; + auto choice = constraint->getOverloadChoice(); + if (choice.getDecl()->getInterfaceType()->isEqual(representativeType)) { + found.push_back(constraint); + break; + } + } } } } diff --git a/lib/Sema/CSStep.cpp b/lib/Sema/CSStep.cpp index 10636456a5a1c..abfa28c2d477f 100644 --- a/lib/Sema/CSStep.cpp +++ b/lib/Sema/CSStep.cpp @@ -615,8 +615,12 @@ bool DisjunctionStep::shortCircuitDisjunctionAt( auto &ctx = CS.getASTContext(); // If the successfully applied constraint is favored, we'll consider that to - // be the "best". - if (lastSuccessfulChoice->isFavored() && !currentChoice->isFavored()) { + // be the "best". If it was only temporarily favored because it matched other + // operator bindings, we can even short-circuit other favored constraints. + if (lastSuccessfulChoice->isFavored() && + (!currentChoice->isFavored() || + (CS.solverState->isTemporarilyFavored(lastSuccessfulChoice) && + !CS.solverState->isTemporarilyFavored(currentChoice)))) { #if !defined(NDEBUG) if (lastSuccessfulChoice->getKind() == ConstraintKind::BindOverload) { auto overloadChoice = lastSuccessfulChoice->getOverloadChoice(); From 34fa9c27866ac888fc84928e87805b281547b242 Mon Sep 17 00:00:00 2001 From: gregomni Date: Wed, 17 Jul 2019 19:13:05 -0700 Subject: [PATCH 06/22] Merge typevars of operators with shared decls after finding a solution to speed up further searching --- lib/Sema/CSStep.cpp | 43 +++++++++++++++++++++++++++++++++++++++---- lib/Sema/CSStep.h | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 4 deletions(-) diff --git a/lib/Sema/CSStep.cpp b/lib/Sema/CSStep.cpp index abfa28c2d477f..180df35df7520 100644 --- a/lib/Sema/CSStep.cpp +++ b/lib/Sema/CSStep.cpp @@ -659,15 +659,50 @@ bool DisjunctionStep::shortCircuitDisjunctionAt( if (currentChoice->getKind() == ConstraintKind::CheckedCast) return true; - // If we have a SIMD operator, and the prior choice was not a SIMD - // Operator, we're done. + // Extra checks for binding of operators if (currentChoice->getKind() == ConstraintKind::BindOverload && - isSIMDOperator(currentChoice->getOverloadChoice().getDecl()) && + currentChoice->getOverloadChoice().getDecl()->isOperator() && lastSuccessfulChoice->getKind() == ConstraintKind::BindOverload && - !isSIMDOperator(lastSuccessfulChoice->getOverloadChoice().getDecl())) { + lastSuccessfulChoice->getOverloadChoice().getDecl()->isOperator()) { return true; } + // If we have a SIMD operator, and the prior choice was not a SIMD + // Operator, we're done. + if (isSIMDOperator(currentChoice->getOverloadChoice().getDecl()) && + !isSIMDOperator(lastSuccessfulChoice->getOverloadChoice().getDecl())) + return true; + + // Otherwise if we have an existing solution, bind tyvars bound to the same + // decl in the solution to the choice tyvar. We can continue finding more + // solutions, but all the instances of the operator that chose the same + // overload as this successful choice will be bound togeter. + if (Solutions.size()) { + auto lastTyvar = + lastSuccessfulChoice->getFirstType()->getAs(); + auto lastRep = CS.getRepresentative(lastTyvar); + + for (auto overload : Solutions[0].overloadChoices) { + auto overloadChoice = overload.getSecond().choice; + if (!overloadChoice.isDecl() || + overloadChoice.getDecl() != + lastSuccessfulChoice->getOverloadChoice().getDecl()) + continue; + + auto choiceTyvar = + CS.getType(simplifyLocatorToAnchor(CS, overload.getFirst())) + ->getAs(); + if (!choiceTyvar) + continue; + + auto rep = CS.getRepresentative(choiceTyvar); + if (lastRep != rep) { + CS.mergeEquivalenceClasses(rep, lastRep); + lastRep = CS.getRepresentative(lastRep); + } + } + } + } return false; } diff --git a/lib/Sema/CSStep.h b/lib/Sema/CSStep.h index 2b59badf1a38a..c3e0f06f4f38b 100644 --- a/lib/Sema/CSStep.h +++ b/lib/Sema/CSStep.h @@ -649,6 +649,7 @@ class DisjunctionStep final : public BindingStep { : BindingStep(cs, {cs, disjunction}, solutions), Disjunction(disjunction), AfterDisjunction(erase(disjunction)) { assert(Disjunction->getKind() == ConstraintKind::Disjunction); + pruneOverloadSet(Disjunction); ++cs.solverState->NumDisjunctions; } @@ -696,6 +697,43 @@ class DisjunctionStep final : public BindingStep { /// simplified further, false otherwise. bool attempt(const DisjunctionChoice &choice) override; + // Check if selected disjunction has a representative + // this might happen when there are multiple binary operators + // chained together. If so, disable choices which differ + // from currently selected representative. + void pruneOverloadSet(Constraint *disjunction) { + auto *choice = disjunction->getNestedConstraints().front(); + auto *typeVar = choice->getFirstType()->getAs(); + if (!typeVar) + return; + + auto *repr = typeVar->getImpl().getRepresentative(nullptr); + if (!repr || repr == typeVar) + return; + + for (auto *resolved = getResolvedOverloads(); resolved; + resolved = resolved->Previous) { + if (!resolved->BoundType->isEqual(repr)) + continue; + + auto &representative = resolved->Choice; + if (!representative.isDecl()) + return; + + // Disable all of the overload choices which are different from + // the one which is currently picked for representative. + for (auto *constraint : disjunction->getNestedConstraints()) { + auto choice = constraint->getOverloadChoice(); + if (!choice.isDecl() || choice.getDecl() == representative.getDecl()) + continue; + + constraint->setDisabled(); + DisabledChoices.push_back(constraint); + } + break; + } + }; + // Figure out which of the solutions has the smallest score. static Optional getBestScore(SmallVectorImpl &solutions) { assert(!solutions.empty()); From cb64d7f715b9550c565131fafdf031e52f42283a Mon Sep 17 00:00:00 2001 From: Holly Borla Date: Wed, 14 Oct 2020 11:21:41 -0700 Subject: [PATCH 07/22] [ConstraintSystem] Fix build failures. --- lib/Sema/CSStep.cpp | 6 ++---- lib/Sema/CSStep.h | 8 ++++---- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/lib/Sema/CSStep.cpp b/lib/Sema/CSStep.cpp index 180df35df7520..f78b92b8dd524 100644 --- a/lib/Sema/CSStep.cpp +++ b/lib/Sema/CSStep.cpp @@ -664,8 +664,6 @@ bool DisjunctionStep::shortCircuitDisjunctionAt( currentChoice->getOverloadChoice().getDecl()->isOperator() && lastSuccessfulChoice->getKind() == ConstraintKind::BindOverload && lastSuccessfulChoice->getOverloadChoice().getDecl()->isOperator()) { - return true; - } // If we have a SIMD operator, and the prior choice was not a SIMD // Operator, we're done. @@ -690,14 +688,14 @@ bool DisjunctionStep::shortCircuitDisjunctionAt( continue; auto choiceTyvar = - CS.getType(simplifyLocatorToAnchor(CS, overload.getFirst())) + CS.getType(simplifyLocatorToAnchor(overload.getFirst())) ->getAs(); if (!choiceTyvar) continue; auto rep = CS.getRepresentative(choiceTyvar); if (lastRep != rep) { - CS.mergeEquivalenceClasses(rep, lastRep); + CS.mergeEquivalenceClasses(rep, lastRep, /*updateWorkList=*/false); lastRep = CS.getRepresentative(lastRep); } } diff --git a/lib/Sema/CSStep.h b/lib/Sema/CSStep.h index c3e0f06f4f38b..053633dfda07c 100644 --- a/lib/Sema/CSStep.h +++ b/lib/Sema/CSStep.h @@ -711,12 +711,12 @@ class DisjunctionStep final : public BindingStep { if (!repr || repr == typeVar) return; - for (auto *resolved = getResolvedOverloads(); resolved; - resolved = resolved->Previous) { - if (!resolved->BoundType->isEqual(repr)) + for (auto overload : CS.getResolvedOverloads()) { + auto resolved = overload.second; + if (!resolved.boundType->isEqual(repr)) continue; - auto &representative = resolved->Choice; + auto &representative = resolved.choice; if (!representative.isDecl()) return; From 91291c77487a11199d7639933007a968cd866637 Mon Sep 17 00:00:00 2001 From: gregomni Date: Fri, 19 Jul 2019 09:00:34 -0700 Subject: [PATCH 08/22] Dump disjunction possibilities on multiple aligned lines for better readability. --- lib/Sema/Constraint.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/Sema/Constraint.cpp b/lib/Sema/Constraint.cpp index a26577dfbe276..357b78d8630c5 100644 --- a/lib/Sema/Constraint.cpp +++ b/lib/Sema/Constraint.cpp @@ -317,15 +317,17 @@ void Constraint::print(llvm::raw_ostream &Out, SourceManager *sm) const { Locator->dump(sm, Out); Out << "]]"; } - Out << ":"; + Out << ":\n"; interleave(getNestedConstraints(), [&](Constraint *constraint) { if (constraint->isDisabled()) - Out << "[disabled] "; + Out << "> [disabled] "; + else + Out << "> "; constraint->print(Out, sm); }, - [&] { Out << " or "; }); + [&] { Out << "\n"; }); return; } From b9e08b23fbbcef742f3b09965d3b150f001c1abc Mon Sep 17 00:00:00 2001 From: Holly Borla Date: Fri, 16 Oct 2020 10:24:51 -0700 Subject: [PATCH 09/22] [ConstraintSystem] Allow the solver to find other solutions after successfully finding a solution by favoring operators already bound elsewhere. Favoring existing operator bindings often lets the solver find a solution fast, but it's not necessarily the best solution. --- include/swift/Sema/ConstraintSystem.h | 10 ------ lib/Sema/CSStep.cpp | 49 --------------------------- lib/Sema/CSStep.h | 38 --------------------- 3 files changed, 97 deletions(-) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 767ec57abdc84..407c71e46d93c 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -2467,16 +2467,6 @@ class ConstraintSystem { favoredConstraints.push_back(constraint); } - /// Whether or not the given constraint is only favored during this scope. - bool isTemporarilyFavored(Constraint *constraint) { - assert(constraint->isFavored()); - - for (auto test : favoredConstraints) - if (test == constraint) - return true; - return false; - } - private: /// The list of constraints that have been retired along the /// current path, this list is used in LIFO fashion when constraints diff --git a/lib/Sema/CSStep.cpp b/lib/Sema/CSStep.cpp index f78b92b8dd524..6a1342ecb2f29 100644 --- a/lib/Sema/CSStep.cpp +++ b/lib/Sema/CSStep.cpp @@ -614,25 +614,6 @@ bool DisjunctionStep::shortCircuitDisjunctionAt( Constraint *currentChoice, Constraint *lastSuccessfulChoice) const { auto &ctx = CS.getASTContext(); - // If the successfully applied constraint is favored, we'll consider that to - // be the "best". If it was only temporarily favored because it matched other - // operator bindings, we can even short-circuit other favored constraints. - if (lastSuccessfulChoice->isFavored() && - (!currentChoice->isFavored() || - (CS.solverState->isTemporarilyFavored(lastSuccessfulChoice) && - !CS.solverState->isTemporarilyFavored(currentChoice)))) { -#if !defined(NDEBUG) - if (lastSuccessfulChoice->getKind() == ConstraintKind::BindOverload) { - auto overloadChoice = lastSuccessfulChoice->getOverloadChoice(); - assert((!overloadChoice.isDecl() || - !overloadChoice.getDecl()->getAttrs().isUnavailable(ctx)) && - "Unavailable decl should not be favored!"); - } -#endif - - return true; - } - // Anything without a fix is better than anything with a fix. if (currentChoice->getFix() && !lastSuccessfulChoice->getFix()) return true; @@ -670,36 +651,6 @@ bool DisjunctionStep::shortCircuitDisjunctionAt( if (isSIMDOperator(currentChoice->getOverloadChoice().getDecl()) && !isSIMDOperator(lastSuccessfulChoice->getOverloadChoice().getDecl())) return true; - - // Otherwise if we have an existing solution, bind tyvars bound to the same - // decl in the solution to the choice tyvar. We can continue finding more - // solutions, but all the instances of the operator that chose the same - // overload as this successful choice will be bound togeter. - if (Solutions.size()) { - auto lastTyvar = - lastSuccessfulChoice->getFirstType()->getAs(); - auto lastRep = CS.getRepresentative(lastTyvar); - - for (auto overload : Solutions[0].overloadChoices) { - auto overloadChoice = overload.getSecond().choice; - if (!overloadChoice.isDecl() || - overloadChoice.getDecl() != - lastSuccessfulChoice->getOverloadChoice().getDecl()) - continue; - - auto choiceTyvar = - CS.getType(simplifyLocatorToAnchor(overload.getFirst())) - ->getAs(); - if (!choiceTyvar) - continue; - - auto rep = CS.getRepresentative(choiceTyvar); - if (lastRep != rep) { - CS.mergeEquivalenceClasses(rep, lastRep, /*updateWorkList=*/false); - lastRep = CS.getRepresentative(lastRep); - } - } - } } return false; } diff --git a/lib/Sema/CSStep.h b/lib/Sema/CSStep.h index 053633dfda07c..2b59badf1a38a 100644 --- a/lib/Sema/CSStep.h +++ b/lib/Sema/CSStep.h @@ -649,7 +649,6 @@ class DisjunctionStep final : public BindingStep { : BindingStep(cs, {cs, disjunction}, solutions), Disjunction(disjunction), AfterDisjunction(erase(disjunction)) { assert(Disjunction->getKind() == ConstraintKind::Disjunction); - pruneOverloadSet(Disjunction); ++cs.solverState->NumDisjunctions; } @@ -697,43 +696,6 @@ class DisjunctionStep final : public BindingStep { /// simplified further, false otherwise. bool attempt(const DisjunctionChoice &choice) override; - // Check if selected disjunction has a representative - // this might happen when there are multiple binary operators - // chained together. If so, disable choices which differ - // from currently selected representative. - void pruneOverloadSet(Constraint *disjunction) { - auto *choice = disjunction->getNestedConstraints().front(); - auto *typeVar = choice->getFirstType()->getAs(); - if (!typeVar) - return; - - auto *repr = typeVar->getImpl().getRepresentative(nullptr); - if (!repr || repr == typeVar) - return; - - for (auto overload : CS.getResolvedOverloads()) { - auto resolved = overload.second; - if (!resolved.boundType->isEqual(repr)) - continue; - - auto &representative = resolved.choice; - if (!representative.isDecl()) - return; - - // Disable all of the overload choices which are different from - // the one which is currently picked for representative. - for (auto *constraint : disjunction->getNestedConstraints()) { - auto choice = constraint->getOverloadChoice(); - if (!choice.isDecl() || choice.getDecl() == representative.getDecl()) - continue; - - constraint->setDisabled(); - DisabledChoices.push_back(constraint); - } - break; - } - }; - // Figure out which of the solutions has the smallest score. static Optional getBestScore(SmallVectorImpl &solutions) { assert(!solutions.empty()); From 8dd2008f7e61c2fb8ce4025801df8ce592b68187 Mon Sep 17 00:00:00 2001 From: Holly Borla Date: Thu, 22 Oct 2020 16:42:18 -0700 Subject: [PATCH 10/22] [Test] Adjust type checker performance tests --- .../{slow => fast}/expression_too_complex_4.swift | 1 - .../Sema/type_checker_perf/fast/rdar18360240.swift.gyb | 2 +- .../Sema/type_checker_perf/{slow => fast}/rdar22022980.swift | 1 - .../Sema/type_checker_perf/{slow => fast}/rdar23429943.swift | 1 - .../Sema/type_checker_perf/{slow => fast}/rdar23682605.swift | 1 - .../Sema/type_checker_perf/{slow => fast}/rdar23861629.swift | 1 - .../type_checker_perf/slow/fast-operator-typechecking.swift | 1 - 7 files changed, 1 insertion(+), 7 deletions(-) rename validation-test/Sema/type_checker_perf/{slow => fast}/expression_too_complex_4.swift (80%) rename validation-test/Sema/type_checker_perf/{slow => fast}/rdar22022980.swift (83%) rename validation-test/Sema/type_checker_perf/{slow => fast}/rdar23429943.swift (69%) rename validation-test/Sema/type_checker_perf/{slow => fast}/rdar23682605.swift (91%) rename validation-test/Sema/type_checker_perf/{slow => fast}/rdar23861629.swift (86%) diff --git a/validation-test/Sema/type_checker_perf/slow/expression_too_complex_4.swift b/validation-test/Sema/type_checker_perf/fast/expression_too_complex_4.swift similarity index 80% rename from validation-test/Sema/type_checker_perf/slow/expression_too_complex_4.swift rename to validation-test/Sema/type_checker_perf/fast/expression_too_complex_4.swift index f404bb6e35d1c..7ce1e003a7a41 100644 --- a/validation-test/Sema/type_checker_perf/slow/expression_too_complex_4.swift +++ b/validation-test/Sema/type_checker_perf/fast/expression_too_complex_4.swift @@ -5,5 +5,4 @@ func test(_ i: Int, _ j: Int) -> Int { return 1 + (((i >> 1) + (i >> 2) + (i >> 3) + (i >> 4) << 1) << 1) & 0x40 + 1 + (((i >> 1) + (i >> 2) + (i >> 3) + (i >> 4) << 1) << 1) & 0x40 + 1 + (((i >> 1) + (i >> 2) + (i >> 3) + (i >> 4) << 1) << 1) & 0x40 - // expected-error@-1 {{the compiler is unable to type-check this expression in reasonable time}} } diff --git a/validation-test/Sema/type_checker_perf/fast/rdar18360240.swift.gyb b/validation-test/Sema/type_checker_perf/fast/rdar18360240.swift.gyb index d3a80b33700c9..a9e1b674c85ec 100644 --- a/validation-test/Sema/type_checker_perf/fast/rdar18360240.swift.gyb +++ b/validation-test/Sema/type_checker_perf/fast/rdar18360240.swift.gyb @@ -1,4 +1,4 @@ -// RUN: %scale-test --begin 2 --end 10 --step 2 --select NumConstraintScopes --polynomial-threshold 1.5 %s +// RUN: %scale-test --begin 2 --end 10 --step 2 --select NumConstraintScopes %s // REQUIRES: asserts,no_asan let empty: [Int] = [] diff --git a/validation-test/Sema/type_checker_perf/slow/rdar22022980.swift b/validation-test/Sema/type_checker_perf/fast/rdar22022980.swift similarity index 83% rename from validation-test/Sema/type_checker_perf/slow/rdar22022980.swift rename to validation-test/Sema/type_checker_perf/fast/rdar22022980.swift index 74b5698def071..c396254f9b227 100644 --- a/validation-test/Sema/type_checker_perf/slow/rdar22022980.swift +++ b/validation-test/Sema/type_checker_perf/fast/rdar22022980.swift @@ -2,4 +2,3 @@ // REQUIRES: tools-release,no_asan _ = [1, 3, 5, 7, 11].filter{ $0 == 1 || $0 == 3 || $0 == 11 || $0 == 1 || $0 == 3 || $0 == 11 } == [ 1, 3, 11 ] -// expected-error@-1 {{unable to type-check}} diff --git a/validation-test/Sema/type_checker_perf/slow/rdar23429943.swift b/validation-test/Sema/type_checker_perf/fast/rdar23429943.swift similarity index 69% rename from validation-test/Sema/type_checker_perf/slow/rdar23429943.swift rename to validation-test/Sema/type_checker_perf/fast/rdar23429943.swift index 7f3efc941f47e..e1edbcda1fd8a 100644 --- a/validation-test/Sema/type_checker_perf/slow/rdar23429943.swift +++ b/validation-test/Sema/type_checker_perf/fast/rdar23429943.swift @@ -1,7 +1,6 @@ // RUN: %target-typecheck-verify-swift -solver-expression-time-threshold=1 // REQUIRES: tools-release,no_asan -// expected-error@+1 {{the compiler is unable to type-check this expression in reasonable time}} let _ = [0].reduce([Int]()) { return $0.count == 0 && ($1 == 0 || $1 == 2 || $1 == 3) ? [] : $0 + [$1] } diff --git a/validation-test/Sema/type_checker_perf/slow/rdar23682605.swift b/validation-test/Sema/type_checker_perf/fast/rdar23682605.swift similarity index 91% rename from validation-test/Sema/type_checker_perf/slow/rdar23682605.swift rename to validation-test/Sema/type_checker_perf/fast/rdar23682605.swift index b7bc757fe1989..4be53b4d2ef83 100644 --- a/validation-test/Sema/type_checker_perf/slow/rdar23682605.swift +++ b/validation-test/Sema/type_checker_perf/fast/rdar23682605.swift @@ -14,7 +14,6 @@ func memoize( body: @escaping ((T)->U, T)->U ) -> (T)->U { } let fibonacci = memoize { - // expected-error@-1 {{reasonable time}} fibonacci, n in n < 2 ? Double(n) : fibonacci(n - 1) + fibonacci(n - 2) } diff --git a/validation-test/Sema/type_checker_perf/slow/rdar23861629.swift b/validation-test/Sema/type_checker_perf/fast/rdar23861629.swift similarity index 86% rename from validation-test/Sema/type_checker_perf/slow/rdar23861629.swift rename to validation-test/Sema/type_checker_perf/fast/rdar23861629.swift index 3f10765e02aa0..3b68b4ac93e11 100644 --- a/validation-test/Sema/type_checker_perf/slow/rdar23861629.swift +++ b/validation-test/Sema/type_checker_perf/fast/rdar23861629.swift @@ -5,7 +5,6 @@ struct S { var s: String? } func rdar23861629(_ a: [S]) { _ = a.reduce("") { - // expected-error@-1 {{reasonable time}} ($0 == "") ? ($1.s ?? "") : ($0 + "," + ($1.s ?? "")) + ($1.s ?? "test") + ($1.s ?? "okay") } } diff --git a/validation-test/Sema/type_checker_perf/slow/fast-operator-typechecking.swift b/validation-test/Sema/type_checker_perf/slow/fast-operator-typechecking.swift index 20b9e029a84d4..df04b5fc59cc0 100644 --- a/validation-test/Sema/type_checker_perf/slow/fast-operator-typechecking.swift +++ b/validation-test/Sema/type_checker_perf/slow/fast-operator-typechecking.swift @@ -18,5 +18,4 @@ func f(tail: UInt64, byteCount: UInt64) { func size(count: Int) { // Size of the buffer we need to allocate let _ = count * MemoryLayout.size * (4 + 3 + 3 + 2 + 4) - // expected-error@-1 {{the compiler is unable to type-check this expression in reasonable time}} } From c9100c46e70a5e0348686f1494a40a09e5b3eec8 Mon Sep 17 00:00:00 2001 From: Holly Borla Date: Thu, 29 Oct 2020 14:39:11 -0700 Subject: [PATCH 11/22] [ConstraintSystem] Instead of favoring existing operator bindings, order them first in the main disjunction partition. --- lib/Sema/CSSolver.cpp | 65 ++++++++++++++++--------------------------- 1 file changed, 24 insertions(+), 41 deletions(-) diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index 7def6c1f0c9e0..075797c241cd3 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -2015,7 +2015,9 @@ static Constraint *tryOptimizeGenericDisjunction( // Performance hack: favor operator overloads with decl or type we're already // binding elsewhere in this expression. -static void existingOperatorBindingsForDisjunction(ConstraintSystem &CS, ArrayRef constraints, SmallVectorImpl &found) { +static void existingOperatorBindingsForDisjunction(ConstraintSystem &CS, + ArrayRef constraints, + SmallVectorImpl &found) { auto *choice = constraints.front(); if (choice->getKind() != ConstraintKind::BindOverload) return; @@ -2026,47 +2028,28 @@ static void existingOperatorBindingsForDisjunction(ConstraintSystem &CS, ArrayRe auto decl = overload.getDecl(); if (!decl->isOperator()) return; - auto baseName = decl->getBaseName(); - - SmallSet typesFound; + SmallSet typesFound; for (auto overload : CS.getResolvedOverloads()) { auto resolved = overload.second; if (!resolved.choice.isDecl()) continue; - auto representativeDecl = resolved.choice.getDecl(); + auto representativeDecl = resolved.choice.getDecl(); if (!representativeDecl->isOperator()) continue; - if (representativeDecl->getBaseName() == baseName) { - // Favor exactly the same decl, if we have a binding to the same name. - for (auto *constraint : constraints) { - if (constraint->isFavored()) - continue; - auto choice = constraint->getOverloadChoice(); - if (choice.getDecl() == representativeDecl) { - found.push_back(constraint); - break; - } - } - } else { - // Favor the same type, if we have a binding to an operator of that type. - auto representativeType = representativeDecl->getInterfaceType(); - if (typesFound.count(representativeType.getPointer())) - continue; - typesFound.insert(representativeType.getPointer()); + typesFound.insert(representativeDecl->getInterfaceType()->getCanonicalType()); + } - for (auto *constraint : constraints) { - if (constraint->isFavored()) - continue; - auto choice = constraint->getOverloadChoice(); - if (choice.getDecl()->getInterfaceType()->isEqual(representativeType)) { - found.push_back(constraint); - break; - } - } - } + for (auto index : indices(constraints)) { + auto *constraint = constraints[index]; + if (constraint->isFavored()) + continue; + + auto *decl = constraint->getOverloadChoice().getDecl(); + if (typesFound.count(decl->getInterfaceType()->getCanonicalType())) + found.push_back(index); } } @@ -2074,11 +2057,6 @@ void ConstraintSystem::partitionDisjunction( ArrayRef Choices, SmallVectorImpl &Ordering, SmallVectorImpl &PartitionBeginning) { - SmallVector existing; - existingOperatorBindingsForDisjunction(*this, Choices, existing); - for (auto constraint : existing) - favorConstraint(constraint); - // Apply a special-case rule for favoring one generic function over // another. if (auto favored = tryOptimizeGenericDisjunction(DC, Choices)) { @@ -2105,12 +2083,18 @@ void ConstraintSystem::partitionDisjunction( // First collect some things that we'll generally put near the beginning or // end of the partitioning. - SmallVector favored; + SmallVector everythingElse; SmallVector simdOperators; SmallVector disabled; SmallVector unavailable; + // Add existing operator bindings to the main partition first. This often + // helps the solver find a solution fast. + existingOperatorBindingsForDisjunction(*this, Choices, everythingElse); + for (auto index : everythingElse) + taken.insert(Choices[index]); + // First collect disabled and favored constraints. forEachChoice(Choices, [&](unsigned index, Constraint *constraint) -> bool { if (constraint->isDisabled()) { @@ -2170,7 +2154,6 @@ void ConstraintSystem::partitionDisjunction( } }; - SmallVector everythingElse; // Gather the remaining options. forEachChoice(Choices, [&](unsigned index, Constraint *constraint) -> bool { everythingElse.push_back(index); @@ -2206,8 +2189,8 @@ Constraint *ConstraintSystem::selectDisjunction() { if (firstFavored == secondFavored) { // Look for additional choices to favor - SmallVector firstExisting; - SmallVector secondExisting; + SmallVector firstExisting; + SmallVector secondExisting; existingOperatorBindingsForDisjunction(*cs, first->getNestedConstraints(), firstExisting); firstFavored = firstExisting.size() ? firstExisting.size() : first->countActiveNestedConstraints(); From a8fcdb8580673742c21953a6a4d30e91e620b59f Mon Sep 17 00:00:00 2001 From: Holly Borla Date: Thu, 29 Oct 2020 17:23:07 -0700 Subject: [PATCH 12/22] [ConstraintSystem] Remove an unnecessary SIMD special case in `DisjunctionStep::shortCircuitDisjunctionAt`. This code is unnecessary because SIMD overloads are in their own partition, so the short circuiting will happen automatically. --- lib/Sema/CSStep.cpp | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/lib/Sema/CSStep.cpp b/lib/Sema/CSStep.cpp index 6a1342ecb2f29..269121cfc6aad 100644 --- a/lib/Sema/CSStep.cpp +++ b/lib/Sema/CSStep.cpp @@ -640,18 +640,6 @@ bool DisjunctionStep::shortCircuitDisjunctionAt( if (currentChoice->getKind() == ConstraintKind::CheckedCast) return true; - // Extra checks for binding of operators - if (currentChoice->getKind() == ConstraintKind::BindOverload && - currentChoice->getOverloadChoice().getDecl()->isOperator() && - lastSuccessfulChoice->getKind() == ConstraintKind::BindOverload && - lastSuccessfulChoice->getOverloadChoice().getDecl()->isOperator()) { - - // If we have a SIMD operator, and the prior choice was not a SIMD - // Operator, we're done. - if (isSIMDOperator(currentChoice->getOverloadChoice().getDecl()) && - !isSIMDOperator(lastSuccessfulChoice->getOverloadChoice().getDecl())) - return true; - } return false; } From 423d6bdff076285b02c7a6152baf6a85149ec8ef Mon Sep 17 00:00:00 2001 From: Holly Borla Date: Thu, 29 Oct 2020 17:25:23 -0700 Subject: [PATCH 13/22] [ConstraintSystem] Re-instate the operator type variable merging hacks for now. There's some more disjunction pruning work to be done before we can remove this. --- lib/Sema/CSGen.cpp | 66 ++++++++++++++++++++++++++++++++++ lib/Sema/CSStep.h | 38 ++++++++++++++++++++ test/Constraints/sr10324.swift | 2 ++ 3 files changed, 106 insertions(+) diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp index 09e9ea5c0827a..987d70541ad60 100644 --- a/lib/Sema/CSGen.cpp +++ b/lib/Sema/CSGen.cpp @@ -303,6 +303,72 @@ namespace { // solver can still make progress. auto favoredTy = (*lti.collectedTypes.begin())->getWithoutSpecifierType(); CS.setFavoredType(expr, favoredTy.getPointer()); + + // If we have a chain of identical binop expressions with homogeneous + // argument types, we can directly simplify the associated constraint + // graph. + auto simplifyBinOpExprTyVars = [&]() { + // Don't attempt to do linking if there are + // literals intermingled with other inferred types. + if (lti.hasLiteral) + return; + + for (auto binExp1 : lti.binaryExprs) { + for (auto binExp2 : lti.binaryExprs) { + if (binExp1 == binExp2) + continue; + + auto fnTy1 = CS.getType(binExp1)->getAs(); + auto fnTy2 = CS.getType(binExp2)->getAs(); + + if (!(fnTy1 && fnTy2)) + return; + + auto ODR1 = dyn_cast(binExp1->getFn()); + auto ODR2 = dyn_cast(binExp2->getFn()); + + if (!(ODR1 && ODR2)) + return; + + // TODO: We currently limit this optimization to known arithmetic + // operators, but we should be able to broaden this out to + // logical operators as well. + if (!isArithmeticOperatorDecl(ODR1->getDecls()[0])) + return; + + if (ODR1->getDecls()[0]->getBaseName() != + ODR2->getDecls()[0]->getBaseName()) + return; + + // All things equal, we can merge the tyvars for the function + // types. + auto rep1 = CS.getRepresentative(fnTy1); + auto rep2 = CS.getRepresentative(fnTy2); + + if (rep1 != rep2) { + CS.mergeEquivalenceClasses(rep1, rep2, + /*updateWorkList*/ false); + } + + auto odTy1 = CS.getType(ODR1)->getAs(); + auto odTy2 = CS.getType(ODR2)->getAs(); + + if (odTy1 && odTy2) { + auto odRep1 = CS.getRepresentative(odTy1); + auto odRep2 = CS.getRepresentative(odTy2); + + // Since we'll be choosing the same overload, we can merge + // the overload tyvar as well. + if (odRep1 != odRep2) + CS.mergeEquivalenceClasses(odRep1, odRep2, + /*updateWorkList*/ false); + } + } + } + }; + + simplifyBinOpExprTyVars(); + return true; } diff --git a/lib/Sema/CSStep.h b/lib/Sema/CSStep.h index 2b59badf1a38a..053633dfda07c 100644 --- a/lib/Sema/CSStep.h +++ b/lib/Sema/CSStep.h @@ -649,6 +649,7 @@ class DisjunctionStep final : public BindingStep { : BindingStep(cs, {cs, disjunction}, solutions), Disjunction(disjunction), AfterDisjunction(erase(disjunction)) { assert(Disjunction->getKind() == ConstraintKind::Disjunction); + pruneOverloadSet(Disjunction); ++cs.solverState->NumDisjunctions; } @@ -696,6 +697,43 @@ class DisjunctionStep final : public BindingStep { /// simplified further, false otherwise. bool attempt(const DisjunctionChoice &choice) override; + // Check if selected disjunction has a representative + // this might happen when there are multiple binary operators + // chained together. If so, disable choices which differ + // from currently selected representative. + void pruneOverloadSet(Constraint *disjunction) { + auto *choice = disjunction->getNestedConstraints().front(); + auto *typeVar = choice->getFirstType()->getAs(); + if (!typeVar) + return; + + auto *repr = typeVar->getImpl().getRepresentative(nullptr); + if (!repr || repr == typeVar) + return; + + for (auto overload : CS.getResolvedOverloads()) { + auto resolved = overload.second; + if (!resolved.boundType->isEqual(repr)) + continue; + + auto &representative = resolved.choice; + if (!representative.isDecl()) + return; + + // Disable all of the overload choices which are different from + // the one which is currently picked for representative. + for (auto *constraint : disjunction->getNestedConstraints()) { + auto choice = constraint->getOverloadChoice(); + if (!choice.isDecl() || choice.getDecl() == representative.getDecl()) + continue; + + constraint->setDisabled(); + DisabledChoices.push_back(constraint); + } + break; + } + }; + // Figure out which of the solutions has the smallest score. static Optional getBestScore(SmallVectorImpl &solutions) { assert(!solutions.empty()); diff --git a/test/Constraints/sr10324.swift b/test/Constraints/sr10324.swift index 5e38121153008..6cacfb2710420 100644 --- a/test/Constraints/sr10324.swift +++ b/test/Constraints/sr10324.swift @@ -1,5 +1,7 @@ // RUN: %target-swift-frontend -typecheck -verify %s +// REQUIRES: rdar65007946 + struct A { static func * (lhs: A, rhs: A) -> B { return B() } static func * (lhs: B, rhs: A) -> B { return B() } From 0d8a1619c6320cec3b918ace1f7befcc73345672 Mon Sep 17 00:00:00 2001 From: Holly Borla Date: Fri, 30 Oct 2020 17:25:57 -0700 Subject: [PATCH 14/22] [NFC][ConstraintSystem] Update the documentation for existingOperatorBindingsForDisjunction. --- lib/Sema/CSSolver.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index 075797c241cd3..62fe99dddd362 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -2013,8 +2013,13 @@ static Constraint *tryOptimizeGenericDisjunction( llvm_unreachable("covered switch"); } -// Performance hack: favor operator overloads with decl or type we're already -// binding elsewhere in this expression. +/// Populates the \c found vector with the indices of the given constraints +/// that have a matching type to an existing operator binding elsewhere in +/// the expression. +/// +/// Operator bindings that have a matching type to an existing binding +/// are attempted first by the solver because it's very common to chain +/// operators of the same type together. static void existingOperatorBindingsForDisjunction(ConstraintSystem &CS, ArrayRef constraints, SmallVectorImpl &found) { @@ -2056,7 +2061,6 @@ static void existingOperatorBindingsForDisjunction(ConstraintSystem &CS, void ConstraintSystem::partitionDisjunction( ArrayRef Choices, SmallVectorImpl &Ordering, SmallVectorImpl &PartitionBeginning) { - // Apply a special-case rule for favoring one generic function over // another. if (auto favored = tryOptimizeGenericDisjunction(DC, Choices)) { From 099560813eb9ffc02273928aee7bfa7e629c9d61 Mon Sep 17 00:00:00 2001 From: Holly Borla Date: Tue, 3 Nov 2020 10:28:38 -0800 Subject: [PATCH 15/22] [ConstraintSystem] Only factor in existing operator bindings in selectDisjunction if both disjunctions are operator bindings. --- lib/Sema/CSSolver.cpp | 12 ++++++++---- .../slow/fast-operator-typechecking.swift | 1 + .../{fast => slow}/rdar23682605.swift | 1 + 3 files changed, 10 insertions(+), 4 deletions(-) rename validation-test/Sema/type_checker_perf/{fast => slow}/rdar23682605.swift (91%) diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index 62fe99dddd362..72f2d873a52d1 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -2191,6 +2191,10 @@ Constraint *ConstraintSystem::selectDisjunction() { unsigned firstFavored = first->countFavoredNestedConstraints(); unsigned secondFavored = second->countFavoredNestedConstraints(); + if (!isOperatorBindOverload(first->getNestedConstraints().front()) || + !isOperatorBindOverload(second->getNestedConstraints().front())) + return first->countActiveNestedConstraints() < second->countActiveNestedConstraints(); + if (firstFavored == secondFavored) { // Look for additional choices to favor SmallVector firstExisting; @@ -2201,12 +2205,12 @@ Constraint *ConstraintSystem::selectDisjunction() { existingOperatorBindingsForDisjunction(*cs, second->getNestedConstraints(), secondExisting); secondFavored = secondExisting.size() ? secondExisting.size() : second->countActiveNestedConstraints(); - return firstFavored < secondFavored; - } else { - firstFavored = firstFavored ? firstFavored : first->countActiveNestedConstraints(); - secondFavored = secondFavored ? secondFavored : second->countActiveNestedConstraints(); return firstFavored < secondFavored; } + + firstFavored = firstFavored ? firstFavored : first->countActiveNestedConstraints(); + secondFavored = secondFavored ? secondFavored : second->countActiveNestedConstraints(); + return firstFavored < secondFavored; }); if (minDisjunction != disjunctions.end()) diff --git a/validation-test/Sema/type_checker_perf/slow/fast-operator-typechecking.swift b/validation-test/Sema/type_checker_perf/slow/fast-operator-typechecking.swift index df04b5fc59cc0..20b9e029a84d4 100644 --- a/validation-test/Sema/type_checker_perf/slow/fast-operator-typechecking.swift +++ b/validation-test/Sema/type_checker_perf/slow/fast-operator-typechecking.swift @@ -18,4 +18,5 @@ func f(tail: UInt64, byteCount: UInt64) { func size(count: Int) { // Size of the buffer we need to allocate let _ = count * MemoryLayout.size * (4 + 3 + 3 + 2 + 4) + // expected-error@-1 {{the compiler is unable to type-check this expression in reasonable time}} } diff --git a/validation-test/Sema/type_checker_perf/fast/rdar23682605.swift b/validation-test/Sema/type_checker_perf/slow/rdar23682605.swift similarity index 91% rename from validation-test/Sema/type_checker_perf/fast/rdar23682605.swift rename to validation-test/Sema/type_checker_perf/slow/rdar23682605.swift index 4be53b4d2ef83..b7bc757fe1989 100644 --- a/validation-test/Sema/type_checker_perf/fast/rdar23682605.swift +++ b/validation-test/Sema/type_checker_perf/slow/rdar23682605.swift @@ -14,6 +14,7 @@ func memoize( body: @escaping ((T)->U, T)->U ) -> (T)->U { } let fibonacci = memoize { + // expected-error@-1 {{reasonable time}} fibonacci, n in n < 2 ? Double(n) : fibonacci(n - 1) + fibonacci(n - 2) } From abfc90303c278ec0124ba7f1be5178fffaacd761 Mon Sep 17 00:00:00 2001 From: Holly Borla Date: Tue, 3 Nov 2020 12:48:06 -0800 Subject: [PATCH 16/22] [ConstraintSystem] For generic operators, only consider exact decls that are already bound elsewhere for existingOperatorBindingsForDisjunction, rather than also considering overload choices with the same type. --- lib/Sema/CSSolver.cpp | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index 72f2d873a52d1..507700632aced 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -2034,7 +2034,14 @@ static void existingOperatorBindingsForDisjunction(ConstraintSystem &CS, if (!decl->isOperator()) return; - SmallSet typesFound; + // For concrete operators, consider overloads that have the same type as + // an existing binding, because it's very common to write mixed operator + // expressions where all operands have the same type, e.g. `(x + 10) / 2`. + // For generic operators, only favor an exact overload that has already + // been bound, because mixed operator expressions are far less common, and + // computing generic canonical types is expensive. + SmallSet concreteTypesFound; + SmallSet genericDeclsFound; for (auto overload : CS.getResolvedOverloads()) { auto resolved = overload.second; if (!resolved.choice.isDecl()) @@ -2044,7 +2051,12 @@ static void existingOperatorBindingsForDisjunction(ConstraintSystem &CS, if (!representativeDecl->isOperator()) continue; - typesFound.insert(representativeDecl->getInterfaceType()->getCanonicalType()); + auto interfaceType = representativeDecl->getInterfaceType(); + if (interfaceType->is()) { + genericDeclsFound.insert(representativeDecl); + } else { + concreteTypesFound.insert(interfaceType->getCanonicalType()); + } } for (auto index : indices(constraints)) { @@ -2053,7 +2065,10 @@ static void existingOperatorBindingsForDisjunction(ConstraintSystem &CS, continue; auto *decl = constraint->getOverloadChoice().getDecl(); - if (typesFound.count(decl->getInterfaceType()->getCanonicalType())) + auto interfaceType = decl->getInterfaceType(); + bool isGeneric = interfaceType->is(); + if ((isGeneric && genericDeclsFound.count(decl)) || + (!isGeneric && concreteTypesFound.count(interfaceType->getCanonicalType()))) found.push_back(index); } } From 12dce859a2e9178ec0d74cce284b70ada65ed815 Mon Sep 17 00:00:00 2001 From: Holly Borla Date: Tue, 3 Nov 2020 13:21:18 -0800 Subject: [PATCH 17/22] [NFC][ConstraintSystem] Use llvm::count_if for the count methods on Constraint. --- include/swift/Sema/Constraint.h | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/include/swift/Sema/Constraint.h b/include/swift/Sema/Constraint.h index c2a6a6a1c67cb..d1f91348f9e60 100644 --- a/include/swift/Sema/Constraint.h +++ b/include/swift/Sema/Constraint.h @@ -673,21 +673,15 @@ class Constraint final : public llvm::ilist_node, } unsigned countFavoredNestedConstraints() const { - unsigned count = 0; - for (auto *constraint : Nested) - if (constraint->isFavored() && !constraint->isDisabled()) - count++; - - return count; + return llvm::count_if(Nested, [](const Constraint *constraint) { + return constraint->isFavored() && !constraint->isDisabled(); + }); } unsigned countActiveNestedConstraints() const { - unsigned count = 0; - for (auto *constraint : Nested) - if (!constraint->isDisabled()) - count++; - - return count; + return llvm::count_if(Nested, [](const Constraint *constraint) { + return !constraint->isDisabled(); + }); } /// Determine if this constraint represents explicit conversion, From 2507a3155d70ea803819473ff0aff46ed2b2130f Mon Sep 17 00:00:00 2001 From: Holly Borla Date: Thu, 5 Nov 2020 10:31:34 -0800 Subject: [PATCH 18/22] [Test] Add a slow type checker test case from source compatibility suite. --- .../slow/mixed_string_array_addition.swift | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 validation-test/Sema/type_checker_perf/slow/mixed_string_array_addition.swift diff --git a/validation-test/Sema/type_checker_perf/slow/mixed_string_array_addition.swift b/validation-test/Sema/type_checker_perf/slow/mixed_string_array_addition.swift new file mode 100644 index 0000000000000..6b7fa19f0cb2b --- /dev/null +++ b/validation-test/Sema/type_checker_perf/slow/mixed_string_array_addition.swift @@ -0,0 +1,12 @@ +// RUN: %target-typecheck-verify-swift -swift-version 5 -solver-expression-time-threshold=1 + +func method(_ arg: String, body: () -> [String]) {} + +func test(str: String, properties: [String]) { + // expected-error@+1 {{the compiler is unable to type-check this expression in reasonable time}} + method(str + "" + str + "") { + properties.map { param in + "" + param + "" + param + "" + } + [""] + } +} From d675c7471bf866349d8b0820a88a2ea4d8c7e34c Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Fri, 6 Nov 2020 17:56:19 -0800 Subject: [PATCH 19/22] [CSApply] Adjust @autoclosure locator to point to argument expression being coerced to result type While applying type inferred for autoclosure to argument use locator that records argument/param indices to make it possible for `coerceToType` to simplify it down to underlying argument expression if necessary. --- lib/Sema/CSApply.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp index 2b08dda1cd2bb..437481c310d4a 100644 --- a/lib/Sema/CSApply.cpp +++ b/lib/Sema/CSApply.cpp @@ -5813,9 +5813,11 @@ Expr *ExprRewriter::coerceCallArguments( // - new types are propagated to constraint system auto *closureType = param.getPlainType()->castTo(); + auto argLoc = getArgLocator(argIdx, paramIdx, param.getParameterFlags()); + arg = coerceToType( arg, closureType->getResult(), - locator.withPathElement(ConstraintLocator::AutoclosureResult)); + argLoc.withPathElement(ConstraintLocator::AutoclosureResult)); if (shouldInjectWrappedValuePlaceholder) { // If init(wrappedValue:) takes an autoclosure, then we want From f8577b2aa884b73eda24e8bed49401b8a2fa9819 Mon Sep 17 00:00:00 2001 From: Xi Ge Date: Sat, 7 Nov 2020 10:09:36 -0800 Subject: [PATCH 20/22] driver: allow SWIFT_USE_NEW_DRIVER to specify a driver executable name --- include/swift/AST/DiagnosticsDriver.def | 6 ++- tools/driver/driver.cpp | 53 +++++++++++++++---------- 2 files changed, 37 insertions(+), 22 deletions(-) diff --git a/include/swift/AST/DiagnosticsDriver.def b/include/swift/AST/DiagnosticsDriver.def index 32e862b3751da..ccce85380c3f7 100644 --- a/include/swift/AST/DiagnosticsDriver.def +++ b/include/swift/AST/DiagnosticsDriver.def @@ -190,7 +190,11 @@ WARNING(warn_drv_darwin_sdk_invalid_settings, none, "SDK settings were ignored because 'SDKSettings.json' could not be parsed", ()) -REMARK(remark_forwarding_to_new_driver, none, "new Swift driver will be used", ()) +REMARK(remark_forwarding_to_new_driver, none, + "new Swift driver at '%0' will be used", (StringRef)) + +REMARK(remark_forwarding_driver_not_there, none, + "new Swift driver at '%0' cannot be found; C++ driver will be used", (StringRef)) #define UNDEFINE_DIAGNOSTIC_MACROS #include "DefineDiagnosticMacros.h" diff --git a/tools/driver/driver.cpp b/tools/driver/driver.cpp index 0fb6062e12ec0..cf6409d2a5633 100644 --- a/tools/driver/driver.cpp +++ b/tools/driver/driver.cpp @@ -160,33 +160,44 @@ static int run_driver(StringRef ExecName, DiagnosticEngine Diags(SM); Diags.addConsumer(PDC); + std::string newDriverName; + if (auto driverNameOp = llvm::sys::Process::GetEnv("SWIFT_USE_NEW_DRIVER")) { + newDriverName = driverNameOp.getValue(); + } + // Forwarding calls to the swift driver if the C++ driver is invoked as `swift` // or `swiftc`, and an environment variable SWIFT_USE_NEW_DRIVER is defined. - if (llvm::sys::Process::GetEnv("SWIFT_USE_NEW_DRIVER") && + if (!newDriverName.empty() && (ExecName == "swift" || ExecName == "swiftc")) { SmallString<256> NewDriverPath(llvm::sys::path::parent_path(Path)); - llvm::sys::path::append(NewDriverPath, "swift-driver"); - SmallVector subCommandArgs; - // Rewrite the program argument. - subCommandArgs.push_back(NewDriverPath.c_str()); - if (ExecName == "swiftc") { - subCommandArgs.push_back("--driver-mode=swiftc"); + llvm::sys::path::append(NewDriverPath, newDriverName); + if (!llvm::sys::fs::exists(NewDriverPath)) { + Diags.diagnose(SourceLoc(), diag::remark_forwarding_driver_not_there, + NewDriverPath); } else { - assert(ExecName == "swift"); - subCommandArgs.push_back("--driver-mode=swift"); + SmallVector subCommandArgs; + // Rewrite the program argument. + subCommandArgs.push_back(NewDriverPath.c_str()); + if (ExecName == "swiftc") { + subCommandArgs.push_back("--driver-mode=swiftc"); + } else { + assert(ExecName == "swift"); + subCommandArgs.push_back("--driver-mode=swift"); + } + subCommandArgs.insert(subCommandArgs.end(), argv.begin() + 1, argv.end()); + + // Execute the subcommand. + subCommandArgs.push_back(nullptr); + Diags.diagnose(SourceLoc(), diag::remark_forwarding_to_new_driver, + NewDriverPath); + ExecuteInPlace(NewDriverPath.c_str(), subCommandArgs.data()); + + // If we reach here then an error occurred (typically a missing path). + std::string ErrorString = llvm::sys::StrError(); + llvm::errs() << "error: unable to invoke subcommand: " << subCommandArgs[0] + << " (" << ErrorString << ")\n"; + return 2; } - subCommandArgs.insert(subCommandArgs.end(), argv.begin() + 1, argv.end()); - - // Execute the subcommand. - subCommandArgs.push_back(nullptr); - Diags.diagnose(SourceLoc(), diag::remark_forwarding_to_new_driver); - ExecuteInPlace(NewDriverPath.c_str(), subCommandArgs.data()); - - // If we reach here then an error occurred (typically a missing path). - std::string ErrorString = llvm::sys::StrError(); - llvm::errs() << "error: unable to invoke subcommand: " << subCommandArgs[0] - << " (" << ErrorString << ")\n"; - return 2; } Driver TheDriver(Path, ExecName, argv, Diags); From f8585d75d1e559a943b10dfbf71a56f5383e2759 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Rodri=CC=81guez=20Troitin=CC=83o?= Date: Sat, 7 Nov 2020 15:57:12 -0800 Subject: [PATCH 21/22] [windows] Partial revert #34143. Remove XFAIL. The test started working in the last week, for unknown reasons. Remove the XFAIL line, but keep the infra for detecting the MSVC version for the tests. It might be useful later. If the test starts failing again, and nobody has any idea why, the best path forward might be marking it as UNSUPPORTED. --- test/Frontend/crash-in-user-code.swift | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/Frontend/crash-in-user-code.swift b/test/Frontend/crash-in-user-code.swift index d36f642c76e46..4a36a0c3b5a4c 100644 --- a/test/Frontend/crash-in-user-code.swift +++ b/test/Frontend/crash-in-user-code.swift @@ -8,8 +8,6 @@ // UNSUPPORTED: OS=tvos // UNSUPPORTED: OS=watchos -// XFAIL: MSVC_VER=15.0 - // CHECK: Stack dump: // CHECK-NEXT: Program arguments: // CHECK-NEXT: Swift version From 5b4cc58f8a5de2c4b680a29da844fdfc927bbb09 Mon Sep 17 00:00:00 2001 From: Xi Ge Date: Sat, 7 Nov 2020 18:24:33 -0800 Subject: [PATCH 22/22] driver: add an option to avoid forwarding to the new driver --- include/swift/Option/Options.td | 4 ++++ tools/driver/driver.cpp | 6 ++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/include/swift/Option/Options.td b/include/swift/Option/Options.td index d0bb1e0cb990f..d200d3061c9a8 100644 --- a/include/swift/Option/Options.td +++ b/include/swift/Option/Options.td @@ -177,6 +177,10 @@ def verify_incremental_dependencies : Flags<[FrontendOption, HelpHidden]>, HelpText<"Enable the dependency verifier for each frontend job">; +def disallow_forwarding_driver : + Flag<["-"], "disallow-use-new-driver">, Flags<[]>, + HelpText<"Disable using new swift-driver">; + def driver_emit_fine_grained_dependency_dot_file_after_every_import : Flag<["-"], "driver-emit-fine-grained-dependency-dot-file-after-every-import">, InternalDebugOpt, diff --git a/tools/driver/driver.cpp b/tools/driver/driver.cpp index cf6409d2a5633..4b7d23f71309a 100644 --- a/tools/driver/driver.cpp +++ b/tools/driver/driver.cpp @@ -164,10 +164,12 @@ static int run_driver(StringRef ExecName, if (auto driverNameOp = llvm::sys::Process::GetEnv("SWIFT_USE_NEW_DRIVER")) { newDriverName = driverNameOp.getValue(); } - + auto disallowForwarding = llvm::find_if(argv, [](const char* arg) { + return StringRef(arg) == "-disallow-use-new-driver"; + }) != argv.end(); // Forwarding calls to the swift driver if the C++ driver is invoked as `swift` // or `swiftc`, and an environment variable SWIFT_USE_NEW_DRIVER is defined. - if (!newDriverName.empty() && + if (!newDriverName.empty() && !disallowForwarding && (ExecName == "swift" || ExecName == "swiftc")) { SmallString<256> NewDriverPath(llvm::sys::path::parent_path(Path)); llvm::sys::path::append(NewDriverPath, newDriverName);