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 macOS: Implement omrsysinfo_get_processor_description() #6742

Merged
merged 1 commit into from
Oct 5, 2022

Conversation

knn-k
Copy link
Contributor

@knn-k knn-k commented Sep 30, 2022

This commit implements omrsysinfo_get_processor_description() for AArch64 macOS.

Closes: #6739

Signed-off-by: KONNO Kazuhiro [email protected]

@knn-k
Copy link
Contributor Author

knn-k commented Oct 3, 2022

The failures with PortSockTest on x86 macOS are known as Issue #6516.

port/unix/omrsysinfo.c Outdated Show resolved Hide resolved
@knn-k
Copy link
Contributor Author

knn-k commented Oct 3, 2022

This commit enables the use of optional atomic instructions (OMR_FEATURE_ARM64_LSE) on AArch64 macOS.

if (comp->target().cpu.supportsFeature(OMR_FEATURE_ARM64_LSE) && (!disableLSE))

https://github.com/eclipse-openj9/openj9/blob/adf9c099f62dd206727721ddbfbe9de931870bff/runtime/compiler/aarch64/codegen/J9TreeEvaluator.cpp#L3680

OMR_FEATURE_ARM64_LSE is the only optional feature that is checked in AArch 64 code generation of OMR and OpenJ9 at the moment.
I ran sanity.functional on AArch64 macOS in the OpenJ9 project, to make sure it does not break anything.

@0xdaryl
Copy link
Contributor

0xdaryl commented Oct 4, 2022

Can't test on macOS until #6637 is ready.

@0xdaryl
Copy link
Contributor

0xdaryl commented Oct 4, 2022

Jenkins build aarch64

@0xdaryl
Copy link
Contributor

0xdaryl commented Oct 4, 2022

@babsingh : could you review the port lib changes please?

Copy link
Contributor

@babsingh babsingh left a comment

Choose a reason for hiding this comment

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

Please only correct code that has been modified and near-by instances for coding standard. Near-by instances should not exceed the limits of the function which was modified.

I have been misinterpreted in the past, and the author ended up fixing the entire file for coding standard.

port/unix/omrsysinfo.c Outdated Show resolved Hide resolved
port/unix/omrsysinfo.c Outdated Show resolved Hide resolved
port/unix/omrsysinfo.c Show resolved Hide resolved
port/unix/omrsysinfo.c Outdated Show resolved Hide resolved
port/unix/omrsysinfo.c Show resolved Hide resolved
port/unix/omrsysinfo.c Outdated Show resolved Hide resolved
port/unix/omrsysinfo.c Outdated Show resolved Hide resolved
port/unix/omrsysinfo.c Outdated Show resolved Hide resolved
port/unix/omrsysinfo.c Outdated Show resolved Hide resolved
port/unix/omrsysinfo.c Outdated Show resolved Hide resolved
@knn-k knn-k force-pushed the aarch64macos_sysinfo1 branch 2 times, most recently from c5a1a85 to 9e59e68 Compare October 4, 2022 23:58
@knn-k
Copy link
Contributor Author

knn-k commented Oct 5, 2022

Jenkins build all

@knn-k
Copy link
Contributor Author

knn-k commented Oct 5, 2022

Jenkins build zos

@knn-k
Copy link
Contributor Author

knn-k commented Oct 5, 2022

Jenkins build riscv

RISC-V timed out.

Copy link
Contributor

@babsingh babsingh left a comment

Choose a reason for hiding this comment

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

This is my last feedback. Everything else looks good. The PR will be merged after this request is addressed. Since there are no amacs on OMR Jenkins, can you confirm if local testing on an amac has passed?

port/unix/omrsysinfo.c Outdated Show resolved Hide resolved
This commit implements omrsysinfo_get_processor_description() for
AArch64 macOS.

Signed-off-by: KONNO Kazuhiro <[email protected]>
@knn-k
Copy link
Contributor Author

knn-k commented Oct 5, 2022

This PR resolves a failure with PortSysinfoTest in omrporttest on AArch64 macOS as the output below shows.
There are two failures with PortSockTest, which need to be addressed by Issue #6516.

[----------] 38 tests from PortSysinfoTest
originalSoftLimit=256
originalHardLimit=24576
soft set to hard limit=24576
[----------] 38 tests from PortSysinfoTest (7457 ms total)


[==========] 244 tests from 21 test cases ran. (45284 ms total)
[  PASSED  ] 242 tests.
[  FAILED  ] 2 tests, listed below:
[  FAILED  ] PortSockTest.poll_functionality_basic
[  FAILED  ] PortSockTest.poll_functionality_many_sockets

@knn-k
Copy link
Contributor Author

knn-k commented Oct 5, 2022

Jenkins build aarch64

Copy link
Contributor

@babsingh babsingh left a comment

Choose a reason for hiding this comment

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

All PR builds have passed in the previous run. The most recent change only adjusts line spacing. The PR will be merged after the recent auto-run and aarch64 builds pass.

@babsingh babsingh merged commit 773e9b5 into eclipse:master Oct 5, 2022
@knn-k knn-k deleted the aarch64macos_sysinfo1 branch October 5, 2022 21:18
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.

Implement omrsysinfo_get_processor_description() for AArch64 macOS
3 participants