RFR: 8241502: Migrate x86_64.ad to MacroAssembler

classic Classic list List threaded Threaded
12 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR: 8241502: Migrate x86_64.ad to MacroAssembler

John Tortugo
Relates to: https://bugs.openjdk.java.net/browse/JDK-8241502
Tested on: Linux tier1, 2 and 3

Can you please take a look whether these changes are going in the direction expected or not? If it is, I'll continue working on the `JDK-8241502` but I'd like to split it in a few PRs since it's a lot of changes.

-------------

Commit messages:
 - Merge branch 'jdk-8241502' of https://github.com/JohnTortugo/jdk into jdk-8241502
 - First part of Migrate x86_64 to MacroAssembler
 - First part of Migrate x86_64 to MacroAssembler
 - Merge pull request #1 from openjdk/master

Changes: https://git.openjdk.java.net/jdk/pull/2420/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2420&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8241502
  Stats: 174 lines in 1 file changed: 44 ins; 21 del; 109 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2420.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2420/head:pull/2420

PR: https://git.openjdk.java.net/jdk/pull/2420
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8241502: Migrate x86_64.ad to MacroAssembler

Vladimir Ivanov-2
On Fri, 5 Feb 2021 03:15:15 GMT, John Tortugo <[hidden email]> wrote:

> Can you please take a look whether these changes are going in the direction expected or not?

Yes, the patch is perfectly aligned with what JDK-8241502 proposes. Thanks for taking care of it.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2420
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8241502: Migrate x86_64.ad to MacroAssembler

Vladimir Kozlov-2
On Fri, 5 Feb 2021 11:02:04 GMT, Vladimir Ivanov <[hidden email]> wrote:

>> Relates to: https://bugs.openjdk.java.net/browse/JDK-8241502
>> Tested on: Linux tier1, 2 and 3
>>
>> Can you please take a look whether these changes are going in the direction expected or not? If it is, I'll continue working on the `JDK-8241502` but I'd like to split it in a few PRs since it's a lot of changes.
>
>> Can you please take a look whether these changes are going in the direction expected or not?
>
> Yes, the patch is perfectly aligned with what JDK-8241502 proposes. Thanks for taking care of it.

I agree. We wanted to do that for long time.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2420
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8241502: Migrate x86_64.ad to MacroAssembler

Ioi Lam-2
On Fri, 5 Feb 2021 17:47:07 GMT, Vladimir Kozlov <[hidden email]> wrote:

>>> Can you please take a look whether these changes are going in the direction expected or not?
>>
>> Yes, the patch is perfectly aligned with what JDK-8241502 proposes. Thanks for taking care of it.
>
> I agree. We wanted to do that for long time.

I am curious if the x86_64.o file changes in any significant way (speed of size).

-------------

PR: https://git.openjdk.java.net/jdk/pull/2420
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8241502: Migrate x86_64.ad to MacroAssembler [v2]

John Tortugo
In reply to this post by John Tortugo
> Relates to: https://bugs.openjdk.java.net/browse/JDK-8241502
> Tested on: Linux tier1, 2 and 3
>
> Can you please take a look whether these changes are going in the direction expected or not? If it is, I'll continue working on the `JDK-8241502` but I'd like to split it in a few PRs since it's a lot of changes.

John Tortugo has updated the pull request incrementally with one additional commit since the last revision:

  Second part of conversions.

-------------

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2420/files
  - new: https://git.openjdk.java.net/jdk/pull/2420/files/8198a988..25824fde

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2420&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2420&range=00-01

  Stats: 141 lines in 1 file changed: 45 ins; 0 del; 96 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2420.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2420/head:pull/2420

PR: https://git.openjdk.java.net/jdk/pull/2420
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8241502: Migrate x86_64.ad to MacroAssembler

Dean Long-2
In reply to this post by Ioi Lam-2
On Fri, 5 Feb 2021 18:43:27 GMT, Ioi Lam <[hidden email]> wrote:

>> I agree. We wanted to do that for long time.
>
> I am curious if the x86_64.o file changes in any significant way (speed of size).

I wish there was a way for the old and new versions to co-exist at the same time, so we could generate the code the old way and and the new way, then compare, for automatic verification of the MacroAssember version.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2420
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8241502: Migrate x86_64.ad to MacroAssembler [v3]

John Tortugo
In reply to this post by John Tortugo
> Relates to: https://bugs.openjdk.java.net/browse/JDK-8241502
> Tested on: Linux tier1, 2 and 3
>
> Can you please take a look whether these changes are going in the direction expected or not? If it is, I'll continue working on the `JDK-8241502` but I'd like to split it in a few PRs since it's a lot of changes.

John Tortugo has updated the pull request incrementally with one additional commit since the last revision:

  Third part of conversions. Small fix in Assembler::cmovl.

