Skip to content

[minor] a -dmatchcomp for matching compiler debug output - #13425

Merged
gasche merged 3 commits into
ocaml:trunkfrom
gasche:dmatchcomp
Sep 9, 2024
Merged

gasche merged 3 commits into
ocaml:trunkfrom
gasche:dmatchcomp

Conversation

@gasche

@gasche gasche commented Sep 9, 2024

Copy link
Copy Markdown
Member

In the course of working on the Pattern-Matching Bug, I frequently needed to enable the matching compiler debug output to understand what was going on. Currently the way to do this is not very nice, one has to modify lambda/matching.ml to turn let dbg = false into let dbg = true, and the resulting compiler produces a ton of debug output when building itself.

This PR adds a -dmatchcomp option to show the matching compilation debug output. This will make the pattern-matching compiler easier to observe and debug. The change is minimal, designed to be useful for compiler hacking only.

Note: -dmatchcomp does not behave like most other -dfoo options,
because it shows course-of-compilation debug output instead of
printing an intermediate representation of a compilation pass -- there
are no separate intermediate representations for pattern-matching
compilation, unfortunately.

In particular, with the current implementation, -dmatchcomp does not
respect the -dump-into-* parameters, it always print to debug
output. Fixing this would be nice, but it requires cumbersome
implementation work, because the matching.ml codebase is not
parametrized over a debug channel.

If someone wanted to tackle this, I would suggest the following route:

  • refactor Translcore to turn the ~scopes parameter into a more
    generic ~ctx parameter, a record with a scopesfield
  • same thing with Translmod
  • add a ppf_dump field to this compilation context
  • add a ctx parameter (at a different type) to all Matching
    functions as well, and pass the ppf_dump alongside it
  • pass the context to the debugf calls

This is left outside the scope of the current PR.

Note: -dmatchcomp does not behave like most other -dfoo options,
because it shows course-of-compilation debug output instead of
printing an intermediate representation of a compilation pass -- there
are no separate intermediate representations for pattern-matching
compilation, unfortunately.

In particular, with the current implementation, -dmatchcomp does not
respect the -dump-into-* parameters, it always print to debug
output. Fixing this would be nice, but it requires cumbersome
implementation work, because the matching.ml codebase is not
parametrized over a debug channel.

If someone wanted to tackle this, we would suggest the following route:

- refactor Translcore to turn the ~scopes parameter into a more
generic ~ctx parameter, a record with a `scopes`field
- same thing with Translmod
- add a `ppf_dump` field to this compilation context
- add a `ctx` parameter (at a different type) to all Matching
  functions as well, and pass the `ppf_dump` alongside it
- pass the context to the `debugf` calls
@gasche

gasche commented Sep 9, 2024

Copy link
Copy Markdown
Member Author

Ah, also: the debug output looks like garbage, and in particular it uses Format but seems to suffer from some pretty-printing bugs, where things are not shown as intended. This is fine: having this semi-broken output is much better than not having it. I try to fix the formatting sometimes, and usually give up because I don't know how to do it and it's not worth the time.

@lthls lthls left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with this PR.
In my experience it can also be useful to add support for passing this kind of flag through the OCAMLPARAM variable; it allows for debugging programs with complex build systems without patching this system (I have used this feature on the testsuite in a few cases).
The code to modify is in the file driver/compenv.ml, if you agree it's worth doing here.

@gasche

gasche commented Sep 9, 2024

Copy link
Copy Markdown
Member Author

@lthls There is already dump_flambda_verbose there, as a standalone flambda-verbose boolean setting, and no other dump option. Do you think we should add each dump option as its own setting, or rather support something like dump=<pass> and dump=-<pass> to enable or disable dumping of specific "passes"?

@nojb nojb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lthls

lthls commented Sep 9, 2024

Copy link
Copy Markdown
Contributor

@lthls There is already dump_flambda_verbose there, as a standalone flambda-verbose boolean setting, and no other dump option. Do you think we should add each dump option as its own setting, or rather support something like dump=<pass> and dump=-<pass> to enable or disable dumping of specific "passes"?

I would be fine with either solution. If I implemented it, I would probably go with the first solution.

If you end up implementing OCAMLPARAM support for all (or most) -dfoo flags, you might want to first merge this PR as it is now and propose your implementation as a new PR.

@gasche

gasche commented Sep 9, 2024

Copy link
Copy Markdown
Member Author

I propose the dump=<option> support in OCAMLPARAM in #13428.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants