Reuse z_color and z_uncolor for x86_64 and aarch64

[xiangzhai]

Hi @stefank

Please review my patch.

Thanks, Leslie Zhai

---

Progress

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

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/zgc pull/7/head:pull/7
$ git checkout pull/7

Update a local copy of the PR:
$ git checkout pull/7
$ git pull https://git.openjdk.org/zgc pull/7/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 7

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/zgc/pull/7.diff

[bridgekeeper[bot]]

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

[openjdk[bot]]

@xiangzhai Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. See OpenJDK Developers’ Guide for more information.

[openjdk[bot]]

@xiangzhai this pull request can not be integrated into zgc_generational due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:

git checkout la_port_zgc_generational
git fetch https://git.openjdk.org/zgc zgc_generational
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge zgc_generational"
git push

[xiangzhai]

PING @stefank

[fisk]

Hi @xiangzhai. I'm not sure how I feel about this change. It defines on a general level how to color/uncolor pointers. But it only really applies to some coloring and uncoloring where you can patch code, such as for nmethods where we have nmethod entry barriers, allowing cross modifying code. But when you scroll around there is other coloring and uncoloring being done, for example vector color/uncolor for arraycopy, and color/uncolor from the interpreter and shared assembly code such as method handle intrinsics, where we do some other coloring/uncoloring. This makes me think this is an abstraction that causes more friction than it really helps. Having a function on the barrier set assembler called e.g. "color", it makes you think this is the way a pointer is colored, but it's really only for nmethods, and you can't use that code for example in the interpreter code, or we will crash. Unfortunately, I don't really know what to suggest. Do you feel strongly about adding this abstraction? I can imagine trying to split up the different clients of barriers into different classes (one for nmethod, one for non-nmethod assembly, one for arraycopy, etc), and for each such class define what load_barrier/store_barrier/uncolor/color etc should do. But that seems like a larger refactoring, and kind of counter to what you wanted to achieve, and I don't feel certain that it is really worth it. What do you think?

[xiangzhai]

Hi @fisk

Thanks for your kind response!

This makes me think this is an abstraction that causes more friction than it really helps.

I agree.

But when you scroll around there is other coloring and uncoloring being done, for example vector color/uncolor for arraycopy.

Sorry I recently implemented LoongArch vector color/uncolor for arraycopy attempt to make the arraycopy code look more like x86 AVX_128/256bit, so I was lack of higher view about other coloring/uncoloring.

But that seems like a larger refactoring, and kind of counter to what you wanted to achieve, and I don't feel certain that it is really worth it.

I agree.

Thanks, Leslie Zhai