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

Tom Rodriguez tom.rodriguez at oracle.com
Thu Jul 14 11:52:00 PDT 2011


> 
> compile.cpp:
> 
> +   if (n->is_Branch() && n->as_Mach()->ideal_Opcode() != Op_Jump) {
> +     // Fake label for branch instruction.
> +     MacroAssembler masm(&buf);
> +     Label fakeL;
> +     masm.bind(fakeL);
> +     n->as_Mach()->label_set(fakeL, 0);
> +   }

The label be cleared after doing the emit too so we don't end up with garbage in there.

tom

> 
> What guarantees do we get about the lifetime of the storage for fakeL since it doesn't appear to be used?  I would assume that at a minimum it should be declared outside of the if.
> 
> You can clean up all the "l ? (l->loc_pos() - (cbuf.insts_size() + 1)) : 0;" code in x86_*.ad after this fix too.  We can even use the MacroAssembler for these instrutions now if we want to though that's probably for another day.
> 
> Should labelOper::label have an assert that it's non-null when called?  That would cover all the other asserts you added though I'm not sure it must be true.
> 
> The overall change looks good.
> 
> tom
> 
> 
> On Jul 13, 2011, at 8:44 PM, 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.
> 



More information about the hotspot-compiler-dev mailing list