8341834: C2 compilation fails with "bad AD file" due to Replicate

[rwestrel]

Superword creates a Replicate node at a ConvL2I node and uses the type of the result of the ConvL2I to pick the type of the Replicate instead of the type of the input to the ConvL2I.

---

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-8341834: C2 compilation fails with "bad AD file" due to Replicate (Bug - P3)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 21660

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

Using diff file

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

Webrev

Link to Webrev Comment

[bridgekeeper[bot]]

:wave: Welcome back roland! 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[bot]]

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

8341834: C2 compilation fails with "bad AD file" due to Replicate

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

• 16feeb7a8e9a02fec4846179fcfbdc3a71b07fe5: 8343547: Restore accidentally removed annotations in LambdaForm from ClassFile API port
• f62fc4844125cc20a91dc2be39ba05a2d3aca8cf: 8342498: Add test for Allocation elimination after use as alignment reference by SuperWord
• f3671beefb3ff07441a905e25619f0d1a0a2fe15: 8335392: C2 MergeStores: enhanced pointer parsing
• 4fc6d4135e795d18a024a6035908f380b81082d1: 8341194: [REDO] Implement C2 VectorizedHashCode on AArch64
• abf2dc7128fc0644e85bca32d8f3beacc876cecb: 8343298: Improve stability of runtime/cds/DeterministicDump.java test
• dafa2e55adb6b054c342d5e723e51087d771e6d6: 8343124: Tests fails with java.lang.IllegalAccessException: class com.sun.javatest.regtest.agent.MainWrapper$MainTask cannot access
• 0f7dd98d9d546e0fc2c7b1df779cef35e5b5852c: 8251926: PPC: Remove an unused variable in assembler_ppc.cpp
• cd91a44500e83f84e8e9ecc2760552dd18860842: 8343549: SeededSecureRandomTest needn't be in a package
• 20f3aaff4470745ff082bc562f4e4e72044090b2: 8343471: RISC-V: compiler/cpuflags/TestAESIntrinsicsOnUnsupportedConfig.java fails after JDK-8334999
• 67907d5e8985ee47ddadb51dae1220404a18dd47: 8343500: Optimize ArrayClassDescImpl computeDescriptor
• ... and 13 more: https://git.openjdk.org/jdk/compare/895a7b64f01dec7248549b127875edcf006457cf...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[bot]]

@rwestrel 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[bot]]

Webrevs

• 02: Full - Incremental (9219a292)
• 01: Full - Incremental (1070696f)
• 00: Full (4678caf2)

[TobiHartmann]

Is this a regression from JDK-8332163 or JDK-8248830?

[eme64]

Hi Roland,

Isn't it a little strange that we have a Replicate before a ConvL2I pack? That means that the ConvL2I did not common, even though they have the same inputs. I guess that is due to the type not being identical - a rather rare case.

 Pack: 8
    0:  739  ConvL2I  === _ 742  [[ 738 ]]  #int:2..99:www !orig=612,467,407,[138],[156] !jvms: Test4::test @ bci:41 (line 11)
    1:  741  ConvL2I  === _ 742  [[ 740 ]]  #int:2..198:www !orig=613,468,440
 Pack: 9
    0:  755  ConvL2I  === _ 756  [[ 738 ]]  #int:2..109:www !orig=594,458,[399],137 !jvms: Test4::test @ bci:37 (line 10)
    1:  754  ConvL2I  === _ 756  [[ 740 ]]  #int:2..208:www !orig=591,514

I played around with the test case as well, reducing it further:

./java -XX:CompileCommand=quiet -XX:CompileCommand=compileonly,Test4::test -Xcomp -XX:+TraceSuperWord -XX:+TraceNewVectors -XX:UseAVX=2 Test4.java

public class Test4 {
    public static long val = 0;

    public static void test(int x) {
        x = Math.max(0, Math.min(10, x)); // type 0..10
        short a[] = new short[500];

        for (long l = 0; l < 100; l++) {
            val = l + x; // store seems required, hmm
            int y = (int)val;
            int z = (int)l; // this becomes multiple ConvL2I
            a[z] = (short)(z - y);
        }
    }

    public static void main(String[] args) {
        Math.min(0, 1);
        Math.max(0, 1);
        test(0);
    }
}

[eme64]

The fix seems correct, but I fear that maybe older versions could be affected, it would just be very difficult to create that ConvL2I pack.

[eme64]

I would add a comment in the VM code. The one-liner is a little dense to read.

Basically the issue is that velt_basic_type(p0) gives us the output type of p0. But what we need is the input type of p0 - this is what we are replicating for.

Now I'm wondering if there are any other p0 nodes that have diverging input/output type?

[eme64]

I found these:

