8352020: [CompileFramework] enable compilation for VectorAPI #24082

eme64 · 2025-03-17T15:53:17Z

During work on JDK-8344942 I discovered that it is currently not possible to compile VectorAPI code because it is still in incubator mode and needs flag "--add-modules=jdk.incubator.vector" for "javac".

Also: "javac" can produce warnings, and that leads to issues like this: JDK-8351998. We should allow such warnings, they are not compile failures.

Example:

javac --add-modules=jdk.incubator.vector Test.java
warning: [incubating] using incubating module(s): jdk.incubator.vector
1 warning

I added an example test as well.

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-8352020: [CompileFramework] enable compilation for VectorAPI (Enhancement - P4)

Reviewers

Vladimir Kozlov (@vnkozlov - Reviewer) Review applies to 3b9eb4ce
Christian Hagedorn (@chhagedorn - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 24082

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

Using diff file

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

Using Webrev

Link to Webrev Comment

bridgekeeper · 2025-03-17T15:54:17Z

👋 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 · 2025-03-17T15:55:06Z

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

8352020: [CompileFramework] enable compilation for VectorAPI

Reviewed-by: chagedorn, 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 10 new commits pushed to the master branch:

38499b3: 8352084: Add more test code in TestSetupAOT.java
19154f7: 8351970: Retire JavaLangAccess::exit
3aa6d62: 8351372: Improve negative tests coverage of jpackage
3239919: 8350835: C2 SuperWord: assert/wrong result when using Float.float16ToFloat with byte instead of short input
47c1960: 8351689: -Xshare:dump with default classlist fails on static JDK
6b82b42: 8348598: Update Libpng to 1.6.47
2674a31: 8351891: Disable TestBreakSignalThreadDump.java#with_jsig and XCheckJSig.java on static JDK
4c6a523: 8352096: Test jdk/jfr/event/profiling/TestFullStackTrace.java shouldn't be executed with -XX:+DeoptimizeALot
d68775d: 8351995: JFR: Leftovers from removal of Security Manager
e62becc: 8350964: Add an ArtifactResolver.fetch(clazz) method

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch.
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-03-17T15:55:49Z

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

mlbridge · 2025-03-17T16:03:10Z

Webrevs

01: Full - Incremental (fb4cd1c2)
00: Full (3b9eb4ce)

vnkozlov

Very good.

chhagedorn

Otherwise, it looks good to me, too.

chhagedorn · 2025-03-17T20:16:59Z

test/hotspot/jtreg/compiler/lib/compile_framework/Compile.java

+        // Note: the output can be non-empty even if the compilation succeeds, e.g. for warnings.
+        if (exitCode != 0) {


Would -XX:-PrintWarnings also work?

Maybe... but I don't want to risk it. There have recently been a few sightings where javac printed some messages, see JDK-8351998. I think it's just not worth it to fail if it prints anything. Exit code should be sufficient.

Agreed, it's safer.

chhagedorn · 2025-03-17T20:21:17Z

...spot/jtreg/testlibrary_tests/compile_framework/examples/IRFrameworkWithVectorAPIExample.java

+
+/**
+ * This test shows that the IR verification can be done on code compiled by the Compile Framework.
+ * The "@compile" command for JTREG is required so that the IRFramework is compiled, other javac


What about a CompileFilework::invokeIRTest() or something like that, that just additionally loads the TestFramework class somehow (for example just creating an instance or accessing a field etc., we could also provide an empty static method in TestFramework.java that can be called to minimize the overhead), would that work? Then we do not need to worry about adding @compile or figuring out why the IR test is not working.

Maybe... but is this kind of magic really worth it? It would also mean that any CompileFraework test always compiles the TestFramework. And if we decide to do this, I think it would be a separate RFE, since I'm just copying from test/hotspot/jtreg/testlibrary_tests/compile_framework/examples/IRFrameworkJavaExample.java here ;)

FYI: the issue will probably get worse over time, as there may be more and more test-library parts that are not used in the JTREG test directly, but only in the CompileFramework compiled code. I currently have a test under development where I have to write this:

/* * @test * @summary Test the Template Library's expression generation for the Vector API. * @modules jdk.incubator.vector * @modules java.base/jdk.internal.misc * @library /test/lib / * @compile ../../../compiler/lib/ir_framework/TestFramework.java * @compile ../../../compiler/lib/generators/Generators.java * @compile ../../../compiler/lib/verify/Verify.java * @run driver template_library.examples.TestFuzzVectorAPI */

But I'm not sure which test libraries we should always load... maybe we can address this down the road, when it really becomes cumbersome for people, and we know more what we want?

It would also mean that any CompileFraework test always compiles the TestFramework

I don't think so, wouldn't it only load it when you call invokeIrTest()? So, when you only call something from the TestFramework class there, I think it will only be loaded and initialized when you actually call invokeIrTest():

Object invokeIrTest(String className, String methodName, Object[] args) { TestFramework.loadClass(); return invoke(className, methodName, args); }

And if we decide to do this, I think it would be a separate RFE,

Sure, that's fine. I only just noticed this when reading the comment :-)

It would also mean that any CompileFraework test always compiles the TestFramework

I don't think so, wouldn't it only load it when you call invokeIrTest()?

JTREG would always compile the TestFramework, but the test would not always load the TestFramework class ;)

And if we decide to do this, I think it would be a separate RFE,

Sure, that's fine. I only just noticed this when reading the comment :-)

Ok, good, I'll keep it in mind. I mean it's bothering me a little too, I'm just not sure yet if or how to fix it best. Especially because there are now multiple test-frameworks, and we may want to compile and load any combination of them...

It would also mean that any CompileFraework test always compiles the TestFramework

I don't think so, wouldn't it only load it when you call invokeIrTest()?

JTREG would always compile the TestFramework, but the test would not always load the TestFramework class ;)

