[GHC] #7902: Add support for byte endianness swapping exposed as a primops
#7902: Add support for byte endianness swapping exposed as a primops -----------------------------+---------------------------------------------- Reporter: tab | Owner: Type: feature request | Status: new Priority: normal | Component: Compiler Version: 7.7 | Keywords: bswap endianness Os: Unknown/Multiple | Architecture: Unknown/Multiple Failure: None/Unknown | Blockedby: Blocking: | Related: -----------------------------+---------------------------------------------- Add support for W32 and W64 endianness swapping as a new primops (bSwap32# and bSwap64#), supported by code generator change in llvm and ncg to generate efficient code (i.e. X86's bswap instruction support). For other NCG architectures (powerpc, sparc) provide support by 2 ghc-prim C functions. -- Ticket URL: http://hackage.haskell.org/trac/ghc/ticket/7902 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#7902: Add support for byte endianness swapping exposed as a primops -----------------------------+---------------------------------------------- Reporter: tab | Owner: Type: feature request | Status: new Priority: normal | Component: Compiler Version: 7.7 | Keywords: bswap endianness Os: Unknown/Multiple | Architecture: Unknown/Multiple Failure: None/Unknown | Blockedby: Blocking: | Related: -----------------------------+---------------------------------------------- Comment(by tab): At this stage this is for RFC, i've quite possibly missed something at any level. Also I didn't want to refactor the llvm codegen popcnt function to support bswap as this might look more scary in the patch diff, but i'll respin the patch if everyone is happy with the direction and such. -- Ticket URL: http://hackage.haskell.org/trac/ghc/ticket/7902#comment:1 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#7902: Add support for byte endianness swapping exposed as a primops -----------------------------+---------------------------------------------- Reporter: tab | Owner: Type: feature request | Status: patch Priority: normal | Component: Compiler Version: 7.7 | Keywords: bswap endianness Os: Unknown/Multiple | Architecture: Unknown/Multiple Failure: None/Unknown | Blockedby: Blocking: | Related: -----------------------------+---------------------------------------------- Changes (by tab): * status: new => patch -- Ticket URL: http://hackage.haskell.org/trac/ghc/ticket/7902#comment:2 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#7902: Add support for byte endianness swapping exposed as a primops -----------------------------+---------------------------------------------- Reporter: tab | Owner: Type: feature request | Status: patch Priority: normal | Component: Compiler Version: 7.7 | Keywords: bswap endianness Os: Unknown/Multiple | Architecture: Unknown/Multiple Failure: None/Unknown | Blockedby: Blocking: | Related: -----------------------------+---------------------------------------------- Comment(by tibbe): Some comments: * Why is there no 16-bit version? LLVM support it and it can be implemented using the XCHG instruction, like so: `xchg al, ah` * Are you also going to make the changes to base (e.g. in Data.Bits) to expose this to the world? -- Ticket URL: http://hackage.haskell.org/trac/ghc/ticket/7902#comment:3 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#7902: Add support for byte endianness swapping exposed as a primops -----------------------------+---------------------------------------------- Reporter: tab | Owner: Type: feature request | Status: patch Priority: normal | Component: Compiler Version: 7.7 | Keywords: bswap endianness Os: Unknown/Multiple | Architecture: Unknown/Multiple Failure: None/Unknown | Blockedby: Blocking: | Related: -----------------------------+---------------------------------------------- Comment(by tab): * I started originally with a bSwap16# too, but i removed it to focus on the 32 bits version and make sure i'ld plumb it in all the correct place. bSwap16# is not very hard indeed and i can re-add it properly quickly now i'm a bit more familiar with ghc's codegen. * I'm not sure how to expose it to the world, as Bits is quite big and not sure what byte endianess reversing mean for signed int and such. So i'll be happy to expose it somewhere following some discussion and direction on this. -- Ticket URL: http://hackage.haskell.org/trac/ghc/ticket/7902#comment:4 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#7902: Add support for byte endianness swapping exposed as a primops -----------------------------+---------------------------------------------- Reporter: tab | Owner: Type: feature request | Status: patch Priority: normal | Component: Compiler Version: 7.7 | Keywords: bswap endianness Os: Unknown/Multiple | Architecture: Unknown/Multiple Failure: None/Unknown | Blockedby: Blocking: | Related: -----------------------------+---------------------------------------------- Comment(by tab): actually signed fixed sized integer is not a problem at all, it's just the same as Word really. -- Ticket URL: http://hackage.haskell.org/trac/ghc/ticket/7902#comment:5 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#7902: Add support for byte endianness swapping exposed as a primops -----------------------------+---------------------------------------------- Reporter: tab | Owner: Type: feature request | Status: patch Priority: normal | Component: Compiler Version: 7.7 | Keywords: bswap endianness Os: Unknown/Multiple | Architecture: Unknown/Multiple Failure: None/Unknown | Blockedby: Blocking: | Related: -----------------------------+---------------------------------------------- Comment(by tab): i've uploaded new versions of the patch. a quick revlog: * i've added a 16 bits bswap. in NCG, i've implemented it as a 32 bits bswap followed by a shr instead of using a xchg, as the higher 8 bits register are not available as a name in the current codebase, and also that limits the number of possible registers to 4. * corrected a bug with bswap's llvm codegen. Use zero extension (zext) instead of signed extension (sext). Technically popcnt got the same problem, but as the number never reach the highest bits, sext is equivalent to zext in this case. (Is there a better way to contribute to ghc than attaching patches here ? Or is this the best thing to do ?) -- Ticket URL: http://hackage.haskell.org/trac/ghc/ticket/7902#comment:6 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#7902: Add support for byte endianness swapping exposed as a primops ---------------------------------+------------------------------------------ Reporter: tab | Owner: Type: feature request | Status: patch Priority: normal | Milestone: Component: Compiler | Version: 7.7 Keywords: bswap endianness | Os: Unknown/Multiple Architecture: Unknown/Multiple | Failure: None/Unknown Difficulty: Unknown | Testcase: Blockedby: | Blocking: Related: | ---------------------------------+------------------------------------------ Changes (by igloo): * difficulty: => Unknown Comment: Attaching patches for GHC here is best, yes. And providing tests too is even better, thanks! I haven't tried them, but these look OK to me, except that I think it would be worth making a helper function to factor out the common code in the LLVM backend, as you suggested. If you'd like to propose exposing the change via the core libraries, then please see http://www.haskell.org/haskellwiki/Library_submissions for details of the process. I'm not sure how best to do it, though: If it goes into `Bits` then you'll need to give an implementation for `Integer`, and in theory could need to give one for a hypothetical `Word13` type. Even `Int` and `Word` aren't guaranteed to be a multiple of 8 bits. -- Ticket URL: http://hackage.haskell.org/trac/ghc/ticket/7902#comment:7 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#7902: Add support for byte endianness swapping exposed as a primops ---------------------------------+------------------------------------------ Reporter: tab | Owner: Type: feature request | Status: patch Priority: normal | Milestone: Component: Compiler | Version: 7.7 Keywords: bswap endianness | Os: Unknown/Multiple Architecture: Unknown/Multiple | Failure: None/Unknown Difficulty: Unknown | Testcase: Blockedby: | Blocking: Related: | ---------------------------------+------------------------------------------ Comment(by tab): I've refreshed the patches once again (refactor the llvm code gen to remove the duplication, and record it with git commit to have a commitlog) BTW, is this feature request depends on the library submission or are they 2 "independent" proposals ? I haven't considered words not being multiple of 8. I don't think it would make sense to do endian swap on those types and it's harder to make a byteSwap : a -> a. I'm not sure Bits is going to be the best fit to expose any of this. -- Ticket URL: http://hackage.haskell.org/trac/ghc/ticket/7902#comment:8 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#7902: Add support for byte endianness swapping exposed as a primops ---------------------------------+------------------------------------------ Reporter: tab | Owner: Type: feature request | Status: patch Priority: normal | Milestone: Component: Compiler | Version: 7.7 Keywords: bswap endianness | Os: Unknown/Multiple Architecture: Unknown/Multiple | Failure: None/Unknown Difficulty: Unknown | Testcase: Blockedby: | Blocking: Related: | ---------------------------------+------------------------------------------ Comment(by tibbe): Patches look good to me. You can start a discussion on libraries@haskell.org about how to expose these primops through the base package. -- Ticket URL: http://hackage.haskell.org/trac/ghc/ticket/7902#comment:9 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#7902: Add support for byte endianness swapping exposed as a primops ---------------------------------+------------------------------------------ Reporter: tab | Owner: Type: feature request | Status: patch Priority: normal | Milestone: Component: Compiler | Version: 7.7 Keywords: bswap endianness | Os: Unknown/Multiple Architecture: Unknown/Multiple | Failure: None/Unknown Difficulty: Unknown | Testcase: Blockedby: | Blocking: Related: | ---------------------------------+------------------------------------------ Comment(by tab): I've refreshed the compiler bits to also expose a generic bSwap# that works on Word, and made some documentation precision on what the primops do for bSwap16 and bSwap32 as currently it will zero all the upper bits of word#. I've added the bits patch here for reference, and it doesn't looks as bad as i imagined. that's what i'll sent to the list ASAP. -- Ticket URL: http://hackage.haskell.org/trac/ghc/ticket/7902#comment:10 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#7902: Add support for byte endianness swapping exposed as a primops -------------------------------+-------------------------------------------- Reporter: tab | Owner: Type: feature request | Status: closed Priority: normal | Milestone: Component: Compiler | Version: 7.7 Resolution: fixed | Keywords: bswap endianness Os: Unknown/Multiple | Architecture: Unknown/Multiple Failure: None/Unknown | Difficulty: Unknown Testcase: | Blockedby: Blocking: | Related: -------------------------------+-------------------------------------------- Changes (by igloo): * status: patch => closed * resolution: => fixed Comment: Applied, thanks! -- Ticket URL: http://hackage.haskell.org/trac/ghc/ticket/7902#comment:11 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#7902: Add support for byte endianness swapping exposed as a primops -------------------------------+-------------------------------------------- Reporter: tab | Owner: Type: feature request | Status: closed Priority: normal | Milestone: Component: Compiler | Version: 7.7 Resolution: fixed | Keywords: bswap endianness Os: Unknown/Multiple | Architecture: Unknown/Multiple Failure: None/Unknown | Difficulty: Unknown Testcase: | Blockedby: Blocking: | Related: -------------------------------+-------------------------------------------- Comment(by tab): Thanks ! Let me know if this cause any problem, i'll be happy to investigate if it does. -- Ticket URL: http://hackage.haskell.org/trac/ghc/ticket/7902#comment:12 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#7902: Add support for byte endianness swapping exposed as a primops
------------------------------------------+---------------------------------
Reporter: tab | Owner:
Type: feature request | Status: new
Priority: normal | Milestone:
Component: Compiler | Version: 7.7
Resolution: | Keywords: bswap endianness
Os: Unknown/Multiple | Architecture: Unknown/Multiple
Failure: None/Unknown | Difficulty: Unknown
Testcase: codeGen/should_run/cgrun072 | Blockedby:
Blocking: | Related:
------------------------------------------+---------------------------------
Changes (by simonpj):
* status: closed => new
* testcase: => codeGen/should_run/cgrun072
* resolution: fixed =>
Comment:
Urgh. These patches caused #7976. So I have reverted them:
{{{
commit 4aa7fc89fbdbe38d362e59c93fe8ec02185c8073
Author: Simon Peyton Jones
#7902: Add support for byte endianness swapping exposed as a primops ------------------------------------------+--------------------------------- Reporter: tab | Owner: Type: feature request | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.7 Resolution: | Keywords: bswap endianness Os: Unknown/Multiple | Architecture: Unknown/Multiple Failure: None/Unknown | Difficulty: Unknown Testcase: codeGen/should_run/cgrun072 | Blockedby: Blocking: | Related: ------------------------------------------+--------------------------------- Comment(by igloo): See also #7976, but I'm not sure if there's a general 32bit problem or an x86-only problem. -- Ticket URL: http://hackage.haskell.org/trac/ghc/ticket/7902#comment:14 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#7902: Add support for byte endianness swapping exposed as a primops ------------------------------------------+--------------------------------- Reporter: tab | Owner: Type: feature request | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.7 Resolution: | Keywords: bswap endianness Os: Unknown/Multiple | Architecture: Unknown/Multiple Failure: None/Unknown | Difficulty: Unknown Testcase: codeGen/should_run/cgrun072 | Blockedby: Blocking: | Related: ------------------------------------------+--------------------------------- Comment(by tab): ah #7976 is already a bit further than my investigation. So far i came to the same conclusion of the problem being the 64 bits flavor of byteSwap. I think the patch make sense (without considering the overlapping), the 32 bit version for i386 definitely need a specific version for bswap64. I don't think it should be an issue with the 32 bits in general, as ppc and sparc (the other NCG platforms) will be handled with the C functions. -- Ticket URL: http://hackage.haskell.org/trac/ghc/ticket/7902#comment:15 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#7902: Add support for byte endianness swapping exposed as a primops ------------------------------------------+--------------------------------- Reporter: tab | Owner: Type: feature request | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.7 Resolution: | Keywords: bswap endianness Os: Unknown/Multiple | Architecture: Unknown/Multiple Failure: None/Unknown | Difficulty: Unknown Testcase: codeGen/should_run/cgrun072 | Blockedby: Blocking: | Related: ------------------------------------------+--------------------------------- Comment(by tab): Attached is an incremental patch to fix the 32bit x86 build. I've verified the 32 bits works and that 64 bits still build and work too. along with the patch, there's a change of byteSwap64 primops from GenPrimOp to Monadic type. I don't think it change anything, but i believe the latter is the correct one in this case. It's inspired by #7976, and i've verified that there's no overlap going on in registers: * my (limited) understanding of codeGen seem iselExpr64 would create a new register pair all the time. * Empirically generating arbitrary haskell code and checking the disassembled code. -- Ticket URL: http://hackage.haskell.org/trac/ghc/ticket/7902#comment:16 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#7902: Add support for byte endianness swapping exposed as a primops ------------------------------------------+--------------------------------- Reporter: tab | Owner: Type: feature request | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.7 Resolution: | Keywords: bswap endianness Os: Unknown/Multiple | Architecture: Unknown/Multiple Failure: None/Unknown | Difficulty: Unknown Testcase: codeGen/should_run/cgrun072 | Blockedby: Blocking: | Related: ------------------------------------------+--------------------------------- Comment(by simonpj): Does an "incremental" patch mean that it's work in progress for other interested parties to look at; or that it's finished and you think it should be merged? If the latter, what remains to be done, and why not do that first? Thanks for doing this! Simon -- Ticket URL: http://hackage.haskell.org/trac/ghc/ticket/7902#comment:17 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#7902: Add support for byte endianness swapping exposed as a primops ------------------------------------------+--------------------------------- Reporter: tab | Owner: Type: feature request | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.7 Resolution: | Keywords: bswap endianness Os: Unknown/Multiple | Architecture: Unknown/Multiple Failure: None/Unknown | Difficulty: Unknown Testcase: codeGen/should_run/cgrun072 | Blockedby: Blocking: | Related: ------------------------------------------+--------------------------------- Comment(by tab): By incremental, I mean it apply on the previously applied (but now reverted) patches. i can fold the patches together in one if it's better, but I thought it could be useful for everyone to see what I changed. I'm reasonably confident it's mergeable now. -- Ticket URL: http://hackage.haskell.org/trac/ghc/ticket/7902#comment:18 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#7902: Add support for byte endianness swapping exposed as a primops ------------------------------------------+--------------------------------- Reporter: tab | Owner: Type: feature request | Status: patch Priority: normal | Milestone: Component: Compiler | Version: 7.7 Resolution: | Keywords: bswap endianness Os: Unknown/Multiple | Architecture: Unknown/Multiple Failure: None/Unknown | Difficulty: Unknown Testcase: codeGen/should_run/cgrun072 | Blockedby: Blocking: | Related: ------------------------------------------+--------------------------------- Changes (by simonpj): * status: new => patch -- Ticket URL: http://hackage.haskell.org/trac/ghc/ticket/7902#comment:19 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#7902: Add support for byte endianness swapping exposed as a primops ------------------------------------------+--------------------------------- Reporter: tab | Owner: Type: feature request | Status: patch Priority: normal | Milestone: Component: Compiler | Version: 7.7 Resolution: | Keywords: bswap endianness Os: Unknown/Multiple | Architecture: Unknown/Multiple Failure: None/Unknown | Difficulty: Unknown Testcase: codeGen/should_run/cgrun072 | Blockedby: Blocking: | Related: ------------------------------------------+--------------------------------- Comment(by tab): Is there anything I can do for those patches ? more tests ? not prodding, just want to make sure this patchset is not stuck on my input, and just in queue for some future reviewer/committer's time. -- Ticket URL: http://hackage.haskell.org/trac/ghc/ticket/7902#comment:20 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
participants (2)
-
GHC -
GHC