Allow packing constructor fields
authorMichal Terepeta <michal.terepeta@gmail.com>
Mon, 30 Oct 2017 00:49:32 +0000 (20:49 -0400)
committerBen Gamari <ben@smart-cactus.org>
Mon, 30 Oct 2017 01:51:05 +0000 (21:51 -0400)
commitcca2d6b78f97bfb79bef4dc3f75d6c4d15b94680
tree9be80ec91082ad99ba79d21a6cd0aac68309a236
parent85aa1f4253163985fe07d172f8da73b784bb7b4b
Allow packing constructor fields

This is another step for fixing #13825 and is based on D38 by Simon
Marlow.

The change allows storing multiple constructor fields within the same
word. This currently applies only to `Float`s, e.g.,
```
data Foo = Foo {-# UNPACK #-} !Float {-# UNPACK #-} !Float
```
on 64-bit arch, will now store both fields within the same constructor
word. For `WordX/IntX` we'll need to introduce new primop types.

Main changes:

- We now use sizes in bytes when we compute the offsets for
  constructor fields in `StgCmmLayout` and introduce padding if
  necessary (word-sized fields are still word-aligned)

- `ByteCodeGen` had to be updated to correctly construct the data
  types. This required some new bytecode instructions to allow pushing
  things that are not full words onto the stack (and updating
  `Interpreter.c`). Note that we only use the packed stuff when
  constructing data types (i.e., for `PACK`), in all other cases the
  behavior should not change.

- `RtClosureInspect` was changed to handle the new layout when
  extracting subterms. This seems to be used by things like `:print`.
  I've also added a test for this.

- I deviated slightly from Simon's approach and use `PrimRep` instead
  of `ArgRep` for computing the size of fields.  This seemed more
  natural and in the future we'll probably want to introduce new
  primitive types (e.g., `Int8#`) and `PrimRep` seems like a better
  place to do that (where we already have `Int64Rep` for example).
  `ArgRep` on the other hand seems to be more focused on calling
  functions.

Signed-off-by: Michal Terepeta <michal.terepeta@gmail.com>
Test Plan: ./validate

Reviewers: bgamari, simonmar, austin, hvr, goldfire, erikd

Reviewed By: bgamari

Subscribers: maoe, rwbarton, thomie

GHC Trac Issues: #13825

Differential Revision: https://phabricator.haskell.org/D3809
30 files changed:
compiler/cmm/CmmCallConv.hs
compiler/cmm/SMRep.hs
compiler/codeGen/StgCmmBind.hs
compiler/codeGen/StgCmmCon.hs
compiler/codeGen/StgCmmHeap.hs
compiler/codeGen/StgCmmLayout.hs
compiler/coreSyn/CoreLint.hs
compiler/ghci/ByteCodeAsm.hs
compiler/ghci/ByteCodeGen.hs
compiler/ghci/ByteCodeInstr.hs
compiler/ghci/RtClosureInspect.hs
compiler/main/Constants.hs
compiler/types/TyCon.hs
includes/rts/Bytecodes.h
includes/stg/Types.h
rts/Disassembler.c
rts/Interpreter.c
testsuite/tests/codeGen/should_run/T13825-unit.hs [new file with mode: 0644]
testsuite/tests/codeGen/should_run/all.T
testsuite/tests/ghci.debugger/scripts/T13825-debugger.hs [new file with mode: 0644]
testsuite/tests/ghci.debugger/scripts/T13825-debugger.script [new file with mode: 0644]
testsuite/tests/ghci.debugger/scripts/T13825-debugger.stdout [new file with mode: 0644]
testsuite/tests/ghci.debugger/scripts/all.T
testsuite/tests/ghci/should_run/T13825-ghci.hs [new file with mode: 0644]
testsuite/tests/ghci/should_run/T13825-ghci.script [new file with mode: 0644]
testsuite/tests/ghci/should_run/T13825-ghci.stdout [new file with mode: 0644]
testsuite/tests/ghci/should_run/all.T
testsuite/tests/primops/should_run/T13825-compile.hs [new file with mode: 0644]
testsuite/tests/primops/should_run/T13825-compile.stdout [new file with mode: 0644]
testsuite/tests/primops/should_run/all.T