Request for reviews (L): 7063628: Use cbcond on T4

[Christian Thalinger]

On Jul 14, 2011, at 5:44 AM, Vladimir Kozlov wrote:
> http://cr.openjdk.java.net/~kvn/7063628/webrev
> 
> Fixed 7063628: Use cbcond on T4
> 
> Added new MacroAssembler instructions cmp_and_br() and old branch instructions are modified to use cbcond on T4 if distance is small (2K bytes). Most of the rest changes are usage of these new branch instructions in Interpreter and C1.
> 
> The prototype was done by Tom and I took some of his additional fixes. The formssel.cpp change is a bug fix where the MatchNode equality test wasn't recursing so it would mistakenly return true for complex matches. Added a fake label for branches generated in temp buffer by MachNode::emit_size(), added assert into .ad files to check label.
> 
> There was problem in is_in_wdisp16_range() (and in initial implementation of use_cbc()) which calls target(L) before emitting branch. Non-bound (forward branch) labels record current pc() as a branch address and later try to patch instruction (which could be 'cmp') at that address.
> 
> Removed unused code in check_klass_subtype_fast_path().
> 
> Modified vm_version string: has_ prefix is removed and only v8 or v9 will be printed as well only niagara_plus or niagara.
> 
> Tested on T4 with CTW, nsk, VM and java invoke regression tests.


src/cpu/sparc/vm/assembler_sparc.hpp:

-  void br_zero( Condition c, bool a, Predict p, Register s1, Label& L);
+  // compares register with zero (32 bit) and branches (V9 and V8 instructions)
+  void br_zero   ( Register s1, Label& L );

Why did you remove the annul and predict argument while you left it with br_null/br_notnull?  Maybe we should add default arguments for these:

+  void br_zero(Register s1, Label& L, bool a = false, Predict p = Assembler::pt);


src/cpu/sparc/vm/assembler_sparc.cpp:

-    cmp(value_reg, top_reg_after_save);
-    br(Assembler::notEqual, false, Assembler::pn, not_same);
-    delayed()->nop();
+    cmp_and_br(value_reg, top_reg_after_save, Assembler::notEqual, false, Assembler::pt, not_same);

You changed the predict bit.  Intentionally?

Here too:

-    cmp(t1, t2);
-    br(Assembler::equal, false, Assembler::pt, ok);
-    delayed()->nop();
+    cmp_and_br(t1, t2, Assembler::equal, false, Assembler::pn, ok);


src/cpu/sparc/vm/c1_LIRAssembler_sparc.cpp:

2608     __ ba(*obj_is_null, false);
2609     __ delayed()->nop();
2610     __ bind(not_null);
2611   } else {
2612     __ br_null(obj, false, Assembler::pn, *obj_is_null, false);
2613     __ delayed()->nop();
2614   }

Just a question:  I guess you are using emit_delayed_nop = false here to not emit a cbcond instruction.  What is the reason for this?  There are also a couple of ba with a nop in the delay slot that use false.

Otherwise this looks good.  Nice cleanup!  And thanks for enabling UseRDPCForConstantTableBase on T4s.

-- Christian