Implement constant references in compiler IL (initially disabled) - #8121
Merged
Conversation
nbhuiyan
requested review from
0xdaryl,
dsouzai,
knn-k,
mstoodle and
vijaysun-omr
as code owners
February 9, 2026 19:25
Contributor
Author
0xdaryl
reviewed
Feb 10, 2026
0xdaryl
reviewed
Feb 12, 2026
|
|
||
| void setLmmdFailed() { _lmmdFailed = true; } | ||
|
|
||
| TR::LabelSymbol *assignConstRefLabel(TR::Node *node); |
Contributor
There was a problem hiding this comment.
Could these functions get some Doxygen comments? In particular, the ones (like this one) that may return NULL instead of a label and the semantics of that.
Contributor
Author
There was a problem hiding this comment.
I have added the doxygen comments in the newly pushed commit. I will squash the last 2 commits after review is complete.
Contributor
|
Jenkins build all I'm just launching a build just to see if it shakes anything out. |
Contributor
|
Jenkins build all |
Contributor
Author
|
Two of the test jobs ended up in failure:
This failure has been present for sometime now, and not introduced by the changes in the PR.
I will now proceed with updating the branch to resolve merge conflicts and to squash the last 2 commits. |
nbhuiyan
force-pushed
the
const-refs-jit-impl
branch
3 times, most recently
from
February 17, 2026 18:05
4308b4b to
9bfce1e
Compare
This commit introduces a representation for non-null reference constants
in the IL. For each known object, there is now a lazily created symbol
reference representing the constant that always refers to that object.
The symref has a known object index in the same way that existing
"improved" symrefs do. It's static, and the IL should load it directly,
i.e. using aload. A constant represented as such a load looks as follows
in tracing:
aload <constant object ref>[eclipse-omr#429 final Static] (obj3)
This new representation is meant to completely supplant the existing
ways of attaching known object information to nodes in the IL, namely
the per-node known object index and the aforementioned "improved"
symrefs. There are two reasons for this change:
1. These reference constants are inherently guaranteed to evaluate to
references to the expected objects, unlike nodes where we've used the
prior techniques, since those left the original operations in place.
This guarantee makes it less error prone to constant-fold loads, and
it makes it possible to do so in more circumstances.
2. These reference constants make the resulting references available
more or less directly. It generally takes only a single load to get
the reference. This improves performance in cases where the reference
was originally coming from a chain of two or more dependent loads
(and where it doesn't become unused before code generation).
The new preprocessor symbol TR_ALLOW_NON_CONST_KNOWN_OBJECTS gates old
functionality that should no longer be used once const refs are enabled.
In particular, if that symbol is not defined, then attempts to get or
set the per-node known object index, and attempts to create an
"improved" version of an existing symref, will cause a build error. This
is useful for making sure that every use of those APIs has alternative
logic implemented to use const refs instead (when enabled). It should
also eventually be useful for eliminating those old APIs. However, it's
expected that there will be builds that need to disable const refs for
some time yet. Indeed, this commit does not even enable const refs by
default.
This commit also implements a cross-platform code generation strategy
for const refs, along with minimal architecture-specific instruction
selection changes. During evaluation, each known object used as a const
ref will be assigned a label, and the generated instructions will use
the label to identify the address from which to load. Later, the
reference slots will be allocated amongst the snippets in the binary
code, in particular between the regular snippets and the data snippets.
Before allocating the slots though, the downstream project needs to add
labels for any necessary keepalives (see RetainedMethodSet), and then it
has an opportunity to specify any arbitrary order for the references to
appear in, as convenient for the downstream implementation. Finally, at
the end of the compilation, the downstream project must arrange for the
slots to be scanned by GC as appropriate, and it must initialize them to
point to their respective objects.
nbhuiyan
force-pushed
the
const-refs-jit-impl
branch
from
February 17, 2026 19:37
9bfce1e to
2583522
Compare
hzongaro
reviewed
Feb 18, 2026
hzongaro
left a comment
hzongaro
left a comment
Contributor
There was a problem hiding this comment.
Just a few very minor comments. I think the changes look, but I did not review the code generation changes.
nbhuiyan
force-pushed
the
const-refs-jit-impl
branch
from
February 19, 2026 17:30
2583522 to
87122fd
Compare
The ifdef guards were specific to a downstream project, and were intended to return NULL in cases where that macro was not defined, as that would imply that const refs support was disabled. However, assignConstRefLabel() would have already returned NULL without the ifdef guards due to the existing useConstRefs() check. In addition, a fatal assert has been added in assignConstRefLabelImpl() to verify that it is not being used with const refs disabled. Also added Doxygen comments for these new functions. Signed-off-by: Nazim Bhuiyan <nubhuiyan@ibm.com>
nbhuiyan
force-pushed
the
const-refs-jit-impl
branch
from
February 19, 2026 18:03
87122fd to
f302433
Compare
Contributor
|
Jenkins build all |
hzongaro
approved these changes
Feb 19, 2026
hzongaro
left a comment
hzongaro
left a comment
Contributor
There was a problem hiding this comment.
The changes look good. Thanks!
0xdaryl
approved these changes
Feb 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR contributes @jdmpapin's const refs implementation in the compiler. Please refer to the commit message for an overview of the changes.
Issue: eclipse-openj9/openj9#16616