RFR(S): 8238384: CTW: C2 compilation fails with "assert(store != load->find_exact_control(load->in(0))) failed: dependence cycle found"

[Tobias Hartmann]

Hi Roland,

I agree with your analysis but I'm confused by your changes to Compile::can_alias(). You are
checking if the alias_idx belongs to a known instance address type but how can line 1909 then ever
return false? Doesn't alias_idx == AliasIdxBot imply known_instance == false?

Some minor suggestions below (no new webrev required).

compile.cpp:
- line 1908: Please add parentheses

macroArrayCopy.cpp
- line 710: "memory on the" -> "memory of the"

CopyOfBrokenAntiDependency.java
- Please name the file TestCopyOfBrokenAntiDependency
- line 35/36: Unused imports
- line 41: Unused allocation
- line 51/52: Indentation is wrong

Best regards,
Tobias

On 17.02.20 14:08, Roland Westrelin wrote:
> 
> http://cr.openjdk.java.net/~roland/8238384/webrev.00/
> 
> In the test case: the newly allocated array of the first copyOf doesn't
> escape and is assigned an instance id. The memory edge of the element
> load for that array is modified to skip over the copyOf. C2 can't
> eliminate the non escaping allocations and the element load. When
> anti-dependencies are added, the load is found to be both control
> dependent on the membar that was added when the second copyOf is
> expanded and anti dependent on it: the assert triggers.
> 
> The bug is similar to 8228772 that Tobias fixed last summer but I
> propose a different fix. I think the root cause is that known instances
> don't alias with bottom memory. That's what allows memory edges for
> known instances to be rewired. So I propose Compile::can_alias() be
> changed to reflect that. AFAICT, ArrayCopy is an exception (it can copy
> to a known instance but has adr_type, the bottom type) for which I've
> made some changes.
> 
> Roland.
>