RFR: 8325520: Vector loads with offsets incorrectly compiled [v3]

Damon Fenacci dfenacci at openjdk.org
Wed Apr 24 10:23:57 UTC 2024


> # Issue
> When loading multiple vectors using offsets or masks (e.g. `LongVector::fromArray(LongVector.SPECIES_256, storage, 0, offsets, 0` or `LongVector::fromArray(LongVector.SPECIES_256, storage, 0, longMask)`) there is an error in the C2 compiled code that makes different vectors be treated as equal even though they are not.
> 
> # Causes
> On vector-capable platforms, vector loads with masks and offsets (for Long, Integer, Float and Double) create specific nodes in the ideal graph (i.e. `LoadVectorGather`, `LoadVectorMasked`, `LoadVectorGatherMasked`). Vector loads without mask or offsets are mapped as `LoadVector` nodes instead.
> While running GVN loops, we can get to the situation in which there are multiple loads from the same address. If successive loads are deemed to be identical to the first one, they might get folded, which is what happens in the problematic examples of this issue. The check for identity happens in
> https://github.com/openjdk/jdk/blob/481c866df87c693a90a1da20e131e5654b084ddd/src/hotspot/share/opto/memnode.cpp#L1253
> This version of `Identity` is defined in `LoadNode` but it is also the one used by the subclasses `LoadVectorGather`, `LoadVectorMasked` and `LoadVectorGatherMasked`. Although this definition of `Identity` is enough for `LoadVector` nodes it is not sufficient for `LoadVectorGather`, `LoadVectorMasked` and `LoadVectorGatherMasked` ones, as the value being loaded also depends on the offsets and mask (different offsets and/or masks load completely different values). This is the reason why these nodes get folded even if they shouldn't.
> 
> # Solution
> `LoadVectorGather`, `LoadVectorMasked` and `LoadVectorGatherMasked` need their own version of the `Identity` method, which specialize `LoadVector::Identity` by restricting the results to nodes that also have the same offsets and masks.
> 
> The same issue exists for _StoreVector_ nodes (i.e. `StoreVectorScatter`, `StoreVectorMasked` and `StoreVectorScatterMasked`). So, `Identity` has to be redefined there as well.
> 
> Regression tests for all versions of `Load/StoreVectorGather/Masked` have been added too.

Damon Fenacci has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 22 commits:

 - Merge branch 'master' into JDK-8325520
 - Update test/hotspot/jtreg/compiler/vectorapi/VectorGatherMaskFoldingTest.java
   
   Co-authored-by: Tobias Hartmann <tobias.hartmann at oracle.com>
 - Update src/hotspot/share/opto/memnode.cpp
   
   Co-authored-by: Emanuel Peter <emanuel.peter at oracle.com>
 - JDK-8325520: fix load gather mask avx condition
 - JDK-8325520: fix tests for small species
 - JDK-8325520: add store tests
 - JDK-8325520: fix copyright notices
 - JDK-8325520: remove trailing whitespaces
 - JDK-8325520: use IR framework in tests
 - JDK-8325520: handle same offsets/masks in store identity
 - ... and 12 more: https://git.openjdk.org/jdk/compare/174d6265...b7e5fe02

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

Changes: https://git.openjdk.org/jdk/pull/18347/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=18347&range=02
  Stats: 1009 lines in 5 files changed: 1006 ins; 0 del; 3 mod
  Patch: https://git.openjdk.org/jdk/pull/18347.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18347/head:pull/18347

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


More information about the hotspot-compiler-dev mailing list