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

Make cast between words and floats real primops #253

Closed
hsyl20 opened this issue Feb 8, 2024 · 6 comments
Closed

Make cast between words and floats real primops #253

hsyl20 opened this issue Feb 8, 2024 · 6 comments
Labels
approved Approved by CLC vote base-4.20 Implemented in base-4.20 (GHC 9.10)

Comments

@hsyl20
Copy link

hsyl20 commented Feb 8, 2024

base:GHC.Float exposes the following functions:

castWord32ToFloat :: Word32 -> Float
castFloatToWord32 :: Float -> Word32
castWord64ToDouble :: Word64 -> Double
castDoubleToWord64 :: Double -> Word64

stgDoubleToWord64 :: Double# -> Word64#
stgWord64ToDouble :: Word64# -> Double#
stgFloatToWord32 :: Float# -> Word32#
stgWord32ToFloat :: Word32# -> Float#

These functions perform bitcast between word and float types.

Issue

While working on bytestring (in bytestring#631) I've noticed that GHC didn't constant-fold application of these functions to constant values. E.g. stgDoubleToWord64 1.0## gives the following Core:

  = case {__ffi_static_ccall_safe base:stg_doubleToWord64zh "stg_doubleToWord64zh" :: Double#
                                                                             -> Word64#}_ii3E
           1.0##
    of ds1_ii3H
    { __DEFAULT ->
    GHC.Word.W64# ds1_ii3H
    }  

Proposal

Replace the current FFI implementation with real primops. This will allow:

  • constant folding support
  • implementing them using inline assembly (e.g. MOVD instruction on x86-64)

What it means for base:

  1. a few more exports from modules that export all primops (GHC.Base, GHC.Exts...)
  2. we could use the opportunity to rename:
stgDoubleToWord64 :: Double# -> Word64#
stgWord64ToDouble :: Word64# -> Double#
stgFloatToWord32 :: Float# -> Word32#
stgWord32ToFloat :: Word32# -> Float#

into (note the trailing #):

castDoubleToWord64# :: Double# -> Word64#
castWord64ToDouble# :: Word64# -> Double#
castFloatToWord32# :: Float# -> Word32#
castWord32ToFloat# :: Word32# -> Float#

The old names would be deprecated for a few releases.

Implementation

@Bodigrim
Copy link
Collaborator

Dear CLC members. Given that the change only cursory touches base and that the linked MR does not contain any breaking changes, I think we can vote as is promptly.

@tomjaguarpaw @mixphix @hasufell @velveteer @angerman @parsonsmatt


+1 from me. I followed Sylvain's work in haskell/bytestring#631 (comment), this patch is a nice optimization indeed.

@tomjaguarpaw
Copy link
Member

+1


Thanks for proposing better new names and keeping around the old ones for compatibility. I think that's the right thing to do.

@Bodigrim Bodigrim added the vote-in-progress The CLC vote is in progress label Feb 11, 2024
@velveteer
Copy link
Contributor

+1

@angerman
Copy link

+1, although I will say that the fact that we need to hold the compilers hand and expose these primops explicitly makes me a bit sad.

@mixphix
Copy link
Collaborator

mixphix commented Feb 13, 2024

+1

@Bodigrim
Copy link
Collaborator

Thanks all, that's enough votes to approve.

@Bodigrim Bodigrim added approved Approved by CLC vote and removed vote-in-progress The CLC vote is in progress labels Feb 13, 2024
@Bodigrim Bodigrim added the base-4.20 Implemented in base-4.20 (GHC 9.10) label Jun 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Approved by CLC vote base-4.20 Implemented in base-4.20 (GHC 9.10)
Projects
None yet
Development

No branches or pull requests

6 participants