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

Fail to buid against GHCJS(or maybe other 32bit platform) #80

Closed
yuchangyuan opened this issue Feb 13, 2020 · 9 comments · Fixed by #82
Closed

Fail to buid against GHCJS(or maybe other 32bit platform) #80

yuchangyuan opened this issue Feb 13, 2020 · 9 comments · Fixed by #82
Labels

Comments

@yuchangyuan
Copy link

Build fail with ghcjs-8.6.0.1, due to type mismatch. ltWord# & eqWord# accept Word# type, but Word# type is not always Word64#.

Below is part of of build message:

[1 of 3] Compiling Data.TypeRepMap.Internal ( src/Data/TypeRepMap/Internal.hs, dist/build/Data/TypeRepMap/Internal.js_o )

src/Data/TypeRepMap/Internal.hs:328:69: error:
    • Couldn't match expected type ‘GHC.Prim.Word#’
                  with actual type ‘GHC.Prim.Word64#’
    • In the first argument of ‘ltWord#’, namely ‘a’
      In the expression: a `ltWord#` valA
      In the expression:
        case a `ltWord#` valA of
          0#
            -> case a `eqWord#` valA of
                 0# -> go (2# *# i +# 2#)
                 _ -> let ... in ...
          _ -> go (2# *# i +# 1#)
    |

@chshersh
Copy link
Contributor

@yuchangyuan Thanks for opening the issue! It's an interesting problem 🤔 We work with Word64 because it's stored inside Fingerprint and Word64 is a wrapper around Word#. Is it correct that when the code is built with GHCJS, it's actually Word64# inside? I wonder, what can be done here to support GHCJS painlessly 🤔

@yuchangyuan
Copy link
Author

I am not familiar with types inside GHC.Prim, but Word in GHCJS is actual 32bit, this can be verifed by below code:

module Main where

import Data.Word

main = do
  print (maxBound :: Word)
  print (maxBound :: Word32)
  print (maxBound :: Word64)

with regular ghc(on 64bit machine), output is

18446744073709551615
4294967295
18446744073709551615

with ghcjs (The Glorious Glasgow Haskell Compilation System for JavaScript, version 8.6.0.1 (GHC 8.6.4)), output is:

4294967295
4294967295
18446744073709551615

@angerman
Copy link

I've run into this as well today. This isn't necessarily about GHCJS only, it's about any 32bit compiler. From the link @chshersh posted:

------------------------------------------------------------------------
-- type Word64
------------------------------------------------------------------------

#if WORD_SIZE_IN_BITS < 64

data {-# CTYPE "HsWord64" #-} Word64 = W64# Word64#
-- ^ 64-bit unsigned integer type
(...)
#else

-- Word64 is represented in the same way as Word.
-- Operations may assume and must ensure that it holds only values
-- from its logical range.

data {-# CTYPE "HsWord64" #-} Word64 = W64# Word#
-- ^ 64-bit unsigned integer type
(...)
#endif

Wouldn't using explicit 64bit ops do?

@angerman
Copy link

@chshersh what's the reason for not using Ord and Eq? Performance? The code seems to be littered with inline pragmas, I would--from a cursory look--have expected that to be optimised anyway? The Ord and Eq instance seem to be implemented for 32 and 64bit handling properly.

@vrom911
Copy link
Member

vrom911 commented Mar 27, 2020

@angerman This "litter" gave us a significant boost in performance. Performance is the primary goal of the project, and it was not released until we achieved acceptable numbers.

You can make a PR to our "littered code" with your suggestions, and we will accept that if the performance is not affected.

@angerman
Copy link

@vrom911 I'm sorry, there has been a misunderstanding. I wrote "The code", and should have written "That code" to make it clear that I did not mean "This code". Specifically I did not mean this repository. I meant this. The Eq and Ord instances for Word are accompanied with INLINE pragmas, hence the question for the use of unboxing. Again the implied assumption was that this was probably done for performance.

The key being that for 32bit platforms we'd need

#include "MachDeps.h"
#if WORD_SIZE_IN_BITS < 64
import GHC.IntWord64
#endif

to be able to use eqWord64# and ltWord64# across 32bit and 64bit architectures.

@vrom911
Copy link
Member

vrom911 commented Mar 28, 2020

Thank for the clarification, @angerman , there was a misunderstanding indeed.

A PR is still welcome 🙂

@Ericson2314
Copy link

I wrote https://gitlab.haskell.org/ghc/ghc/-/merge_requests/1102 so we could avoid the CPP workaround #82, but there were test failures I did not know how to fix. I was hoping to figure it out at GHC Week before Covid-19 killed that off. If anyone here feels motivated to give it a shot, please do!

@chshersh
Copy link
Contributor

typerep-map-0.3.3.0, that builds on 32-bit platforms, has been released to Hackage. Moreover, we now build typerep-map on CI with both 32-bit and 64-bit platforms to make sure this is true.

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.

5 participants