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

[Vladimir Kozlov]

Christian Thalinger wrote:
> On Jul 14, 2011, at 5:44 AM, Vladimir Kozlov wrote:
> 
> 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);

In all usage cases annul was false but there were cases where predict was pn but 
those case had far label so I replaced them with normal code. But I agree that I 
should add default parameters. Done.

> 
> 
> 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);
> 

copy-paste bug. Fixed.

> 
> 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.

Correct. *obj_is_null is far label so I don't want to use cbcond.

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

Thanks,
Vladimir