[GHC] #8996: mark more things as const in C codegen

#8996: mark more things as const in C codegen -------------------------------------------+------------------------------- Reporter: slyfox | Owner: Type: feature request | Status: new Priority: normal | Milestone: Component: Compiler (CodeGen) | Version: 7.8.2 Keywords: | Operating System: Architecture: Unknown/Multiple | Unknown/Multiple Difficulty: Easy (less than 1 hour) | Type of failure: Blocked By: | None/Unknown Related Tickets: | Test Case: | Blocking: -------------------------------------------+------------------------------- Recently i've tried to build ghc-7.8.2 on ia64 and it overflows so called "short data" section. (it's where global 'static' variables live). I've looked at low-hanging fruits of squashing such global "variables". One source of them seems to be a string literal. Consider one-line module: {{{ module B (v) where v = "hello" }}} in '''-fvia-C''' (or unreg) mode it generates code like {{{ static char gibberish_str[] = "hello"; }}} It uselessly eats data section. The patch switches generator to emit: {{{ static const char gibberish_str[] = "hello"; }}} Some notes: 1. as far as I see native codegens already put ot to .rodata, thus it should be safe. 2. I likely didn't cover more cases, like - RelocatableReadOnlyData - and mysterious '''pprTop (CmmData _section (Statics lbl [CmmUninitialised size]))''' Size of unreg-stage2 on amd64 before and after the patch: {{{ $ size inplace/lib/bin/ghc-stage2 (unpatched) text data bss dec hex filename 81986382 20776344 44096 102806822 620b526 inplace/lib/bin/ghc-stage2 $ size inplace/lib/bin/ghc-stage2 (patched) text data bss dec hex filename 83648494 19062936 44096 102755526 61fecc6 inplace/lib/bin/ghc-stage2 }}} Text section increased for 1.6MBs (consts moved here, likely .rodata actually), data section decreased for 1.7MBs. I think with minor fiddling with linker flags we can merge equal constants later on. Thanks! -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8996 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#8996: mark more things as const in C codegen -------------------------------------+------------------------------------- Reporter: slyfox | Owner: Type: feature request | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.8.2 (CodeGen) | Keywords: Resolution: | Architecture: Unknown/Multiple Operating System: Unknown/Multiple | Difficulty: Easy (less than 1 Type of failure: None/Unknown | hour) Test Case: | Blocked By: Blocking: | Related Tickets: -------------------------------------+------------------------------------- Comment (by ezyang): I think the intent is good. However, I'd probably go about doing it slightly differently: we want this to apply to all read-only data (so `ReadOnlyData`, `RelocatableReadOnlyData` and `ReadOnlyData16`), so it sounds like what you should do is go through all the PprC branches looking for places where a 'const' could be inserted, and then check if the relevant section is readonly. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8996#comment:1 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#8996: mark more things as const in C codegen -------------------------------------+------------------------------------- Reporter: slyfox | Owner: Type: feature request | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.8.2 (CodeGen) | Keywords: Resolution: | Architecture: Unknown/Multiple Operating System: Unknown/Multiple | Difficulty: Easy (less than 1 Type of failure: None/Unknown | hour) Test Case: | Blocked By: Blocking: | Related Tickets: -------------------------------------+------------------------------------- Comment (by slyfox): I think something, like use cases of {{{ pprLocalness :: CLabel -> SDoc pprLocalness lbl | not $ externallyVisibleCLabel lbl = ptext (sLit "static ") | otherwise = empty }}} might be very close to 'const' case. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8996#comment:2 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#8996: mark more things as const in C codegen ---------------------------------------+----------------------------------- Reporter: slyfox | Owner: Type: feature request | Status: new Priority: normal | Milestone: Component: Compiler (CodeGen) | Version: 7.8.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: None/Unknown | Unknown/Multiple Test Case: | Difficulty: Unknown Blocking: | Blocked By: | Related Tickets: ---------------------------------------+----------------------------------- Changes (by slyfox): * difficulty: Easy (less than 1 hour) => Unknown Comment: This patch attempts to mark everything ghc knows as RODATA, what raises a couple of questions: Patch can be tested as './configure --enable-unregisterised' in amd64. Example of build failure: {{{ HC [stage 1] libraries/ghc-prim/dist-install/build/GHC/Types.o In file included from /tmp/ghc26762_0/ghc26762_1.hc:3:0: /tmp/ghc26762_0/ghc26762_1.hc:295:6: error: conflicting types for 'ghczmprim_GHCziTypes_False_closure' CEI_(ghczmprim_GHCziTypes_False_closure); ^ /home/slyfox/dev/git/ghc/includes/Stg.h:213:52: note: in definition of macro 'CEI_' #define CEI_(X) extern const StgWordArray (X) GNU_ATTRIBUTE(aligned (8)) ^ /tmp/ghc26762_0/ghc26762_1.hc:27:9: note: previous definition of 'ghczmprim_GHCziTypes_False_closure' was here StgWord ghczmprim_GHCziTypes_False_closure[] = { ^ }}} GHC thinks that extern points to rodata, but actually place or to .data (rwdata). I think it's due to '''rts/StgMiscClosures.cmm''': {{{ /* put these in the *data* section, since the garbage collector relies * on the fact that static closures live in the data section. */ #if !(defined(COMPILING_WINDOWS_DLL)) section "data" { stg_CHARLIKE_closure: CHARLIKE_HDR(0) CHARLIKE_HDR(1) CHARLIKE_HDR(2) CHARLIKE_HDR(3) ... }}} Is it an absolute need to use mutable data section for it? Could GC be tuned to scan for certain rodata block? Or if it's not an option, fix ghc to know that those closure externs are really in "data" section. Thanks! -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8996#comment:3 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#8996: mark more things as const in C codegen ---------------------------------------+----------------------------------- Reporter: slyfox | Owner: Type: feature request | Status: new Priority: normal | Milestone: Component: Compiler (CodeGen) | Version: 7.8.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: None/Unknown | Unknown/Multiple Test Case: | Difficulty: Unknown Blocking: | Blocked By: | Related Tickets: ---------------------------------------+----------------------------------- Comment (by ezyang): The comment dates back to GHC 4.01, so it might be wrong; I can't think of any reason for the GC to be doing any scribbling here (Chars and Ints are NOCAF, so we don't need to link them together on the static_objects list; ditto for all of the other similar miscellaneous closures.). Maybe Simon knows better. In any case, it will be a moot point when is #8199 fixed, after which "static closures" absolutely must be writable since we need to update them to indirect onto blocks we've allocated. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8996#comment:4 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#8996: mark more things as const in C codegen ---------------------------------------+----------------------------------- Reporter: slyfox | Owner: Type: feature request | Status: new Priority: normal | Milestone: Component: Compiler (CodeGen) | Version: 7.8.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: None/Unknown | Unknown/Multiple Test Case: | Difficulty: Unknown Blocking: | Blocked By: | Related Tickets: ---------------------------------------+----------------------------------- Comment (by simonmar): I think the comment dates back to when we used to figure out what was static data by knowing the boundaries of sections. We haven't done that for a long time, so the comment is out of date. Let's kill it! -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8996#comment:5 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#8996: mark more things as const in C codegen -------------------------------------+------------------------------------- Reporter: slyfox | Owner: (none) Type: feature request | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.8.2 (CodeGen) | Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3481 Wiki Page: | -------------------------------------+------------------------------------- Changes (by slyfox): * differential: => Phab:D3481 Comment: Three years later I now have a vague idea how things fit togeter now :) Sent https://phabricator.haskell.org/D3481 for review. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8996#comment:6 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#8996: mark more things as const in C codegen
-------------------------------------+-------------------------------------
Reporter: slyfox | Owner: (none)
Type: feature request | Status: new
Priority: normal | Milestone:
Component: Compiler | Version: 7.8.2
(CodeGen) |
Resolution: | Keywords:
Operating System: Unknown/Multiple | Architecture:
| Unknown/Multiple
Type of failure: None/Unknown | Test Case:
Blocked By: | Blocking:
Related Tickets: | Differential Rev(s): Phab:D3481
Wiki Page: |
-------------------------------------+-------------------------------------
Comment (by Ben Gamari

#8996: mark more things as const in C codegen -------------------------------------+------------------------------------- Reporter: slyfox | Owner: (none) Type: feature request | Status: closed Priority: normal | Milestone: 8.4.1 Component: Compiler | Version: 7.8.2 (CodeGen) | Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3481 Wiki Page: | -------------------------------------+------------------------------------- Changes (by bgamari): * status: new => closed * resolution: => fixed * milestone: => 8.4.1 -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8996#comment:8 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#8996: mark more things as const in C codegen -------------------------------------+------------------------------------- Reporter: slyfox | Owner: (none) Type: feature request | Status: closed Priority: normal | Milestone: 8.4.1 Component: Compiler | Version: 7.8.2 (CodeGen) | Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3481 Wiki Page: | -------------------------------------+------------------------------------- Comment (by bgamari): trofi, feel free to reopen if you know of anything else that hasn't yet been marked as const. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8996#comment:9 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
participants (1)
-
GHC