8371297: C2: assert triggered in BoolTest::BoolTest #28141

Hamlin-Li · 2025-11-04T22:18:13Z

Hi,
Can you help to review this patch?

Not sure how this one slipped in (#28047), I think it passed my local test and github CI test at that time.
Please check https://bugs.openjdk.org/browse/JDK-8371297 for details.

Thanks!

Progress

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

Issue

JDK-8371297: C2: assert triggered in BoolTest::BoolTest (Bug - P3)

Reviewers

Dean Long (@dean-long - Reviewer)
Ludovic Henry (@luhenry - Committer)
Emanuel Peter (@eme64 - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 28141

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

Using diff file

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

Using Webrev

Link to Webrev Comment

bridgekeeper · 2025-11-04T22:19:20Z

👋 Welcome back mli! 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 · 2025-11-04T22:20:54Z

@Hamlin-Li 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:

8371297: C2: assert triggered in BoolTest::BoolTest

Reviewed-by: dlong, luhenry, epeter

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

cbd77fc: 8370244: [PPC64] Several vector tests fail on Power8
2902436: 8371019: G1: Support heap expansion during startup
2910032: 8370966: Create regression test for the hierarchical memory limit fix in JDK-8370572
... and 98 more: https://git.openjdk.org/jdk/compare/325082302266f25d4fac33d0d4a9492c72de3ffc...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.

openjdk · 2025-11-04T22:22:11Z

@Hamlin-Li 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 · 2025-11-05T09:23:33Z

Webrevs

00: Full (0c877e26)

dean-long · 2025-11-05T21:20:46Z

src/hotspot/share/opto/vtransform.cpp

+  BoolTest::mask m = BoolTest::mask(_test._mask & ~BoolTest::unsigned_compare);
+  const BoolTest bt(m);
+  tty->print(" test=%s", m == _test._mask ? "" : "unsigned ");
  bt.dump_on(tty);


I was wondering why we pass the raw mask around instead of keeping it encapsulated in a BoolTest object. Elsewhere I saw code like this:
cond->get_con() & (BoolTest::unsigned_compare - 1)
which seems to be making fragile assumptions about BoolTest internals.

@dean-long Yes, I have the same feeling that BoolTest is currently used in a fragile way. The reasons could be, BoolTest itself is by design a struct and expose all its status, and unsigned_compare is indeed not formally supported but needed somewhere e.g. in vector intrinsic, and auto-vectorization (after #28047).
I think it's worth to do more investigation about the refactoring of BoolTest. Besides of several places using unsigned_compare, there are more places using signed BoolTest, it might be helpful to do it in another specific pr, so file https://bugs.openjdk.org/browse/JDK-8371396 to track it, feel free to take it if you already have a solution or idea.

This issue (in fact it's #27942) blocks several other prs in my backlog for a while, e.g. #25336, #25341. It helps to resolve this assert in a quick (although ugly) way. Please kindly let me know how you think about it. Thanks!

dean-long

OK, I'm fine with cleaning this up as a separate issue.

dean-long · 2025-11-07T00:01:05Z

/reviewers 2

openjdk · 2025-11-07T00:02:30Z

@dean-long
The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 1 Reviewer, 1 Author).

Hamlin-Li · 2025-11-07T09:05:53Z

OK, I'm fine with cleaning this up as a separate issue.

@dean-long Thank you for reviewing!

Hamlin-Li · 2025-11-07T14:00:37Z

@luhenry Thank you for reviewing!

Hamlin-Li · 2025-11-07T14:01:44Z

@eme64 Can you have a look? Thanks! :)

eme64

Looks hacky but reasonable for now. Though maybe it would be nicer if we had some printing method that works directly on the mask? Or some other way of passing around the mask.

eme64 · 2025-11-11T14:48:35Z

@Hamlin-Li You have a PR title mismatch. And you should always give a quick description about what went wrong in your PR description ;)

Hamlin-Li · 2025-11-11T14:53:37Z

Looks hacky but reasonable for now. Though maybe it would be nicer if we had some printing method that works directly on the mask? Or some other way of passing around the mask.

Yes, you're right. I created https://bugs.openjdk.org/browse/JDK-8371396 to track it.

Hamlin-Li · 2025-11-11T14:54:30Z

@Hamlin-Li You have a PR title mismatch. And you should always give a quick description about what went wrong in your PR description ;)

Thank you for reminding and reviewing! I'll pay attention later. :)

