6443505: Ideal() function for CmpLTMask

Summary: Repair wrong code generation, added new matching rule
Reviewed-by: kvn, twisti

src/cpu/sparc/vm/sparc.ad

 //
-// Copyright (c) 1998, 2012, Oracle and/or its affiliates. All rights reserved.
+// Copyright (c) 1998, 2013, Oracle and/or its affiliates. All rights reserved.
 // DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
 //
 // This code is free software; you can redistribute it and/or modify it
@@ -8223,10 +8223,25 @@ instruct cadd_cmpLTMask( iRegI p, iRegI q, iRegI y, iRegI tmp, flagsReg ccr ) %{
   format %{ "SUBcc  $p,$q,$p\t! p' = p-q\n\t"
             "ADD    $p,$y,$tmp\t! g3=p-q+y\n\t"
             "MOVlt  $tmp,$p\t! p' < 0 ? p'+y : p'" %}
-  ins_encode( enc_cadd_cmpLTMask(p, q, y, tmp) );
-  ins_pipe( cadd_cmpltmask );
+  ins_encode(enc_cadd_cmpLTMask(p, q, y, tmp));
+  ins_pipe(cadd_cmpltmask);
 %}
 
+instruct and_cmpLTMask(iRegI p, iRegI q, iRegI y, flagsReg ccr) %{
+  match(Set p (AndI (CmpLTMask p q) y));
+  effect(KILL ccr);
+  ins_cost(DEFAULT_COST*3);
+
+  format %{ "CMP  $p,$q\n\t"
+            "MOV  $y,$p\n\t"
+            "MOVge G0,$p" %}
+  ins_encode %{
+    __ cmp($p$$Register, $q$$Register);
+    __ mov($y$$Register, $p$$Register);
+    __ movcc(Assembler::greaterEqual, false, Assembler::icc, G0, $p$$Register);
+  %}
+  ins_pipe(ialu_reg_reg_ialu);
+%}
 
 //-----------------------------------------------------------------
 // Direct raw moves between float and general registers using VIS3.

src/cpu/x86/vm/x86_32.ad

 //
-// Copyright (c) 1997, 2012, Oracle and/or its affiliates. All rights reserved.
+// Copyright (c) 1997, 2013, Oracle and/or its affiliates. All rights reserved.
 // DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
 //
 // This code is free software; you can redistribute it and/or modify it
@@ -2317,30 +2317,6 @@ encode %{
     emit_rm(cbuf, 0x3, $p$$reg, tmpReg);
   %}
 
-  enc_class enc_cmpLTP_mem(rRegI p, rRegI q, memory mem, eCXRegI tmp) %{    // cadd_cmpLT
-    int tmpReg = $tmp$$reg;
-
-    // SUB $p,$q
-    emit_opcode(cbuf,0x2B);
-    emit_rm(cbuf, 0x3, $p$$reg, $q$$reg);
-    // SBB $tmp,$tmp
-    emit_opcode(cbuf,0x1B);
-    emit_rm(cbuf, 0x3, tmpReg, tmpReg);
-    // AND $tmp,$y
-    cbuf.set_insts_mark();       // Mark start of opcode for reloc info in mem operand
-    emit_opcode(cbuf,0x23);
-    int reg_encoding = tmpReg;
-    int base  = $mem$$base;
-    int index = $mem$$index;
-    int scale = $mem$$scale;
-    int displace = $mem$$disp;
-    relocInfo::relocType disp_reloc = $mem->disp_reloc();
-    encode_RegMem(cbuf, reg_encoding, base, index, scale, displace, disp_reloc);
-    // ADD $p,$tmp
-    emit_opcode(cbuf,0x03);
-    emit_rm(cbuf, 0x3, $p$$reg, tmpReg);
-  %}
-
   enc_class shift_left_long( eRegL dst, eCXRegI shift ) %{
     // TEST shift,32
     emit_opcode(cbuf,0xF7);
@@ -8922,9 +8898,9 @@ instruct convP2B( rRegI dst, eRegP src, eFlagsReg cr ) %{
   %}
 %}
 
