[Git][ghc/ghc][wip/T23070-dicts] Two fast paths
Simon Peyton Jones (@simonpj)
gitlab at gitlab.haskell.org
Sun May 14 21:53:45 UTC 2023
Simon Peyton Jones pushed to branch wip/T23070-dicts at Glasgow Haskell Compiler / GHC
Commits:
c2619514 by Simon Peyton Jones at 2023-05-14T22:53:00+01:00
Two fast paths
* Naturally coherent constraints
* Hole-fits
- - - - -
2 changed files:
- compiler/GHC/Tc/Errors/Hole.hs
- compiler/GHC/Tc/Solver/Dict.hs
Changes:
=====================================
compiler/GHC/Tc/Errors/Hole.hs
=====================================
@@ -1,5 +1,6 @@
{-# LANGUAGE CPP #-}
{-# LANGUAGE RecordWildCards #-}
+{-# LANGUAGE MultiWayIf #-}
{-# LANGUAGE ExistentialQuantification #-}
module GHC.Tc.Errors.Hole
@@ -40,7 +41,10 @@ import GHC.Tc.Utils.TcMType
import GHC.Tc.Types.Evidence
import GHC.Tc.Utils.TcType
import GHC.Core.Type
+import GHC.Core.TyCon( TyCon, isGenerativeTyCon )
+import GHC.Core.TyCo.Rep( Type(..) )
import GHC.Core.DataCon
+import GHC.Core.Predicate( Pred(..), classifyPredType, eqRelRole )
import GHC.Types.Name
import GHC.Types.Name.Reader
import GHC.Builtin.Names ( gHC_ERR )
@@ -981,28 +985,33 @@ tcCheckHoleFit (TypedHole {..}) hole_ty ty = discardErrs $
tcSubTypeSigma orig (ExprSigCtxt NoRRC) ty hole_ty
; traceTc "Checking hole fit {" empty
; traceTc "wanteds are: " $ ppr wanted
- ; if isEmptyWC wanted && isEmptyBag th_relevant_cts
- then do { traceTc "}" empty
- ; return (True, wrap) }
- else do { fresh_binds <- newTcEvBinds
- -- The relevant constraints may contain HoleDests, so we must
- -- take care to clone them as well (to avoid #15370).
- ; cloned_relevants <- mapBagM cloneWantedCtEv th_relevant_cts
- -- We wrap the WC in the nested implications, for details, see
- -- Note [Checking hole fits]
- ; let wrapInImpls cts = foldl (flip (setWCAndBinds fresh_binds)) cts th_implics
- final_wc = wrapInImpls $ addSimples wanted $
- mapBag mkNonCanonical cloned_relevants
- -- We add the cloned relevants to the wanteds generated
- -- by the call to tcSubType_NC, for details, see
- -- Note [Relevant constraints]. There's no need to clone
- -- the wanteds, because they are freshly generated by the
- -- call to`tcSubtype_NC`.
- ; traceTc "final_wc is: " $ ppr final_wc
- -- See Note [Speeding up valid hole-fits]
- ; (rem, _) <- tryTc $ runTcSEarlyAbort $ simplifyTopWanteds final_wc
- ; traceTc "}" empty
- ; return (any isSolvedWC rem, wrap) } }
+ ; if | isEmptyWC wanted, isEmptyBag th_relevant_cts
+ -> do { traceTc "}" empty
+ ; return (True, wrap) }
+
+ | checkInsoluble wanted
+ -> return (False, wrap)
+
+ | otherwise
+ -> do { fresh_binds <- newTcEvBinds
+ -- The relevant constraints may contain HoleDests, so we must
+ -- take care to clone them as well (to avoid #15370).
+ ; cloned_relevants <- mapBagM cloneWantedCtEv th_relevant_cts
+ -- We wrap the WC in the nested implications, for details, see
+ -- Note [Checking hole fits]
+ ; let wrapInImpls cts = foldl (flip (setWCAndBinds fresh_binds)) cts th_implics
+ final_wc = wrapInImpls $ addSimples wanted $
+ mapBag mkNonCanonical cloned_relevants
+ -- We add the cloned relevants to the wanteds generated
+ -- by the call to tcSubType_NC, for details, see
+ -- Note [Relevant constraints]. There's no need to clone
+ -- the wanteds, because they are freshly generated by the
+ -- call to`tcSubtype_NC`.
+ ; traceTc "final_wc is: " $ ppr final_wc
+ -- See Note [Speeding up valid hole-fits]
+ ; (rem, _) <- tryTc $ runTcSEarlyAbort $ simplifyTopWanteds final_wc
+ ; traceTc "}" empty
+ ; return (any isSolvedWC rem, wrap) } }
where
orig = ExprHoleOrigin (hole_occ <$> th_hole)
@@ -1012,3 +1021,31 @@ tcCheckHoleFit (TypedHole {..}) hole_ty ty = discardErrs $
-> WantedConstraints -- The new constraints.
setWCAndBinds binds imp wc
= mkImplicWC $ unitBag $ imp { ic_wanted = wc , ic_binds = binds }
+
+checkInsoluble :: WantedConstraints -> Bool
+checkInsoluble (WC { wc_simple = simples })
+ = any is_insol simples
+ where
+ is_insol ct = case classifyPredType (ctPred ct) of
+ EqPred r t1 t2 -> definitelyNotEqual (eqRelRole r) t1 t2
+ _ -> False
+
+definitelyNotEqual :: Role -> TcType -> TcType -> Bool
+definitelyNotEqual r t1 t2
+ = go t1 t2
+ where
+ go t1 t2
+ | Just t1' <- coreView t1 = go t1' t2
+ | Just t2' <- coreView t2 = go t1 t2'
+
+ go (TyConApp tc _) t2 | isGenerativeTyCon tc r = go_tc tc t2
+ go t1 (TyConApp tc _) | isGenerativeTyCon tc r = go_tc tc t1
+ go (FunTy {ft_af = af1}) (FunTy {ft_af = af2}) = af1 /= af2
+ go _ _ = False
+
+ go_tc :: TyCon -> TcType -> Bool
+ -- The TyCon is generative, and is not a saturated FunTy
+ go_tc tc1 (TyConApp tc2 _) | isGenerativeTyCon tc2 r = tc1 /= tc2
+ go_tc _ (FunTy {}) = True
+ go_tc _ (ForAllTy {}) = True
+ go_tc _ _ = False
\ No newline at end of file
=====================================
compiler/GHC/Tc/Solver/Dict.hs
=====================================
@@ -69,7 +69,7 @@ solveDictNC :: CtEvidence -> Class -> [Type] -> SolverStage ()
-- NC: this comes from CNonCanonical or CIrredCan
-- Precondition: already rewritten by inert set
solveDictNC ev cls tys
- = do { dict_ct <- simpleStage (canDictCt ev cls tys)
+ = do { dict_ct <- canDictCt ev cls tys
; solveDict dict_ct }
solveDict :: DictCt -> SolverStage ()
@@ -100,18 +100,29 @@ updInertDicts dict_ct@(DictCt { di_cls = cls, di_ev = ev })
-- Add the new constraint to the inert set
; updInertCans (updDicts (addDict dict_ct)) }
-canDictCt :: CtEvidence -> Class -> [Type] -> TcS DictCt
+canDictCt :: CtEvidence -> Class -> [Type] -> SolverStage DictCt
-- Once-only processing of Dict constraints:
-- * expand superclasses
-- * deal with CallStack
canDictCt ev cls tys
| isGiven ev -- See Note [Eagerly expand given superclasses]
- = do { dflags <- getDynFlags
+ = Stage $
+ do { dflags <- getDynFlags
; sc_cts <- mkStrictSuperClasses (givensFuel dflags) ev [] [] cls tys
- -- givensFuel dflags: See Note [Expanding Recursive Superclasses and ExpansionFuel]
- ; emitWork (listToBag sc_cts)
- ; return (DictCt { di_ev = ev, di_cls = cls
- , di_tys = tys, di_pend_sc = doNotExpand }) }
+ -- givensFuel dflags: See Note [Expanding Recursive Superclasses and ExpansionFuel]
+
+ -- For equality classes, /replace/ the current constraint with its
+ -- superclasses, rather than /adding/ them.
+ -- See (NC1) in Note [Naturally coherent classes]
+ ; if isEqualityClass cls
+ then case sc_cts of
+ [ct] -> startAgainWith ct
+ _ -> pprPanic "canDictCt" (ppr cls)
+ else
+
+ do { emitWork (listToBag sc_cts)
+ ; continueWith (DictCt { di_ev = ev, di_cls = cls
+ , di_tys = tys, di_pend_sc = doNotExpand }) } }
-- doNotExpand: We have already expanded superclasses for /this/ dict
-- so set the fuel to doNotExpand to avoid repeating expansion
@@ -123,7 +134,8 @@ canDictCt ev cls tys
-- of solving it directly from a given.
-- See Note [Overview of implicit CallStacks] in GHC.Tc.Types.Evidence
-- and Note [Solving CallStack constraints] in GHC.Tc.Solver.Types
- = do { -- First we emit a new constraint that will capture the
+ = Stage $
+ do { -- First we emit a new constraint that will capture the
-- given CallStack.
let new_loc = setCtLocOrigin loc (IPOccOrigin (HsIPName ip_name))
-- We change the origin to IPOccOrigin so
@@ -139,18 +151,19 @@ canDictCt ev cls tys
(ctLocSpan loc) (ctEvExpr new_ev)
; solveCallStack ev ev_cs
- ; return (DictCt { di_ev = new_ev, di_cls = cls
- , di_tys = tys, di_pend_sc = doNotExpand }) }
+ ; continueWith (DictCt { di_ev = new_ev, di_cls = cls
+ , di_tys = tys, di_pend_sc = doNotExpand }) }
-- doNotExpand: No superclasses for class CallStack
-- See invariants in CDictCan.cc_pend_sc
| otherwise
- = do { dflags <- getDynFlags
+ = Stage $
+ do { dflags <- getDynFlags
; let fuel | classHasSCs cls = wantedsFuel dflags
| otherwise = doNotExpand
-- See Invariants in `CCDictCan.cc_pend_sc`
- ; return (DictCt { di_ev = ev, di_cls = cls
- , di_tys = tys, di_pend_sc = fuel }) }
+ ; continueWith (DictCt { di_ev = ev, di_cls = cls
+ , di_tys = tys, di_pend_sc = fuel }) }
where
loc = ctEvLoc ev
orig = ctLocOrigin loc
@@ -790,7 +803,7 @@ matchClassInst dflags inerts clas tys loc
-- whether top level, or local quantified constraints.
-- See Note [Instance and Given overlap]
| not (xopt LangExt.IncoherentInstances dflags)
- , not (naturallyCoherentClass clas)
+ , not (naturallyCoherentClass clas) -- See (NC3) in Note [Naturally coherent classes]
, not (noMatchableGivenDicts inerts loc clas tys)
= do { traceTcS "Delaying instance application" $
vcat [ text "Work item=" <+> pprClassPred clas tys ]
@@ -822,8 +835,13 @@ matchClassInst dflags inerts clas tys loc
-- See also Note [The equality types story] in GHC.Builtin.Types.Prim.
naturallyCoherentClass :: Class -> Bool
naturallyCoherentClass cls
- = isCTupleClass cls
- || cls `hasKey` heqTyConKey
+ = isCTupleClass cls || isEqualityClass cls
+
+isEqualityClass :: Class -> Bool
+-- True of (~), (~~), and Coercible
+-- These all have a single primitive-equality superclass, either (~N# or ~R#)
+isEqualityClass cls
+ = cls `hasKey` heqTyConKey
|| cls `hasKey` eqTyConKey
|| cls `hasKey` coercibleTyConKey
@@ -917,14 +935,50 @@ this:
instance a ~# b => a ~~ b
(See Note [The equality types story] in GHC.Builtin.Types.Prim.)
-Faced with [W] t1 ~~ t2, it's always OK to reduce it to [W] t1 ~# t2,
-without worrying about Note [Instance and Given overlap]. Why? Because
-if we had [G] s1 ~~ s2, then we'd get the superclass [G] s1 ~# s2, and
-so the reduction of the [W] constraint does not risk losing any solutions.
+PS: the term "naturally coherent" doesn't really seem helpful.
+Perhaps "invertible" or something? I left it for now though.
-On the other hand, it can be fatal to /fail/ to reduce such
-equalities, on the grounds of Note [Instance and Given overlap],
-because many good things flow from [W] t1 ~# t2.
+For naturally coherent classes:
+
+(NC1) For Givens, when expanding superclasses, we /replace/ the constraint
+ with its superclasses (which, remember, are equally powerful) rather than
+ /adding/ them. This can make a huge difference. Consider T17836, which
+ has a constraint like
+ forall b,c. a ~ (b,c) =>
+ forall d,e. c ~ (d,e) =>
+ ...etc...
+ If we just /add/ the superclasses of [G] g1:a ~ (b,c), we'll put
+ [G] g1:(a~(b,c)) in the inert set and emit [G] g2:a ~# (b,c). That will
+ kick out g1, and it'll be re-inserted as [G] g1':(b,c)~(b,c) which does
+ no good to anyone. When the implication is deeply nested, this has
+ quadratic cost, and no benefit. Just replace!
+
+ Originally I tried this for all naturally-coherent classes, including
+ tuples. But discarding the tuple Given (which "replacing" does) means that
+ we may have to reconstruct it for a recursive call, and the optimiser isn't
+ quite clever enought to figure that out: see #10359 and its test case.
+ This is less pressing for equality classes because they have to be unpacked
+ strictly, so CSE-ing away the reconstuction works fine. Hence the use
+ of isEqualityClass rather than naturallyCoherentClass in canDictCt.
+ A bit ad-hoc.
+
+(NC2) Because of this replacement, we don't need do the fancy footwork
+ of Note [Solving superclass constraints], so the computation of `sc_loc`
+ in `mk_strict_superclasses` can be simpler.
+
+ For tuple predicates, this matters, because their size can be large,
+ and we don't want to add a big class to the size of the dictionaries
+ in the chain. When we get down to a base predicate, we'll include
+ its size. See #10335
+
+(NC3) Faced with [W] t1 ~ t2, it's always OK to reduce it to [W] t1 ~# t2,
+ without worrying about Note [Instance and Given overlap]. Why? Because
+ if we had [G] s1 ~ s2, then we'd get the superclass [G] s1 ~# s2, and
+ so the reduction of the [W] constraint does not risk losing any solutions.
+
+ On the other hand, it can be fatal to /fail/ to reduce such equalities
+ on the grounds of Note [Instance and Given overlap], fbecause many good
+ things flow from [W] t1 ~# t2.
The same reasoning applies to
@@ -947,9 +1001,6 @@ And less obviously to:
Examples: T5853, T10432, T5315, T9222, T2627b, T3028b
-PS: the term "naturally coherent" doesn't really seem helpful.
-Perhaps "invertible" or something? I left it for now though.
-
Note [Local instances and incoherence]
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Consider
@@ -1934,18 +1985,8 @@ mk_strict_superclasses fuel rec_clss ev@(CtGiven { ctev_evar = evar, ctev_loc =
`App` (evId evar `mkVarApps` (tvs ++ dict_ids))
`mkVarApps` sc_tvs
- sc_loc | isCTupleClass cls
- = loc -- For tuple predicates, just take them apart, without
- -- adding their (large) size into the chain. When we
- -- get down to a base predicate, we'll include its size.
- -- #10335
-
- | isEqPredClass cls || cls `hasKey` coercibleTyConKey
- = loc -- The only superclasses of ~, ~~, and Coercible are primitive
- -- equalities, and they don't use the GivenSCOrigin mechanism
- -- detailed in Note [Solving superclass constraints] in
- -- GHC.Tc.TyCl.Instance. Skip for a tiny performance win.
-
+ sc_loc | naturallyCoherentClass cls
+ = loc -- See (NC2) in Note [Naturally coherent classes]
| otherwise
= loc { ctl_origin = mk_sc_origin (ctLocOrigin loc) }
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/c2619514d5418fe21b19ac3ec7ab740175d2b63e
--
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/c2619514d5418fe21b19ac3ec7ab740175d2b63e
You're receiving this email because of your account on gitlab.haskell.org.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.haskell.org/pipermail/ghc-commits/attachments/20230514/55f006ae/attachment-0001.html>
More information about the ghc-commits
mailing list