Suboptimal C2 code generation

[Vladimir Ivanov]

Me too. I'd expect that IfNode::search_identical() handles that, but 
evidently it doesn't. Filed JDK-8143542 [1] to track the problem.

Thanks for the report!

Best regards,
Vladimir Ivanov

[1] https://bugs.openjdk.java.net/browse/JDK-8143542

On 11/20/15 4:19 PM, Vitaly Davidovich wrote:
> Hi Vladimir,
>
> Yes, this is exactly it -- "halfway to desired shape" is a great
> description.
>
> The motivating example, from which I extracted this small illustrative
> one, is wrapping an instance of some interface and providing a
> different/narrowed view on top of it; the wrapped interface object has
> strong profile towards one type, with a very small occurrence of it
> being null.  After wrapping it, the wrapper is then used to invoke
> another method in the following manner:
>
> Wrapper w = ...;
> callAnotherMethod(w.value(), w.value2(), w.value3(), w.value4()...);
>
> Each of these valueX() methods internally does that null check.  I
> wanted to see if the JIT would effectively transform that chain into:
>
> if (w.i != null) {
>      callAnotherMethod(<no repeated null checks, just call into the
> interface methods>);
> } else {
>     callAnotherMethod(<use the null value branches from all those
> accessors>);
> }
>
> After the above transformation and knowing that there's a single
> receiver type (when it's not null) with very simple code backing the
> interface methods, I was checking if the interface calls were
> devirtualized.  What I found was a bit surprising, which warranted this
> email :).
>
> Would it be difficult to enhance this?
>
> Thanks
>
>
> On Fri, Nov 20, 2015 at 7:54 AM, Vladimir Ivanov
> <vladimir.x.ivanov at oracle.com <mailto:vladimir.x.ivanov at oracle.com>> wrote:
>
>     It looks like C2 stops halfway to desired shape.
>     The load is commoned, but repeated null check is not eliminated.
>
>     Final IR shape looks like:
>        w = Load this._w
>
>        If (w == NULL) deopt
>
>        i = Load w._i
>
>        If (i != NULL) {
>          If (!i instanceof C) deopt
>        } v1 = Phi(T:1,F:-1);
>
>        If (i != NULL) {
>          If (!i instanceof C) deopt
>        } v2 = Phi(T:2,F:-1)
>
>        return v1+v2
>
>     The next transformation would be:
>        w = Load this._w
>        If (w == NULL) deopt
>        i = Load w._i
>        If (i != NULL) {
>          If (!i instanceof C) deopt
>          If (!i instanceof C) deopt
>        } v1 = Phi(T:1,F:-1)
>          v2 = Phi(T:2,F:-1)
>        return v1+v2
>
>     And finally:
>        w = Load this._w
>        If (w == NULL) deopt
>        i = Load w._i
>        If (i != NULL) {
>          If (!i instanceof C) deopt
>        } v1 = Phi(T:1,F:-1)
>          v2 = Phi(T:2,F:-1)
>        return i1+i2;
>
>     Best regards,
>     Vladimir Ivanov
>
>     On 11/20/15 5:29 AM, John Rose wrote:
>
>         On Nov 19, 2015, at 2:58 PM, Vitaly Davidovich
>         <vitalyd at gmail.com <mailto:vitalyd at gmail.com>
>         <mailto:vitalyd at gmail.com <mailto:vitalyd at gmail.com>>> wrote:
>
>
>                      return _w.value() + _w.value2();
>
>
>         Which is (ignoring non-taken null branches):
>
>                   return _w._i.value() + _w._i.value2();
>
>         There are two independent fetches of _w._i in the bytecode.
>         The machine code is treating them as independent, where
>         we would want the optimizer to use a common value.
>
>         The machine code is optimistic that _w._i is always a C,
>         but it appears to be leaving open the possibility that some
>         activity not tracked by the JIT could store some other C2 <: I.
>
>         What happens at 0x00007ff5f82e00f9?  Does it de-opt,
>         or does it merge back into 0x00007ff5f82e00b2?  In the
>         latter case, the JIT cannot assume that _w._i is a C.
>
>         So, it could be a failure to de-opt, or it could be some
>         fluff in the IR which is preventing the two _w._i from
>         commoning.  Or something else.
>
>         — John
>
>