-------------

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2420/files
  - new: https://git.openjdk.java.net/jdk/pull/2420/files/25824fde..1e8361cc

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2420&range=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2420&range=01-02

  Stats: 156 lines in 2 files changed: 46 ins; 13 del; 97 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2420.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2420/head:pull/2420

PR: https://git.openjdk.java.net/jdk/pull/2420
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8241502: Migrate x86_64.ad to MacroAssembler

John Tortugo
In reply to this post by Dean Long-2
On Tue, 9 Feb 2021 01:35:35 GMT, Dean Long <[hidden email]> wrote:

>> I am curious if the x86_64.o file changes in any significant way (speed of size).
>
> I wish there was a way for the old and new versions to co-exist at the same time, so we could generate the code the old way and and the new way, then compare, for automatic verification of the MacroAssember version.

Thank you all for the feedback!

@iklam - I'll check that and let you know once I make more conversions.

@dean-long - That would be great. I'm all ears for the best way to test this!

-------------

PR: https://git.openjdk.java.net/jdk/pull/2420
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8241502: Migrate x86_64.ad to MacroAssembler [v4]

John Tortugo
In reply to this post by John Tortugo
> Relates to: https://bugs.openjdk.java.net/browse/JDK-8241502
> Tested on: Linux tier1, 2 and 3
>
> Can you please take a look whether these changes are going in the direction expected or not? If it is, I'll continue working on the `JDK-8241502` but I'd like to split it in a few PRs since it's a lot of changes.

John Tortugo has updated the pull request incrementally with one additional commit since the last revision:

  Some div and shift instructions.

-------------

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2420/files
  - new: https://git.openjdk.java.net/jdk/pull/2420/files/1e8361cc..0e72dfe0

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2420&range=03
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2420&range=02-03

  Stats: 419 lines in 3 files changed: 221 ins; 136 del; 62 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2420.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2420/head:pull/2420

PR: https://git.openjdk.java.net/jdk/pull/2420
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8241502: Migrate x86_64.ad to MacroAssembler [v5]

John Tortugo
In reply to this post by John Tortugo
> Relates to: https://bugs.openjdk.java.net/browse/JDK-8241502
> Tested on: Linux tier1, 2 and 3
>
> Can you please take a look whether these changes are going in the direction expected or not? If it is, I'll continue working on the `JDK-8241502` but I'd like to split it in a few PRs since it's a lot of changes.

John Tortugo has updated the pull request incrementally with one additional commit since the last revision:

  More shifts; logic operations and movs.

-------------

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2420/files
  - new: https://git.openjdk.java.net/jdk/pull/2420/files/0e72dfe0..b776b9f6

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2420&range=04
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2420&range=03-04

  Stats: 117 lines in 3 files changed: 68 ins; 7 del; 42 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2420.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2420/head:pull/2420

PR: https://git.openjdk.java.net/jdk/pull/2420
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8241502: Migrate x86_64.ad to MacroAssembler

Dean Long-2
In reply to this post by John Tortugo
On Thu, 11 Feb 2021 05:12:55 GMT, John Tortugo <[hidden email]> wrote:

>> I wish there was a way for the old and new versions to co-exist at the same time, so we could generate the code the old way and and the new way, then compare, for automatic verification of the MacroAssember version.
>
> Thank you all for the feedback!
>
> @iklam - I'll check that and let you know once I make more conversions.
>
> @dean-long - That would be great. I'm all ears for the best way to test this!

Here's one way to test both versions, using loadRange as an example.  It's not exactly pretty, but it seems to work.
[loadRange.txt](https://github.com/openjdk/jdk/files/5994725/loadRange.txt)

-------------

PR: https://git.openjdk.java.net/jdk/pull/2420
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8241502: Migrate x86_64.ad to MacroAssembler

John Tortugo
On Wed, 17 Feb 2021 09:47:13 GMT, Dean Long <[hidden email]> wrote:

>> Thank you all for the feedback!
>>
>> @iklam - I'll check that and let you know once I make more conversions.
>>
>> @dean-long - That would be great. I'm all ears for the best way to test this!
>
> Here's one way to test both versions, using loadRange as an example.  It's not exactly pretty, but it seems to work.
> [loadRange.txt](https://github.com/openjdk/jdk/files/5994725/loadRange.txt)

@dean-long - I recently hacked something pretty similar - basically, I added a flag to the CodeSection class to make it print emitted bytes whenever the flag was set, then I created two encoding classes in the AD file to toggle the print flag. That way I was able to _visually_ compare the output of the two versions of the code. Your approach with the CRC is much better. Thanks a lot!

-------------

PR: https://git.openjdk.java.net/jdk/pull/2420