[GHC] #12370: Implement LetUp in DmdAnal (or document why we do not do it)

#12370: Implement LetUp in DmdAnal (or document why we do not do it) -------------------------------------+------------------------------------- Reporter: nomeata | Owner: Type: task | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.1 Keywords: | Operating System: Unknown/Multiple Architecture: | Type of failure: None/Unknown Unknown/Multiple | Test Case: | Blocked By: Blocking: | Related Tickets: Differential Rev(s): | Wiki Page: -------------------------------------+------------------------------------- The paper “Modular, Higher-Order Cardinality Analysis in Theory and Practice” has two rules for analizing let-bindings: LetDown, used for functions, that determines the function’s strictness signature, and passes it down, and LetUp, used for thunks, which uses the demand coming from the scope. The implementation uses only LetDown for all let-bindings. I wonder why this is so. I implemented something that looks like LetUp and will validate and performance-test it soon, in order to either make the implementation match the analysis, or alternatively document why this is the case. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/12370 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#12370: Implement LetUp in DmdAnal (or document why we do not do it)
-------------------------------------+-------------------------------------
Reporter: nomeata | Owner:
Type: task | Status: new
Priority: normal | Milestone:
Component: Compiler | Version: 8.1
Resolution: | Keywords:
Operating System: Unknown/Multiple | Architecture:
| Unknown/Multiple
Type of failure: None/Unknown | Test Case:
Blocked By: | Blocking:
Related Tickets: | Differential Rev(s):
Wiki Page: |
-------------------------------------+-------------------------------------
Comment (by nomeata):
Work in progress in branch `wip/T12370`.
It already works in this simple case:
{{{
foo :: (Int, Int) -> Int
foo (x,y) = x + y
{-# NOINLINE foo #-}
bar n m =
let p = (n,m)
{-# NOINLINE p #-}
in foo p
}}}
Here, bar now gets the very detailed signature
`m` instead of just `

#12370: Implement LetUp in DmdAnal (or document why we do not do it)
-------------------------------------+-------------------------------------
Reporter: nomeata | Owner:
Type: task | Status: new
Priority: normal | Milestone:
Component: Compiler | Version: 8.1
Resolution: | Keywords:
Operating System: Unknown/Multiple | Architecture:
| Unknown/Multiple
Type of failure: None/Unknown | Test Case:
Blocked By: | Blocking:
Related Tickets: | Differential Rev(s):
Wiki Page: |
-------------------------------------+-------------------------------------
Comment (by nomeata):
I have an implementation now, and I think it does what it should.
Performance measurements, though, indicate no significant changes
whatsoever:
https://perf.haskell.org/ghc/#revision/04ded5e3c0bcc359f4b46958b5942b8230af1...
The programs get consistently smaller by 0.02%.
The test suite goes through with the exception of a single test:
{{{
Actual stderr output differs from expected:
--- ./simplCore/should_compile/spec-inline.run/spec-
inline.stderr.normalised 2016-07-06 18:06:21.855289459 +0200
+++ ./simplCore/should_compile/spec-inline.run/spec-
inline.comp.stderr.normalised 2016-07-06 18:06:21.855289459 +0200
@@ -43,7 +43,7 @@
-- RHS size: {terms: 55, types: 9, coercions: 0}
Roman.foo_$s$wgo [Occ=LoopBreaker]
:: GHC.Prim.Int# -> GHC.Prim.Int# -> GHC.Prim.Int#
-[GblId, Arity=2, Caf=NoCafRefs, Str=]
+[GblId, Arity=2, Caf=NoCafRefs, Str=]
Roman.foo_$s$wgo =
/ (sc :: GHC.Prim.Int#) (sc1 :: GHC.Prim.Int#) ->
let {
*** unexpected failure for spec-inline(optasm)
}}}
And this “better” signature does not matter, as the argument is unlifted
anyways.
I will separately measure if this increases the number of thunks
determined to be one-shot. But even if it did, it does not matter much, as
shown by nofib.
The code change (changeset:04ded5e3c0bcc359f4b46958b5942b8230af1b28/ghc)
is relatively small and brings it closer to the paper. Is it worth getting
that in shape for master, or should we not bother?
--
Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/12370#comment:2
GHC http://www.haskell.org/ghc/
The Glasgow Haskell Compiler

#12370: Implement LetUp in DmdAnal (or document why we do not do it) -------------------------------------+------------------------------------- Reporter: nomeata | Owner: Type: task | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by simonpj): That changeset isn't in the repo. Perhaps you meant [https://git.haskell.org/ghc.git/commitdiff/aa472d7bf13bbeb390e857c95c8b92d90... this one]? Yes, that looks like a good change to me; simple and worth doing. Make `isLam` look through casts perhaps. I'd had hoped that we could eliminate the `is_thunk` stuff in `dmdAnalRhs`, which would make the change a bigger win. But for recursive RHSs we might still see a non-lambda in `dmdAnalRhs`. But I'm hazy about the `trimCPR` and `splitFVs` stuff (which is ill-documented) so it may be that for the recursive case we can just do something simpler and more conservative. If we have mutual recursion with a non-lambda, I doubt we are going to get much useful. There must be simple examples where there really is a win; maybe add one as a regression test so we will see if we lose it? For the triv-rhs part, there's a bit of fancy footwork in `dmdAnalRhs` that you don't seem to be doing here... why? Another infelicity in the existing setup. What about `foo = g x` where `g` has arity 2? This RHS is a partial application and morally we should get the same as `foo = \y. g x y`. But I doubt we do. Fixing this might be a small win too. GHC's policy right now is NOT to eta-expand partial applications (I forget why; we could consider revisiting that decision). -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/12370#comment:3 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#12370: Implement LetUp in DmdAnal (or document why we do not do it) -------------------------------------+------------------------------------- Reporter: nomeata | Owner: Type: task | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by nomeata):
That changeset isn't in the repo. Perhaps you meant this one?
Yes; there is no good stable way to refer to changesets on rebasing branches, unfortunately.
Make isLam look through casts perhaps.
As the comment indicates, it mirros the behavior of `collectBinders e`, because if `collectBinders` does not find any binders, then LetDown is not going to perform well. I will expand the comment to explain this reasoning.
There must be simple examples where there really is a win; maybe add one as a regression test so we will see if we lose it?
For the triv-rhs part, there's a bit of fancy footwork in dmdAnalRhs
I will produce an example where we can see a difference in the analysis result. that you don't seem to be doing here... why? I simply don’t handle the case and let it fall through to `dmdAnalRhs`. Will add a note, or refactor. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/12370#comment:4 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#12370: Implement LetUp in DmdAnal (or document why we do not do it) -------------------------------------+------------------------------------- Reporter: nomeata | Owner: Type: task | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by nomeata):
I will separately measure if this increases the number of thunks determined to be one-shot. But even if it did, it does not matter much, as shown by nofib.
Again, only insignificant changes, not worth mentioning. The number of thunks in total goes up by 0.06%, precision of the analysis is now 24.9% instead of 24.8%. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/12370#comment:5 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#12370: Implement LetUp in DmdAnal (or document why we do not do it) -------------------------------------+------------------------------------- Reporter: nomeata | Owner: Type: task | Status: patch Priority: normal | Milestone: Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D2395 Wiki Page: | -------------------------------------+------------------------------------- Changes (by nomeata): * status: new => patch * differential: => Phab:D2395 -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/12370#comment:6 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#12370: Implement LetUp in DmdAnal (or document why we do not do it)
-------------------------------------+-------------------------------------
Reporter: nomeata | Owner:
Type: task | Status: patch
Priority: normal | Milestone:
Component: Compiler | Version: 8.1
Resolution: | Keywords:
Operating System: Unknown/Multiple | Architecture:
| Unknown/Multiple
Type of failure: None/Unknown | Test Case:
Blocked By: | Blocking:
Related Tickets: | Differential Rev(s): Phab:D2395
Wiki Page: |
-------------------------------------+-------------------------------------
Comment (by Joachim Breitner

#12370: Implement LetUp in DmdAnal (or document why we do not do it) -------------------------------------+------------------------------------- Reporter: nomeata | Owner: Type: task | Status: closed Priority: normal | Milestone: Component: Compiler | Version: 8.1 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D2395 Wiki Page: | -------------------------------------+------------------------------------- Changes (by nomeata): * status: patch => closed * resolution: => fixed -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/12370#comment:8 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
participants (1)
-
GHC