jdk17u-dev
jdk17u-dev copied to clipboard
8324123: aarch64: fix prfm literal encoding in assembler
Unclean backport of fixing aarch64 PRFM (literal) encoding.
Additional testing:
- [x] Linux aarch64 server fastdebug, tier1
Progress
- [ ] Change must be properly reviewed (1 review required, with at least 1 Reviewer)
- [ ] JDK-8324123 needs maintainer approval
- [x] Change must not contain extraneous whitespace
- [x] Commit message must refer to an issue
Issue
- JDK-8324123: aarch64: fix prfm literal encoding in assembler (Bug - P5)
Reviewing
Using git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk17u-dev.git pull/2256/head:pull/2256
$ git checkout pull/2256
Update a local copy of the PR:
$ git checkout pull/2256
$ git pull https://git.openjdk.org/jdk17u-dev.git pull/2256/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 2256
View PR using the GUI difftool:
$ git pr show -t 2256
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk17u-dev/pull/2256.diff
Webrev
:wave: Welcome back lmao! 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.
This backport pull request has now been updated with issue from the original commit.
Could reviewers please help with the review? The risk is very low as the original wrong encoding is not used in code base. riscv64 build failed because of the risc GHA infra which is also seen in JDK21u.
Could reviewers please help with the review? The risk is very low as the original wrong encoding is not used in code base.
There is, therefore, no reason to backport this. It should not be approved.
Could reviewers please help with the review? The risk is very low as the original wrong encoding is not used in code base.
There is, therefore, no reason to backport this. It should not be approved.
Hi @theRealAph , I guess we might have enhancements in the future depending on this correct instruction encoding. 17u still accepts enhancements with substantial benefit.
Could reviewers please help with the review? The risk is very low as the original wrong encoding is not used in code base.
There is, therefore, no reason to backport this. It should not be approved.
Hi @theRealAph , I guess we might have enhancements in the future depending on this correct instruction encoding. 17u still accepts enhancements with substantial benefit.
... to the users. I guess what you say is possible, and applying this patch now might avoid some future risk. Having said that, if we can speculate about any and every possible future we'd backport an awful lot of stuff, with the risk that implies.
As it stands, this backport is outside the current rules. The maintainers are allowed to make an exception for a special case, though.
Could reviewers please help with the review? The risk is very low as the original wrong encoding is not used in code base.
There is, therefore, no reason to backport this. It should not be approved.
Hi @theRealAph , I guess we might have enhancements in the future depending on this correct instruction encoding. 17u still accepts enhancements with substantial benefit.
... to the users. I guess what you say is possible, and applying this patch now might avoid some future risk. Having said that, if we can speculate about any and every possible future we'd backport an awful lot of stuff, with the risk that implies.
As it stands, this backport is outside the current rules. The maintainers are allowed to make an exception for a special case, though.
Sure. We can still have a review before maintainer makes the decision. It's not just a normal pre-requisite enhancement which seems unnecessary as you described but an incorrect instruction encoding mistake which really makes compiler engineers nervous. The wrong encoding may not lead to crash but cannot perform the prefetch as expected and very difficult for engineers to find out the root cause.
❗ This change is not yet ready to be integrated. See the Progress checklist in the description for automated requirements.
@mmyxym 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!
@mmyxym This pull request has been inactive for more than 8 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the /open
pull request command.