[aarch64-port-dev ] RFR: Fix for jtreg test case TestIntAtomicCAS

[Edward Nevill]

Hi,

The patch below fixes the failures I am seeing in the jtreg test case TestIntAtomicCAS with the C1 compiler.

The patch is based on the code from aarch64.ad

    Label retry_load, done;
    __ bind(retry_load);
    __ ldaxr(rscratch1, addr_reg);
    __ cmp(rscratch1, old_reg);
    __ br(Assembler::NE, done);
    // if we store+flush with no intervening write tmp will be zero
    __ stlxr(rscratch1, new_reg, addr_reg);
    __ cmpw(rscratch1, zr);       // cannot use cbzw as must set flag
    __ br(Assembler::EQ, done);
    // retry so we only ever return after a load fails to compare
    // ensures we don't return a stale value after a failed write.
    __ b(retry_load);
    __ bind(done);

whereas the equivalent code in C1 does

  Label nope;
  // flush and load exclusive from the memory location
  // and fail if it is not what we expect
  __ ldaxr(rscratch1, addr);
  __ cmp(rscratch1, cmpval);
  __ cset(rscratch1, Assembler::NE);
  __ br(Assembler::NE, nope);
  // if we store+flush with no intervening write rscratch1 wil be zero
  __ stlxr(rscratch1, newval, addr);
  __ bind(nope);

IE. it does not do the branch to retry_load, but just fails if the stlxr fails, so I have modified the C1 code in the patch below to replicate the C2 behavior.

Below is a much reduced reproducer of the test case. I have also added a 'loop until fail' so that the test will continue executing until it fails (this may take several minutes).

Ok to push this patch?
Ed.

---- CUT HERE -------
/*
 * 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.
 *
 */
import java.util.concurrent.atomic.AtomicIntegerArray;

public class TestIntAtomicCAS {
  private static final int ARRLEN = 97;

  public static void main(String args[]) {
    int errn = test(false);
  }

  static int test(boolean test_only) {
    AtomicIntegerArray a1 = new AtomicIntegerArray(ARRLEN);
    AtomicIntegerArray a2 = new AtomicIntegerArray(ARRLEN);
    // Initialize

    int errn = 0;

 while (errn == 0) {
    // Initialize
    for (int i=0; i<ARRLEN; i++) {
      a1.set(i, -1);
      a2.set(i, -1);
    }
    // Test and verify results
    {
      test_ci(a1);
      for (int i=0; i<ARRLEN; i++) {
        errn += verify("test_ci: a1", i, a1.get(i), -123);
      }

    }
  }

      return errn;

  }

  static void test_ci(AtomicIntegerArray a) {
    for (int i = 0; i < ARRLEN; i+=1) {
      a.compareAndSet(i, -1, -123);
    }
  }

  static int verify(String text, int i, int elem, int val) {
    if (elem != val) {
      System.err.println(text + "[" + i + "] = " + elem + " != " + val);
      return 1;
    }
    return 0;
  }
}
--- CUT HERE ----

And here is the patch

--- CUT HERE ---
# HG changeset patch
# User Edward Nevill edward.nevill at linaro.org
# Date 1387469288 0
#      Thu Dec 19 16:08:08 2013 +0000
# Node ID a1f632c91185437aaac51531a184cde461fd89b7
# Parent  8f7ab4f357559285bd7db4c20e35bd8853778cf5
Fix jtreg test case TestIntAtomicCAS

diff -r 8f7ab4f35755 -r a1f632c91185
src/cpu/aarch64/vm/c1_LIRAssembler_aarch64.cpp
--- a/src/cpu/aarch64/vm/c1_LIRAssembler_aarch64.cpp	Tue Dec 17 05:47:59
2013 -0500
+++ b/src/cpu/aarch64/vm/c1_LIRAssembler_aarch64.cpp	Thu Dec 19 16:08:08
2013 +0000
@@ -1567,28 +1567,36 @@
 }
 
 void LIR_Assembler::casw(Register addr, Register newval, Register
cmpval) {
-  Label nope;
+  Label retry_load, nope;
   // flush and load exclusive from the memory location
   // and fail if it is not what we expect
+  __ bind(retry_load);
   __ ldaxrw(rscratch1, addr);
   __ cmpw(rscratch1, cmpval);
   __ cset(rscratch1, Assembler::NE);
   __ br(Assembler::NE, nope);
   // if we store+flush with no intervening write rscratch1 wil be zero
   __ stlxrw(rscratch1, newval, addr);
+  // retry so we only ever return after a load fails to compare
+  // ensures we don't return a stale value after a failed write.
+  __ cbnz(rscratch1, retry_load);
   __ bind(nope);
 }
 
 void LIR_Assembler::casl(Register addr, Register newval, Register
cmpval) {
-  Label nope;
+  Label retry_load, nope;
   // flush and load exclusive from the memory location
   // and fail if it is not what we expect
+  __ bind(retry_load);
   __ ldaxr(rscratch1, addr);
   __ cmp(rscratch1, cmpval);
   __ cset(rscratch1, Assembler::NE);
   __ br(Assembler::NE, nope);
   // if we store+flush with no intervening write rscratch1 wil be zero
   __ stlxr(rscratch1, newval, addr);
+  // retry so we only ever return after a load fails to compare
+  // ensures we don't return a stale value after a failed write.
+  __ cbnz(rscratch1, retry_load);
   __ bind(nope);
 }
 
--- CUT HERE ---