RFR: 8240281: Remove failing assertion code when selecting first memory state in SuperWord::co_locate_pack
Christian Hagedorn
chagedorn at openjdk.java.net
Thu Feb 11 13:03:38 UTC 2021
On Thu, 11 Feb 2021 07:44:30 GMT, Tobias Hartmann <thartmann at openjdk.org> wrote:
>> While working on [JDK-8238438](https://bugs.openjdk.java.net/browse/JDK-8238438), it was not clear if there is a case where `SuperWord::co_locate_pack()` should pick the memory state of the first load of a pack. We could not find an example and therefore added an `assert(false)` and left the code there to clean it up at some point if the assert was never hit.
>>
>> Now, a newly found fuzzer test showed that are cases where we need to pick the first memory state, triggering the `assert(false)`. In the test case `test()`, a store and a load pack are created but the store pack is filtered in `SuperWord::filter_packs()`. The load `x += iArrFld[j]` must read the old value before the store `iArrFld[j] = j` overrides it. Therefore, the load vector must be executed before any of the stores to `iArrFld[j]`. This, however, is not the case if we pick the memory state of the last load which results in wrong values for `iArrFld`: The stores and the loads are dependent and some of the stores are already executed before the load vector.
>>
>> The fix is to remove the assertion code added by JDK-8238438 and keep the code for selecting the first memory state of a load pack.
>>
>> Thanks,
>> Christian
>
> Looks good. Great that we finally have a test for this case!
Thanks Vladimir and Tobias for your reviews! Yes, it's good to finally have a testcase for it. Was already thinking about cleaning this code up soon - good that I've waited.
-------------
PR: https://git.openjdk.java.net/jdk/pull/2495
More information about the hotspot-compiler-dev
mailing list