8277324: C2 compilation fails with "bad AD file" on x86-32 after JDK-8276162 due to missing match rule #6427

TobiHartmann · 2021-11-17T09:25:26Z

JDK-8276162 introduced an optimization that creates CMoveI (Bool (CmpUL ...) ...) shapes but x86-32 misses the corresponding match rules in C2's backend.

I also fixed two comments incorrectly referring to ints instead of ptrs.

Thanks,
Tobias

Progress

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

Issue

JDK-8277324: C2 compilation fails with "bad AD file" on x86-32 after JDK-8276162 due to missing match rule

Reviewers

Christian Hagedorn (@chhagedorn - Reviewer)
Roland Westrelin (@rwestrel - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/6427/head:pull/6427
$ git checkout pull/6427

Update a local copy of the PR:
$ git checkout pull/6427
$ git pull https://git.openjdk.java.net/jdk pull/6427/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 6427

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/6427.diff

…8276162 due to missing match rule

bridgekeeper · 2021-11-17T09:25:35Z

👋 Welcome back thartmann! 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 · 2021-11-17T09:28:34Z

@TobiHartmann 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.

mlbridge · 2021-11-17T09:35:47Z

Webrevs

01: Full - Incremental (1e55fd19)
00: Full (8b315cdc)

chhagedorn

Otherwise, the fix looks good!

chhagedorn · 2021-11-17T11:12:10Z

src/hotspot/cpu/x86/x86_32.ad

+  format %{ "CMOV$cmp $dst,$src" %}
+  opcode(0x0F,0x40);
+  ins_encode( enc_cmov(cmp), RegReg( dst, src ) );
+  ins_pipe( pipe_cmov_reg );


Since this code is shared with cmovII_reg_LTGE, we could directly use it with expand:

expand %{ cmovII_reg_LTGE(cmp, flags, dst, src); %}

Could also be done for the other cases.

Thanks Christian, that's a good suggestion. I thought expand does not work because the argument types differ but it does. I updated the patch.

Looks good! AFAICT the two operands flagsReg_(u)long_LTGE only differ in their format string which is probably why it works. But maybe someone else can jump in here who knows the code better.

openjdk · 2021-11-17T11:17:53Z

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

8277324: C2 compilation fails with "bad AD file" on x86-32 after JDK-8276162 due to missing match rule

Reviewed-by: chagedorn, roland

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 43 new commits pushed to the master branch:

2f20b0d: 8273039: JShell crashes when naming variable or method "abstract" or "strictfp"
f34f119: 8277213: CompileTask_lock is acquired out of order with MethodCompileQueue_lock
47564ca: 8275643: C2's unaryOp vector intrinsic does not properly handle LongVector.neg
2f0bde1: 8277102: Dubious PrintCompilation output
839033b: 8276976: Rename LIR_OprDesc to LIR_Opr
89b125f: 8275527: Refactor forward pointer access
36bd4a3: 8277404: Test VMDeprecatedOptions.java failing with Unable to create shared archive file
57eb864: 8276927: [PPC64] Port shenandoahgc to linux on ppc64le
8db0c36: 8277414: ProblemList runtime/CommandLine/VMDeprecatedOptions.java on windows-x64
03473b4: 8270874: JFrame paint artifacts when dragged from standard monitor to HiDPI monitor
... and 33 more: https://git.openjdk.java.net/jdk/compare/08f65a59a7bd387974d94253ec7093524a3e92f1...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.

rwestrel

Looks good to me.

TobiHartmann · 2021-11-19T08:22:24Z

Thanks for the reviews, Christian and Roland!

TobiHartmann · 2021-11-19T08:23:02Z

/integrate

openjdk · 2021-11-19T08:24:00Z

Going to push as commit 3a76d39.
Since your change was applied there have been 43 commits pushed to the master branch:

2f20b0d: 8273039: JShell crashes when naming variable or method "abstract" or "strictfp"
f34f119: 8277213: CompileTask_lock is acquired out of order with MethodCompileQueue_lock
47564ca: 8275643: C2's unaryOp vector intrinsic does not properly handle LongVector.neg
2f0bde1: 8277102: Dubious PrintCompilation output
839033b: 8276976: Rename LIR_OprDesc to LIR_Opr
89b125f: 8275527: Refactor forward pointer access
36bd4a3: 8277404: Test VMDeprecatedOptions.java failing with Unable to create shared archive file
57eb864: 8276927: [PPC64] Port shenandoahgc to linux on ppc64le
8db0c36: 8277414: ProblemList runtime/CommandLine/VMDeprecatedOptions.java on windows-x64
03473b4: 8270874: JFrame paint artifacts when dragged from standard monitor to HiDPI monitor
... and 33 more: https://git.openjdk.java.net/jdk/compare/08f65a59a7bd387974d94253ec7093524a3e92f1...master

Your commit was automatically rebased without conflicts.

openjdk · 2021-11-19T08:24:18Z

@TobiHartmann Pushed as commit 3a76d39.

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

8277324: C2 compilation fails with "bad AD file" on x86-32 after JDK-…

8b315cd

…8276162 due to missing match rule

TobiHartmann mentioned this pull request Nov 17, 2021

8276162: Optimise unsigned comparison pattern #6101

Closed

3 tasks

openjdk bot added the rfr Pull request is ready for review label Nov 17, 2021

openjdk bot added the hotspot-compiler [email protected] label Nov 17, 2021

chhagedorn approved these changes Nov 17, 2021

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Nov 17, 2021

Use expand

1e55fd1

chhagedorn approved these changes Nov 17, 2021

View reviewed changes

rwestrel approved these changes Nov 19, 2021

View reviewed changes

openjdk bot closed this Nov 19, 2021

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 Nov 19, 2021

CptGit mentioned this pull request Dec 3, 2021

8277882: New subnode ideal optimization: converting "c0 - (x + c1)" into "(c0 - c1) - x" #6441

Closed

3 tasks

vnkozlov mentioned this pull request Dec 6, 2021

8276116: C2: optimize long range checks in int counted loops #6576

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

8277324: C2 compilation fails with "bad AD file" on x86-32 after JDK-8276162 due to missing match rule #6427

8277324: C2 compilation fails with "bad AD file" on x86-32 after JDK-8276162 due to missing match rule #6427

Uh oh!

TobiHartmann commented Nov 17, 2021 •

edited by openjdk bot

Loading

Uh oh!

bridgekeeper bot commented Nov 17, 2021

Uh oh!

openjdk bot commented Nov 17, 2021

Uh oh!

mlbridge bot commented Nov 17, 2021 •

edited

Loading

Uh oh!

chhagedorn left a comment

Uh oh!

chhagedorn Nov 17, 2021

Uh oh!

TobiHartmann Nov 17, 2021

Uh oh!

chhagedorn Nov 17, 2021

Uh oh!

openjdk bot commented Nov 17, 2021 •

edited

Loading

Uh oh!

rwestrel left a comment

Uh oh!

TobiHartmann commented Nov 19, 2021

Uh oh!

TobiHartmann commented Nov 19, 2021

Uh oh!

openjdk bot commented Nov 19, 2021

Uh oh!

openjdk bot commented Nov 19, 2021

Uh oh!

Uh oh!

8277324: C2 compilation fails with "bad AD file" on x86-32 after JDK-8276162 due to missing match rule #6427

8277324: C2 compilation fails with "bad AD file" on x86-32 after JDK-8276162 due to missing match rule #6427

Uh oh!

Conversation

TobiHartmann commented Nov 17, 2021 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented Nov 17, 2021

Uh oh!

openjdk bot commented Nov 17, 2021

Uh oh!

mlbridge bot commented Nov 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

chhagedorn left a comment

Choose a reason for hiding this comment

Uh oh!

chhagedorn Nov 17, 2021

Choose a reason for hiding this comment

Uh oh!

TobiHartmann Nov 17, 2021

Choose a reason for hiding this comment

Uh oh!

chhagedorn Nov 17, 2021

Choose a reason for hiding this comment

Uh oh!

openjdk bot commented Nov 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rwestrel left a comment

Choose a reason for hiding this comment

Uh oh!

TobiHartmann commented Nov 19, 2021

Uh oh!

TobiHartmann commented Nov 19, 2021

Uh oh!

openjdk bot commented Nov 19, 2021

Uh oh!

openjdk bot commented Nov 19, 2021

Uh oh!

Uh oh!

TobiHartmann commented Nov 17, 2021 •

edited by openjdk bot

Loading

mlbridge bot commented Nov 17, 2021 •

edited

Loading

openjdk bot commented Nov 17, 2021 •

edited

Loading