Rodrigo Mesquita pushed to branch wip/romes/step-out-11 at Glasgow Haskell Compiler / GHC Commits: addf49bb by Rodrigo Mesquita at 2025-07-17T10:38:59+01:00 cabal-reinstall: Pass --strict to Happy This is necessary to make the generated Parser build successfully This mimics Hadrian, which always passes --strict to happy. Fixes #26202 - - - - - a0aacf65 by Rodrigo Mesquita at 2025-07-17T10:39:04+01:00 genprimopcode: Require higher happy version I've bumped the happy version to forbid deprecated Happy versions which don't successfully compile. - - - - - 85d450f4 by Rodrigo Mesquita at 2025-07-29T18:51:32+01:00 debugger: Uniquely identify breakpoints by internal id Since b85b11994e0130ff2401dd4bbdf52330e0bcf776 (support inlining breakpoints), a breakpoint has been identified at runtime by *two* pairs of <module,index>. - The first, aka a 'BreakpointId', uniquely identifies a breakpoint in the source of a module by using the Tick index. A Tick index can index into ModBreaks.modBreaks_xxx to fetch source-level information about where that tick originated. - When a user specifies e.g. a line breakpoint using :break, we'll reverse engineer what a Tick index for that line - We update the `BreakArray` of that module (got from the LoaderState) at that tick index to `breakOn`. - A BCO we can stop at is headed by a BRK_FUN instruction. This instruction stores in an operand the `tick index` it is associated to. We look it up in the associated `BreakArray` (also an operand) and check wheter it was set to `breakOn`. - The second, aka the `ibi_info_mod` + `ibi_info_ix` of the `InternalBreakpointId`, uniquely index into the `imodBreaks_breakInfo` -- the information we gathered during code generation about the existing breakpoint *ocurrences*. - Note that with optimisation there may be many occurrences of the same source-tick-breakpoint across different modules. The `ibi_info_ix` is unique per occurrence, but the `bi_tick_ix` may be shared. See Note [Breakpoint identifiers] about this. - Note that besides the tick ids, info ids are also stored in `BRK_FUN` so the break handler can refer to the associated `CgBreakInfo`. In light of that, the driving changes come from the desire to have the info_id uniquely identify the breakpoint at runtime, and the source tick id being derived from it: - An InternalBreakpointId should uniquely identify a breakpoint just from the code-generation identifiers of `ibi_info_ix` and `ibi_info_mod`. So we drop `ibi_tick_mod` and `ibi_tick_ix`. - A BRK_FUN instruction need only record the "internal breakpoint id", not the tick-level id. So we drop the tick mod and tick index operands. - A BreakArray should be indexed by InternalBreakpointId rather than BreakpointId That means we need to do some more work when setting a breakpoint. Specifically, we need to figure out the internal ids (occurrences of a breakpoint) from the source-level BreakpointId we want to set the breakpoint at (recall :break refers to breaks at the source level). Besides this change being an improvement to the handling of breakpoints (it's clearer to have a single unique identifier than two competing ones), it unlocks the possibility of generating "internal" breakpoints during Cg (needed for #26042). It should also be easier to introduce multi-threaded-aware `BreakArrays` following this change (needed for #26064). Se also the new Note [ModBreaks vs InternalModBreaks] - - - - - d2813113 by Rodrigo Mesquita at 2025-07-29T18:51:32+01:00 cleanup: Move dehydrateCgBreakInfo to Stg2Bc This no longer has anything to do with Core. - - - - - 71e625fd by Ben Gamari at 2025-07-29T18:51:32+01:00 rts/Interpreter: Factor out ctoi tuple info tables into data Instead of a massive case let's put this into data which we can reuse elsewhere. - - - - - aec0fb68 by Rodrigo Mesquita at 2025-07-29T18:51:32+01:00 rts/Disassembler: Fix spacing of BRK_FUN - - - - - 10c533ca by Rodrigo Mesquita at 2025-07-29T18:51:32+01:00 debugger: Fix bciPtr in Step-out We need to use `BCO_NEXT` to move bciPtr to ix=1, because ix=0 points to the instruction itself! I do not understand how this didn't crash before. - - - - - 5ea0c8ce by Rodrigo Mesquita at 2025-07-29T18:51:32+01:00 debugger: Allow BRK_FUNs to head case continuation BCOs When we start executing a BCO, we may want to yield to the scheduler: this may be triggered by a heap/stack check, context switch, or a breakpoint. To yield, we need to put the stack in a state such that when execution is resumed we are back to where we yielded from. Previously, a BKR_FUN could only head a function BCO because we only knew how to construct a valid stack for yielding from one -- simply add `apply_interp_info` + the BCO to resume executing. This is valid because the stack at the start of run_BCO is headed by that BCO's arguments. However, in case continuation BCOs (as per Note [Case continuation BCOs]), we couldn't easily reconstruct a valid stack that could be resumed because we dropped too soon the stack frames regarding the value returned (stg_ret) and received (stg_ctoi) by that continuation. This is especially tricky because of the variable type and size return frames (e.g. pointer ret_p/ctoi_R1p vs a tuple ret_t/ctoi_t2). The trick to being able to yield from a BRK_FUN at the start of a case cont BCO is to stop removing the ret frame headers eagerly and instead keep them until the BCO starts executing. The new layout at the start of a case cont. BCO is described by the new Note [Stack layout when entering run_BCO]. Now, we keep the ret_* and ctoi_* frames when entering run_BCO. A BRK_FUN is then executed if found, and the stack is yielded as-is with the preserved ret and ctoi frames. Then, a case cont BCO's instructions always SLIDE off the headers of the ret and ctoi frames, in StgToByteCode.doCase, turning a stack like | .... | +---------------+ | fv2 | +---------------+ | fv1 | +---------------+ | BCO | +---------------+ | stg_ctoi_ret_ | +---------------+ | retval | +---------------+ | stg_ret_..... | +---------------+ into | .... | +---------------+ | fv2 | +---------------+ | fv1 | +---------------+ | retval | +---------------+ for the remainder of the BCO. Moreover, this more uniform approach of keeping the ret and ctoi frames means we need less ad-hoc logic concerning the variable size of ret_tuple vs ret_p/np frames in the code generator and interpreter: Always keep the return to cont. stack intact at the start of run_BCO, and the statically generated instructions will take care of adjusting it. Unlocks BRK_FUNs at the start of case cont. BCOs which will enable a better user-facing step-out (#26042) which is free of the bugs the current BRK_ALTS implementation suffers from (namely, using BRK_FUN rather than BRK_ALTS in a case cont. means we'll never accidentally end up in a breakpoint "deeper" than the continuation, because we stop at the case cont itself rather than on the first breakpoint we evaluate after it). - - - - - b568cc20 by Rodrigo Mesquita at 2025-07-29T18:51:33+01:00 Add InternalBreakLocs for code-generation time Brk locations T26042d2 is a simple example displaying how this approach is not good enough e.g. for do blocks because the cases continuations currently end up not surrounded by a tick. TODO: Figure out how to add BRK_FUNs to all case continuations where it is relevant that we can step out to TODO: Test step-out from a continuation which receives an unboxed tuple as an argument TODO: A few comments - - - - - 5184f125 by Rodrigo Mesquita at 2025-07-29T18:51:33+01:00 bytecode: Don't PUSH_L 0; SLIDE 1 1 While looking through bytecode I noticed a quite common unfortunate pattern: ... PUSH_L 0 SLIDE 1 1 We do this often by generically constructing a tail call from a function atom that may be somewhere arbitrary on the stack. However, for the special case that the function can be found directly on top of the stack, as part of the arguments, it's plain redundant to push then slide it. In this commit we add a small optimisation to the generation of tailcalls in bytecode. Simply: lookahead for the function in the stack. If it is the first thing on the stack and it is part of the arguments which would be dropped as we entered the tail call, then don't push then slide it. In a simple example (T26042b), this already produced a drastic improvement in generated code (left is old, right is with this patch): ```diff 3c3 < 2025-07-29 10:14:02.081277 UTC ---
2025-07-29 10:50:36.560949 UTC 160,161c160 < PUSH_L 0 < SLIDE 1 2
SLIDE 1 1
164,165d162 < PUSH_L 0 < SLIDE 1 1 175,176c172 < PUSH_L 0 < SLIDE 1 2 ---
SLIDE 1 1
179,180d174 < PUSH_L 0 < SLIDE 1 1 206,207d199 < PUSH_L 0 < SLIDE 1 1 210,211d201 < PUSH_L 0 < SLIDE 1 1 214,215d203 < PUSH_L 0 < SLIDE 1 1 218,219d205 < PUSH_L 0 < SLIDE 1 1 222,223d207 < PUSH_L 0 < SLIDE 1 1 333,334c317 < PUSH_L 0 < SLIDE 1 2 ---
SLIDE 1 1
337,338d319 < PUSH_L 0 < SLIDE 1 1 367,368c348 < PUSH_L 0 < SLIDE 1 2 ---
SLIDE 1 1
371,372c351 < PUSH_L 0 < SLIDE 1 2 ---
SLIDE 1 1
375,376d353 < PUSH_L 0 < SLIDE 1 1 379,380c356 < PUSH_L 0 < SLIDE 1 2 ---
SLIDE 1 1
383,384c359 < PUSH_L 0 < SLIDE 1 3 ---
SLIDE 1 2
387,388c362 < PUSH_L 0 < SLIDE 1 2 ---
SLIDE 1 1 417,418d390 < PUSH_L 0 < SLIDE 1 1 421,422c393 < PUSH_L 0 < SLIDE 1 4
SLIDE 1 3
442,443c413 < PUSH_L 0 < SLIDE 1 2 ---
SLIDE 1 1
446,447c416 < PUSH_L 0 < SLIDE 1 4 ---
SLIDE 1 3
510,511c479 < PUSH_L 0 < SLIDE 1 2 ---
SLIDE 1 1
514,515d481 < PUSH_L 0 < SLIDE 1 1 600,601c566 < PUSH_L 0 < SLIDE 1 2 ---
SLIDE 1 1
604,605d568 < PUSH_L 0 < SLIDE 1 1 632,633d594 < PUSH_L 0 < SLIDE 1 1 636,637d596 < PUSH_L 0 < SLIDE 1 1 640,641d598 < PUSH_L 0 < SLIDE 1 1 644,645d600 < PUSH_L 0 < SLIDE 1 1 648,649d602 < PUSH_L 0 < SLIDE 1 1 652,653d604 < PUSH_L 0 < SLIDE 1 1 656,657d606 < PUSH_L 0 < SLIDE 1 1 660,661d608 < PUSH_L 0 < SLIDE 1 1 664,665d610 < PUSH_L 0 < SLIDE 1 1 ``` - - - - - 35 changed files: - cabal.project-reinstall - compiler/GHC/ByteCode/Asm.hs - compiler/GHC/ByteCode/Breakpoints.hs - compiler/GHC/ByteCode/Instr.hs - compiler/GHC/ByteCode/Linker.hs - compiler/GHC/ByteCode/Types.hs - compiler/GHC/CoreToIface.hs - compiler/GHC/HsToCore/Breakpoints.hs - compiler/GHC/Linker/Loader.hs - compiler/GHC/Runtime/Debugger/Breakpoints.hs - compiler/GHC/Runtime/Eval.hs - compiler/GHC/Runtime/Interpreter.hs - compiler/GHC/StgToByteCode.hs - ghc/GHCi/UI.hs - ghc/GHCi/UI/Monad.hs - libraries/ghci/GHCi/Debugger.hs - libraries/ghci/GHCi/Message.hs - libraries/ghci/GHCi/Run.hs - rts/Disassembler.c - rts/Exception.cmm - rts/Interpreter.c - rts/Profiling.c - testsuite/tests/count-deps/CountDepsAst.stdout - testsuite/tests/count-deps/CountDepsParser.stdout - testsuite/tests/ghci.debugger/scripts/T26042b.stdout - testsuite/tests/ghci.debugger/scripts/T26042c.script - testsuite/tests/ghci.debugger/scripts/T26042c.stdout - + testsuite/tests/ghci.debugger/scripts/T26042d2.hs - + testsuite/tests/ghci.debugger/scripts/T26042d2.script - + testsuite/tests/ghci.debugger/scripts/T26042d2.stdout - testsuite/tests/ghci.debugger/scripts/T26042e.stdout - testsuite/tests/ghci.debugger/scripts/T26042f2.stdout - testsuite/tests/ghci.debugger/scripts/T26042g.stdout - testsuite/tests/ghci.debugger/scripts/all.T - utils/genprimopcode/genprimopcode.cabal The diff was not included because it is too large. View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/6a546b33d0df3812114266225b249d7... -- View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/6a546b33d0df3812114266225b249d7... You're receiving this email because of your account on gitlab.haskell.org.