8270832: Aarch64: Update algorithm annotations for MacroAssembler::fill_words #4809

Wanghuang-Huawei · 2021-07-16T11:20:45Z

It is found that the comments of MacroAssembler::fill_words is not right here. Let's fix that.

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed

Issue

JDK-8270832: Aarch64: Update algorithm annotations for MacroAssembler::fill_words

Reviewers

Nick Gasson (@nick-arm - Reviewer)
Andrew Haley (@theRealAph - Reviewer)

Contributors

Wang Huang <whuang@openjdk.org>
Miu Zhuojun <mouzhuojun@huawei.com>
Wu Yan <wuyan@openjdk.org>

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/4809/head:pull/4809
$ git checkout pull/4809

Update a local copy of the PR:
$ git checkout pull/4809
$ git pull https://git.openjdk.java.net/jdk pull/4809/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 4809

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/4809.diff

…ll_words

bridgekeeper · 2021-07-16T11:21:55Z

👋 Welcome back whuang! 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.

Wanghuang-Huawei · 2021-07-16T11:22:34Z

/contributor add Wang Huang whuang@openjdk.org
/contributor add Miu Zhuojun mouzhuojun@huawei.com

openjdk · 2021-07-16T11:23:20Z

@Wanghuang-Huawei
Contributor Wang Huang <whuang@openjdk.org> successfully added.

openjdk · 2021-07-16T11:24:21Z

@Wanghuang-Huawei
Contributor Miu Zhuojun <mouzhuojun@huawei.com> successfully added.

openjdk · 2021-07-16T11:26:45Z

@Wanghuang-Huawei The following label will be automatically applied to this pull request:

hotspot

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2021-07-16T11:30:24Z

Webrevs

01: Full - Incremental (faf96722)
00: Full (f4cf8949)

nick-arm · 2021-07-19T03:03:57Z

src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp

