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

Enable warnings as errors on Aarch64 in the JIT #18382

Merged
merged 1 commit into from
Nov 2, 2023

Conversation

dylanjtuttle
Copy link
Contributor

@dylanjtuttle dylanjtuttle commented Oct 31, 2023

"Warnings as errors" is enabled on a component-by-component basis throughout OpenJ9 and OMR. For the JIT, warnings as errors is enabled on every platform in OMR and on x86 and Z in OpenJ9.

This PR enables warnings as errors on Aarch64 in OpenJ9 by:

  • Turning the OMR_WARNINGS_AS_ERRORS flag on if OMR_ARCH_AARCH64 is true
  • Removing the --disable-warnings-as-errors flag which is passed into Aarch64 Mac builds on Jenkins by default

This will ensure all builds on Aarch64 compile code with the -Werror flag, which will halt compilation if a warning is reported.

Enable `OMR_WARNINGS_AS_ERRORS` on Aarch64 in the JIT and remove
`--disable-warnings-as-errors` flag by default on Aarch64 for JCL
and OpenJDK Jenkins builds

Signed-off-by: Dylan Tuttle <[email protected]>
@dylanjtuttle
Copy link
Contributor Author

@0xdaryl as we discussed earlier today...

@0xdaryl
Copy link
Contributor

0xdaryl commented Nov 1, 2023

@knn-k : any objections to enabling this now?

@0xdaryl 0xdaryl self-assigned this Nov 1, 2023
Copy link
Contributor

@knn-k knn-k left a comment

Choose a reason for hiding this comment

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

No objection.
I built AArch64 OpenJ9 JDK 17 for Linux and macOS locally, and both of them were OK with warnings-as-errors.

@knn-k
Copy link
Contributor

knn-k commented Nov 2, 2023

Build jobs at internal servers finished successfully:

  • job/Build_JDK17_aarch64_linux_Personal/461/ (Linux)
  • job/Build_JDK17_aarch64_mac_Personal/193/ (macOS)

@0xdaryl
Copy link
Contributor

0xdaryl commented Nov 2, 2023

There are no community build jobs to test this, but internal testing runs fine.

@0xdaryl 0xdaryl merged commit 1217913 into eclipse-openj9:master Nov 2, 2023
@pshipton
Copy link
Member

pshipton commented Nov 2, 2023

There are no community build jobs to test this, but internal testing runs fine.

@0xdaryl are you referring to the UNB testing problems or something else? alinux isn't at UNB, and we can do builds at UNB, the main problem is running testing.

@0xdaryl
Copy link
Contributor

0xdaryl commented Nov 2, 2023

Yes, I was referring to the UNB problems. My mistake about alinux.

Konno-san tested this internally on a couple of configurations before we merged this so I don't think we'll see any problems.

@pshipton
Copy link
Member

pshipton commented Nov 2, 2023

I wasn't concerned, I just wanted you to be aware that we can build JVMs at UNB even if we can't test them all concurrently (we can run a limited amount of testing).

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.

4 participants