diff options
author | Simon Peyton Jones <simonpj@microsoft.com> | 2022-04-12 08:59:19 +0100 |
---|---|---|
committer | Marge Bot <ben+marge-bot@smart-cactus.org> | 2022-04-28 18:57:13 -0400 |
commit | 43bd897dd9fc41265ad5d5ce3eb6cf09b9224f90 (patch) | |
tree | ae73ea62cf6eab1ca16eef37f7c019f7df2d9f96 /compiler/GHC | |
parent | a8c993910ea79264775105a09ad6c80fb52400db (diff) | |
download | haskell-43bd897dd9fc41265ad5d5ce3eb6cf09b9224f90.tar.gz |
Add INLINE pragmas for Enum helper methods
As #21343 showed, we need to be super-certain that the "helper
methods" for Enum instances are actually inlined or specialised.
I also tripped over this when I discovered that numericEnumFromTo
and friends had no pragmas at all, so their performance was very
fragile. If they weren't inlined, all bets were off. So I've added
INLINE pragmas for them too.
See new Note [Inline Enum method helpers] in GHC.Enum.
I also expanded Note [Checking for INLINE loop breakers] in
GHC.Core.Lint to explain why an INLINE function might temporarily
be a loop breaker -- this was the initial bug report in #21343.
Strangely we get a 16% runtime allocation decrease in
perf/should_run/T15185, but only on i386. Since it moves in the right
direction I'm disinclined to investigate, so I'll accept it.
Metric Decrease:
T15185
Diffstat (limited to 'compiler/GHC')
-rw-r--r-- | compiler/GHC/Core/Lint.hs | 60 |
1 files changed, 59 insertions, 1 deletions
diff --git a/compiler/GHC/Core/Lint.hs b/compiler/GHC/Core/Lint.hs index 8ee39cbe88..037940eac2 100644 --- a/compiler/GHC/Core/Lint.hs +++ b/compiler/GHC/Core/Lint.hs @@ -767,7 +767,65 @@ It's very suspicious if a strong loop breaker is marked INLINE. However, the desugarer generates instance methods with INLINE pragmas that form a mutually recursive group. Only after a round of simplification are they unravelled. So we suppress the test for -the desugarer. +the desugarer. Here is an example: + instance Eq T where + t1 == t2 = blah + t1 /= t2 = not (t1 == t2) + {-# INLINE (/=) #-} + +This will generate something like + -- From the class decl for Eq + data Eq a = EqDict (a->a->Bool) (a->a->Bool) + eq_sel :: Eq a -> (a->a->Bool) + eq_sel (EqDict eq _) = eq + + -- From the instance Eq T + $ceq :: T -> T -> Bool + $ceq = blah + + Rec { $dfEqT :: Eq T {-# DFunId #-} + $dfEqT = EqDict $ceq $cnoteq + + $cnoteq :: T -> T -> Bool {-# INLINE #-} + $cnoteq x y = not (eq_sel $dfEqT x y) } + +Notice that + +* `$dfEqT` and `$cnotEq` are mutually recursive. + +* We do not want `$dfEqT` to be the loop breaker: it's a DFunId, and + we want to let it "cancel" with "eq_sel" (see Note [ClassOp/DFun + selection] in GHC.Tc.TyCl.Instance, which it can't do if it's a loop + breaker. + +So we make `$cnoteq` into the loop breaker. That means it can't +inline, despite the INLINE pragma. That's what gives rise to the +warning, which is perfectly appropriate for, say + Rec { {-# INLINE f #-} f = \x -> ...f.... } +We can't inline a recursive function -- it's a loop breaker. + +But now we can optimise `eq_sel $dfEqT` to `$ceq`, so we get + Rec { + $dfEqT :: Eq T {-# DFunId #-} + $dfEqT = EqDict $ceq $cnoteq + + $cnoteq :: T -> T -> Bool {-# INLINE #-} + $cnoteq x y = not ($ceq x y) } + +and now the dependencies of the Rec have gone, and we can split it up to give + NonRec { $dfEqT :: Eq T {-# DFunId #-} + $dfEqT = EqDict $ceq $cnoteq } + + NonRec { $cnoteq :: T -> T -> Bool {-# INLINE #-} + $cnoteq x y = not ($ceq x y) } + +Now $cnoteq is not a loop breaker any more, so the INLINE pragma can +take effect -- the warning turned out to be temporary. + +To stop excessive warnings, this warning for INLINE loop breakers is +switched off when linting the the result of the desugarer. See +lf_check_inline_loop_breakers in GHC.Core.Lint. + Note [Checking for representation polymorphism] ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |