Skip to content

Conversation

@theRealAph
Copy link
Contributor

@theRealAph theRealAph commented Apr 29, 2022

This is a backport of 8285802 from the openjdk/jdk repository. It's almost clean, but upstream mainline has a few
cleanups of integer type handling, so there is some additional backporting risk.


Progress

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

Issue

  • JDK-8285802: AArch64: Consistently handle offsets in MacroAssembler as 64-bit quantities

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk8u-dev pull/50/head:pull/50
$ git checkout pull/50

Update a local copy of the PR:
$ git checkout pull/50
$ git pull https://git.openjdk.java.net/jdk8u-dev pull/50/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 50

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk8u-dev/pull/50.diff

@bridgekeeper
Copy link

bridgekeeper bot commented Apr 29, 2022

👋 Welcome back aph! 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 added the rfr Pull request is ready for review label Apr 29, 2022
@mlbridge
Copy link

mlbridge bot commented Apr 29, 2022

Webrevs

@gnu-andrew
Copy link
Member

@theRealAph please change the PR title to "Backport df4d5cf5f53c1451487e6301d31c196fac029f7a" so SKARA correctly picks this up as a backport. See https://wiki.openjdk.java.net/display/SKARA/Backports Thanks.

@gnu-andrew
Copy link
Member

Also it looks like you need to enable GitHub Actions on this repository so the test builds run. See https://wiki.openjdk.java.net/display/SKARA/Testing

Not too big a deal on this one though, as there is not yet any AArch64 testing.

@theRealAph theRealAph changed the title 8285802: AArch64: Consistently handle offsets in MacroAssembler as 64-bit quantities Backport df4d5cf5f53c1451487e6301d31c196fac029f7a May 2, 2022
@theRealAph theRealAph marked this pull request as draft May 2, 2022 09:07
@openjdk openjdk bot changed the title Backport df4d5cf5f53c1451487e6301d31c196fac029f7a 8285802: AArch64: Consistently handle offsets in MacroAssembler as 64-bit quantities May 2, 2022
@openjdk
Copy link

openjdk bot commented May 2, 2022

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

@openjdk openjdk bot added the backport Port of a pull request already in a different code base label May 2, 2022
@theRealAph
Copy link
Contributor Author

I'm going to have to do some more work on this one.

Is there something we can do to make testing work automatically?

@openjdk openjdk bot removed the rfr Pull request is ready for review label May 2, 2022
@bridgekeeper
Copy link

bridgekeeper bot commented Jun 27, 2022

@theRealAph This pull request has been inactive for more than 8 weeks and will be automatically closed if another 8 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!

@gnu-andrew
Copy link
Member

I'm going to have to do some more work on this one.

Is there something we can do to make testing work automatically?

The PR should at least do builds on Linux x86, Linux x86_64, Windows and Mac once GitHub Actions are turned on. To run the JTreg tests and produce cross-compiled builds, we need to be able to pass the built JDK from one stage to another, which requires backporting bundles or some alternative I'll try and look that again after the July update is out of the way.

It won't really help on AArch64 though, as I don't believe there's any AArch64 hardware on Gitlab. So the most we can do is cross-compile for AArch64.

@gnu-andrew
Copy link
Member

I take this should now be replaced by https://bugs.openjdk.org/browse/JDK-8285923 ?

@theRealAph
Copy link
Contributor Author

I take this should now be replaced by https://bugs.openjdk.org/browse/JDK-8285923 ?

Possibly not. Mainline has changed a great deal, so backporting becomes very tricky and involves a bunch of other changes if it is to be stable. At minimum, there were additional patches. Unless this is seriously biting people on 8 I wouldn't.

@gnu-andrew
Copy link
Member

I take this should now be replaced by https://bugs.openjdk.org/browse/JDK-8285923 ?

Possibly not. Mainline has changed a great deal, so backporting becomes very tricky and involves a bunch of other changes if it is to be stable. At minimum, there were additional patches. Unless this is seriously biting people on 8 I wouldn't.

Ok, I'm just trying to understand where we are with this, given 8285802 in trunk was backed out and replaced with 8285923. Do you still plan to backport any of this to 8u or shall we just close this PR?

@theRealAph
Copy link
Contributor Author

Ok, I'm just trying to understand where we are with this, given 8285802 in trunk was backed out and replaced with 8285923. Do you still plan to backport any of this to 8u or shall we just close this PR?

Yes, we should close it, unless someone actually needs it in production.

@bridgekeeper
Copy link

bridgekeeper bot commented Aug 25, 2022

@theRealAph This pull request has been inactive for more than 8 weeks and will be automatically closed if another 8 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!

@theRealAph theRealAph closed this Aug 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport Port of a pull request already in a different code base

Development

Successfully merging this pull request may close these issues.

2 participants