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

8270947: AArch64: C1: use zero_words to initialize all objects #1502

Closed
wants to merge 1 commit into from

Conversation

navyxliu
Copy link
Member

@navyxliu navyxliu commented Nov 4, 2022

We would like to backport JDK-8270947 to jdk11u. It's not only less code to maintain, but also better generated code. C1 can leverage DC ZVA in large allocations on aarch64. The secondary consideration is that openjdk11u aligns with oraclejdk11u.

This isn't a clean backport. Small cosmetic changes are made in MacroAssembler::zero_words(Register ptr, Register cnt) chunk.

There are also 3 trailing bugfixes. We will post them together as stacked PRs


Progress

  • Change must be properly reviewed (1 review required, with at least 1 Reviewer)
  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue

Issue

  • JDK-8270947: AArch64: C1: use zero_words to initialize all objects

Reviewers

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk11u-dev pull/1502/head:pull/1502
$ git checkout pull/1502

Update a local copy of the PR:
$ git checkout pull/1502
$ git pull https://git.openjdk.org/jdk11u-dev pull/1502/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 1502

View PR using the GUI difftool:
$ git pr show -t 1502

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk11u-dev/pull/1502.diff

@bridgekeeper
Copy link

bridgekeeper bot commented Nov 4, 2022

👋 Welcome back xliu! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

@openjdk openjdk bot changed the title Backport 6c68ce2d396c6fe02201daf2bdb8c164de807cc1 8270947: AArch64: C1: use zero_words to initialize all objects Nov 4, 2022
@openjdk
Copy link

openjdk bot commented Nov 4, 2022

This backport pull request has now been updated with issue from the original commit.

@openjdk openjdk bot added backport rfr Pull request is ready for review labels Nov 4, 2022
@mlbridge
Copy link

mlbridge bot commented Nov 4, 2022

Webrevs

@GoeLin
Copy link
Member

GoeLin commented Nov 7, 2022

Hi
I think this has four follow ups:
What about 8280234 and 8271956?

Copy link
Member

@phohensee phohensee left a comment

Choose a reason for hiding this comment

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

3-patch bundle passes tier1. Lgtm.

Pre-submit failure looks to be an infrastructure issue.

@openjdk
Copy link

openjdk bot commented Nov 7, 2022

@navyxliu This change now passes all automated pre-integration checks.

After integration, the commit message for the final commit will be:

8270947: AArch64: C1: use zero_words to initialize all objects

Reviewed-by: phh

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 12 new commits pushed to the master branch:

  • c09c433: 8292866: Java_sun_awt_shell_Win32ShellFolder2_getLinkLocation check MultiByteToWideChar return value for failures
  • 03e1247: 8273685: Remove jtreg tag manual=yesno for java/awt/Graphics/LCDTextAndGraphicsState.java & show test instruction
  • df9c46a: 8261445: Use memory_order_relaxed for os::random().
  • 8f5ae91: 8277881: Missing SessionID in TLS1.3 resumption in compatibility mode
  • f07e691: 8277970: Test jdk/sun/security/ssl/SSLSessionImpl/NoInvalidateSocketException.java fails with "tag mismatch"
  • 3e3f665: 8293826: Closed test fails after JDK-8276108 on aarch64
  • ca988a6: 8273236: keytool does not accurately warn about algorithms that are disabled but have additional constraints
  • 188ab59: 8295554: Move the "sizecalc.h" to the correct location
  • f24340a: 8292682: Code change of JDK-8282730 not updated to reflect CSR update
  • 6b2e6ae: 8282730: LdapLoginModule throw NPE from logout method after login failure
  • ... and 2 more: https://git.openjdk.org/jdk11u-dev/compare/7c5f9c6831e356fc10aac948a6abbc5cb07b56e0...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@phohensee) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

@openjdk openjdk bot added the ready Pull request is ready to be integrated label Nov 7, 2022
@navyxliu
Copy link
Member Author

navyxliu commented Nov 7, 2022

Hi I think this has four follow ups: What about 8280234 and 8271956?

Hi, @GoeLin
Thank you for the reminder. I didn't notice them at the beginning.

Those two fixed compilation issues for other variants.

  1. 8271956 is for variant client.
  2. 8280234 is for variant core.

