8319690: [AArch64] C2 compilation hits offset_ok_for_immed: assert "c2 compiler bug" #18103

eme64 · 2024-03-04T09:46:56Z

Taking this over from @fg1417, she seems to be "on leave" according to her GitHub account.

I'm taking her regression test (she improved the reproducer that I had originally reported the bug with).

It is ok to just remove the assert, because the address is "sanitized" after the assert, i.e. we use a lea instruction to compute the address.

But I'm simply removing the assert, as suggested in the comments of the previous PR.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8319690: [AArch64] C2 compilation hits offset_ok_for_immed: assert "c2 compiler bug" (Bug - P4)

Reviewers

Andrew Haley (@theRealAph - Reviewer)
Vladimir Kozlov (@vnkozlov - Reviewer)
Guoxiong Li (@lgxbslgx - Committer)

Contributors

Fei Gao <fgao@openjdk.org>

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/18103/head:pull/18103
$ git checkout pull/18103

Update a local copy of the PR:
$ git checkout pull/18103
$ git pull https://git.openjdk.org/jdk.git pull/18103/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 18103

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/18103.diff

Webrev

Link to Webrev Comment

bridgekeeper · 2024-03-04T09:47:30Z

👋 Welcome back epeter! 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 · 2024-03-04T09:48:15Z

@eme64 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 pull request command.

eme64 · 2024-03-04T09:51:45Z

/contributor add fg1417

@fg1417 wrote a first PR, but gave it up. I'm taking over the test, but not her fix.
(I had found the original reproducer, but she improved the test further, so I want to give her credit for that)

openjdk · 2024-03-04T09:52:08Z

@eme64 fg1417 was not found in the census.

Syntax: /contributor (add|remove) [@user | openjdk-user | Full Name <email@address>]. For example:

/contributor add @openjdk-bot
/contributor add duke
/contributor add J. Duke <duke@openjdk.org>

User names can only be used for users in the census associated with this repository. For other contributors you need to supply the full name and email address.

eme64 · 2024-03-04T09:54:35Z

/contributor add faye.gao@arm.com

openjdk · 2024-03-04T09:55:22Z

@eme64 feigao.aha@gmail.com is not a valid name and email string.

Syntax: /contributor (add|remove) [@user | openjdk-user | Full Name <email@address>]. For example:

/contributor add @openjdk-bot
/contributor add duke
/contributor add J. Duke <duke@openjdk.org>

User names can only be used for users in the census associated with this repository. For other contributors you need to supply the full name and email address.

eme64 · 2024-03-04T09:57:29Z

/contributor add @fg1417

openjdk · 2024-03-04T09:57:44Z

@eme64
Contributor Fei Gao <fgao@openjdk.org> successfully added.

mlbridge · 2024-03-04T11:53:06Z

Webrevs

00: Full (c2c368f7)

theRealAph

Yes, that's right. I wrote that assertion for my own information, this isn't really a bug. I might rework this whole area of the compiler in the future, but there's no urgency. Thanks.

openjdk · 2024-03-04T16:54:54Z

@eme64 This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8319690: [AArch64] C2 compilation hits offset_ok_for_immed: assert "c2 compiler bug"

Co-authored-by: Fei Gao <fgao@openjdk.org>
Reviewed-by: aph, kvn, gli

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 50 new commits pushed to the master branch:

e9adceb: 8327208: Remove unused method java.util.jar.Manifest.make72Safe
d6f2a17: 8325881: Require minimum gcc version 10
0b95909: 8327224: G1: comment in G1BarrierSetC2::post_barrier() refers to nonexistent new_deferred_store_barrier()
c589555: 8325095: C2: bailout message broken: ResourceArea allocated string used after free
b7540df: 8327007: javax/swing/JSpinner/8008657/bug8008657.java fails
e1b661f: 8319900: Recursive lightweight locking: riscv64 implementation
045eea1: 8327057: Parallel: Refactor ParMarkBitMap::iterate
f615ac4: 8309622: Re-examine the cache mechanism in BaseLocale
6f8d351: 8325725: Parallel: Refactor PSParallelCompact::fill_dense_prefix_end
8cfaceb: 8327125: SpinYield.report should report microseconds
... and 40 more: https://git.openjdk.org/jdk/compare/e772e7817981b7b5c01b95245cf2022372494619...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

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