Ah yes, you're right, that's true.

And if we decide to do this, I think it would be a separate RFE,

Sure, that's fine. I only just noticed this when reading the comment :-)

Ok, good, I'll keep it in mind. I mean it's bothering me a little too, I'm just not sure yet if or how to fix it best. Especially because there are now multiple test-frameworks, and we may want to compile and load any combination of them...

Yeah, me too - maybe it's worth to revisit this again and discuss possible low-overhead solutions.

But I'm not sure which test libraries we should always load... maybe we can address this down the road, when it really becomes cumbersome for people, and we know more what we want?

That's really a lot of compile statements you need to make sure to add. Let's discuss this again later and go with what we have now.

eme64 · 2025-03-20T06:13:20Z

@vnkozlov @chhagedorn thanks for the reviews!

/integrate

openjdk · 2025-03-20T06:13:52Z

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

a5d06a1: 8352249: Remove incidental whitespace in traditional doc comments
74df384: 8352428: GenShen: Old-gen cycles are still looping
fcc2a24: 8350813: Rendering of bulky sound bank from MIDI sequence can cause OutOfMemoryError
ac3ad03: 8350589: Investigate cleaner implementation of AArch64 ML-DSA intrinsic introduced in JDK-8348561
8a1c85e: 8350898: Shenandoah: Eliminate final roots safepoint
8e999b8: 8351969: Add Public Identifiers to the JDK built-in Catalog
c7f3338: 8352109: java/awt/Desktop/MailTest.java fails in platforms where Action.MAIL is not supported
fed34e4: 8351933: Inaccurate masking of TC subfield decrement in ForkJoinPool
8f64ccc: 8350485: C2: factor out common code in Node::grow() and Node::out_grow()
c2be19c: 8351902: RISC-V: Several tests fail after JDK-8351145
... and 28 more: https://git.openjdk.org/jdk/compare/dbf47d6c6c9573a143e0158a0664dd3bbab8e251...master

Your commit was automatically rebased without conflicts.

openjdk · 2025-03-20T06:14:01Z

@eme64 Pushed as commit 3ed010a.

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

JDK-8352020

Loading
Loading status checks…

0dbcd3e

openjdk bot changed the title ~~JDK-8352020~~ 8352020: [CompileFramework] enable compilation for VectorAPI Mar 17, 2025

openjdk bot added the hotspot-compiler label Mar 17, 2025

whitespace

Loading
Loading status checks…

3b9eb4c

eme64 marked this pull request as ready for review March 17, 2025 15:58

openjdk bot added the rfr label Mar 17, 2025

vnkozlov approved these changes Mar 17, 2025

View reviewed changes

openjdk bot added the ready label Mar 17, 2025

chhagedorn reviewed Mar 17, 2025

View reviewed changes

typo

Loading
Loading status checks…

fb4cd1c

openjdk bot removed the ready label Mar 18, 2025

chhagedorn approved these changes Mar 18, 2025

View reviewed changes

openjdk bot added the ready label Mar 18, 2025

openjdk bot added the integrated label Mar 20, 2025

openjdk bot closed this Mar 20, 2025

openjdk bot removed ready rfr labels Mar 20, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8352020: [CompileFramework] enable compilation for VectorAPI #24082

8352020: [CompileFramework] enable compilation for VectorAPI #24082

eme64 commented Mar 17, 2025 •

edited by openjdk bot

Loading

bridgekeeper bot commented Mar 17, 2025

openjdk bot commented Mar 17, 2025 •

edited

Loading

openjdk bot commented Mar 17, 2025

mlbridge bot commented Mar 17, 2025 •

edited

Loading

vnkozlov left a comment

chhagedorn left a comment

chhagedorn Mar 17, 2025

eme64 Mar 18, 2025

chhagedorn Mar 18, 2025

chhagedorn Mar 17, 2025

eme64 Mar 18, 2025 •

edited

Loading

eme64 Mar 18, 2025

chhagedorn Mar 18, 2025 •

edited

Loading

eme64 Mar 18, 2025

chhagedorn Mar 18, 2025 •

edited

Loading

eme64 commented Mar 20, 2025

openjdk bot commented Mar 20, 2025

openjdk bot commented Mar 20, 2025

		// Note: the output can be non-empty even if the compilation succeeds, e.g. for warnings.
		if (exitCode != 0) {

8352020: [CompileFramework] enable compilation for VectorAPI #24082

8352020: [CompileFramework] enable compilation for VectorAPI #24082

Conversation

eme64 commented Mar 17, 2025 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

bridgekeeper bot commented Mar 17, 2025

openjdk bot commented Mar 17, 2025 • edited Loading

openjdk bot commented Mar 17, 2025

mlbridge bot commented Mar 17, 2025 • edited Loading

Webrevs

vnkozlov left a comment

Choose a reason for hiding this comment

chhagedorn left a comment

Choose a reason for hiding this comment

chhagedorn Mar 17, 2025

Choose a reason for hiding this comment

eme64 Mar 18, 2025

Choose a reason for hiding this comment

chhagedorn Mar 18, 2025

Choose a reason for hiding this comment

chhagedorn Mar 17, 2025

Choose a reason for hiding this comment

eme64 Mar 18, 2025 • edited Loading

Choose a reason for hiding this comment

eme64 Mar 18, 2025

Choose a reason for hiding this comment

chhagedorn Mar 18, 2025 • edited Loading

Choose a reason for hiding this comment

eme64 Mar 18, 2025

Choose a reason for hiding this comment

chhagedorn Mar 18, 2025 • edited Loading

Choose a reason for hiding this comment

eme64 commented Mar 20, 2025

openjdk bot commented Mar 20, 2025

openjdk bot commented Mar 20, 2025

eme64 commented Mar 17, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Mar 17, 2025 •

edited

Loading

mlbridge bot commented Mar 17, 2025 •

edited

Loading

eme64 Mar 18, 2025 •

edited

Loading

chhagedorn Mar 18, 2025 •

edited

Loading

chhagedorn Mar 18, 2025 •

edited

Loading