jdk icon indicating copy to clipboard operation
jdk copied to clipboard

8342768: GTest AssemblerX86.validate_vm failed: assert(VM_Version::supports_bmi1()) failed: tzcnt instruction not supported

Open hanklo6 opened this issue 1 year ago • 9 comments

The tzcnt instruction requires the VM to support bmi1 feature, which we set directly when running the test.


Progress

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

Issue

  • JDK-8342768: GTest AssemblerX86.validate_vm failed: assert(VM_Version::supports_bmi1()) failed: tzcnt instruction not supported (Bug - P3)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 21644

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

Using diff file

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

Webrev

Link to Webrev Comment

hanklo6 avatar Oct 22 '24 17:10 hanklo6

:wave: Welcome back hanklo6! 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.

bridgekeeper[bot] avatar Oct 22 '24 17:10 bridgekeeper[bot]

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

8342768: GTest AssemblerX86.validate_vm failed: assert(VM_Version::supports_bmi1()) failed: tzcnt instruction not supported

Reviewed-by: sviswanathan, thartmann, kvn

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

  • 3c14c2babbdfb46a77636ed80e083ef2f8be2b45: 8341566: Add Reader.of(CharSequence)
  • b0ac633b2d0076d64b463b2a6ce19abf6b12c50f: 8342075: HttpClient: improve HTTP/2 flow control checks
  • 85774b713edf8782f162ac25b61ce99a77e116f4: 8342882: RISC-V: Unify handling of jumps to runtime
  • 2c31c8eeb42188ad6fd15eca50db4342cd791fb2: 8339730: Windows regression after removing ObjectMonitor Responsible
  • f0b130e54f33d3190640ce33c991e35f27e9f812: 8339296: Record deconstruction pattern in switch fails to compile
  • e96b4cf0a81914c6a615bb4f62ea3f139a4737f3: 8342387: C2 SuperWord: refactor and improve compiler/loopopts/superword/TestDependencyOffsets.java
  • f7a61fce949ea2d15ec09d295c643d0c2eabea1e: 8342931: ProblemList failing tests from JDK-8335912
  • 25c2f48d458bfd92423c311a887679ad3e1e4041: 8338544: Dedicated Array class descriptor implementation
  • 158b93d19a518d2b9d3d185e2d4c4dbff9c82aab: 8335912: Add an operation mode to the jar command when extracting to not overwriting existing files
  • 28d23ada6dde007ed60b8538cc159afc62d76db3: 8340177: Malformed system classes loaded by bootloader crash the JVM in product builds
  • ... and 19 more: https://git.openjdk.org/jdk/compare/c61d2c5a3403ad5f48daad22bdfb031f42bc3dd5...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.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@sviswa7, @TobiHartmann, @vnkozlov) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

openjdk[bot] avatar Oct 22 '24 17:10 openjdk[bot]

@hanklo6 The following label will be automatically applied to this pull request:

  • hotspot

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.

openjdk[bot] avatar Oct 22 '24 17:10 openjdk[bot]

Webrevs

mlbridge[bot] avatar Oct 22 '24 17:10 mlbridge[bot]

@dholmes-ora Could you please verify that the test now passes on your MacPro6?

sviswa7 avatar Oct 22 '24 18:10 sviswa7

There is an other issue found with this test: https://bugs.openjdk.org/browse/JDK-8342862

vnkozlov avatar Oct 23 '24 00:10 vnkozlov

Question: why we run this test on not APX machines? The original PR was "for testing APX encoding of extended gpr instructions".

vnkozlov avatar Oct 23 '24 00:10 vnkozlov

@dholmes-ora Could you please verify that the test now passes on your MacPro6?

I will test it once it seems to be in a final state.

dholmes-ora avatar Oct 23 '24 07:10 dholmes-ora

"This test is including a huge, generated file asmtest.out.h. I'm guessing this is the culprit of the long compile time."

May be the test should be compiled conditionally to avoid affect on JVM build.

vnkozlov avatar Oct 23 '24 15:10 vnkozlov

What is holding this PR?

vnkozlov avatar Oct 23 '24 22:10 vnkozlov

I will submit this to our CI to see if we can verify it on the macMini

dholmes-ora avatar Oct 24 '24 07:10 dholmes-ora

Not 100% sure I hit the same machine but it passed on a number of different mac systems so ... worth a shot. Thanks

dholmes-ora avatar Oct 24 '24 10:10 dholmes-ora

Let's integrate this asap to get our testing back to a clean state.

TobiHartmann avatar Oct 24 '24 12:10 TobiHartmann

/integrate

hanklo6 avatar Oct 24 '24 16:10 hanklo6

@hanklo6 Your change (at version d564a588763c618647cd93546c308209cc87dc2e) is now ready to be sponsored by a Committer.

openjdk[bot] avatar Oct 24 '24 16:10 openjdk[bot]

/sponsor

sviswa7 avatar Oct 24 '24 16:10 sviswa7

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

  • 3c14c2babbdfb46a77636ed80e083ef2f8be2b45: 8341566: Add Reader.of(CharSequence)
  • b0ac633b2d0076d64b463b2a6ce19abf6b12c50f: 8342075: HttpClient: improve HTTP/2 flow control checks
  • 85774b713edf8782f162ac25b61ce99a77e116f4: 8342882: RISC-V: Unify handling of jumps to runtime
  • 2c31c8eeb42188ad6fd15eca50db4342cd791fb2: 8339730: Windows regression after removing ObjectMonitor Responsible
  • f0b130e54f33d3190640ce33c991e35f27e9f812: 8339296: Record deconstruction pattern in switch fails to compile
  • e96b4cf0a81914c6a615bb4f62ea3f139a4737f3: 8342387: C2 SuperWord: refactor and improve compiler/loopopts/superword/TestDependencyOffsets.java
  • f7a61fce949ea2d15ec09d295c643d0c2eabea1e: 8342931: ProblemList failing tests from JDK-8335912
  • 25c2f48d458bfd92423c311a887679ad3e1e4041: 8338544: Dedicated Array class descriptor implementation
  • 158b93d19a518d2b9d3d185e2d4c4dbff9c82aab: 8335912: Add an operation mode to the jar command when extracting to not overwriting existing files
  • 28d23ada6dde007ed60b8538cc159afc62d76db3: 8340177: Malformed system classes loaded by bootloader crash the JVM in product builds
  • ... and 19 more: https://git.openjdk.org/jdk/compare/c61d2c5a3403ad5f48daad22bdfb031f42bc3dd5...master

Your commit was automatically rebased without conflicts.

openjdk[bot] avatar Oct 24 '24 16:10 openjdk[bot]

@sviswa7 @hanklo6 Pushed as commit d8c3b0f834c603fe115ef4ca442727948b7a834e.

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

openjdk[bot] avatar Oct 24 '24 16:10 openjdk[bot]