// Java API for Long.bitCount/numberOfLeadingZeros/numberOfTrailingZeros
// returns int type, but Vector API for them returns long type. To unify
// the implementation in backend, AutoVectorization splits the vector
// implementation for Java API into an execution node with long type plus
// another node converting long to int.
bool VectorNode::is_scalar_op_that_returns_int_but_vector_op_returns_long(int opc) {
  switch (opc) {
    case Op_PopCountL:
    case Op_CountLeadingZerosL:
    case Op_CountTrailingZerosL:
      return true;
    default:
      return false;
  }
}

But they are single-input ops, so if they have the same inputs, they would common, and not create a pack with an input replicate node.

Hmm. I tried to play with MulAddS2I, but so far no success with getting an example that vectorizes with Replicate...

Do you think there are any other cases than Conv where input and output do not match?

[rwestrel]

Do you know what JDK versions are affected?

The failure doesn't reproduce with jdk21u. But that seems to be because we need JDK-8326139 (and JDK-8331575) for the bug to show up.

[rwestrel]

/integrate

[openjdk[bot]]

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

• 57c3bb6091f8ba0caced6f5ecf21dc998ffeee9f: 8343068: C2: CastX2P Ideal transformation not always applied
• 83f3d42d6bcefac80449987f4d951f8280eeee3a: 8339303: C2: dead node after failing to match cloned address expression
• ead0116f2624e0e34529e47e4f509142d588b994: 8331341: secondary_super_cache does not scale well: C1 and interpreter
• 06d8216a4ef6b883119459da7e52b37d16cd2f03: 8318442: java/net/httpclient/ManyRequests2.java fails intermittently on Linux
• bdd68163df4d9b63694bfc0900e4b5ddb2475834: 8343502: RISC-V: SIGBUS in updateBytesCRC32 after JDK-8339738
• 4431852a880b06241231d346311170331c20ab2d: 8342943: Replace predicate walking and cloning code for main/post loops with a predicate visitor
• 1b0281dc77f41fc5df323c7f7b25a4138b1ffb9e: 8333427: langtools/tools/javac/newlines/NewLineTest.java is failing on Japanese Windows
• 471f112bca715d04304cbe35c6ed63df8c7b7fee: 8342577: Clean up JVMTI breakpoint support
• 69bc0887741a7dd7eda234f5b3252c3c5e46d87e: 8343071: Broken anchors to restricted method page and some redundant ids
• d4d9831c9075c1a157d8375e6902bfc6c731389a: 8340454: C2 EA asserts with "previous reducible Phi is no longer reducible before SUT"
• ... and 34 more: https://git.openjdk.org/jdk/compare/895a7b64f01dec7248549b127875edcf006457cf...master

Your commit was automatically rebased without conflicts.

[openjdk[bot]]

@rwestrel Pushed as commit 72a45ddbad9c343200197348ccfcf74105e6fefa.

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

[shqking]

Hi @rwestrel

My JBS account is inactive recently. Hence I'd like to report the bug here.

I encountered the following error with -XX:MaxVectorSize=8 on both AArch64 and x86_64. Could you help take a look at this issue? Thanks.

Test command:

make test JTREG="VM_OPTIONS=-XX:MaxVectorSize=8" TEST=test/hotspot/jtreg/compiler/vectorization/TestReplicateAtConv.java

Error message:

CompileCommand: compileonly TestReplicateAtConv.test bool compileonly = true            
#                                                                                       
# A fatal error has been detected by the Java Runtime Environment:
#
#  Internal Error (/tmp/jdk-dev/src/hotspot/share/opto/type.cpp:2499), pid=1424540, tid=1424557
#  assert(Matcher::vector_size_supported(elem_bt, length)) failed: length in range
#
# JRE version: OpenJDK Runtime Environment (24.0) (fastdebug build 24-internal-git-63c19d3db58)
# Java VM: OpenJDK 64-Bit Server VM (fastdebug 24-internal-git-63c19d3db58, compiled mode, sharing, tiered, compressed oops, compressed class ptrs, g1 gc, linux-aarch64)
# Problematic frame:
# V  [libjvm.so+0x17bca30]  TypeVect::make(BasicType, unsigned int, bool)+0x150
#
# Core dump will be written. Default location: Core dumps may be processed with "/usr/share/apport/apport -p%p -s%s -c%c -d%d -P%P -u%u -g%g -- %E" (or dumping to /tmp/jdk-build/test-support/jtreg_test_hotspot_jtreg_compiler_vectorization_TestReplicateAtConv_java/scratch/0/core.1424540)
#
# An error report file with more information is saved as:
# /tmp/jdk-build/test-support/jtreg_test_hotspot_jtreg_compiler_vectorization_TestReplicateAtConv_java/scratch/0/hs_err_pid1424540.log
#
# Compiler replay data is saved as:
# /tmp/jdk-build/test-support/jtreg_test_hotspot_jtreg_compiler_vectorization_TestReplicateAtConv_java/scratch/0/replay_pid1424540.log
#
# If you would like to submit a bug report, please visit:
#   https://bugreport.java.com/bugreport/crash.jsp

[rwestrel]

@shqking thanks for the report. I filed https://bugs.openjdk.org/browse/JDK-8343747