RFR: 8293833: Error mixing types with -XX:+UseCMoveUnconditionally -XX:+UseVectorCmov [v2]

Fei Gao fgao at openjdk.org
Wed Oct 12 01:54:15 UTC 2022


> After JDK-8139340, JDK-8192846 and JDK-8289422, we can vectorize
> the case below by enabling -XX:+UseCMoveUnconditionally and
> -XX:+UseVectorCmov:
> 
> // double[] a, double[] b, double[] c;
> for (int i = 0; i < a.length; i++) {
>     c[i] = (a[i] > b[i]) ? a[i] : b[i];
> }
> 
> 
> But we don't support the case like:
> 
> // double[] a;
> // int seed;
> for (int i = 0; i < a.length; i++) {
>     a[i] = (i % 2 == 0) ? seed + i : seed - i;
> }
> 
> because the IR nodes for the CMoveD in the loop is:
> 
>   AddI  AndI     AddD   SubD
>      \  /         /     /
>      CmpI        /    /
>        \        /   /
>       Bool     /  /
>           \   / /
>           CMoveD
> 
> 
> and it is not our target pattern, which requires that the inputs
> of Cmp node must be the same as the inputs of CMove node
> as commented in CMoveKit::make_cmovevd_pack(). Because
> we can't vectorize the CMoveD pack, we shouldn't vectorize
> its inputs, AddD and SubD. But the current function
> CMoveKit::make_cmovevd_pack() doesn't clear the unqualified
> CMoveD pack from the packset. In this way, superword wrongly
> vectorizes AddD and SubD. Finally, we get a scalar CMoveD
> node with two vector inputs, AddVD and SubVD, which has
> wrong mixing types, then the assertion fails.
> 
> To fix it, we need to remove the unvectorized CMoveD pack
> from the packset and clear related map info.

Fei Gao has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision:

 - Refine the function and clean up the code style
 - Merge branch 'master' into fg8293833
 - 8293833: Error mixing types with -XX:+UseCMoveUnconditionally -XX:+UseVectorCmov
   
   After JDK-8139340, JDK-8192846 and JDK-8289422, we can vectorize
   the case below by enabling -XX:+UseCMoveUnconditionally and
   -XX:+UseVectorCmov:
   ```
   // double[] a, double[] b, double[] c;
   for (int i = 0; i < a.length; i++) {
       c[i] = (a[i] > b[i]) ? a[i] : b[i];
   }
   ```
   
   But we don't support the case like:
   ```
   // double[] a;
   // int seed;
   for (int i = 0; i < a.length; i++)
       a[i] = (i % 2 == 0) ? seed + i : seed - i;
   }
   ```
   because the IR nodes for the CMoveD in the loop is:
   ```
     AddI  AndI     AddD   SubD
        \  /         /     /
        CmpI        /    /
          \        /   /
         Bool     /  /
             \   / /
             CMoveD
   ```
   
   and it is not our target pattern, which requires that the inputs
   of Cmp node must be the same as the inputs of CMove node as
   commented in CMoveKit::make_cmovevd_pack(). Because we can't
   vectorize the CMoveD pack, we shouldn't vectorize its inputs,
   AddD and SubD. But the current function
   CMoveKit::make_cmovevd_pack() doesn't clear the unqualified
   CMoveD pack from the packset. In this way, superword wrongly
   vectorizes AddD and SubD. Finally, we get a scalar CMoveD node
   with two vector inputs, AddVD and SubVD, which has wrong mixing
   types, then the assertion fails.
   
   To fix it, we need to remove the unvectorized CMoveD pack from
   the packset and clear related map info.

-------------

Changes:
  - all: https://git.openjdk.org/jdk/pull/10627/files
  - new: https://git.openjdk.org/jdk/pull/10627/files/1b615da3..a113675d

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=10627&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=10627&range=00-01

  Stats: 1024 lines in 55 files changed: 486 ins; 230 del; 308 mod
  Patch: https://git.openjdk.org/jdk/pull/10627.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/10627/head:pull/10627

PR: https://git.openjdk.org/jdk/pull/10627


More information about the hotspot-compiler-dev mailing list