8329273: C2 SuperWord: Some basic MemorySegment IR tests #18535
Conversation
|
👋 Welcome back epeter! A progress list of the required criteria for merging this PR into |
|
@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: 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 1 new commit pushed to the
Please see this link for an up-to-date comparison between the source branch of this pull request and the ➡️ To integrate this PR with the above commit message to the |
openjdk
bot
changed the title
openjdk
bot
changed the title
|
@eme64 This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration! |
Webrevs
|
|
|
||
| /* | ||
| * @test id=byte-array | ||
| * @bug 8310190 |
There was a problem hiding this comment.
Should be updated to 8329273. Same for other runs
| static { | ||
| String providerName = System.getProperty("memorySegmentProviderNameForTestVM"); | ||
| provider = switch (providerName) { | ||
| case "ByteArray" -> ( () -> { return newMemorySegmentOfByteArray(); } ); | ||
| case "CharArray" -> ( () -> { return newMemorySegmentOfCharArray(); } ); | ||
| case "ShortArray" -> ( () -> { return newMemorySegmentOfShortArray(); } ); | ||
| case "IntArray" -> ( () -> { return newMemorySegmentOfIntArray(); } ); | ||
| case "LongArray" -> ( () -> { return newMemorySegmentOfLongArray(); } ); | ||
| case "FloatArray" -> ( () -> { return newMemorySegmentOfFloatArray(); } ); | ||
| case "DoubleArray" -> ( () -> { return newMemorySegmentOfDoubleArray(); } ); | ||
| case "ByteBuffer" -> ( () -> { return newMemorySegmentOfByteBuffer(); } ); | ||
| case "ByteBufferDirect" -> ( () -> { return newMemorySegmentOfByteBufferDirect(); } ); | ||
| case "Native" -> ( () -> { return newMemorySegmentOfNative(); } ); | ||
| case "MixedArray" -> ( () -> { return newMemorySegmentOfMixedArray(); } ); | ||
| case "MixedBuffer" -> ( () -> { return newMemorySegmentOfMixedBuffer(); } ); | ||
| case "Mixed" -> ( () -> { return newMemorySegmentOfMixed(); } ); | ||
| default -> throw new RuntimeException("Test argument not recognized: " + providerName); | ||
| }; | ||
| } |
There was a problem hiding this comment.
As discussed offline, this is an interesting workaround. Maybe the IR framework could be extended at some point to simplify this.
| @IR(counts = {IRNode.LOAD_VECTOR_B, "= 0", | ||
| IRNode.ADD_VB, "= 0", | ||
| IRNode.STORE_VECTOR, "= 0"}, |
There was a problem hiding this comment.
You should use failOn instead of = 0. Same for other tests.
There was a problem hiding this comment.
I honestly prefer "= 0", because it is easier to flip to "> 0", and keeps the same style that way. But I guess that is really a matter of taste.
| static { | ||
| String providerName = System.getProperty("memorySegmentProviderNameForTestVM"); | ||
| provider = switch (providerName) { | ||
| case "ByteArray" -> ( () -> { return newMemorySegmentOfByteArray(); } ); |
There was a problem hiding this comment.
You can directly use an expression lambda without return:
case "ByteArray" -> (() -> newMemorySegmentOfByteArray());
But I think you can go even further and directly use a method reference:
| case "ByteArray" -> ( () -> { return newMemorySegmentOfByteArray(); } ); | |
| case "ByteArray" -> (TestMemorySegmentImpl::newMemorySegmentOfByteArray); |
Same for others.
test/hotspot/jtreg/compiler/loopopts/superword/TestMemorySegment.java
Outdated
Show resolved
Hide resolved
| tests.put("testMemorySegmentBadExitCheck", () -> { | ||
| return testMemorySegmentBadExitCheck(copy(a)); | ||
| }); |
There was a problem hiding this comment.
Same as above, you can replace this with an expression lambda:
| tests.put("testMemorySegmentBadExitCheck", () -> { | |
| return testMemorySegmentBadExitCheck(copy(a)); | |
| }); | |
| tests.put("testIntLoop_longIndex_intInvar_sameAdr_byte", | |
| () -> testIntLoop_longIndex_intInvar_sameAdr_byte(copy(a), 0)); |
Same for others.
test/hotspot/jtreg/compiler/loopopts/superword/TestMemorySegment.java
Outdated
Show resolved
Hide resolved
test/hotspot/jtreg/compiler/loopopts/superword/TestMemorySegment.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Christian Hagedorn <christian.hagedorn@oracle.com>
…com/eme64/jdk into JDK-8329273-memory-segment-ir-tests
|
|
|
Thanks for the reviews @vnkozlov @chhagedorn ! |
|
Going to push as commit c4867c6.
Your commit was automatically rebased without conflicts. |
openjdk
bot
removed
ready
I could not find any IR vectorization tests for
MemorySegmentyet.I make sure to exercise different backing types:
I filed a follow-up RFE, to eventually make all cases where I have "FAILS" vectorize:
JDK-8331659: C2 SuperWord: investicate failed vectorization in compiler/loopopts/superword/TestMemorySegment.java
Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/18535/head:pull/18535$ git checkout pull/18535Update a local copy of the PR:
$ git checkout pull/18535$ git pull https://git.openjdk.org/jdk.git pull/18535/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 18535View PR using the GUI difftool:
$ git pr show -t 18535Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/18535.diff
Webrev
Link to Webrev Comment