RFR: 8193567: Conversion of comparison nodes affects local slots in optimistic continuation

[Attila Szegedi]

I completely agree with taking the safe path in JDK 10. I still want to figure out what might be the root problem with this. Both RuntimeNode for comparisons and BinaryNode take two arguments in the same order. Even if there were some pushing/popping involved around them for UOE handlers, the continuation handler code did evolve over time (and all the corner cases we encountered earlier on) into a creature that can very well handle changes in local variable types across recompilations and slot numbering too (e.g. as an int evolves to double), so I doubt there’s a systemic problem here but rather another corner case to track down. This is in no way meant as a criticism of your work – I totally agree it’s better to take this approach for JDK 10, it’s more a note to myself that I should dig into this next time I have some cycles and see if I can find a smaller-impact solution.

Attila.

> On 2018. Jan 8., at 16:44, Hannes Wallnöfer <hannes.wallnoefer at oracle.com> wrote:
> 
> Thanks for the review, Attila. 
> 
> I didn’t look at the contents of the local slots. I think the root of the problem is that the comparison is represented as RuntimeNode in some cases and as BinaryNode in others, and it can switch between the two between optimistic recompilations (see LocalVariableTypesCalculator.leaveBinaryNode on how comparison nodes are replaced with RuntimeNodes depending on the types of sub-expressions). I doubt it is possible to guarantee these representations use a compatible slot layout, and even if there is, I think for JDK 10 it’s best to take the safe path.
> 
> Hannes
> 
> 
>> Am 25.12.2017 um 17:27 schrieb Attila Szegedi <szegedia at gmail.com>:
>> 
>> A sad +1. I too wish this were easier to solve. Can you point me to the code parts that cause differences in local slot assignment? I guess I could try to figure it out myself from that terrible expression in the test, but if you have a quick explanation…
>> 
>> Thanks,
>> Attila.
>> 
>>> On Dec 21, 2017, at 4:36 PM, Hannes Wallnöfer <hannes.wallnoefer at oracle.com> wrote:
>>> 
>>> Please review:
>>> 
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8193567
>>> Webrev: http://cr.openjdk.java.net/~hannesw/8193567/webrev.00/
>>> 
>>> I’ve tried finding a smaller fix like just tagging the child nodes of all comparisons as non-optimistic, but that didn't fix the problem as those nodes could still have optimistic children.
>>> 
>>> Hannes
>> 
>