Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AArch64: Add extraRegister to OMRMemoryReference #4225

Merged
merged 1 commit into from
Nov 26, 2019

Conversation

knn-k
Copy link
Contributor

@knn-k knn-k commented Aug 26, 2019

This commit adds extraRegister and related code to
OMRMemoryReference for AArch64.
It will be used by UnresolvedSnippet.

Signed-off-by: KONNO Kazuhiro konno@jp.ibm.com

@knn-k
Copy link
Contributor Author

knn-k commented Aug 26, 2019

This is a revisit of #3953.
UnresolvedDataSnippet requires the introduction of modBase register.

@knn-k knn-k changed the title AArch64: Add modBase register to OMRMemoryReference WIP: AArch64: Add modBase register to OMRMemoryReference Aug 26, 2019
@0xdaryl 0xdaryl self-assigned this Aug 29, 2019
@knn-k
Copy link
Contributor Author

knn-k commented Oct 29, 2019

Cancelling this PR.
I am adding the modBase register to J9MemoryReference now. No need to introduce J9_PROJECT_SPECIFIC code to OMRMemoryReference.

@knn-k
Copy link
Contributor Author

knn-k commented Nov 5, 2019

Reopened this PR.

I have to add the _modBase member to OMR::ARM64::MemoryReference instead of J9::ARM64::MemoryReference because:

  • Two constructors of OMR::ARM64::MemoryReference call addToOffset().
  • addToOffset() is defined in the OMR side. It calls decNodeReferenceCounts().
  • decNodeReferenceCounts() needs to call stopUsingRegister(_modBase).
  • If I add _modBase to J9::ARM64::MemoryReference, and override the function decNodeReferenceCounts() in J9::ARM64::MemoryReference for adding the code stopUsingRegister(_modBase), that does not work as expected. _modBase is not initialized when the constructor of OMR::ARM64::MemoryReference calls addToOffset().
    "Never call virtual functions during construction or destruction."

@knn-k knn-k changed the title WIP: AArch64: Add modBase register to OMRMemoryReference AArch64: Add modBase register to OMRMemoryReference Nov 5, 2019
@knn-k
Copy link
Contributor Author

knn-k commented Nov 19, 2019

Rebased the code.

@knn-k knn-k changed the title AArch64: Add modBase register to OMRMemoryReference AArch64: Add extraRegister to OMRMemoryReference Nov 20, 2019
@knn-k
Copy link
Contributor Author

knn-k commented Nov 20, 2019

Renamed modBase to extraRegister, and removed the guards with J9_PROJECT_SPECIFIC.
Also changed the title and the commit comment.

@@ -63,6 +63,7 @@ class OMR_EXTENSIBLE MemoryReference : public OMR::MemoryReference

TR::UnresolvedDataSnippet *_unresolvedSnippet;
TR::SymbolReference *_symbolReference;
TR::Register *_extraRegister; // for use with unresolved snippet
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be useful to document what the extra register is for. Any downstream project can use it to associate an additional assigned register to a memory reference that can be used for project-specific purposes. This register is in addition to the base and index registers and isn't directly part of the addressing expression in the memory reference. An example of what this could be used for is to help synthesize unresolved data reference addresses at runtime.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added comments in the code on the potential use of the extra register.

@0xdaryl
Copy link
Contributor

0xdaryl commented Nov 20, 2019

@genie-omr build aarch64

This commit adds extraRegister and related code to
OMRMemoryReference for AArch64.
It will be used by UnresolvedSnippet.

Signed-off-by: KONNO Kazuhiro <konno@jp.ibm.com>
@0xdaryl 0xdaryl merged commit 9ad00ba into eclipse-omr:master Nov 26, 2019
@knn-k knn-k deleted the aarch64memref7 branch May 11, 2021 04:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants