Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Investigate suboptimal code generated by AsData #6770

Open
zliu41 opened this issue Jan 2, 2025 · 4 comments
Open

Investigate suboptimal code generated by AsData #6770

zliu41 opened this issue Jan 2, 2025 · 4 comments

Comments

@zliu41
Copy link
Member

zliu41 commented Jan 2, 2025

I found two issues when looking at the generated code in patternMatching.pir.golden

  1. x, y, z, w are all non-strict bindings. This causes their bodies to be evaluated every time they are referenced, which is why patternMatching is slower than recordFields. If CSE is turned on, then patternMatching does become much cheaper, because force x, force y etc. are considered common subexpressions and are factored out. However, it is not great to have to rely on CSE, which is unreliable. Can we generate more strict code from AsData?
  2. The use of Tuple4 also has some overhead. Is it possible to generate code that doesn't use tuples? Ideally it should look like this:
!tup = unConstrData scrut
!list = sndPair tup
!x = unIData (headList list)
!list' = tailList list
!y = unIData (headList list')
!list'' = tailList list'
!z = unIData (headList list'')
!list''' = tailList list''
!w = unIData (headList list''')
@github-actions github-actions bot added the status: needs triage GH issues that requires triage label Jan 2, 2025
@zliu41 zliu41 added Internal and removed status: needs triage GH issues that requires triage labels Jan 2, 2025
@zliu41
Copy link
Member Author

zliu41 commented Jan 2, 2025

@ana-pantilie this is the performance issue I talked to you about earlier.

@zliu41
Copy link
Member Author

zliu41 commented Jan 17, 2025

@ana-pantilie I looked a bit into this, and wrote a more efficient pattern synonym by hand: see IntsManualPattern in b31eab9.

To answer the two questions above:

  1. I don't think there's an easy way to make x etc. strict, since that's just how GHC handles pattern synonyms - arguably it is a GHC bug. So, we should advise users to strictify them by binding each of them to another strict variable - see patternMatchingManual.
  2. I also don't think there's an easy way to avoid the Tuple4 - as you can see, I had to use a 4-tuple even in my hand written pattern synonym.

That said, the hand written pattern synonym is more efficient than the TH generate one, and we should modify the TH to generate the more efficient version.

@zliu41
Copy link
Member Author

zliu41 commented Jan 19, 2025

I opened a GHC issue regarding 1: https://gitlab.haskell.org/ghc/ghc/-/issues/25666

@zliu41
Copy link
Member Author

zliu41 commented Jan 19, 2025

So, we should advise users to strictify them by binding each of them to another strict variable - see patternMatchingManual.

There's actually a much simpler solution: use case ... of IntsManualPattern x y z w instead of let (IntsManualPattern x y z w) = .... The latter somehow doesn't work, even with bang patterns all over the place (let !(IntsManualPattern !x !y !z !w) = ...).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants