Skip to content

Conversation

@SingleAccretion
Copy link
Contributor

@SingleAccretion SingleAccretion commented Jan 29, 2022

Recently, I've compressed all "oper kinds" into an unsigned char, however, that meant all 8 bits were taken, while not all of the kinds are actually needed in Release builds. This change fixes that by introducing another, DEBUG-only, type that serves the same purpose - GenTreeDebugOperKind and frees 2 bits (GTK_EXOP requires a bit more work) in the process. It also introduces a new "kind" for opers that should not appear in HIR - DBK_NOTHIR, counterpart to DBK_NOTLIR.

It also cleans up gtlist.h a little by getting rid of redundant parenthesis and moving opers around to more logical places. This speeds up OperIsIndir by a good margin.

No diffs.

@ghost ghost added community-contribution Indicates that the PR has been added by a community member area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI labels Jan 29, 2022
@ghost
Copy link

ghost commented Jan 29, 2022

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

Issue Details

Recently, I've compressed all "oper kinds" into an unsigned char, however, that meant all 8 bits were taken, while not all of the kinds are actually needed in Release builds. This change fixes that by introducing another, DEBUG-only, type that serves the same purpose - GenTreeDebugOperKind and frees 2 bits (GTK_EXOP requires a bit more work) in the process. It also introduces a new "kind" for opers that should not appear in HIR - DBK_NOTHIR, counterpart to DBK_NOTLIR.

It also cleans up gtlist.h a little by getting rid of redundant parenthesis and moving opers around to more logical places. This speeds up OperIsIndir by a good margin.

Diffs are not expected.

Author: SingleAccretion
Assignees: -
Labels:

area-CodeGen-coreclr, community-contribution

Milestone: -

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would have been possible to make the values here part of GenTreeOperKind, but I think having a separate enum is clearer overall.

Copy link
Contributor Author

@SingleAccretion SingleAccretion Jan 29, 2022

Choose a reason for hiding this comment

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

We also have "early HIR" in the compiler, i. e. HIR before morph: GT_FIELD, GT_PUTARG_TYPE, GT_RET_EXPR, GT_RUNTIMELOOKUP, GT_CNS_STR, GT_FTN_ADDR, GT_INDEX, so something like DBK_EHIR can now be easily added, if people feel like it is valuable (I personally do not see a lot of value).

@SingleAccretion SingleAccretion marked this pull request as ready for review January 30, 2022 00:14
@SingleAccretion
Copy link
Contributor Author

@dotnet/jit-contrib

To track invariants related to opers in asserts
without increasing the size of the primary oper
kind table.

Some shuffling of the oper table to make it look better.
Put all OperIsIdir opers together, fix up formatting,
move opers around to more logical places.
There is not a lot of point in this being a "release"
oper kind, as it is really only useful for debug checks.
@AndyAyersMS
Copy link
Member

Looks good to me, but want to give the rest @dotnet/jit-contrib one last chance to weigh in...

@AndyAyersMS AndyAyersMS merged commit 5422e1f into dotnet:main Feb 11, 2022
@SingleAccretion SingleAccretion deleted the Not-HIR branch February 11, 2022 18:11
@ghost ghost locked as resolved and limited conversation to collaborators Mar 13, 2022
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 community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants