JIT code generation for Long/Integer.compare

[Ian Rogers]

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> wrote:

> On Sep 25, 2015, at 7:35 PM, Vladimir Kozlov <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/20150927/3c4ce84b/attachment-0001.html>