Decouple and refactor DynFlags in CmmToLlvm
What
Adds two modules:
-
GHC.CmmToLlvm.Config-- module that defines theLCGConfigtype (The DynFlags for CmmToLlvm pass, read as Llvm Code Gen Config) -
GHC.Driver.Config.CmmToLlvm-- module that defines the initialization function, i.e.,initLCGConfig :: DynFlags -> LCGConfigthis will be the site of the future handshake - Absorb
LlvmOptsinto new typeLCGConfig
Removes all references to DynFlags in the CmmToLlvm pass, swapping out these calls to a new record LCGConfig which is defined to hold only what CmmToLlvm needs in order to operate.
[Edit]: refactor llvm code generation to ensure GHC.Data.FastString length is calculated at compile time. See this comment and commit 89718b7c
Why
- see issue #20730, specifically this comment
- This MR implements
Goal 1: IsolationforCmmToLlvm, Goal 2 and 3 left to do.
Other things and where I'm unsure
- I've changed a lot of code that was only using
DynFlagsfor error messages viapanicorpprPanic, most of these changes swap lines likeshowSDoc dflags <something>torenderWithContext (lcgContext cfg) <something>, wherelcgContextis defined initialized toinitSDocContext dflags (PprCode CStyle). This is different then what is on master, on mastershowSDocuses a default context:showSDoc dflags sdoc = renderWithContext (initSDocContext dflags defaultUserStyle) sdoc.
So:
- I am not sure if this is a problematic change, please advise
- That
showSDoctakesDynFlagsat all seems quite bad to me. I think that this is actually a driver of the coupling ofDynFlagsto a bunch of code throughout the compiler. Imagine you're writing some code then you hit a point and think "Oh I need to panic here", and then see "oh panic takes a String", and then you lookup how to make yourSDocand then show it withshowSDocand we are off to the races. - The majority of changes are in
CmmToLlvm(obviously) but I'm unsure how to test and verify that these changes are sound (i.e., I haven't introduced bugs). Are there tests that specficially test-fllvm? Or, what is the best way to verify (for example CI fails on Windows, but passes everywhere else, is this expected?)
Todos
Once these todos are closed I'll mark as ready to merge:
-
confirmation that (1) is not problematic or has been addressed -
confirmation that (3) has been addressed and the MR has been sufficiently tested before merge.
[Edit]: Todos closed, see this comment
Edited by jeffrey young