-
Notifications
You must be signed in to change notification settings - Fork 102
Add disjoint
#559
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
Add disjoint
#559
Conversation
...to improve the readability of the generated code.
| disjointSubtrees _s _a Empty = True | ||
| disjointSubtrees s a (Leaf hB (L kB _)) = | ||
| lookupCont (\_ -> True) (\_ _ -> False) hB kB s a | ||
| disjointSubtrees s a b@Collision{} = disjointSubtrees s b a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately GHC fails to specialize this and we end up with stuff like
disjoint_$s$wdisjointSubtrees @a @b @Int bx bx1 ww $fEqInt wild
or
disjoint_$s$wdisjointSubtrees @a @b @String bx bx1 ww $fEqList_$s$fEqList1 wild
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I misunderstood.
This function seems to be the result of case liberation (EDIT: it's not, it's due to -fspec-constr):
disjoint_$s$wdisjointSubtrees [Occ=LoopBreaker]
:: forall b a k.
Word#
-> SmallArray# (Leaf k a) -> Int# -> Eq k => HashMap k b -> Bool
and I suspect the specialization failure is related to the Eq dict being tucked in the middle of the other parameters instead of being first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can replicate the issue with -O1 -fspec-constr.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have made a GHC issue: https://gitlab.haskell.org/ghc/ghc/-/issues/26615
| -- Note that as of GHC 9.12, SpecConstr creates a specialized worker for | ||
| -- handling the Collision vs. {BitmapIndexed,Full} and vice-versa cases, | ||
| -- but this worker fails to be properly specialized for different key | ||
| -- types. See https://gitlab.haskell.org/ghc/ghc/-/issues/26615. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we perform constructor specialization manually to work around this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't mind if you or someone else wants to work on this, but I won't.
No description provided.