# HG changeset patch # User drchase # Date 1365819287 25200 # Node ID 886d1fd67dc34bbd1adc670b44be7966372d4e0d # Parent bc63dd2539a42c1d6669401534e36d7ea042c5b6 6443505: Ideal() function for CmpLTMask Summary: Repair wrong code generation, added new matching rule Reviewed-by: kvn, twisti diff -r bc63dd2539a4 -r 886d1fd67dc3 src/cpu/sparc/vm/sparc.ad --- a/src/cpu/sparc/vm/sparc.ad Fri Apr 12 20:37:18 2013 -0400 +++ b/src/cpu/sparc/vm/sparc.ad Fri Apr 12 19:14:47 2013 -0700 @@ -1,5 +1,5 @@ // -// 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 @@ 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. diff -r bc63dd2539a4 -r 886d1fd67dc3 src/cpu/x86/vm/x86_32.ad --- a/src/cpu/x86/vm/x86_32.ad Fri Apr 12 20:37:18 2013 -0400 +++ b/src/cpu/x86/vm/x86_32.ad Fri Apr 12 19:14:47 2013 -0700 @@ -1,5 +1,5 @@ // -// 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 @@ 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 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 @@ "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 ); -%} - -instruct cmpLTMask0( rRegI dst, immI0 zero, eFlagsReg cr ) %{ + 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) %{ 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 ); -%} - - -instruct cadd_cmpLTMask( ncxRegI p, ncxRegI q, ncxRegI y, eCXRegI tmp, eFlagsReg cr ) %{ + format %{ "SAR $dst,31\t# cmpLTMask0" %} + ins_encode %{ + __ sarl($dst$$Register, 31); + %} + ins_pipe(ialu_reg); +%} + +/* 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); +%} + +/* 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); + %} + + 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 ) %{ +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); - - 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) ); -%} */ //----------Long Instructions------------------------------------------------ diff -r bc63dd2539a4 -r 886d1fd67dc3 src/cpu/x86/vm/x86_64.ad --- a/src/cpu/x86/vm/x86_64.ad Fri Apr 12 20:37:18 2013 -0400 +++ b/src/cpu/x86/vm/x86_64.ad Fri Apr 12 19:14:47 2013 -0700 @@ -1,5 +1,5 @@ // -// 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 @@ 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 @@ 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) diff -r bc63dd2539a4 -r 886d1fd67dc3 src/share/vm/opto/cfgnode.cpp --- a/src/share/vm/opto/cfgnode.cpp Fri Apr 12 20:37:18 2013 -0400 +++ b/src/share/vm/opto/cfgnode.cpp Fri Apr 12 19:14:47 2013 -0700 @@ -1,5 +1,5 @@ /* - * 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 @@ 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 diff -r bc63dd2539a4 -r 886d1fd67dc3 test/compiler/6443505/Test6443505.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/compiler/6443505/Test6443505.java Fri Apr 12 19:14:47 2013 -0700 @@ -0,0 +1,107 @@ +/* + * 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; + } + +}