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

Non-exhaustive patterns in DEC #1669

Closed
alex-mckenna opened this issue Feb 16, 2021 · 5 comments · Fixed by #1727
Closed

Non-exhaustive patterns in DEC #1669

alex-mckenna opened this issue Feb 16, 2021 · 5 comments · Fixed by #1727
Assignees
Labels

Comments

@alex-mckenna
Copy link
Contributor

alex-mckenna commented Feb 16, 2021

From the mailing list:

System verilog synthesis with clash 1.2.4 yields this error. No obvious cause or location in the code for synthesis, which compiles and runs fine, and should contain no non-synthesizable constructs, but is of reasonable size (2570 lines, plus pulled in modules). Synthesis usually works for me and has worked for all other modules in this project.

GHC: Parsing and optimising modules took: 2m50.393s
GHC: Loading external modules from interface files took: 0.037s
GHC: Parsing annotations took: 0.111s
Clash: Parsing and compiling primitives took 0.282s
GHC+Clash: Loading modules cumulatively took 4m49.327s
Clash: Compiling KPU.Read.topEntity
: error:
Other error:
src/Clash/Normalize/DEC.hs:385:13-45: Non-exhaustive patterns in Just tupTcNm

That is:

        Just tupTcNm = IM.lookup m tupTcm               <--- here
        Just tupTc   = lookupUniqMap tupTcNm tcm

So I presume lookup has returned Nothing. An unnamed tuple value for synthesis? I would suggest a case statement instead of a pattern match, plus an error message as to what it is looking up in the case of a Nothing result, which may give a clue.

@christiaanb
Copy link
Member

Interesting, that basically means we cannot find the tuple TyCon for that tuple size; but that cache contains the tuple names from 2-tuples to 62-tuples:

tupTcCache = IMS.fromList (zip [2..62] (drop 3 tcNames))

So I guess GHC allows tuples larger than 62 these days (that used to be the max tuple size)

@christiaanb
Copy link
Member

Seems the max tuple size is 64 these days: https://gitlab.haskell.org/ghc/ghc/-/blob/master/compiler/GHC/Settings/Constants.hs#L14

@christiaanb
Copy link
Member

Ah, but that's on recent HEAD, 9.0 is still at 62 as max tuple size: https://gitlab.haskell.org/ghc/ghc/-/blob/ghc-9.0/compiler/GHC/Settings/Constants.hs#L14

@christiaanb
Copy link
Member

Ah, I have a better understanding now. DEC actually groups all the arguments to a "lifted"/"shared" function into a tuple: so if a function is applied to more than 62 arguments we exceed the maximum tuple size. i.e. the original code does not contain a tuple larger than size 62; it's purely a Clash internal thing.

@christiaanb
Copy link
Member

Also the original code doesn't even need to contain a function with more than 62 arguments, it could be that Clash creates one through specialisation. The solution is for DEC to use nested tuples in the same way that GHC does: https://gitlab.haskell.org/ghc/ghc/-/blob/a04179e74bf18837236fea02040438a2c29c8d56/compiler/GHC/Hs/Utils.hs#L620-637

@leonschoorl leonschoorl self-assigned this Feb 23, 2021
leonschoorl added a commit that referenced this issue Mar 26, 2021
All (non-shared) arguments to DEC'ed functions are combined into tuples,
so they can all be defined via a single case expression.
But because GHC's tuples are limited to 62 elements, this fails for arguments with many arguments.

This patch uses GHC's mkChunkified to create (2-levels of) nested tuples for a maximum of 62^2=3844 arguments.

Fixes #1669
leonschoorl added a commit that referenced this issue Mar 26, 2021
All (non-shared) arguments to DEC'ed functions are combined into tuples,
so they can all be defined via a single case expression.
But because GHC's tuples are limited to 62 elements, this fails for arguments with many arguments.

This patch uses GHC's mkChunkified to create (2-levels of) nested tuples for a maximum of 62^2=3844 arguments.

Fixes #1669
leonschoorl added a commit that referenced this issue Mar 26, 2021
All (non-shared) arguments to DEC'ed functions are combined into tuples,
so they can all be defined via a single case expression.
But because GHC's tuples are limited to 62 elements, this fails for functions with many arguments.

This patch uses GHC's mkChunkified to create (2-levels of) nested tuples for a maximum of 62^2=3844 arguments.

Fixes #1669
christiaanb pushed a commit that referenced this issue Mar 27, 2021
All (non-shared) arguments to DEC'ed functions are combined into tuples,
so they can all be defined via a single case expression.
But because GHC's tuples are limited to 62 elements, this fails for functions with many arguments.

This patch uses GHC's mkChunkified to create (2-levels of) nested tuples for a maximum of 62^2=3844 arguments.

Fixes #1669
christiaanb pushed a commit that referenced this issue Mar 27, 2021
All (non-shared) arguments to DEC'ed functions are combined into tuples,
so they can all be defined via a single case expression.
But because GHC's tuples are limited to 62 elements, this fails for functions with many arguments.

This patch uses GHC's mkChunkified to create (2-levels of) nested tuples for a maximum of 62^2=3844 arguments.

Fixes #1669
mergify bot pushed a commit that referenced this issue Mar 27, 2021
All (non-shared) arguments to DEC'ed functions are combined into tuples,
so they can all be defined via a single case expression.
But because GHC's tuples are limited to 62 elements, this fails for functions with many arguments.

This patch uses GHC's mkChunkified to create (2-levels of) nested tuples for a maximum of 62^2=3844 arguments.

Fixes #1669

(cherry picked from commit 28b9a57)
christiaanb pushed a commit that referenced this issue Mar 27, 2021
All (non-shared) arguments to DEC'ed functions are combined into tuples,
so they can all be defined via a single case expression.
But because GHC's tuples are limited to 62 elements, this fails for functions with many arguments.

This patch uses GHC's mkChunkified to create (2-levels of) nested tuples for a maximum of 62^2=3844 arguments.

Fixes #1669

(cherry picked from commit 28b9a57)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants