[9] RFR(S): 8181742: Load that bypasses arraycopy has wrong memory state

[Tobias Hartmann]

Hi Roland,

this looks good to me but can you elaborate a bit on why this also fixes 8179678?

I think Vladimir should have a look as well. I'll run some pre-integration testing in the meantime.

Best regards,
Tobias

On 09.06.2017 10:36, Roland Westrelin wrote:
> 
> http://cr.openjdk.java.net/~roland/8181742/webrev.00/
> 
> In:
> 
> static int test1(int[] src) {
>      int[] dst = new int[10];
>      System.arraycopy(src, 0, dst, 0, 10);
>      src[1] = 0x42;
>      return dst[1];
> }
> 
> LoadNode::can_see_arraycopy_value() replaces the load dst[1] by src[1]
> but it does that by cloning the load and doesn't change the memory
> edge. C2 then wrongly optimizes the src[1] load right after the store to
> src[1] as 0x42. This is fixed by setting the memory edge to the memory
> state right before the arraycopy.
> 
> This fix is also a nicer fix for 8179678 (ArrayCopy with same src and
> dst can cause incorrect execution or compiler crash). Most of the
> previous fix is no longer necessary and I removed it.
> 
> With that change, the following test breaks:
> 
> static Object test5_src = null;
> static int test3() {
>     int[] dst = new int[10];
>     System.arraycopy(test5_src, 0, dst, 0, 10);
>     ((int[])test5_src)[1] = 0x42;
>     System.arraycopy(test5_src, 0, dst, 0, 10);
>     return dst[1];
> }
> 
> The input to the arraycopy is not an array. The dst[1] load is replaced
> by test5_src[1] which is of type Object + some offset so that load
> before the second arraycopy won't be on the same slice as the store to
> the array and they can reorder. That failure can be reproduced with
> -XX:+StressGCM and -XX:+StressLCM by running the test enough
> times. LoadNode::can_see_arraycopy_value() now bails out if the source
> type is not an array. If guards are emitted before the ArrayCopyNode we
> can safely assume test5_src is of array type. To cover that case,
> I added a CheckCast to the intrinsification code.
> 
> Roland.
>