For item-1, I can reproduce it in jdk11u. The backport is trivial. I will append it to the Stack PRs.

for item-2, I can't reproduce it! it's because current jdk11u can't build variant 'core'.
Here is the error message.

=== Output from failing command(s) repeated here ===
/usr/bin/printf "* For target hotspot_variant-core_libjvm_objs_globals.o:\n"
* For target hotspot_variant-core_libjvm_objs_globals.o:
(/usr/bin/grep -v -e "^Note: including file:" <  /home/ubuntu/Devel/jdk11u-dev/build/linux-aarch64-normal-core-release/make-support/failure-logs/hotspot_variant-core_libjvm_objs_globals.o.log || true) | /usr/bin/head -n 15
In file included from /home/ubuntu/Devel/jdk11u-dev/src/hotspot/share/memory/allocation.hpp:28,
                 from /home/ubuntu/Devel/jdk11u-dev/src/hotspot/share/oops/array.hpp:28,
                 from /home/ubuntu/Devel/jdk11u-dev/src/hotspot/share/memory/universe.hpp:28,
                 from /home/ubuntu/Devel/jdk11u-dev/src/hotspot/share/code/oopRecorder.hpp:28,
                 from /home/ubuntu/Devel/jdk11u-dev/src/hotspot/share/asm/codeBuffer.hpp:28,
                 from /home/ubuntu/Devel/jdk11u-dev/src/hotspot/share/asm/assembler.hpp:28,
                 from /home/ubuntu/Devel/jdk11u-dev/src/hotspot/share/precompiled/precompiled.hpp:31:
/home/ubuntu/Devel/jdk11u-dev/src/hotspot/share/runtime/globals.hpp:2765:75: error: 'pd_InlineSmallCode' was not declared in this scope; did you mean 'InlineSmallCode'?
 2765 | #define MATERIALIZE_PD_PRODUCT_FLAG(type, name, doc)          type name = pd_##name;
      |                                                                           ^~~
/home/ubuntu/Devel/jdk11u-dev/src/hotspot/share/runtime/globals.hpp:1702:3: note: in expansion of macro 'MATERIALIZE_PD_PRODUCT_FLAG'
 1702 |   product_pd(intx, InlineSmallCode,                                         \
      |   ^~~~~~~~~~
/home/ubuntu/Devel/jdk11u-dev/src/hotspot/share/runtime/globals.hpp:2703:3: note: in expansion of macro 'RUNTIME_FLAGS'
 2703 |   RUNTIME_FLAGS(                                                            \
if test `/usr/bin/wc -l < /home/ubuntu/Devel/jdk11u-dev/build/linux-aarch64-normal-core-release/make-support/failure-logs/hotspot_variant-core_libjvm_objs_globals.o.log` -gt 15; then /usr/bin/echo "   ... (rest of output omitted)" ; fi
   ... (rest of output omitted)
/usr/bin/printf "\n* All command lines available in /home/ubuntu/Devel/jdk11u-dev/build/linux-aarch64-normal-core-release/make-support/failure-logs.\n"

* All command lines available in /home/ubuntu/Devel/jdk11u-dev/build/linux-aarch64-normal-core-release/make-support/failure-logs.
/usr/bin/printf "=== End of repeated output ===\n"
=== End of repeated output ===

I would like to sideline 8280234 for time being. if variant core matters to jdk11u, I will seek to fix build core first.

what do you think?

@navyxliu
Copy link
Member Author

navyxliu commented Nov 7, 2022

I filed an issue about the build failure of variant=core.
https://bugs.openjdk.org/browse/JDK-8296502

@phohensee
Copy link
Member

There may be an existing JBS issue patch that fixes the problem.

@navyxliu
Copy link
Member Author

navyxliu commented Nov 8, 2022

There may be an existing JBS issue patch that fixes the problem.

Yes, I think JDK-8235673 can fix this build issue, but it isn't trivial.

@phohensee
Copy link
Member

I took a look at JDK-8235673 and agree. Better to do the minimum fix you propose in JDK-8296502.

@navyxliu navyxliu closed this Nov 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport ready Pull request is ready to be integrated rfr Pull request is ready for review
Development

Successfully merging this pull request may close these issues.

3 participants