[aarch64-port-dev ] Guarantee failures since 8144028: Use AArch64 bit-test instructions in C2

Andrew Haley aph at redhat.com
Wed Dec 9 19:00:00 UTC 2015


On 12/08/2015 06:22 PM, Edward Nevill wrote:
> Hi,
> 
> Since "8144028: Use AArch64 bit-test instructions in C2" I have been seeing occasional guarantee failures of the form.
> 
> #  Internal Error (assembler_aarch64.hpp:223), pid=4241, tid=4595
> #  guarantee(chk == -1 || chk == 0) failed: Field too big for insn
> 
> These are being generated by the following call from pd_patch_instruction_size in macroAssembler_aarch64.cpp
> 
>     // Test & branch (immediate)
>     Instruction_aarch64::spatch(branch, 18, 5, offset);
> 
> The problem is that test and branch instructions only have a 14 bit offset giving a range of +/- 32Kb which is not sufficient for large C2 methods.
> 
> What can we do about this? It seems a shame to backout this optimization but I cannot see any easy way around it.

Please try this patch.

Andrew.




-------------- next part --------------
diff --git a/src/cpu/aarch64/vm/aarch64.ad b/src/cpu/aarch64/vm/aarch64.ad
--- a/src/cpu/aarch64/vm/aarch64.ad
+++ b/src/cpu/aarch64/vm/aarch64.ad
@@ -3484,10 +3484,17 @@
   return 0;
 }
 
