8251152: ARM32: jtreg c2 Test8202414 test crash #48

fzhinkin · 2020-09-07T11:14:04Z

Some CPUs (like ARM32) does not support unaligned memory accesses. To avoid JVM crashes tests that perform such accesses should be skipped on corresponding platforms.

Progress

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

Issue

JDK-8251152: ARM32: jtreg c2 Test8202414 test crash

Reviewers

Igor Ignatyev (@iignatev - Reviewer)
clanger - Reviewer ⚠️ Added manually

Download

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

…upport Reviewed-by: iignatyev, clanger

bridgekeeper · 2020-09-07T11:15:26Z

👋 Welcome back fzhinkin! 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 · 2020-09-07T11:16:16Z

@fzhinkin Setting summary to Some CPUs (like ARM32) does not support unaligned memory accesses. To avoid JVM crashes tests that perform such accesses should be skipped on corresponding platforms.

openjdk · 2020-09-07T11:16:35Z

@fzhinkin This issue is referenced in the PR title - it will now be updated.

openjdk · 2020-09-07T11:16:54Z

@fzhinkin The command reviewer cannot be used in the pull request body. Please use it in a new comment.

openjdk · 2020-09-07T11:17:14Z

@fzhinkin The command reviewer cannot be used in the pull request body. Please use it in a new comment.

fzhinkin · 2020-09-07T11:17:26Z

/reviewer add iignatyev
/reviewer add clanger

openjdk · 2020-09-07T11:18:01Z

@fzhinkin
Reviewer iignatyev successfully added.

openjdk · 2020-09-07T11:18:19Z

@fzhinkin 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:

8251152: ARM32: jtreg c2 Test8202414 test crash

Some CPUs (like ARM32) does not support unaligned memory accesses. To avoid JVM crashes tests that perform such accesses should be skipped on corresponding platforms.

Reviewed-by: iignatyev, clanger

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.

Since the source branch of this PR was last updated there have been 2 commits pushed to the master branch:

e0d5b5f: 8252627: Make it safe for JFR thread to read threadObj
e29c3f6: 8252661: Change SafepointMechanism terminology to talk less about "blocking"

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid automatic rebasing, please merge master into your branch, and then specify the current head hash when integrating, like this: /integrate e0d5b5f7f2c7290db0680d060acad66066b83499.

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

openjdk · 2020-09-07T11:18:24Z

@fzhinkin
Reviewer clanger successfully added.

fzhinkin · 2020-09-07T11:18:50Z

/issue add 8251152

openjdk · 2020-09-07T11:19:09Z

@fzhinkin This issue is referenced in the PR title - it will now be updated.

openjdk · 2020-09-07T11:19:43Z

@fzhinkin The following label will be automatically applied to this pull request: hotspot-compiler.

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

mlbridge · 2020-09-07T11:22:52Z

Webrevs

00: Full (e5ff3a6)

iignatev · 2020-09-07T12:53:03Z

test/hotspot/jtreg/compiler/c2/Test8202414.java

+        // memory accesses. This test may cause JVM crash due to
+        // alignment check failure on such CPUs.
+        if (!jdk.internal.misc.Unsafe.getUnsafe().unalignedAccess()) {
+          throw new SkippedException(


nit: I don't think we need a line break here.

Previously, all lines within this file were no longer than 80 chars, so I decided to follow the same restriction.

fzhinkin · 2020-09-07T14:03:46Z

/reviewer remove clanger
/reviewer add @RealCLanger

openjdk · 2020-09-07T14:04:20Z

@fzhinkin
Reviewer clanger successfully removed.

openjdk · 2020-09-07T14:05:10Z

@fzhinkin
Reviewer clanger successfully added.

fzhinkin · 2020-09-07T15:32:23Z

/integrate

openjdk · 2020-09-07T15:33:38Z

@fzhinkin Since your change was applied there have been 2 commits pushed to the master branch:

e0d5b5f: 8252627: Make it safe for JFR thread to read threadObj
e29c3f6: 8252661: Change SafepointMechanism terminology to talk less about "blocking"

Your commit was automatically rebased without conflicts.

Pushed as commit 70d5cac.

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

Restore looks like this now: ``` 0x0000000106e4dfcc: movk x9, #0x5e4, lsl openjdk#16 0x0000000106e4dfd0: movk x9, #0x1, lsl openjdk#32 0x0000000106e4dfd4: blr x9 0x0000000106e4dfd8: ldp x2, x3, [sp, openjdk#16] 0x0000000106e4dfdc: ldp x4, x5, [sp, openjdk#32] 0x0000000106e4dfe0: ldp x6, x7, [sp, openjdk#48] 0x0000000106e4dfe4: ldp x8, x9, [sp, openjdk#64] 0x0000000106e4dfe8: ldp x10, x11, [sp, openjdk#80] 0x0000000106e4dfec: ldp x12, x13, [sp, openjdk#96] 0x0000000106e4dff0: ldp x14, x15, [sp, openjdk#112] 0x0000000106e4dff4: ldp x16, x17, [sp, openjdk#128] 0x0000000106e4dff8: ldp x0, x1, [sp], openjdk#144 0x0000000106e4dffc: ldp xzr, x19, [sp], openjdk#16 0x0000000106e4e000: ldp x22, x23, [sp, openjdk#16] 0x0000000106e4e004: ldp x24, x25, [sp, openjdk#32] 0x0000000106e4e008: ldp x26, x27, [sp, openjdk#48] 0x0000000106e4e00c: ldp x28, x29, [sp, openjdk#64] 0x0000000106e4e010: ldp x30, xzr, [sp, openjdk#80] 0x0000000106e4e014: ldp x20, x21, [sp], openjdk#96 0x0000000106e4e018: ldur x12, [x29, #-24] 0x0000000106e4e01c: ldr x22, [x12, openjdk#16] 0x0000000106e4e020: add x22, x22, #0x30 0x0000000106e4e024: ldr x8, [x28, openjdk#8] ```

This patch optimizes the backend implementation of VectorMaskToLong for AArch64, given a more efficient approach to mov value bits from predicate register to general purpose register as x86 PMOVMSK[1] does, by using BEXT[2] which is available in SVE2. With this patch, the final code (input mask is byte type with SPECIESE_512, generated on an SVE vector reg size of 512-bit QEMU emulator) changes as below: Before: mov z16.b, p0/z, #1 fmov x0, d16 orr x0, x0, x0, lsr openjdk#7 orr x0, x0, x0, lsr openjdk#14 orr x0, x0, x0, lsr openjdk#28 and x0, x0, #0xff fmov x8, v16.d[1] orr x8, x8, x8, lsr openjdk#7 orr x8, x8, x8, lsr openjdk#14 orr x8, x8, x8, lsr openjdk#28 and x8, x8, #0xff orr x0, x0, x8, lsl openjdk#8 orr x8, xzr, #0x2 whilele p1.d, xzr, x8 lastb x8, p1, z16.d orr x8, x8, x8, lsr openjdk#7 orr x8, x8, x8, lsr openjdk#14 orr x8, x8, x8, lsr openjdk#28 and x8, x8, #0xff orr x0, x0, x8, lsl openjdk#16 orr x8, xzr, #0x3 whilele p1.d, xzr, x8 lastb x8, p1, z16.d orr x8, x8, x8, lsr openjdk#7 orr x8, x8, x8, lsr openjdk#14 orr x8, x8, x8, lsr openjdk#28 and x8, x8, #0xff orr x0, x0, x8, lsl openjdk#24 orr x8, xzr, #0x4 whilele p1.d, xzr, x8 lastb x8, p1, z16.d orr x8, x8, x8, lsr openjdk#7 orr x8, x8, x8, lsr openjdk#14 orr x8, x8, x8, lsr openjdk#28 and x8, x8, #0xff orr x0, x0, x8, lsl openjdk#32 mov x8, #0x5 whilele p1.d, xzr, x8 lastb x8, p1, z16.d orr x8, x8, x8, lsr openjdk#7 orr x8, x8, x8, lsr openjdk#14 orr x8, x8, x8, lsr openjdk#28 and x8, x8, #0xff orr x0, x0, x8, lsl openjdk#40 orr x8, xzr, #0x6 whilele p1.d, xzr, x8 lastb x8, p1, z16.d orr x8, x8, x8, lsr openjdk#7 orr x8, x8, x8, lsr openjdk#14 orr x8, x8, x8, lsr openjdk#28 and x8, x8, #0xff orr x0, x0, x8, lsl openjdk#48 orr x8, xzr, #0x7 whilele p1.d, xzr, x8 lastb x8, p1, z16.d orr x8, x8, x8, lsr openjdk#7 orr x8, x8, x8, lsr openjdk#14 orr x8, x8, x8, lsr openjdk#28 and x8, x8, #0xff orr x0, x0, x8, lsl openjdk#56 After: mov z16.b, p0/z, #1 mov z17.b, #1 bext z16.d, z16.d, z17.d mov z17.d, #0 uzp1 z16.s, z16.s, z17.s uzp1 z16.h, z16.h, z17.h uzp1 z16.b, z16.b, z17.b mov x0, v16.d[0] [1] https://www.felixcloutier.com/x86/pmovmskb [2] https://developer.arm.com/documentation/ddi0602/2020-12/SVE-Instructions/BEXT--Gather-lower-bits-from-positions-selected-by-bitmask- Change-Id: Ia983a20c89f76403e557ac21328f2f2e05dd08e0

Co-authored-by: Xin Liu <xxinliu@amazon.com>

…ng into ldp/stp on AArch64 Macro-assembler on aarch64 can merge adjacent loads or stores into ldp/stp[1]. For example, it can merge: ``` str w20, [sp, openjdk#16] str w10, [sp, openjdk#20] ``` into ``` stp w20, w10, [sp, openjdk#16] ``` But C2 may generate a sequence like: ``` str x21, [sp, openjdk#8] str w20, [sp, openjdk#16] str x19, [sp, openjdk#24] <--- str w10, [sp, openjdk#20] <--- Before sorting str x11, [sp, openjdk#40] str w13, [sp, openjdk#48] str x16, [sp, openjdk#56] ``` We can't do any merging for non-adjacent loads or stores. The patch is to sort the spilling or unspilling sequence in the order of offset during instruction scheduling and bundling phase. After that, we can get a new sequence: ``` str x21, [sp, openjdk#8] str w20, [sp, openjdk#16] str w10, [sp, openjdk#20] <--- str x19, [sp, openjdk#24] <--- After sorting str x11, [sp, openjdk#40] str w13, [sp, openjdk#48] str x16, [sp, openjdk#56] ``` Then macro-assembler can do ld/st merging: ``` str x21, [sp, openjdk#8] stp w20, w10, [sp, openjdk#16] <--- Merged str x19, [sp, openjdk#24] str x11, [sp, openjdk#40] str w13, [sp, openjdk#48] str x16, [sp, openjdk#56] ``` To justify the patch, we run `HelloWorld.java` ``` public class HelloWorld { public static void main(String [] args) { System.out.println("Hello World!"); } } ``` with `java -Xcomp -XX:-TieredCompilation HelloWorld`. Before the patch, macro-assembler can do ld/st merging for 3688 times. After the patch, the number of ld/st merging increases to 3871 times, by ~5 %. Tested tier1~3 on x86 and AArch64. [1] https://github.com/openjdk/jdk/blob/a95062b39a431b4937ab6e9e73de4d2b8ea1ac49/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp#L2079

8251152: Skip Test8202414 on CPUs missing unaligned memory accesses s…

e5ff3a6

…upport Reviewed-by: iignatyev, clanger

openjdk bot changed the title ~~8251152: Skip Test8202414 on CPUs missing unaligned memory accesses support~~ 8251152: ARM32: jtreg c2 Test8202414 test crash Sep 7, 2020

openjdk bot added the ready Pull request is ready to be integrated label Sep 7, 2020

fzhinkin marked this pull request as ready for review September 7, 2020 11:19

openjdk bot added hotspot-compiler hotspot-compiler-dev@openjdk.org rfr Pull request is ready for review labels Sep 7, 2020

iignatev reviewed Sep 7, 2020

View reviewed changes

iignatev approved these changes Sep 7, 2020

View reviewed changes

openjdk bot closed this Sep 7, 2020

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

mlbridge bot mentioned this pull request Nov 9, 2020

8255949: AArch64: Add support for vectorized shift right and accumulate #1087

Closed

3 tasks

asotona added a commit to asotona/jdk that referenced this pull request Feb 9, 2023

AttributeElement.Kind removal (openjdk#48)

753e684

asotona added a commit to asotona/jdk that referenced this pull request Feb 15, 2023

AttributeElement.Kind removal (openjdk#48)

d4a8460

caojoshua pushed a commit to caojoshua/jdk that referenced this pull request Jun 9, 2023

JVM-2081: PEA materializes the object at bytecode aastore (openjdk#48)

3d9bfb0

Co-authored-by: Xin Liu <xxinliu@amazon.com>

robehn pushed a commit to robehn/jdk that referenced this pull request Aug 15, 2023

Fix ::set-output being deprecated warning (openjdk#48)

564ac55

8251152: ARM32: jtreg c2 Test8202414 test crash #48

8251152: ARM32: jtreg c2 Test8202414 test crash #48

fzhinkin commented Sep 7, 2020 •

edited by openjdk bot

bridgekeeper bot commented Sep 7, 2020

openjdk bot commented Sep 7, 2020

openjdk bot commented Sep 7, 2020

openjdk bot commented Sep 7, 2020

openjdk bot commented Sep 7, 2020

fzhinkin commented Sep 7, 2020

openjdk bot commented Sep 7, 2020

openjdk bot commented Sep 7, 2020 •

edited

openjdk bot commented Sep 7, 2020

fzhinkin commented Sep 7, 2020

openjdk bot commented Sep 7, 2020

openjdk bot commented Sep 7, 2020

mlbridge bot commented Sep 7, 2020

iignatev Sep 7, 2020

fzhinkin Sep 7, 2020

fzhinkin commented Sep 7, 2020

openjdk bot commented Sep 7, 2020

openjdk bot commented Sep 7, 2020

fzhinkin commented Sep 7, 2020

openjdk bot commented Sep 7, 2020

8251152: ARM32: jtreg c2 Test8202414 test crash #48

8251152: ARM32: jtreg c2 Test8202414 test crash #48

Conversation

fzhinkin commented Sep 7, 2020 • edited by openjdk bot

Progress

Issue

Reviewers

Download

bridgekeeper bot commented Sep 7, 2020

openjdk bot commented Sep 7, 2020

openjdk bot commented Sep 7, 2020

openjdk bot commented Sep 7, 2020

openjdk bot commented Sep 7, 2020

fzhinkin commented Sep 7, 2020

openjdk bot commented Sep 7, 2020

openjdk bot commented Sep 7, 2020 • edited

openjdk bot commented Sep 7, 2020

fzhinkin commented Sep 7, 2020

openjdk bot commented Sep 7, 2020

openjdk bot commented Sep 7, 2020

mlbridge bot commented Sep 7, 2020

Webrevs

iignatev Sep 7, 2020

Choose a reason for hiding this comment

fzhinkin Sep 7, 2020

Choose a reason for hiding this comment

fzhinkin commented Sep 7, 2020

openjdk bot commented Sep 7, 2020

openjdk bot commented Sep 7, 2020

fzhinkin commented Sep 7, 2020

openjdk bot commented Sep 7, 2020

fzhinkin commented Sep 7, 2020 •

edited by openjdk bot

openjdk bot commented Sep 7, 2020 •

edited