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

Add External Relocation Record TR_StaticDefaultValueInstance #6641

Merged

Conversation

a7ehuo
Copy link
Contributor

@a7ehuo a7ehuo commented Aug 4, 2022

This relocation record is used to materialize value type default value instance slot address:
&clazz->flattenedClassCache->defaultValue

IL:

  • Add StaticDefaultValueInstance flag to Symbol to indicate
    if the symbol is for default value instance slot address

X86:

  • Add relocation record in AMD64RegImm64SymInstruction for
    TR_StaticDefaultValueInstance relocation type.

Signed-off-by: Annabelle Huo [email protected]

@a7ehuo
Copy link
Contributor Author

a7ehuo commented Aug 4, 2022

This PR should be review along with eclipse-openj9/openj9#15666 which is the change that implements the new TR_RelocationRecordStaticDefaultValueInstance relocation record.

@a7ehuo
Copy link
Contributor Author

a7ehuo commented Aug 4, 2022

@dsouzai @hzongaro May I ask you to review this change along with eclipse-openj9/openj9#15666? Thank you!

compiler/il/OMRSymbol.hpp Outdated Show resolved Hide resolved
compiler/ras/Debug.cpp Outdated Show resolved Hide resolved
compiler/ras/Debug.cpp Outdated Show resolved Hide resolved
compiler/il/OMRSymbol.hpp Outdated Show resolved Hide resolved
Copy link
Member

@dsouzai dsouzai left a comment

Choose a reason for hiding this comment

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

Looks ok to me, minor change requested.

compiler/ras/Debug.cpp Outdated Show resolved Hide resolved
@a7ehuo a7ehuo force-pushed the static-allocation-defaultvalue-aot-2 branch from dc8be8d to 1cd0e50 Compare August 8, 2022 22:14
@a7ehuo
Copy link
Contributor Author

a7ehuo commented Aug 8, 2022

@hzongaro @dsouzai All comments are addressed. Ready for another review. Thanks!

Copy link
Member

@hzongaro hzongaro left a comment

Choose a reason for hiding this comment

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

Looks good overall. Just one request regarding a code comment

This relocation record is used to materialize value type
default value instance slot address:
`&clazz->flattenedClassCache->defaultValue`

IL:
- Add `StaticDefaultValueInstance` flag to `Symbol` to indicate
if the symbol is for default value instance slot address

X86:
- Add relocation record in AMD64RegImm64SymInstruction for
`TR_StaticDefaultValueInstance` relocation type.

Signed-off-by: Annabelle Huo <[email protected]>
@a7ehuo a7ehuo force-pushed the static-allocation-defaultvalue-aot-2 branch from 1cd0e50 to c2b24eb Compare August 9, 2022 14:31
Copy link
Member

@hzongaro hzongaro left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks!

@dsouzai
Copy link
Member

dsouzai commented Aug 9, 2022

osx failures seem to be due to #6516

jenkins build all

@dsouzai dsouzai self-assigned this Aug 9, 2022
@dsouzai
Copy link
Member

dsouzai commented Aug 9, 2022

[2022-08-09T15:20:48.980Z] test 30
[2022-08-09T15:20:48.980Z]       Start 30: compunittest
[2022-08-09T15:20:48.980Z] 
[2022-08-09T15:20:48.980Z] 30: Test command: /home/jenkins/jenkins-agent/workspace/Build/build/fvtest/compilerunittest/compunittest "--gtest_color=yes" "--gtest_output=xml:/home/jenkins/jenkins-agent/workspace/Build/build/fvtest/compilerunittest/compunittest-results.xml"
[2022-08-09T15:20:48.980Z] 30: Test timeout computed to be: 10000000
[2022-08-09T15:20:48.980Z] 30: �[0;32m[==========] �[mRunning 9043 tests from 117 test cases.
[2022-08-09T15:20:48.980Z] 30: �[0;32m[----------] �[m26 tests from PPCMemInstructionExpansionTest
[2022-08-09T15:20:48.980Z] 30: �[0;32m[----------] �[m26 tests from PPCMemInstructionExpansionTest (10 ms total)
[2022-08-09T15:20:48.980Z] 30: 
[2022-08-09T15:20:48.980Z] 30: �[0;32m[----------] �[m2 tests from PeepholeTest
[2022-08-09T15:20:48.980Z] 30: �[0;32m[----------] �[m2 tests from PeepholeTest (0 ms total)
[2022-08-09T15:20:48.980Z] 30: 
[2022-08-09T15:20:48.980Z] 30: �[0;32m[----------] �[m16 tests from AbsVPValueTest
[2022-08-09T15:20:48.980Z] 30/30 Test #30: compunittest ......................***Exception: SegFault  0.30 sec

failure due to #6556

@dsouzai dsouzai merged commit 325d650 into eclipse:master Aug 9, 2022
@a7ehuo a7ehuo deleted the static-allocation-defaultvalue-aot-2 branch October 12, 2022 00:47
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.

3 participants