-instruct cmpLTMask( eCXRegI dst, ncxRegI p, ncxRegI q, eFlagsReg cr ) %{
+instruct cmpLTMask(eCXRegI dst, ncxRegI p, ncxRegI q, eFlagsReg cr) %{
   match(Set dst (CmpLTMask p q));
-  effect( KILL cr );
+  effect(KILL cr);
   ins_cost(400);
 
   // SETlt can only use low byte of EAX,EBX, ECX, or EDX as destination
@@ -8932,50 +8908,83 @@ instruct cmpLTMask( eCXRegI dst, ncxRegI p, ncxRegI q, eFlagsReg cr ) %{
             "CMP    $p,$q\n\t"
             "SETlt  $dst\n\t"
             "NEG    $dst" %}
-  ins_encode( OpcRegReg(0x33,dst,dst),
-              OpcRegReg(0x3B,p,q),
-              setLT_reg(dst), neg_reg(dst) );
-  ins_pipe( pipe_slow );
+  ins_encode %{
+    Register Rp = $p$$Register;
+    Register Rq = $q$$Register;
+    Register Rd = $dst$$Register;
+    Label done;
+    __ xorl(Rd, Rd);
+    __ cmpl(Rp, Rq);
+    __ setb(Assembler::less, Rd);
+    __ negl(Rd);
+  %}
+
+  ins_pipe(pipe_slow);
 %}
 
-instruct cmpLTMask0( rRegI dst, immI0 zero, eFlagsReg cr ) %{
+instruct cmpLTMask0(rRegI dst, immI0 zero, eFlagsReg cr) %{
   match(Set dst (CmpLTMask dst zero));
-  effect( DEF dst, KILL cr );
+  effect(DEF dst, KILL cr);
   ins_cost(100);
 
-  format %{ "SAR    $dst,31" %}
-  opcode(0xC1, 0x7);  /* C1 /7 ib */
-  ins_encode( RegOpcImm( dst, 0x1F ) );
-  ins_pipe( ialu_reg );
+  format %{ "SAR    $dst,31\t# cmpLTMask0" %}
+  ins_encode %{
+  __ sarl($dst$$Register, 31);
+  %}
+  ins_pipe(ialu_reg);
 %}
 
-
-instruct cadd_cmpLTMask( ncxRegI p, ncxRegI q, ncxRegI y, eCXRegI tmp, eFlagsReg cr ) %{
+/* better to save a register than avoid a branch */
+instruct cadd_cmpLTMask(rRegI p, rRegI q, rRegI y, eFlagsReg cr) %{
   match(Set p (AddI (AndI (CmpLTMask p q) y) (SubI p q)));
-  effect( KILL tmp, KILL cr );
+  effect(KILL cr);
   ins_cost(400);
-  // annoyingly, $tmp has no edges so you cant ask for it in
-  // any format or encoding
-  format %{ "SUB    $p,$q\n\t"
-            "SBB    ECX,ECX\n\t"
-            "AND    ECX,$y\n\t"
-            "ADD    $p,ECX" %}
-  ins_encode( enc_cmpLTP(p,q,y,tmp) );
-  ins_pipe( pipe_cmplt );
+  format %{ "SUB    $p,$q\t# cadd_cmpLTMask\n\t"
+            "JGE    done\n\t"
+            "ADD    $p,$y\n"
+            "done:  " %}
+  ins_encode %{
+    Register Rp = $p$$Register;
+    Register Rq = $q$$Register;
+    Register Ry = $y$$Register;
+    Label done;
+    __ subl(Rp, Rq);
+    __ jccb(Assembler::greaterEqual, done);
+    __ addl(Rp, Ry);
+    __ bind(done);
+  %}
+
+  ins_pipe(pipe_cmplt);
 %}
 
-/* If I enable this, I encourage spilling in the inner loop of compress.
-instruct cadd_cmpLTMask_mem( ncxRegI p, ncxRegI q, memory y, eCXRegI tmp, eFlagsReg cr ) %{
-  match(Set p (AddI (AndI (CmpLTMask p q) (LoadI y)) (SubI p q)));
-  effect( USE_KILL tmp, KILL cr );
-  ins_cost(400);
+/* better to save a register than avoid a branch */
+instruct and_cmpLTMask(rRegI p, rRegI q, rRegI y, eFlagsReg cr) %{
+  match(Set y (AndI (CmpLTMask p q) y));
+  effect(KILL cr);
+
+  ins_cost(300);
+
+  format %{ "CMPL     $p, $q\t# and_cmpLTMask\n\t"
+            "JLT      done\n\t"
+            "XORL     $y, $y\n"
+            "done:  " %}
+  ins_encode %{
+    Register Rp = $p$$Register;
+    Register Rq = $q$$Register;
+    Register Ry = $y$$Register;
+    Label done;
+    __ cmpl(Rp, Rq);
+    __ jccb(Assembler::less, done);
+    __ xorl(Ry, Ry);
+    __ bind(done);
+  %}
 
-  format %{ "SUB    $p,$q\n\t"
-            "SBB    ECX,ECX\n\t"
-            "AND    ECX,$y\n\t"
-            "ADD    $p,ECX" %}
-  ins_encode( enc_cmpLTP_mem(p,q,y,tmp) );
+  ins_pipe(pipe_cmplt);
 %}
+
+/* If I enable this, I encourage spilling in the inner loop of compress.
+instruct cadd_cmpLTMask_mem(ncxRegI p, ncxRegI q, memory y, eCXRegI tmp, eFlagsReg cr) %{
+  match(Set p (AddI (AndI (CmpLTMask p q) (LoadI y)) (SubI p q)));
 */
 
 //----------Long Instructions------------------------------------------------

src/cpu/x86/vm/x86_64.ad

 //
-// Copyright (c) 2003, 2012, Oracle and/or its affiliates. All rights reserved.
+// Copyright (c) 2003, 2013, Oracle and/or its affiliates. All rights reserved.
 // DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
 //
 // This code is free software; you can redistribute it and/or modify it
@@ -9434,7 +9434,7 @@ instruct cmpLTMask(rRegI dst, rRegI p, rRegI q, rFlagsReg cr)
   match(Set dst (CmpLTMask p q));
   effect(KILL cr);
 
-  ins_cost(400); // XXX
+  ins_cost(400);
   format %{ "cmpl    $p, $q\t# cmpLTMask\n\t"
             "setlt   $dst\n\t"
             "movzbl  $dst, $dst\n\t"
@@ -9452,37 +9452,63 @@ instruct cmpLTMask0(rRegI dst, immI0 zero, rFlagsReg cr)
   match(Set dst (CmpLTMask dst zero));
   effect(KILL cr);
 
-  ins_cost(100); // XXX
+  ins_cost(100);
   format %{ "sarl    $dst, #31\t# cmpLTMask0" %}
-  opcode(0xC1, 0x7);  /* C1 /7 ib */
-  ins_encode(reg_opc_imm(dst, 0x1F));
+  ins_encode %{
+  __ sarl($dst$$Register, 31);
+  %}
   ins_pipe(ialu_reg);
 %}
 
-
-instruct cadd_cmpLTMask(rRegI p, rRegI q, rRegI y, rRegI tmp, rFlagsReg cr)
+/* Better to save a register than avoid a branch */
+instruct cadd_cmpLTMask(rRegI p, rRegI q, rRegI y, rFlagsReg cr)
 %{
   match(Set p (AddI (AndI (CmpLTMask p q) y) (SubI p q)));
-  effect(TEMP tmp, KILL cr);
-
-  ins_cost(400); // XXX
-  format %{ "subl    $p, $q\t# cadd_cmpLTMask1\n\t"
-            "sbbl    $tmp, $tmp\n\t"
-            "andl    $tmp, $y\n\t"
-            "addl    $p, $tmp" %}
+  effect(KILL cr);
+  ins_cost(300);
+  format %{ "subl   $p,$q\t# cadd_cmpLTMask\n\t"
+            "jge    done\n\t"
+            "addl   $p,$y\n"
+            "done:  " %}
   ins_encode %{
     Register Rp = $p$$Register;
     Register Rq = $q$$Register;
     Register Ry = $y$$Register;
-    Register Rt = $tmp$$Register;
+    Label done;
     __ subl(Rp, Rq);
-    __ sbbl(Rt, Rt);
-    __ andl(Rt, Ry);
-    __ addl(Rp, Rt);
+    __ jccb(Assembler::greaterEqual, done);
+    __ addl(Rp, Ry);
+    __ bind(done);
+  %}
+  ins_pipe(pipe_cmplt);
+%}
+
+/* Better to save a register than avoid a branch */
+instruct and_cmpLTMask(rRegI p, rRegI q, rRegI y, rFlagsReg cr)
+%{
+  match(Set y (AndI (CmpLTMask p q) y));
+  effect(KILL cr);
+
+  ins_cost(300);
+
+  format %{ "cmpl     $p, $q\t# and_cmpLTMask\n\t"
+            "jlt      done\n\t"
+            "xorl     $y, $y\n"
+            "done:  " %}
+  ins_encode %{
+    Register Rp = $p$$Register;
+    Register Rq = $q$$Register;
+    Register Ry = $y$$Register;
+    Label done;
+    __ cmpl(Rp, Rq);
+    __ jccb(Assembler::less, done);
+    __ xorl(Ry, Ry);
+    __ bind(done);
   %}
   ins_pipe(pipe_cmplt);
 %}
 
+
 //---------- FP Instructions------------------------------------------------
 
 instruct cmpF_cc_reg(rFlagsRegU cr, regF src1, regF src2)

src/share/vm/opto/cfgnode.cpp

 /*
- * Copyright (c) 1997, 2012, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2013, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -1306,10 +1306,11 @@ static Node* is_cond_add(PhaseGVN *phase, PhiNode *phi, int true_path) {
     return NULL;
 
   Node *x = n2;
-  Node *y = n1->in(1);
-  if( n2 == n1->in(1) ) {
+  Node *y = NULL;
+  if( x == n1->in(1) ) {
     y = n1->in(2);
-  } else if( n2 == n1->in(1) ) {
+  } else if( x == n1->in(2) ) {
+    y = n1->in(1);
   } else return NULL;
 
   // Not so profitable if compare and add are constants

test/compiler/6443505/Test6443505.java

+/*
+ * Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ *
+ */
+
+/**
+ * @test
+ * @bug 6443505
+ * @summary Some cases for CmpLTMask missed; also wrong code.
+ *
+ * @run main/othervm -Xcomp -XX:CompileOnly="Test6443505.compiled" Test6443505
+ */
+
+public class Test6443505 {
+
+    public static void main(String[] args) throws InterruptedException {
+        test(Integer.MIN_VALUE, 0);
+        test(0, Integer.MIN_VALUE);
+        test(Integer.MIN_VALUE, -1);
+        test(-1, Integer.MIN_VALUE);
+        test(Integer.MIN_VALUE, 1);
+        test(1, Integer.MIN_VALUE);
+
+        test(Integer.MAX_VALUE, 0);
+        test(0, Integer.MAX_VALUE);
+        test(Integer.MAX_VALUE, -1);
+        test(-1, Integer.MAX_VALUE);
+        test(Integer.MAX_VALUE, 1);
+        test(1, Integer.MAX_VALUE);
+
+        test(Integer.MIN_VALUE, Integer.MAX_VALUE);
+        test(Integer.MAX_VALUE, Integer.MIN_VALUE);
+
+        test(1, -1);
+        test(1, 0);
+        test(1, 1);
+        test(-1, -1);
+        test(-1, 0);
+        test(-1, 1);
+        test(0, -1);
+        test(0, 0);
+        test(0, 1);
+    }
+
+    public static void test(int a, int b) throws InterruptedException {
+        int C = compiled(4, a, b);
+        int I = interpreted(4, a, b);
+        if (C != I) {
+            System.err.println("#1 C = " + C + ", I = " + I);
+            System.err.println("#1 C != I, FAIL");
+            System.exit(97);
+        }
+
+        C = compiled(a, b, q, 4);
+        I = interpreted(a, b, q, 4);
+        if (C != I) {
+            System.err.println("#2 C = " + C + ", I = " + I);
+            System.err.println("#2 C != I, FAIL");
+            System.exit(97);
+        }
+
+    }
+
+    static int q = 4;
+
+    // If improperly compiled, uses carry/borrow bit, which is wrong.
+    // with -XX:+PrintOptoAssembly, look for cadd_cmpLTMask
+    static int compiled(int p, int x, int y) {
+        return (x < y) ? q + (x - y) : (x - y);
+    }
+
+    // interpreted reference
+    static int interpreted(int p, int x, int y) {
+        return (x < y) ? q + (x - y) : (x - y);
+    }
+
+    // Test new code with a range of cases
+    // with -XX:+PrintOptoAssembly, look for and_cmpLTMask
+    static int compiled(int x, int y, int q, int p) {
+        return (x < y) ? p + q : q;
+    }
+
+    // interpreted reference
+    static int interpreted(int x, int y, int q, int p) {
+        return (x < y) ? p + q : q;
+    }
+
+}