RFR: 8325520: Vector loads with offsets incorrectly compiled [v12]
Damon Fenacci
dfenacci at openjdk.org
Tue May 21 13:47:32 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.
> The same is true for `StoreVector`s.
> When running GVN loops we can get to the situation where we check if a Load node is preceded by a Store of the same address to be able to replace the Load with the input of the Store (in `LoadNode::Identity`). Here we call
>
> https://github.com/openjdk/jdk/blob/87e864bf21d71daae4e001ec4edbb4ef1f60c36d/src/hotspot/share/opto/memnode.cpp#L1258
>
> where we do an extra check for types if we deal with vectors but we don’t make sure that neither masks nor offsets interfere.
> Similarly, in `StoreNode::Identity` we first check if there is a Load and then a Store:
>
> https://github.com/openjdk/jdk/blob/87e864bf21d71daae4e001ec4edbb4ef1f60c36d/src/hotspot/share/opto/memnode.cpp#L3509-L3515
>
> but we don’t make sure that there are no masks or offsets.
> A few lines below, we check if there are 2 stores for the same value in a row. We need to check for masks and offsets here too but in this case we can include these cases if the masks and offsets of the vector stores are equivalent.
>
> # Solution
> To avoid folding `Load`- and `StoreVector`s with masks and offsets we add a specific `store_Opcode` method to `LoadVectorGatherNode`, `LoadVectorMaskedNode` and `LoadVectorGatherMaskedNode` that doesn’t return a store opcode but instead returns its own (to avoid ever being the same as a store node). In this way, the checks in `MemNode::can_see_stored_value`
>
> https://github.com/openjdk/jdk/blob/87e864bf21d71daae4e001ec4edbb4ef1f60c36d/src/hotspot/share/opto/memnode.cpp#L1164-L1166
>
> and `StoreNode::Identity`
>
> https://github.com/openjdk/jdk/blob/87e864bf21d71daae4e001ec4edbb4ef1f60c36d/src/hotspot/share/opto/memnode.cpp#L3509-L3515
>
> will fail if masks or offsets are used.
> For 2 stores of the same value we instead check for mask and offset equality.
>
> Regression tests for all versions of `Load/StoreVectorGather/Masked` have been add...
Damon Fenacci has updated the pull request incrementally with one additional commit since the last revision:
JDK-8325520: add override keyword
-------------
Changes:
- all: https://git.openjdk.org/jdk/pull/18347/files
- new: https://git.openjdk.org/jdk/pull/18347/files/e676bcb1..df3a49ae
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=18347&range=11
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=18347&range=10-11
Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 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