-bool Matcher::is_short_branch_offset(int rule, int br_size, int offset)
-{
-  Unimplemented();
-  return false;
+// Is this branch offset short enough that a short branch can be used?
+//
+// NOTE: If the platform does not provide any short branch variants, then
+//       this method should return false for offset 0.
+bool Matcher::is_short_branch_offset(int rule, int br_size, int offset) {
+  // The passed offset is relative to address of the branch.  On
+  // AArch64 a branch displacement is calculated relative to address
+  // of the next instruction.
+  offset -= br_size;
+
+  return (-32768 <= offset && offset < 32768);
 }
 
 const bool Matcher::isSimpleConstant64(jlong value) {
@@ -13845,7 +13852,8 @@
 
 // Test bit and Branch
 
-instruct cmpL_branch_sign(cmpOp cmp, iRegL op1, immL0 op2, label labl, rFlagsReg cr) %{
+// Patterns for short (< 32KiB) variants
+instruct cmpL_branch_sign(cmpOp cmp, iRegL op1, immL0 op2, label labl) %{
   match(If cmp (CmpL op1 op2));
   predicate(n->in(1)->as_Bool()->_test._test == BoolTest::lt
             || n->in(1)->as_Bool()->_test._test == BoolTest::ge);
@@ -13855,16 +13863,15 @@
   format %{ "cb$cmp   $op1, $labl # long" %}
   ins_encode %{
     Label* L = $labl$$label;
-    Assembler::Condition cond = (Assembler::Condition)$cmp$$cmpcode;
-    if (cond == Assembler::LT)
-      __ tbnz($op1$$Register, 63, *L);
-    else
-      __ tbz($op1$$Register, 63, *L);
+    Assembler::Condition cond =
+      ((Assembler::Condition)$cmp$$cmpcode == Assembler::LT) ? Assembler::NE : Assembler::EQ;
+    __ tbr($op1$$Register, cond, 63, *L);
   %}
   ins_pipe(pipe_cmp_branch);
-%}
-
-instruct cmpI_branch_sign(cmpOp cmp, iRegIorL2I op1, immI0 op2, label labl, rFlagsReg cr) %{
+  ins_short_branch(1);
+%}
+
+instruct cmpI_branch_sign(cmpOp cmp, iRegIorL2I op1, immI0 op2, label labl) %{
   match(If cmp (CmpI op1 op2));
   predicate(n->in(1)->as_Bool()->_test._test == BoolTest::lt
             || n->in(1)->as_Bool()->_test._test == BoolTest::ge);
@@ -13874,16 +13881,15 @@
   format %{ "cb$cmp   $op1, $labl # int" %}
   ins_encode %{
     Label* L = $labl$$label;
-    Assembler::Condition cond = (Assembler::Condition)$cmp$$cmpcode;
-    if (cond == Assembler::LT)
-      __ tbnz($op1$$Register, 31, *L);
-    else
-      __ tbz($op1$$Register, 31, *L);
+    Assembler::Condition cond =
+      ((Assembler::Condition)$cmp$$cmpcode == Assembler::LT) ? Assembler::NE : Assembler::EQ;
+    __ tbr($op1$$Register, cond, 31, *L);
   %}
   ins_pipe(pipe_cmp_branch);
-%}
-
-instruct cmpL_branch_bit(cmpOp cmp, iRegL op1, immL op2, immL0 op3, label labl, rFlagsReg cr) %{
+  ins_short_branch(1);
+%}
+
+instruct cmpL_branch_bit(cmpOp cmp, iRegL op1, immL op2, immL0 op3, label labl) %{
   match(If cmp (CmpL (AndL op1 op2) op3));
   predicate((n->in(1)->as_Bool()->_test._test == BoolTest::ne
             || n->in(1)->as_Bool()->_test._test == BoolTest::eq)
@@ -13896,15 +13902,13 @@
     Label* L = $labl$$label;
     Assembler::Condition cond = (Assembler::Condition)$cmp$$cmpcode;
     int bit = exact_log2($op2$$constant);
-    if (cond == Assembler::EQ)
-      __ tbz($op1$$Register, bit, *L);
-    else
-      __ tbnz($op1$$Register, bit, *L);
+    __ tbr($op1$$Register, cond, bit, *L);
   %}
   ins_pipe(pipe_cmp_branch);
-%}
-
-instruct cmpI_branch_bit(cmpOp cmp, iRegIorL2I op1, immI op2, immI0 op3, label labl, rFlagsReg cr) %{
+  ins_short_branch(1);
+%}
+
+instruct cmpI_branch_bit(cmpOp cmp, iRegIorL2I op1, immI op2, immI0 op3, label labl) %{
   match(If cmp (CmpI (AndI op1 op2) op3));
   predicate((n->in(1)->as_Bool()->_test._test == BoolTest::ne
             || n->in(1)->as_Bool()->_test._test == BoolTest::eq)
@@ -13917,10 +13921,79 @@
     Label* L = $labl$$label;
     Assembler::Condition cond = (Assembler::Condition)$cmp$$cmpcode;
     int bit = exact_log2($op2$$constant);
-    if (cond == Assembler::EQ)
-      __ tbz($op1$$Register, bit, *L);
-    else
-      __ tbnz($op1$$Register, bit, *L);
+    __ tbr($op1$$Register, cond, bit, *L);
+  %}
+  ins_pipe(pipe_cmp_branch);
+  ins_short_branch(1);
+%}
+
+// And far variants
+instruct far_cmpL_branch_sign(cmpOp cmp, iRegL op1, immL0 op2, label labl) %{
+  match(If cmp (CmpL op1 op2));
+  predicate(n->in(1)->as_Bool()->_test._test == BoolTest::lt
+            || n->in(1)->as_Bool()->_test._test == BoolTest::ge);
+  effect(USE labl);
+
+  ins_cost(BRANCH_COST);
+  format %{ "cb$cmp   $op1, $labl # long" %}
+  ins_encode %{
+    Label* L = $labl$$label;
+    Assembler::Condition cond =
+      ((Assembler::Condition)$cmp$$cmpcode == Assembler::LT) ? Assembler::NE : Assembler::EQ;
+    __ tbr($op1$$Register, cond, 63, *L, /*far*/true);
+  %}
+  ins_pipe(pipe_cmp_branch);
+%}
+
+instruct far_cmpI_branch_sign(cmpOp cmp, iRegIorL2I op1, immI0 op2, label labl) %{
+  match(If cmp (CmpI op1 op2));
+  predicate(n->in(1)->as_Bool()->_test._test == BoolTest::lt
+            || n->in(1)->as_Bool()->_test._test == BoolTest::ge);
+  effect(USE labl);
+
+  ins_cost(BRANCH_COST);
+  format %{ "cb$cmp   $op1, $labl # int" %}
+  ins_encode %{
+    Label* L = $labl$$label;
+    Assembler::Condition cond =
+      ((Assembler::Condition)$cmp$$cmpcode == Assembler::LT) ? Assembler::NE : Assembler::EQ;
+    __ tbr($op1$$Register, cond, 31, *L, /*far*/true);
+  %}
+  ins_pipe(pipe_cmp_branch);
+%}
+
+instruct far_cmpL_branch_bit(cmpOp cmp, iRegL op1, immL op2, immL0 op3, label labl) %{
+  match(If cmp (CmpL (AndL op1 op2) op3));
+  predicate((n->in(1)->as_Bool()->_test._test == BoolTest::ne
+            || n->in(1)->as_Bool()->_test._test == BoolTest::eq)
+            && is_power_of_2(n->in(2)->in(1)->in(2)->get_long()));
+  effect(USE labl);
+
+  ins_cost(BRANCH_COST);
+  format %{ "tb$cmp   $op1, $op2, $labl" %}
+  ins_encode %{
+    Label* L = $labl$$label;
+    Assembler::Condition cond = (Assembler::Condition)$cmp$$cmpcode;
+    int bit = exact_log2($op2$$constant);
+    __ tbr($op1$$Register, cond, bit, *L, /*far*/true);
+  %}
+  ins_pipe(pipe_cmp_branch);
+%}
+
+instruct far_cmpI_branch_bit(cmpOp cmp, iRegIorL2I op1, immI op2, immI0 op3, label labl) %{
+  match(If cmp (CmpI (AndI op1 op2) op3));
+  predicate((n->in(1)->as_Bool()->_test._test == BoolTest::ne
+            || n->in(1)->as_Bool()->_test._test == BoolTest::eq)
+            && is_power_of_2(n->in(2)->in(1)->in(2)->get_int()));
+  effect(USE labl);
+
+  ins_cost(BRANCH_COST);
+  format %{ "tb$cmp   $op1, $op2, $labl" %}
+  ins_encode %{
+    Label* L = $labl$$label;
+    Assembler::Condition cond = (Assembler::Condition)$cmp$$cmpcode;
+    int bit = exact_log2($op2$$constant);
+    __ tbr($op1$$Register, cond, bit, *L, /*far*/true);
   %}
   ins_pipe(pipe_cmp_branch);
 %}
diff --git a/src/cpu/aarch64/vm/c1_MacroAssembler_aarch64.hpp b/src/cpu/aarch64/vm/c1_MacroAssembler_aarch64.hpp
--- a/src/cpu/aarch64/vm/c1_MacroAssembler_aarch64.hpp
+++ b/src/cpu/aarch64/vm/c1_MacroAssembler_aarch64.hpp
@@ -27,6 +27,7 @@
 #define CPU_AARCH64_VM_C1_MACROASSEMBLER_AARCH64_HPP
 
 using MacroAssembler::build_frame;
+using MacroAssembler::null_check;
 
 // C1_MacroAssembler contains high-level macros for C1
 
diff --git a/src/cpu/aarch64/vm/macroAssembler_aarch64.hpp b/src/cpu/aarch64/vm/macroAssembler_aarch64.hpp
--- a/src/cpu/aarch64/vm/macroAssembler_aarch64.hpp
+++ b/src/cpu/aarch64/vm/macroAssembler_aarch64.hpp
@@ -487,6 +487,32 @@
     orr(Vd, T, Vn, Vn);
   }
 
+public:
+
+  // Generalized Test Bit And Branch, including a "far" variety which
+  // spans more than 32KiB.
+  void tbr(Register Rt, Condition cond, int bitpos, Label &dest, bool far = false) {
+    assert(cond == Assembler::EQ || cond == Assembler::NE, "must be");
+
+    if (far)
+      cond = ~cond;
+
+    void (Assembler::* branch)(Register Rt, int bitpos, Label &L);
+    if (cond == Assembler::EQ)
+      branch = &Assembler::tbz;
+    else
+      branch = &Assembler::tbnz;
+
+    if (far) {
+      Label L;
+      (this->*branch)(Rt, bitpos, L);
+      b(dest);
+      bind(L);
+    } else {
+      (this->*branch)(Rt, bitpos, dest);
+    }
+  }
+
   // macro instructions for accessing and updating floating point
   // status register
   //
diff --git a/src/share/vm/adlc/formssel.cpp b/src/share/vm/adlc/formssel.cpp
--- a/src/share/vm/adlc/formssel.cpp
+++ b/src/share/vm/adlc/formssel.cpp
@@ -1246,7 +1246,8 @@
       !is_short_branch() &&     // Don't match another short branch variant
       reduce_result() != NULL &&
       strcmp(reduce_result(), short_branch->reduce_result()) == 0 &&
-      _matrule->equivalent(AD.globalNames(), short_branch->_matrule)) {
+      _matrule->equivalent(AD.globalNames(), short_branch->_matrule) &&
+      equivalent_predicates(this, short_branch)) {
     // The instructions are equivalent.
 
     // Now verify that both instructions have the same parameters and


More information about the aarch64-port-dev mailing list