@@ -4809,23 +4809,30 @@ void MacroAssembler::fill_words(Register base, Register cnt, Register value)
 {
 //  Algorithm:
 //


There's an extra store at the beginning to align base if bit 3 is set. Do you want to document that here too?

Sure. I will fix that.

nick-arm · 2021-07-19T03:03:57Z

src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp

 //      } while (cnt);
+//    }
+//    if (cnt & 1 == 1) {


Should be (cnt & 1) == 1.

Thank you for your suggestion.

nick-arm · 2021-07-19T03:03:57Z

src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp

 //      } while (cnt);
+//    }
+//    if (cnt & 1 == 1) {
+//      p[0] = v;


How about *p++ = v as base also gets incremented here.

Sure. I will fix that.

Wanghuang-Huawei · 2021-07-19T08:53:27Z

/contributor add Wu Yan wuyan@openjdk.org

openjdk · 2021-07-19T09:31:36Z

@Wanghuang-Huawei
Contributor Wu Yan <wuyan@openjdk.org> successfully added.

bridgekeeper · 2021-08-17T08:26:13Z

@Wanghuang-Huawei This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

wuyan0 · 2021-09-02T02:24:13Z

Could you do me a favor to review the patch? @theRealAph @nick-arm Thank you.

nick-arm

Looks OK.

openjdk · 2021-09-02T02:47:27Z

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

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

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

8270832: Aarch64: Update algorithm annotations for MacroAssembler::fill_words

Co-authored-by: Wang Huang <whuang@openjdk.org>
Co-authored-by: Miu Zhuojun <mouzhuojun@huawei.com>
Co-authored-by: Wu Yan <wuyan@openjdk.org>
Reviewed-by: ngasson, aph

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 512 new commits pushed to the master branch:

fa9c865: 8273112: -Xloggc: should override -verbose:gc
dd87181: 8214761: Bug in parallel Kahan summation implementation
7fff22a: 8272805: Avoid looking up standard charsets
92b05fe: 8273251: Call check_possible_safepoint() from SafepointMechanism::process_if_requested()
29e0f13: 8272385: Enforce ECPrivateKey d value to be in the range [1, n-1] for SunEC provider
aaa6f69: 8273250: Address javadoc issues in Deflater::setDictionationary
5ee5dd9: 8272914: Create hotspot:tier2 and hotspot:tier3 test groups
5245c1c: 8273147: Update and restructure TestGCLogMessages log message list
632a7e0: 8273165: GraphKit::combine_exception_states fails with "matching stack sizes" assert
c2e015c: 8273229: Update OS detection code to recognize Windows Server 2022
... and 502 more: https://git.openjdk.java.net/jdk/compare/90c219f37bc7da2a556d1733a148a7d445e900e3...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 (@nick-arm, @theRealAph) 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).

Wanghuang-Huawei · 2021-09-03T06:54:53Z

/integrate

openjdk · 2021-09-03T06:56:14Z

@Wanghuang-Huawei
Your change (at version faf9672) is now ready to be sponsored by a Committer.

nick-arm · 2021-09-07T01:39:01Z

/sponsor

openjdk · 2021-09-07T01:40:35Z

Going to push as commit 649c22c.
Since your change was applied there have been 534 commits pushed to the master branch:

eb22181: 8273386: Remove duplicated code in G1DCQS::abandon_completed_buffers
fc546d6: 8273378: Shenandoah: Remove the remaining uses of os::is_MP
7bd4f49: 8264207: CodeStrings does not honour fixed address assumption.
2cabec8: 8253343: Extract G1 Young GC algorithm related code from G1CollectedHeap
44c5c23: 8272164: DumpAllocStats shouldn't subclass from ResourceObj
1bf5bda: 8269418: jdk/jfr/event/oldobject/TestObjectSize.java failed with "RuntimeException: No events: expected false, was true"
fb5b144: 8272985: Reference discovery is confused about atomicity and degree of parallelism
70ed6c5: 8272878: JEP 381 cleanup: Remove unused Solaris code in sun.font.TrueTypeGlyphMapper
b4e5b28: 8273221: Guard GCIdMark against nested calls
4d25e6f: 8273335: compiler/blackhole tests should not run with interpreter-only VMs
... and 524 more: https://git.openjdk.java.net/jdk/compare/90c219f37bc7da2a556d1733a148a7d445e900e3...master

Your commit was automatically rebased without conflicts.

openjdk · 2021-09-07T01:40:49Z

@nick-arm @Wanghuang-Huawei Pushed as commit 649c22c.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

8270832: Aarch64: Update algorithm annotations for MacroAssembler::fi…

f4cf894

…ll_words

openjdk bot added the rfr label Jul 16, 2021

openjdk bot added the hotspot label Jul 16, 2021

nick-arm reviewed Jul 19, 2021

View changes

fix comments

faf9672

nick-arm approved these changes Sep 2, 2021

View changes

openjdk bot added the ready label Sep 2, 2021

theRealAph approved these changes Sep 2, 2021

View changes

openjdk bot added the sponsor label Sep 3, 2021

openjdk bot closed this Sep 7, 2021

openjdk bot added integrated and removed ready rfr sponsor labels Sep 7, 2021

8270832: Aarch64: Update algorithm annotations for MacroAssembler::fill_words #4809

8270832: Aarch64: Update algorithm annotations for MacroAssembler::fill_words #4809

Wanghuang-Huawei commented Jul 16, 2021 •

edited by openjdk[bot] bot

bridgekeeper bot commented Jul 16, 2021

Wanghuang-Huawei commented Jul 16, 2021

openjdk bot commented Jul 16, 2021

openjdk bot commented Jul 16, 2021

openjdk bot commented Jul 16, 2021

mlbridge bot commented Jul 16, 2021 •

edited

nick-arm Jul 19, 2021

Wanghuang-Huawei Jul 20, 2021

nick-arm Jul 19, 2021

Wanghuang-Huawei Jul 20, 2021

nick-arm Jul 19, 2021

Wanghuang-Huawei Jul 20, 2021

Wanghuang-Huawei commented Jul 19, 2021

openjdk bot commented Jul 19, 2021

bridgekeeper bot commented Aug 17, 2021

wuyan0 commented Sep 2, 2021

nick-arm left a comment

openjdk bot commented Sep 2, 2021 •

edited

Wanghuang-Huawei commented Sep 3, 2021

openjdk bot commented Sep 3, 2021

nick-arm commented Sep 7, 2021

openjdk bot commented Sep 7, 2021

openjdk bot commented Sep 7, 2021

8270832: Aarch64: Update algorithm annotations for MacroAssembler::fill_words #4809

8270832: Aarch64: Update algorithm annotations for MacroAssembler::fill_words #4809

Conversation

Wanghuang-Huawei commented Jul 16, 2021 • edited by openjdk[bot] bot

Progress

Issue

Reviewers

Contributors

Reviewing

bridgekeeper bot commented Jul 16, 2021

Wanghuang-Huawei commented Jul 16, 2021

openjdk bot commented Jul 16, 2021

openjdk bot commented Jul 16, 2021

openjdk bot commented Jul 16, 2021

mlbridge bot commented Jul 16, 2021 • edited

Webrevs

nick-arm Jul 19, 2021

Choose a reason for hiding this comment

Wanghuang-Huawei Jul 20, 2021

Choose a reason for hiding this comment

nick-arm Jul 19, 2021

Choose a reason for hiding this comment

Wanghuang-Huawei Jul 20, 2021

Choose a reason for hiding this comment

nick-arm Jul 19, 2021

Choose a reason for hiding this comment

Wanghuang-Huawei Jul 20, 2021

Choose a reason for hiding this comment

Wanghuang-Huawei commented Jul 19, 2021

openjdk bot commented Jul 19, 2021

bridgekeeper bot commented Aug 17, 2021

wuyan0 commented Sep 2, 2021

nick-arm left a comment

openjdk bot commented Sep 2, 2021 • edited

Wanghuang-Huawei commented Sep 3, 2021

openjdk bot commented Sep 3, 2021

nick-arm commented Sep 7, 2021

openjdk bot commented Sep 7, 2021

openjdk bot commented Sep 7, 2021

Wanghuang-Huawei commented Jul 16, 2021 •

edited by openjdk[bot] bot

mlbridge bot commented Jul 16, 2021 •

edited

openjdk bot commented Sep 2, 2021 •

edited