Hamlin-Li · 2025-11-11T14:54:48Z

/integrate

openjdk · 2025-11-11T14:56:24Z

Going to push as commit 405d5f7.
Since your change was applied there have been 101 commits pushed to the master branch:

cbd77fc: 8370244: [PPC64] Several vector tests fail on Power8
2902436: 8371019: G1: Support heap expansion during startup
2910032: 8370966: Create regression test for the hierarchical memory limit fix in JDK-8370572
... and 98 more: https://git.openjdk.org/jdk/compare/325082302266f25d4fac33d0d4a9492c72de3ffc...master

Your commit was automatically rebased without conflicts.

openjdk · 2025-11-11T14:56:33Z

@Hamlin-Li Pushed as commit 405d5f7.

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

hamlin and others added 18 commits September 12, 2025 09:04

initial commit

e27247b

Merge branch 'openjdk:master' into master

4a75e87

Merge branch 'openjdk:master' into master

4ee1df1

Merge branch 'openjdk:master' into master

0ff5e42

Merge branch 'openjdk:master' into master

924c4c9

Merge branch 'openjdk:master' into master

75dee02

Merge branch 'openjdk:master' into master

57973f4

Merge branch 'openjdk:master' into master

4b058ce

Merge branch 'openjdk:master' into master

b73a502

Merge branch 'openjdk:master' into master

8eba0c0

Merge branch 'openjdk:master' into master

7f36f23

Merge branch 'openjdk:master' into master

3089ec9

Merge branch 'openjdk:master' into master

2238d76

Merge branch 'openjdk:master' into master

c0358cf

Merge branch 'openjdk:master' into master

f54562f

Merge branch 'openjdk:master' into master

6635678

Merge branch 'openjdk:master' into master

736425c

initial commit

0c877e2

openjdk bot added the hotspot-compiler [email protected] label Nov 4, 2025

Hamlin-Li marked this pull request as ready for review November 5, 2025 09:17

openjdk bot added the rfr Pull request is ready for review label Nov 5, 2025

dean-long reviewed Nov 5, 2025

View reviewed changes

dean-long approved these changes Nov 7, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Nov 7, 2025

openjdk bot removed the ready Pull request is ready to be integrated label Nov 7, 2025

luhenry approved these changes Nov 7, 2025

View reviewed changes

Hamlin-Li mentioned this pull request Nov 11, 2025

8357554: Enable vectorization of Bool -> CMove with different type size (on riscv) #28231

Open

3 tasks

eme64 approved these changes Nov 11, 2025

View reviewed changes

Hamlin-Li changed the title ~~8371297: C2: assert triggerred in BoolTest::BoolTest~~ 8371297: C2: assert triggered in BoolTest::BoolTest Nov 11, 2025

openjdk bot added the ready Pull request is ready to be integrated label Nov 11, 2025

openjdk bot added the integrated Pull request has been integrated label Nov 11, 2025

openjdk bot closed this Nov 11, 2025

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Nov 11, 2025

8371297: C2: assert triggered in BoolTest::BoolTest #28141

8371297: C2: assert triggered in BoolTest::BoolTest #28141

Uh oh!

Conversation

Hamlin-Li commented Nov 4, 2025 • 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 4, 2025

Uh oh!

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

Uh oh!

Uh oh!

openjdk bot commented Nov 4, 2025

Uh oh!

mlbridge bot commented Nov 5, 2025

Webrevs

Uh oh!

dean-long Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

Hamlin-Li Nov 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dean-long left a comment

Choose a reason for hiding this comment

Uh oh!

dean-long commented Nov 7, 2025

Uh oh!

openjdk bot commented Nov 7, 2025

Uh oh!

Hamlin-Li commented Nov 7, 2025

Uh oh!

Hamlin-Li commented Nov 7, 2025

Uh oh!

Hamlin-Li commented Nov 7, 2025

Uh oh!

eme64 left a comment

Choose a reason for hiding this comment

Uh oh!

eme64 commented Nov 11, 2025

Uh oh!

Hamlin-Li commented Nov 11, 2025

Uh oh!

Hamlin-Li commented Nov 11, 2025

Uh oh!

Hamlin-Li commented Nov 11, 2025

Uh oh!

openjdk bot commented Nov 11, 2025

Uh oh!

openjdk bot commented Nov 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

Hamlin-Li commented Nov 4, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Nov 4, 2025 •

edited

Loading

Hamlin-Li Nov 6, 2025 •

edited

Loading