Skip to content

Conversation

BruceForstall
Copy link
Contributor

It's been confusing to know if a configuration switch is available in DEBUG or non-DEBUG (Release) builds because you need to figure out which ifdef section it is defined in.

This simplifies it by eliminating #ifdef DEBUG in jitconfigvalues.h. Each config variable is explicitly using CONFIG_... for DEBUG or RELEASE_CONFIG_... for non-DEBUG builds.

I reformatted some of the CONFIG definitions to put the comments before the CONFIG definition instead of on the same line, so they read better after jit-format processes them.

@ghost ghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Apr 5, 2024
Copy link
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

It's been confusing to know if a configuration switch is available
in DEBUG or non-DEBUG (Release) builds because you need to figure
out which `ifdef` section it is defined in.

This simplifies it by eliminating `#ifdef DEBUG` in jitconfigvalues.h.
Each config variable is explicitly using `CONFIG_...` for DEBUG or
`RELEASE_CONFIG_...` for non-DEBUG builds.

I reformatted some of the CONFIG definitions to put the comments before
the CONFIG definition instead of on the same line, so they read better
after jit-format processes them.
@BruceForstall BruceForstall force-pushed the SimplifyJitConfigValues branch from 6e636e9 to ad378f4 Compare April 5, 2024 20:08
@BruceForstall BruceForstall requested a review from a team April 5, 2024 23:19
@BruceForstall
Copy link
Contributor Author

@dotnet/jit-contrib PTAL

@BruceForstall
Copy link
Contributor Author

No diffs of course

Copy link
Member

@jakobbotsch jakobbotsch left a comment

Choose a reason for hiding this comment

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

Nice, this looks much better.

I was surprised by how many configs we expose in release. Presumably a bunch of those weren't even intended to be exposed but just happened to be by accident of where they were placed.

@BruceForstall BruceForstall merged commit f6f9199 into dotnet:main Apr 9, 2024
@BruceForstall BruceForstall deleted the SimplifyJitConfigValues branch April 9, 2024 00:49
matouskozak pushed a commit to matouskozak/runtime that referenced this pull request Apr 30, 2024
* Simplify jitconfigvalues.h

It's been confusing to know if a configuration switch is available
in DEBUG or non-DEBUG (Release) builds because you need to figure
out which `ifdef` section it is defined in.

This simplifies it by eliminating `#ifdef DEBUG` in jitconfigvalues.h.
Each config variable is explicitly using `CONFIG_...` for DEBUG or
`RELEASE_CONFIG_...` for non-DEBUG builds.

I reformatted some of the CONFIG definitions to put the comments before
the CONFIG definition instead of on the same line, so they read better
after jit-format processes them.

* Fix OPT_CONFIG defines
@github-actions github-actions bot locked and limited conversation to collaborators May 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants