Skip to content

8251193: bin/idea.sh is generating wrong folder definitions for JVMCI modules #63

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

jerboaa
Copy link
Contributor

@jerboaa jerboaa commented Sep 7, 2020

Reviewed-by: mcimadamore
Contributed-by: Galder Zamarreno galder@redhat.com


Progress

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

Issue

  • JDK-8251193: bin/idea.sh is generating wrong folder definitions for JVMCI modules

Reviewers

Contributors

  • Galder Zamarreno <galder@redhat.com>

Download

$ git fetch https://git.openjdk.java.net/jdk pull/63/head:pull/63
$ git checkout pull/63

… modules

Reviewed-by: mcimadamore
Contributed-by: Galder Zamarreno <galder@redhat.com>
@bridgekeeper
Copy link

bridgekeeper bot commented Sep 7, 2020

👋 Welcome back sgehwolf! 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 Sep 7, 2020
@openjdk
Copy link

openjdk bot commented Sep 7, 2020

@jerboaa The following labels will be automatically applied to this pull request: build core-libs.

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

@openjdk openjdk bot added build build-dev@openjdk.org core-libs core-libs-dev@openjdk.org labels Sep 7, 2020
@jerboaa
Copy link
Contributor Author

jerboaa commented Sep 7, 2020

/label remove "rfr"

@openjdk
Copy link

openjdk bot commented Sep 7, 2020

@jerboaa Usage: /label <add|remove> [label[, label, ...]]wherelabel` is an additional classification that should be applied to this PR. These labels are valid:

  • serviceability
  • hotspot
  • sound
  • hotspot-compiler
  • kulla
  • i18n
  • shenandoah
  • jdk
  • javadoc
  • 2d
  • security
  • swing
  • hotspot-runtime
  • jmx
  • build
  • nio
  • beans
  • core-libs
  • compiler
  • net
  • hotspot-gc
  • hotspot-jfr
  • awt

@jerboaa
Copy link
Contributor Author

jerboaa commented Sep 7, 2020

/label remove "core-libs"

@openjdk
Copy link

openjdk bot commented Sep 7, 2020

@jerboaa Usage: /label <add|remove> [label[, label, ...]]wherelabel` is an additional classification that should be applied to this PR. These labels are valid:

  • serviceability
  • hotspot
  • sound
  • hotspot-compiler
  • kulla
  • i18n
  • shenandoah
  • jdk
  • javadoc
  • 2d
  • security
  • swing
  • hotspot-runtime
  • jmx
  • build
  • nio
  • beans
  • core-libs
  • compiler
  • net
  • hotspot-gc
  • hotspot-jfr
  • awt

@jerboaa
Copy link
Contributor Author

jerboaa commented Sep 7, 2020

/label remove "build"

@openjdk
Copy link

openjdk bot commented Sep 7, 2020

@jerboaa Usage: /label <add|remove> [label[, label, ...]]wherelabel` is an additional classification that should be applied to this PR. These labels are valid:

  • serviceability
  • hotspot
  • sound
  • hotspot-compiler
  • kulla
  • i18n
  • shenandoah
  • jdk
  • javadoc
  • 2d
  • security
  • swing
  • hotspot-runtime
  • jmx
  • build
  • nio
  • beans
  • core-libs
  • compiler
  • net
  • hotspot-gc
  • hotspot-jfr
  • awt

@jerboaa
Copy link
Contributor Author

jerboaa commented Sep 7, 2020

/label remove build

@openjdk openjdk bot removed the build build-dev@openjdk.org label Sep 7, 2020
@openjdk
Copy link

openjdk bot commented Sep 7, 2020

@jerboaa
The build label was successfully removed.

@jerboaa
Copy link
Contributor Author

jerboaa commented Sep 7, 2020

/label remove core-libs

@openjdk openjdk bot removed the core-libs core-libs-dev@openjdk.org label Sep 7, 2020
@openjdk
Copy link

openjdk bot commented Sep 7, 2020

@jerboaa
The core-libs label was successfully removed.

@jerboaa
Copy link
Contributor Author

jerboaa commented Sep 7, 2020

/contributor add Galder Zamarreno <galder@redhat.com>

@openjdk
Copy link

openjdk bot commented Sep 7, 2020

@jerboaa
Contributor Galder Zamarreno \ <galder@redhat.com\> successfully added.

@jerboaa
Copy link
Contributor Author

jerboaa commented Sep 7, 2020

/contributor remove Galder Zamarreno <galder@redhat.com>

@openjdk
Copy link

openjdk bot commented Sep 7, 2020

@jerboaa
Contributor Galder Zamarreno \ <galder@redhat.com\> successfully removed.

@jerboaa
Copy link
Contributor Author

jerboaa commented Sep 7, 2020

/contributor add Galder Zamarreno galder@redhat.com

@openjdk
Copy link

openjdk bot commented Sep 7, 2020

@jerboaa
Contributor Galder Zamarreno <galder@redhat.com> successfully added.

@mcimadamore
Copy link
Contributor

/sponsor

@openjdk
Copy link

openjdk bot commented Sep 7, 2020

@mcimadamore This change does not need sponsoring - the author is allowed to integrate it.

Copy link
Contributor

@mcimadamore mcimadamore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@openjdk
Copy link

openjdk bot commented Sep 7, 2020

@jerboaa This change now passes all automated pre-integration checks. When the change also fulfills all project specific requirements, type /integrate in a new comment to proceed. After integration, the commit message will be:

8251193: bin/idea.sh is generating wrong folder definitions for JVMCI modules

Co-authored-by: Galder Zamarreno <galder@redhat.com>
Reviewed-by: mcimadamore
  • If you would like to add a summary, use the /summary command.
  • To credit additional contributors, use the /contributor command.
  • To add additional solved issues, use the /issue command.

There are currently no new commits on the master branch since the last update of the source branch of this PR. If another commit should be pushed before you perform the /integrate command, your PR will be automatically rebased. If you would like to avoid potential automatic rebasing, specify the current head hash when integrating, like this: /integrate 70d5cac961db663f96223bd9efbc8b5ca77cc88a.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

@openjdk openjdk bot added the ready Pull request is ready to be integrated label Sep 7, 2020
@jerboaa
Copy link
Contributor Author

jerboaa commented Sep 7, 2020

/integrate

@openjdk openjdk bot closed this Sep 7, 2020
@openjdk openjdk bot added integrated Pull request has been integrated and removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Sep 7, 2020
@openjdk
Copy link

openjdk bot commented Sep 7, 2020

@jerboaa Pushed as commit 8d6d43c.

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

e1iu pushed a commit to e1iu/jdk that referenced this pull request Mar 1, 2022
This patch aims to optimize extract operation on vector for AArch64
according to Neoverse N2 and V1 software optimization guide[1][2].

Currently, extract operation is used by "Vector.lane"[3]. As SVE
doesn’t have direct instruction support for such operation like
"pextr"[4] in x86, the final code is as below:

```
	Byte512Vector.lane(7)

        orr     x8, xzr, #0x7
        whilele p0.b, xzr, x8
        lastb   w10, p0, z16.b
        sxtb    w10, w10
```

This patch uses NEON instruction instead if the target lane is located
in the NEON 128b range. For the same example above, the generated code
is much simpler:

```
        smov    x11, v16.b[7]
```

For those cases that target lane is located out of the NEON 128b range,
this patch uses EXT to shift the target to the lowest. The generated
code is as below:

```
        Byte512Vector.lane(63)

        mov     z17.d, z16.d
        ext     z17.b, z17.b, z17.b, openjdk#63
        smov    x10, v17.b[0]
```

hotspot/compiler/vectorapi, jdk/incubator/vector passed on SVE machine.

Refs:

 // From Arm Neoverse N2 and V1 Software Optimization Guide
 // NOTE: The data inside "()" belongs to V1
 +--------------+---------+------------+--------------------+
 |  Instruction | Latency | Throughput | Utilized Pipelines |
 +--------------+---------+------------+--------------------+
 |WHILELE       |        3|      1(1/2)|               M(M0)|
 +--------------+---------+------------+--------------------+
 |LASTB(scalar) |     5(6)|           1|               V1,M0|
 +--------------+---------+------------+--------------------+
 |EXT           |        2|           2|              V(V01)|
 +--------------+---------+------------+--------------------+
 |UMOV,SMOV     |        2|           1|                   V|
 +--------------+---------+------------+--------------------+
 |ORR           |        2|           2|              V(V01)|
 +--------------+---------+------------+--------------------+
 |INS           |        2|        2(4)|                   V|
 +--------------+---------+------------+--------------------+

[1] https://developer.arm.com/documentation/pjdoc466751330-9685/latest/
[2] https://developer.arm.com/documentation/PJDOC-466751330-18256/0001
[3] https://github.com/openjdk/jdk/blob/master/src/jdk.incubator.vector/share/classes/jdk/incubator/vector/IntVector.java#L2693
[4] https://www.felixcloutier.com/x86/pextrb:pextrd:pextrq

Change-Id: I90cfc1f8deb84145f42132d58d3b211c4a8933ad
e1iu pushed a commit to e1iu/jdk that referenced this pull request Mar 24, 2022
…or 64/128-bit vector sizes

This patch optimizes the SVE backend implementations of Vector.lane and
Vector.withLane for 64/128-bit vector size. The basic idea is to use
lower costs NEON instructions when the vector size is 64/128 bits.

1. Vector.lane(int i) (Gets the lane element at lane index i)

As SVE doesn’t have direct instruction support for extraction like
"pextr"[1] in x86, the final code was shown as below:

```
        Byte512Vector.lane(7)

        orr     x8, xzr, #0x7
        whilele p0.b, xzr, x8
        lastb   w10, p0, z16.b
        sxtb    w10, w10
```

This patch uses NEON instruction instead if the target lane is located
in the NEON 128b range. For the same example above, the generated code
now is much simpler:

```
        smov    x11, v16.b[7]
```

For those cases that target lane is located out of the NEON 128b range,
this patch uses EXT to shift the target to the lowest. The generated
code is as below:

```
        Byte512Vector.lane(63)

        mov     z17.d, z16.d
        ext     z17.b, z17.b, z17.b, openjdk#63
        smov    x10, v17.b[0]
```

2. Vector.withLane(int i, E e) (Replaces the lane element of this vector
                                at lane index i with value e)

For 64/128-bit vector, insert operation could be implemented by NEON
instructions to get better performance. E.g., for IntVector.SPECIES_128,
"IntVector.withLane(0, (int)4)" generates code as below:

```
        Before:
        orr     w10, wzr, #0x4
        index   z17.s, #-16, #1
        cmpeq   p0.s, p7/z, z17.s, #-16
        mov     z17.d, z16.d
        mov     z17.s, p0/m, w10

        After
        orr     w10, wzr, #0x4
        mov     v16.s[0], w10
```

This patch also does a small enhancement for vectors whose sizes are
greater than 128 bits. It can save 1 "DUP" if the target index is
smaller than 32. E.g., For ByteVector.SPECIES_512,
"ByteVector.withLane(0, (byte)4)" generates code as below:

```
        Before:
        index   z18.b, #0, #1
        mov     z17.b, #0
        cmpeq   p0.b, p7/z, z18.b, z17.b
        mov     z17.d, z16.d
        mov     z17.b, p0/m, w16

        After:
        index   z17.b, #-16, #1
        cmpeq   p0.b, p7/z, z17.b, #-16
        mov     z17.d, z16.d
        mov     z17.b, p0/m, w16
```

With this patch, we can see up to 200% performance gain for specific
vector micro benchmarks in my SVE testing system.

[TEST]
test/jdk/jdk/incubator/vector, test/hotspot/jtreg/compiler/vectorapi
passed without failure.

[1] https://www.felixcloutier.com/x86/pextrb:pextrd:pextrq

Change-Id: Ic2a48f852011978d0f252db040371431a339d73c
caojoshua pushed a commit to caojoshua/jdk that referenced this pull request Jul 28, 2023
* Rename all to smoketest.

This patch changes JAVA_PATH to Conditional Variable Assignment, so we
can define it using environment variable.

We also refactor how to compile classes. we only recompile the class files
when the java files have changed.

This patch also brings back run_crazy_exception.sh.  we drop it
accidentally.

* use JAVA_HOME instead of JDK_PATH.

---------

Co-authored-by: Xin Liu <xxinliu@amazon.com>
dansmithcode pushed a commit to dansmithcode/jdk that referenced this pull request Aug 31, 2024
* 7903536: jasm: the tool ignores options: -nowarn, -strict

* Method getResourceString in the public void warning(int where, String id, Object... args) could get wrong result
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integrated Pull request has been integrated
Development

Successfully merging this pull request may close these issues.

2 participants