From b9f75f3566fbc5767e939673b6cbaa741157313b Mon Sep 17 00:00:00 2001 From: iveresov Date: Mon, 27 Dec 2010 21:51:31 -0800 Subject: [PATCH] 7009231: C1: Incorrect CAS code for longs on SPARC 32bit Summary: Fix CAS of longs on SPARC 32bit and cmove on SPARC 64bit. Reviewed-by: kvn --- src/cpu/sparc/vm/c1_LIRAssembler_sparc.cpp | 23 +++-- src/cpu/sparc/vm/c1_LIRGenerator_sparc.cpp | 6 +- src/cpu/x86/vm/c1_LIRAssembler_x86.cpp | 2 +- src/cpu/x86/vm/c1_LIRGenerator_x86.cpp | 5 +- src/share/vm/c1/c1_LIR.hpp | 9 +- src/share/vm/c1/c1_LIRAssembler.cpp | 2 +- src/share/vm/c1/c1_LIRAssembler.hpp | 2 +- src/share/vm/c1/c1_LIRGenerator.cpp | 4 +- test/compiler/7009231/Test7009231.java | 100 +++++++++++++++++++++ 9 files changed, 133 insertions(+), 20 deletions(-) create mode 100644 test/compiler/7009231/Test7009231.java diff --git a/src/cpu/sparc/vm/c1_LIRAssembler_sparc.cpp b/src/cpu/sparc/vm/c1_LIRAssembler_sparc.cpp index 1e02e0a68..15ca82f28 100644 --- a/src/cpu/sparc/vm/c1_LIRAssembler_sparc.cpp +++ b/src/cpu/sparc/vm/c1_LIRAssembler_sparc.cpp @@ -1705,8 +1705,7 @@ void LIR_Assembler::comp_fl2i(LIR_Code code, LIR_Opr left, LIR_Opr right, LIR_Op } -void LIR_Assembler::cmove(LIR_Condition condition, LIR_Opr opr1, LIR_Opr opr2, LIR_Opr result) { - +void LIR_Assembler::cmove(LIR_Condition condition, LIR_Opr opr1, LIR_Opr opr2, LIR_Opr result, BasicType type) { Assembler::Condition acond; switch (condition) { case lir_cond_equal: acond = Assembler::equal; break; @@ -1737,7 +1736,12 @@ void LIR_Assembler::cmove(LIR_Condition condition, LIR_Opr opr1, LIR_Opr opr2, L ShouldNotReachHere(); } Label skip; - __ br(acond, false, Assembler::pt, skip); +#ifdef _LP64 + if (type == T_INT) { + __ br(acond, false, Assembler::pt, skip); + } else +#endif + __ brx(acond, false, Assembler::pt, skip); // checks icc on 32bit and xcc on 64bit if (opr1->is_constant() && opr1->type() == T_INT) { Register dest = result->as_register(); if (Assembler::is_simm13(opr1->as_jint())) { @@ -2688,6 +2692,11 @@ void LIR_Assembler::emit_compare_and_swap(LIR_OpCompareAndSwap* op) { #ifdef _LP64 __ mov(cmp_value_lo, t1); __ mov(new_value_lo, t2); + // perform the compare and swap operation + __ casx(addr, t1, t2); + // generate condition code - if the swap succeeded, t2 ("new value" reg) was + // overwritten with the original value in "addr" and will be equal to t1. + __ cmp(t1, t2); #else // move high and low halves of long values into single registers __ sllx(cmp_value_hi, 32, t1); // shift high half into temp reg @@ -2696,13 +2705,15 @@ void LIR_Assembler::emit_compare_and_swap(LIR_OpCompareAndSwap* op) { __ sllx(new_value_hi, 32, t2); __ srl(new_value_lo, 0, new_value_lo); __ or3(t2, new_value_lo, t2); // t2 holds 64-bit value to swap -#endif // perform the compare and swap operation __ casx(addr, t1, t2); // generate condition code - if the swap succeeded, t2 ("new value" reg) was // overwritten with the original value in "addr" and will be equal to t1. - __ cmp(t1, t2); - + // Produce icc flag for 32bit. + __ sub(t1, t2, t2); + __ srlx(t2, 32, t1); + __ orcc(t2, t1, G0); +#endif } else if (op->code() == lir_cas_int || op->code() == lir_cas_obj) { Register addr = op->addr()->as_pointer_register(); Register cmp_value = op->cmp_value()->as_register(); diff --git a/src/cpu/sparc/vm/c1_LIRGenerator_sparc.cpp b/src/cpu/sparc/vm/c1_LIRGenerator_sparc.cpp index 1abbdb51d..8869de5e8 100644 --- a/src/cpu/sparc/vm/c1_LIRGenerator_sparc.cpp +++ b/src/cpu/sparc/vm/c1_LIRGenerator_sparc.cpp @@ -662,7 +662,7 @@ void LIRGenerator::do_AttemptUpdate(Intrinsic* x) { // generate conditional move of boolean result LIR_Opr result = rlock_result(x); - __ cmove(lir_cond_equal, LIR_OprFact::intConst(1), LIR_OprFact::intConst(0), result); + __ cmove(lir_cond_equal, LIR_OprFact::intConst(1), LIR_OprFact::intConst(0), result, T_LONG); } @@ -699,10 +699,10 @@ void LIRGenerator::do_CompareAndSwap(Intrinsic* x, ValueType* type) { else { ShouldNotReachHere(); } - // generate conditional move of boolean result LIR_Opr result = rlock_result(x); - __ cmove(lir_cond_equal, LIR_OprFact::intConst(1), LIR_OprFact::intConst(0), result); + __ cmove(lir_cond_equal, LIR_OprFact::intConst(1), LIR_OprFact::intConst(0), + result, as_BasicType(type)); if (type == objectType) { // Write-barrier needed for Object fields. // Precise card mark since could either be object or array post_barrier(addr, val.result()); diff --git a/src/cpu/x86/vm/c1_LIRAssembler_x86.cpp b/src/cpu/x86/vm/c1_LIRAssembler_x86.cpp index 724d8411b..f3ba0a2ce 100644 --- a/src/cpu/x86/vm/c1_LIRAssembler_x86.cpp +++ b/src/cpu/x86/vm/c1_LIRAssembler_x86.cpp @@ -2036,7 +2036,7 @@ void LIR_Assembler::emit_compare_and_swap(LIR_OpCompareAndSwap* op) { } } -void LIR_Assembler::cmove(LIR_Condition condition, LIR_Opr opr1, LIR_Opr opr2, LIR_Opr result) { +void LIR_Assembler::cmove(LIR_Condition condition, LIR_Opr opr1, LIR_Opr opr2, LIR_Opr result, BasicType type) { Assembler::Condition acond, ncond; switch (condition) { case lir_cond_equal: acond = Assembler::equal; ncond = Assembler::notEqual; break; diff --git a/src/cpu/x86/vm/c1_LIRGenerator_x86.cpp b/src/cpu/x86/vm/c1_LIRGenerator_x86.cpp index d792a0f62..cdea66f38 100644 --- a/src/cpu/x86/vm/c1_LIRGenerator_x86.cpp +++ b/src/cpu/x86/vm/c1_LIRGenerator_x86.cpp @@ -741,7 +741,7 @@ void LIRGenerator::do_AttemptUpdate(Intrinsic* x) { // generate conditional move of boolean result LIR_Opr result = rlock_result(x); - __ cmove(lir_cond_equal, LIR_OprFact::intConst(1), LIR_OprFact::intConst(0), result); + __ cmove(lir_cond_equal, LIR_OprFact::intConst(1), LIR_OprFact::intConst(0), result, T_LONG); } @@ -810,7 +810,8 @@ void LIRGenerator::do_CompareAndSwap(Intrinsic* x, ValueType* type) { // generate conditional move of boolean result LIR_Opr result = rlock_result(x); - __ cmove(lir_cond_equal, LIR_OprFact::intConst(1), LIR_OprFact::intConst(0), result); + __ cmove(lir_cond_equal, LIR_OprFact::intConst(1), LIR_OprFact::intConst(0), + result, as_BasicType(type)); if (type == objectType) { // Write-barrier needed for Object fields. // Seems to be precise post_barrier(addr, val.result()); diff --git a/src/share/vm/c1/c1_LIR.hpp b/src/share/vm/c1/c1_LIR.hpp index 7a3574cfa..50bf4c4e9 100644 --- a/src/share/vm/c1/c1_LIR.hpp +++ b/src/share/vm/c1/c1_LIR.hpp @@ -1568,15 +1568,16 @@ class LIR_Op2: public LIR_Op { assert(code == lir_cmp, "code check"); } - LIR_Op2(LIR_Code code, LIR_Condition condition, LIR_Opr opr1, LIR_Opr opr2, LIR_Opr result) + LIR_Op2(LIR_Code code, LIR_Condition condition, LIR_Opr opr1, LIR_Opr opr2, LIR_Opr result, BasicType type) : LIR_Op(code, result, NULL) , _opr1(opr1) , _opr2(opr2) - , _type(T_ILLEGAL) + , _type(type) , _condition(condition) , _fpu_stack_size(0) , _tmp(LIR_OprFact::illegalOpr) { assert(code == lir_cmove, "code check"); + assert(type != T_ILLEGAL, "cmove should have type"); } LIR_Op2(LIR_Code code, LIR_Opr opr1, LIR_Opr opr2, LIR_Opr result = LIR_OprFact::illegalOpr, @@ -1993,8 +1994,8 @@ class LIR_List: public CompilationResourceObj { void cmp_mem_int(LIR_Condition condition, LIR_Opr base, int disp, int c, CodeEmitInfo* info); void cmp_reg_mem(LIR_Condition condition, LIR_Opr reg, LIR_Address* addr, CodeEmitInfo* info); - void cmove(LIR_Condition condition, LIR_Opr src1, LIR_Opr src2, LIR_Opr dst) { - append(new LIR_Op2(lir_cmove, condition, src1, src2, dst)); + void cmove(LIR_Condition condition, LIR_Opr src1, LIR_Opr src2, LIR_Opr dst, BasicType type) { + append(new LIR_Op2(lir_cmove, condition, src1, src2, dst, type)); } void cas_long(LIR_Opr addr, LIR_Opr cmp_value, LIR_Opr new_value, diff --git a/src/share/vm/c1/c1_LIRAssembler.cpp b/src/share/vm/c1/c1_LIRAssembler.cpp index c42281997..1b57ea2f0 100644 --- a/src/share/vm/c1/c1_LIRAssembler.cpp +++ b/src/share/vm/c1/c1_LIRAssembler.cpp @@ -685,7 +685,7 @@ void LIR_Assembler::emit_op2(LIR_Op2* op) { break; case lir_cmove: - cmove(op->condition(), op->in_opr1(), op->in_opr2(), op->result_opr()); + cmove(op->condition(), op->in_opr1(), op->in_opr2(), op->result_opr(), op->type()); break; case lir_shl: diff --git a/src/share/vm/c1/c1_LIRAssembler.hpp b/src/share/vm/c1/c1_LIRAssembler.hpp index 24f62862c..bb18c253e 100644 --- a/src/share/vm/c1/c1_LIRAssembler.hpp +++ b/src/share/vm/c1/c1_LIRAssembler.hpp @@ -217,7 +217,7 @@ class LIR_Assembler: public CompilationResourceObj { void volatile_move_op(LIR_Opr src, LIR_Opr result, BasicType type, CodeEmitInfo* info); void comp_mem_op(LIR_Opr src, LIR_Opr result, BasicType type, CodeEmitInfo* info); // info set for null exceptions void comp_fl2i(LIR_Code code, LIR_Opr left, LIR_Opr right, LIR_Opr result, LIR_Op2* op); - void cmove(LIR_Condition code, LIR_Opr left, LIR_Opr right, LIR_Opr result); + void cmove(LIR_Condition code, LIR_Opr left, LIR_Opr right, LIR_Opr result, BasicType type); void call( LIR_OpJavaCall* op, relocInfo::relocType rtype); void ic_call( LIR_OpJavaCall* op); diff --git a/src/share/vm/c1/c1_LIRGenerator.cpp b/src/share/vm/c1/c1_LIRGenerator.cpp index 56b22f686..8dc579d0d 100644 --- a/src/share/vm/c1/c1_LIRGenerator.cpp +++ b/src/share/vm/c1/c1_LIRGenerator.cpp @@ -856,7 +856,7 @@ void LIRGenerator::profile_branch(If* if_instr, If::Condition cond) { __ cmove(lir_cond(cond), LIR_OprFact::intptrConst(taken_count_offset), LIR_OprFact::intptrConst(not_taken_count_offset), - data_offset_reg); + data_offset_reg, as_BasicType(if_instr->x()->type())); // MDO cells are intptr_t, so the data_reg width is arch-dependent. LIR_Opr data_reg = new_pointer_register(); @@ -2591,7 +2591,7 @@ void LIRGenerator::do_IfOp(IfOp* x) { LIR_Opr reg = rlock_result(x); __ cmp(lir_cond(x->cond()), left.result(), right.result()); - __ cmove(lir_cond(x->cond()), t_val.result(), f_val.result(), reg); + __ cmove(lir_cond(x->cond()), t_val.result(), f_val.result(), reg, as_BasicType(x->x()->type())); } diff --git a/test/compiler/7009231/Test7009231.java b/test/compiler/7009231/Test7009231.java new file mode 100644 index 000000000..64afc6601 --- /dev/null +++ b/test/compiler/7009231/Test7009231.java @@ -0,0 +1,100 @@ +/* + * Copyright (c) 2010, 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 7009231 + * @summary C1: Incorrect CAS code for longs on SPARC 32bit + * + * @run main/othervm -Xbatch Test7009231 + * + */ + +import java.util.Random; +import java.util.concurrent.atomic.AtomicLong; + + +public class Test7009231 { + public static void main(String[] args) throws InterruptedException { + doTest(8); + } + + private static void doTest(int nThreads) throws InterruptedException { + Thread[] aThreads = new Thread[nThreads]; + final AtomicLong atl = new AtomicLong(); + + for (int i = 0; i < nThreads; i++) { + aThreads[i] = new RunnerThread(atl, 1L << (8 * i)); + } + + for (int i = 0; i < nThreads; i++) { + aThreads[i].start(); + } + + for (int i = 0; i < nThreads; i++) { + aThreads[i].join(); + } + } + + public static class RunnerThread extends Thread { + public RunnerThread(AtomicLong atomic, long lMask) { + m_lMask = lMask; + m_atomic = atomic; + } + + public void run() { + AtomicLong atomic = m_atomic; + long lMask = m_lMask; + for (int i = 0; i < 100000; i++) { + setBit(atomic, lMask); + clearBit(atomic, lMask); + } + } + + protected void setBit(AtomicLong atomic, long lMask) { + long lWord; + do { + lWord = atomic.get(); + } while (!atomic.compareAndSet(lWord, lWord | lMask)); + + if ((atomic.get() & lMask) == 0L) { + throw new InternalError(); + } + } + + protected void clearBit(AtomicLong atomic, long lMask) { + long lWord; + do { + lWord = atomic.get(); + } while (!atomic.compareAndSet(lWord, lWord & ~lMask)); + + if ((atomic.get() & lMask) != 0L) { + throw new InternalError(); + } + } + + private long m_lMask; + private AtomicLong m_atomic; + } +} -- GitLab