vnkozlov

Good.

lgxbslgx

Looks good.

eme64 · 2024-03-05T13:35:04Z

Thanks @vnkozlov @theRealAph @lgxbslgx for the reviews!
Thanks @fg1417 for the original PR and patching up the test.

/integrate

openjdk · 2024-03-05T13:37:10Z

Going to push as commit 98f0b86.
Since your change was applied there have been 55 commits pushed to the master branch:

2372aba: 8326172: Dubious claim on long[]/double[] alignment in MemorySegment javadoc
c653e67: 8327225: Revert DataInputStream.readUTF to static final
a089ed2: 8326936: RISC-V: Shenandoah GC crashes due to incorrect atomic memory operations
560cf59: 8327287: Remove unused FLSVerifyDictionary debug option
fec51d4: 8327130: Serial: Remove Generation::record_spaces_top
e9adceb: 8327208: Remove unused method java.util.jar.Manifest.make72Safe
d6f2a17: 8325881: Require minimum gcc version 10
0b95909: 8327224: G1: comment in G1BarrierSetC2::post_barrier() refers to nonexistent new_deferred_store_barrier()
c589555: 8325095: C2: bailout message broken: ResourceArea allocated string used after free
b7540df: 8327007: javax/swing/JSpinner/8008657/bug8008657.java fails
... and 45 more: https://git.openjdk.org/jdk/compare/e772e7817981b7b5c01b95245cf2022372494619...master

Your commit was automatically rebased without conflicts.

openjdk · 2024-03-05T13:37:17Z

@eme64 Pushed as commit 98f0b86.

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

8319690

314a1ca

openjdk bot changed the title ~~8319690~~ 8319690: [AArch64] C2 compilation hits offset_ok_for_immed: assert "c2 compiler bug" Mar 4, 2024

openjdk bot added the hotspot-compiler hotspot-compiler-dev@openjdk.org label Mar 4, 2024

remove the assert

c2c368f

eme64 marked this pull request as ready for review March 4, 2024 11:48

openjdk bot added the rfr Pull request is ready for review label Mar 4, 2024

theRealAph approved these changes Mar 4, 2024

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Mar 4, 2024

vnkozlov approved these changes Mar 4, 2024

View reviewed changes

lgxbslgx approved these changes Mar 5, 2024

View reviewed changes

openjdk bot added the integrated Pull request has been integrated label Mar 5, 2024

openjdk bot closed this Mar 5, 2024

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Mar 5, 2024

eme64 mentioned this pull request Mar 5, 2024

8318446: C2: optimize stores into primitive arrays by combining values into larger store #16245

Closed

3 tasks

8319690: [AArch64] C2 compilation hits offset_ok_for_immed: assert "c2 compiler bug" #18103

8319690: [AArch64] C2 compilation hits offset_ok_for_immed: assert "c2 compiler bug" #18103

Uh oh!

Conversation

eme64 commented Mar 4, 2024 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Contributors

Reviewing

Webrev

Uh oh!

bridgekeeper bot commented Mar 4, 2024

Uh oh!

openjdk bot commented Mar 4, 2024

Uh oh!

eme64 commented Mar 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openjdk bot commented Mar 4, 2024

Uh oh!

eme64 commented Mar 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openjdk bot commented Mar 4, 2024

Uh oh!

eme64 commented Mar 4, 2024

Uh oh!

openjdk bot commented Mar 4, 2024

Uh oh!

mlbridge bot commented Mar 4, 2024

Webrevs

Uh oh!

theRealAph left a comment

Choose a reason for hiding this comment

Uh oh!

openjdk bot commented Mar 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vnkozlov left a comment

Choose a reason for hiding this comment

Uh oh!

lgxbslgx left a comment

Choose a reason for hiding this comment

Uh oh!

eme64 commented Mar 5, 2024

Uh oh!

openjdk bot commented Mar 5, 2024

Uh oh!

openjdk bot commented Mar 5, 2024

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

eme64 commented Mar 4, 2024 •

edited by openjdk bot

Loading

eme64 commented Mar 4, 2024 •

edited

Loading

eme64 commented Mar 4, 2024 •

edited

Loading

openjdk bot commented Mar 4, 2024 •

edited

Loading