[9 or 10?] RFR(M): 8176506: C2: loop unswitching and unsafe accesses cause crash

[Vladimir Ivanov]

>> I find it problematic to decide whether an unsafe access is always
>> on-heap or not during parsing. Considering we plan to remove membars
>> around suspicious accesses (as a fix for JDK-8176513), I would like to
>> avoid conservatively treating them as raw.
>
> Isn't the plan to remove the membars for 9 and put it back in 10?

JDK-8176513 removed membars around (possibly) mixed accesses and the fix 
we are discussing makes them obsolete.

> I think it's a problem that we build a broken graph and try to plug the
> holes as we find them because there will always be holes that we are not
> yet aware of. That's why my suggestion is to be conservative. I also
> think we should use arguments profiling and a null check to
> speculatively cast base to non null when it makes sense. We have the
> machinery to do that and it's a very simple change.

I agree with your concerns and fully support using argument profiling 
and speculative casts to improve the code w/ unsafe accesses.

>> FTR I faced a similar problem before (JDK-8155635 [1]) and initially
>> experimented with a different approach: convert null-based oop pointers
>> to raw pointers [2], but after additional discussions decided to abandon it.
>
> I noticed that discussion. When we hit a similar problem with Shenandoah
> I used a fix similar to the one you suggested. If you read Vladimir K's
> answer:
>
> http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2016-April/022745.html
>
> "I think we should track which pointers are really RAW when creating
> them."
>
> which is what the fix I'm proposing now is doing.

Ok, after thinking about it more, I agree with your arguments.

My last concern is that by marking possibly mixed accesses as RAW during 
parsing, we limit what optimizations can be applied later.

I thought about allowing rawptr => oopptr conversions for mixed accesses 
once they become on-heap (+ some cleanups; full patch [1]):

http://cr.openjdk.java.net/~vlivanov/roland/8176506/webrev.01_00/

What do you think about it?

Also, I think the checks in LoadNode::split_through_phi & 
MemNode::Ideal_common I mentioned before affect normal accesses as well, 
so probably the problem with loop unswitching you encountered isn't 
specific to unsafe accesses. But let's keep it separate.

Best regards,
Vladimir Ivanov

[1] http://cr.openjdk.java.net/~vlivanov/roland/8176506/webrev.01/