JIT code generation for Long/Integer.compare

[Vitaly Davidovich]

Speaking of redundant cmps, I've seen them as well in various cases.  One
different example that I brought up on this list is for switches, which
John Rose explained here:
http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2014-September/015524.html

I don't know if the switch case is ultimately the same issue or not, but
there is definitely a tendency to see redundant back-to-back cmp with only
the jump predicate varying.

sent from my phone
On Sep 28, 2015 6:12 PM, "Aleksey Shipilev" <aleksey.shipilev at oracle.com>
wrote:

> Hi Ian,
>
> I filed:
>   https://bugs.openjdk.java.net/browse/JDK-8137309
>
> I can see some improvement with your patch, mostly when Longs are
> random, and therefore branch profile is polluted, and current generated
> code is not laid out well. Current Hotspot seems to generate multiple
> redundant cmp-s, which looks like a generic issue that should be
> addressed. See e.g. the very first disassembly here:
>   http://cr.openjdk.java.net/~shade/8137309/BiasedCompareTo.java
>
> In some other cases, patched version is regressing. It seems to be the
> interaction with intrinsic not using any branch profile to figure out
> which option is more frequent, and some minor regalloc/constant
> propagation issues. See e.g.:
>   http://cr.openjdk.java.net/~shade/8137309/PredictedCompareTo.java
>
> With that, fixing the optimizer/codegen without resorting to point
> intrinsics does look like a better course of action.
>
> Thanks,
> -Aleksey
>
> On 09/28/2015 04:44 AM, Ian Rogers wrote:
> > Thanks. I'm not disagreeing with anything here. is_x2logic is 45 lines
> > long and doing less (at least half) pattern matching work than would be
> > required for compare. Wrt implementation approach, ReverseBytesI is
> > purely an intrinsic but could also be implemented by pattern matching,
> > which fwiw is how its implemented in Jikes RVM as part of instruction
> > selection. With hindsight a high/ideal level BURS matching phase would
> > be nice. The intrinsic implementation is trivially correct and easy for
> > developers to target, whereas pattern matching can be buggy, hard to
> > test, and so on. Its inherently more LOC complex. I have long thought
> > pattern matching is the more laudable approach, but there's also a
> > certain amount of pragmatism when selecting between an intrinsic and
> > pattern matching. The scale of the proposed pattern match is beyond the
> > size of any equivalent in C2, so I have concerns around correctness and
> > in the consistency of the codebase in implementing it this way.
> >
> > If I may explain some of the background for this change, aside pure
> > performance. There is a common bug pattern of:
> >
> > class MyFoo implements Comparable<MyFoo> {
> >   private long x;
> >   int compareTo(MyFoo o) {
> >     return (int)(o.x - x);
> >   }
> > }
> >
> > As the int cast doesn't preserve the sign of the long subtract, subtract
> > results that overflow 31-bits may produce incorrect sort orders.
> > Rewriting the above code to use Long.compare is currently a performance
> > loss, although far more correct. What's true of long is also somewhat
> > true for ints as the subtract approach doesn't behave correctly around
> > Integer.MIN_VALUE. Having Long.compare and Integer.compare flagged as
> > intrinsics would go someway to remove developers reticence to prefer it
> > over the pervasive subtract approach which is naively considered more
> > performant.
> >
> > Thanks,
> > Ian
> >
> >
> > On Sat, Sep 26, 2015 at 12:02 PM, John Rose <john.r.rose at oracle.com
> > <mailto:john.r.rose at oracle.com>> wrote:
> >
> >     On Sep 25, 2015, at 7:35 PM, Vladimir Kozlov
> >     <vladimir.kozlov at oracle.com <mailto:vladimir.kozlov at oracle.com>>
> wrote:
> >>
> >>     For pattern matching I mean ideal transformation in ideal graph.
> >>     See, fro example, is_x2logic() in cfgnode.cpp and other
> >>     transformations for Phi node.
> >>
> >>     You will still need new CmpI3 node and changes in .ad file.
> >
> >     +1  Ideally, we pattern-match the body of the method we know about
> and
> >     generate the new IR, and then two good things happen:  Other
> expressions
> >     we didn't know about also pattern match, and after looking at the
> >     pattern
> >     logic we discover straightforward generalizations that go beyond the
> >     method
> >     we knew about at first.
> >
> >     — John
> >
> >
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20150928/7028b7ec/attachment.html>