Constant propagation of floating-point min/max left over?
Vladimir Kozlov
vladimir.kozlov at oracle.com
Thu Feb 14 16:55:08 UTC 2019
Yes, please file JBS issue. Optimization looks reasonable.
Thanks,
Vladimir
On 2/14/19 7:03 AM, B. Blaser wrote:
> Hi,
>
> I've seen in [1] that constant propagation of floating-point min/max
> intrinsic has been left over.
>
> I tried a quick (naive?) patch on double values, here under, and I did
> the following small experiment on x86_64, currently without any
> matching rule:
>
> class MinMax {
> public static void main(String... args) {
> for (double d=0; d<100_000; d++) {
> test();
> }
> }
>
> static double[] d = new double[5];
>
> public static double[] test() {
> d[0] = Math.max(Math.max(-0.0, 0.0), -0.0);
> d[1] = Math.min(Math.min( 0.0, -0.0), 0.0);
>
> d[2] = Math.max(Math.max(1.0, Double.NaN), 1.0);
> d[3] = Math.min(Math.min(1.0, Double.NaN), 1.0);
>
> d[4] = Math.max(Math.min(Math.max(1.0, 2.0), -1.0), 0.5);
>
> return d;
> }
> }
>
>
> Running it with:
>
> $ java -Xcomp -XX:-TieredCompilation
> -XX:CompileCommand=print,MinMax.test* MinMax
>
>
> Before we had one bloc per min/max invocation like:
>
> movsd XMM0, [constant table base + #0] # load from constant
> table: double=#-0.000000
> movsd [rsp + #0], XMM0 # spill
> movq R10, java/lang/Class:exact * # ptr
> movl RBP, [R10 + #112 (8-bit)] # compressed ptr ! Field: MinMax.d
> xorpd XMM1, XMM1 # double 0.0
> call,static java.lang.Math::max
>
> [ 10 other min/max invocations snipped... ]
>
>
> And after we'd have:
>
> movsd XMM0, [constant table base + #0] # load from constant
> table: double=#nan
> movsd XMM1, [constant table base + #8] # load from constant
> table: double=#0.500000
> movsd XMM2, [constant table base + #16] # load from constant
> table: double=#-0.000000
> xorpd XMM3, XMM3 # double 0.0
> movsd [R12 + R11 << 3 + #16] (compressed oop addressing), XMM3 # double
> movsd [R12 + R11 << 3 + #24] (compressed oop addressing), XMM2 # double
> movsd [R12 + R11 << 3 + #32] (compressed oop addressing), XMM0 # double
> movsd [R12 + R11 << 3 + #40] (compressed oop addressing), XMM0 # double
> movsd [R12 + R11 << 3 + #48] (compressed oop addressing), XMM1 # double
>
>
> Does this attempt look reasonable?
> If yes, I'll create a JBS issue and probably send out a RFR soon (for
> float and double).
>
> Note that hotspot:tier1 is OK on x86_64 but all architectures would
> benefit from this enhancement.
>
> Thanks,
> Bernard
>
>
> [1] http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2018-October/031025.html
>
>
> diff --git a/src/hotspot/cpu/x86/x86_64.ad b/src/hotspot/cpu/x86/x86_64.ad
> --- a/src/hotspot/cpu/x86/x86_64.ad
> +++ b/src/hotspot/cpu/x86/x86_64.ad
> @@ -5456,6 +5456,22 @@
> ins_pipe(pipe_slow); // XXX
> %}
>
> +instruct minD_reg(regD dst, regD a, regD b) %{
> + predicate(false);
> + match(Set dst (MinD a b));
> + format %{ "# NOP" %}
> + ins_encode %{ %}
> + ins_pipe( pipe_slow );
> +%}
> +
> +instruct maxD_reg(regD dst, regD a, regD b) %{
> + predicate(false);
> + match(Set dst (MaxD a b));
> + format %{ "# NOP" %}
> + ins_encode %{ %}
> + ins_pipe( pipe_slow );
> +%}
> +
> // Load Double
> instruct MoveD2VL(vlRegD dst, regD src) %{
> match(Set dst src);
> diff --git a/src/hotspot/share/opto/addnode.cpp
> b/src/hotspot/share/opto/addnode.cpp
> --- a/src/hotspot/share/opto/addnode.cpp
> +++ b/src/hotspot/share/opto/addnode.cpp
> @@ -929,3 +929,37 @@
> // Otherwise just MIN them bits.
> return TypeInt::make( MIN2(r0->_lo,r1->_lo), MIN2(r0->_hi,r1->_hi),
> MAX2(r0->_widen,r1->_widen) );
> }
> +
> +const Type *MaxDNode::add_ring( const Type *t0, const Type *t1 ) const {
> + const TypeD *ta = t0->isa_double_constant();
> + const TypeD *tb = t1->isa_double_constant();
> +
> + if (ta == NULL || tb == NULL) return Type::DOUBLE;
> +
> + const jlong a = jlong_cast(ta->getd());
> + const jlong b = jlong_cast(tb->getd());
> +
> + return
> + a == (jlong) 0x8000000000000000L && b == 0L ? tb :
> + b == (jlong) 0x8000000000000000L && a == 0L ? ta :
> + ta->is_nan() ? ta :
> + tb->is_nan() ? tb :
> + ta->getd() > tb->getd() ? ta : tb;
> +}
> +
> +const Type *MinDNode::add_ring( const Type *t0, const Type *t1 ) const {
> + const TypeD *ta = t0->isa_double_constant();
> + const TypeD *tb = t1->isa_double_constant();
> +
> + if (ta == NULL || tb == NULL) return Type::DOUBLE;
> +
> + const jlong a = jlong_cast(ta->getd());
> + const jlong b = jlong_cast(tb->getd());
> +
> + return
> + a == (jlong) 0x8000000000000000L && b == 0L ? ta :
> + b == (jlong) 0x8000000000000000L && a == 0L ? tb :
> + ta->is_nan() ? ta :
> + tb->is_nan() ? tb :
> + ta->getd() < tb->getd() ? ta : tb;
> +}
> diff --git a/src/hotspot/share/opto/addnode.hpp
> b/src/hotspot/share/opto/addnode.hpp
> --- a/src/hotspot/share/opto/addnode.hpp
> +++ b/src/hotspot/share/opto/addnode.hpp
> @@ -279,7 +279,7 @@
> public:
> MaxDNode(Node *in1, Node *in2) : MaxNode(in1, in2) {}
> virtual int Opcode() const;
> - virtual const Type *add_ring(const Type*, const Type*) const {
> return Type::DOUBLE; }
> + virtual const Type *add_ring(const Type*, const Type*) const;
> virtual const Type *add_id() const { return TypeD::NEG_INF; }
> virtual const Type *bottom_type() const { return Type::DOUBLE; }
> virtual uint ideal_reg() const { return Op_RegD; }
> @@ -291,7 +291,7 @@
> public:
> MinDNode(Node *in1, Node *in2) : MaxNode(in1, in2) {}
> virtual int Opcode() const;
> - virtual const Type *add_ring(const Type*, const Type*) const {
> return Type::DOUBLE; }
> + virtual const Type *add_ring(const Type*, const Type*) const;
> virtual const Type *add_id() const { return TypeD::POS_INF; }
> virtual const Type *bottom_type() const { return Type::DOUBLE; }
> virtual uint ideal_reg() const { return Op_RegD; }
> diff --git a/src/hotspot/share/opto/library_call.cpp
> b/src/hotspot/share/opto/library_call.cpp
> --- a/src/hotspot/share/opto/library_call.cpp
> +++ b/src/hotspot/share/opto/library_call.cpp
> @@ -6609,6 +6609,7 @@
>
> //------------------------------inline_fp_min_max------------------------------
> bool LibraryCallKit::inline_fp_min_max(vmIntrinsics::ID id) {
> +printf("inline_fp_min_max\n");
> Node *a = NULL;
> Node *b = NULL;
> Node *n = NULL;
> @@ -6629,7 +6630,7 @@
> fatal_unexpected_iid(id);
> break;
> }
> - if (a->is_Con() || b->is_Con()) {
> + if (id != vmIntrinsics::_maxD && id != vmIntrinsics::_minD &&
> (a->is_Con() || b->is_Con())) {
> return false;
> }
> switch (id) {
>
More information about the hotspot-compiler-dev
mailing list