[Git][ghc/ghc][wip/amg/tweak-co-opt] Address review feedback

[Adam Gundry (@adamgundry)]

Adam Gundry pushed to branch wip/amg/tweak-co-opt at Glasgow Haskell Compiler / GHC


Commits:
6846ec0e by Adam Gundry at 2023-05-09T09:05:33+01:00
Address review feedback

- - - - -


1 changed file:

- compiler/GHC/Core/Coercion/Opt.hs


Changes:

=====================================
compiler/GHC/Core/Coercion/Opt.hs
=====================================
@@ -803,9 +803,11 @@ opt_trans_rule is co1 co2
 -- Push transitivity inside axioms
 opt_trans_rule is co1 co2
 
-  -- See Note [Push transitivity inside axioms]
+  -- See Note [Push transitivity inside axioms] and
+  -- Note [Push transitivity inside newtype axioms only]
   -- TrPushSymAxR
   | Just (sym, con, ind, cos1) <- co1_is_axiom_maybe
+  , isNewTyCon (coAxiomTyCon con)
   , True <- sym
   , Just cos2 <- matchAxiom sym con ind co2
   , let newAxInst = AxiomInstCo con ind (opt_transList is (map mkSymCo cos2) cos1)
@@ -813,6 +815,7 @@ opt_trans_rule is co1 co2
 
   -- TrPushAxR
   | Just (sym, con, ind, cos1) <- co1_is_axiom_maybe
+  , isNewTyCon (coAxiomTyCon con)
   , False <- sym
   , Just cos2 <- matchAxiom sym con ind co2
   , let newAxInst = AxiomInstCo con ind (opt_transList is cos1 cos2)
@@ -820,6 +823,7 @@ opt_trans_rule is co1 co2
 
   -- TrPushSymAxL
   | Just (sym, con, ind, cos2) <- co2_is_axiom_maybe
+  , isNewTyCon (coAxiomTyCon con)
   , True <- sym
   , Just cos1 <- matchAxiom (not sym) con ind co1
   , let newAxInst = AxiomInstCo con ind (opt_transList is cos2 (map mkSymCo cos1))
@@ -827,6 +831,7 @@ opt_trans_rule is co1 co2
 
   -- TrPushAxL
   | Just (sym, con, ind, cos2) <- co2_is_axiom_maybe
+  , isNewTyCon (coAxiomTyCon con)
   , False <- sym
   , Just cos1 <- matchAxiom (not sym) con ind co1
   , let newAxInst = AxiomInstCo con ind (opt_transList is cos1 cos2)
@@ -970,13 +975,13 @@ Not only are there no cancellation opportunities here, but calling matchAxiom
 repeatedly down the transitive chain is very expensive.  Hence we do not attempt
 to push transitivity inside type family axioms.  See #8095, !9210 and related tickets.
 
-This is implemented by isAxiom_maybe checking that the axiom is for a newtype
-constructor (i.e. not a type family).  Adding this single guard substantially
+This is implemented by opt_trans_rule checking that the axiom is for a newtype
+constructor (i.e. not a type family).  Adding these guards substantially
 improved performance (reduced bytes allocated by more than 10%) for the tests
 CoOpt_Singletons, LargeRecord, T12227, T12545, T13386, T15703, T5030, T8095.
 
-A side benefit is that we do not encounter difficulties with confict checking
-for branched axioms; see Note [Why call checkAxInstCo during optimisation].
+A side benefit is that we do not risk accidentally creating an ill-typed
+coercion; see Note [Why call checkAxInstCo during optimisation].
 
 There may exist programs that previously relied on pushing transitivity inside
 type family axioms to avoid creating huge coercions, which will regress in
@@ -1107,7 +1112,6 @@ isAxiom_maybe (SymCo co)
   | Just (sym, con, ind, cos) <- isAxiom_maybe co
   = Just (not sym, con, ind, cos)
 isAxiom_maybe (AxiomInstCo con ind cos)
-  | isNewTyCon (coAxiomTyCon con)  -- See Note [Push transitivity inside newtype axioms only]
   = Just (False, con, ind, cos)
 isAxiom_maybe _ = Nothing
 



View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/6846ec0e2d21011e6119414a5002aab1395cca77

-- 
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/6846ec0e2d21011e6119414a5002aab1395cca77
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/20230512/3fbd4210/attachment-0001.html>