Request for reviews (L): 7063629: use cbcond in C2 generated code on T4

[Tom Rodriguez]

This looks really good.

This might be for another day but now that label must be non-NULL, maybe it should be a Label& instead of a Label*.  That would make it easier to use it directly during code generation, as in:

+     __ jmpb($labl$$label);

sparc.ad:

It might be nice to factor this out:

      Assembler::Predict predict_taken =
+       cbuf.is_backward_branch(*L) ? Assembler::pt : Assembler::pn;

x86_32.ad:

Would you get averse to inlining Jcc and JccShort?

output.cpp:

Why does the first round of shorten_branches occur in the middle of init_buffer?  Couldn't it be done right afterwards?  It's just odd that it's buried inside there.

That first round is conservative since we haven't done all padding yet, right?  Then shorten_branches_final does a last pass based on the real offsets?  shorten_branches_final isn't a great name.  Maybe finalize_offsets_and_shorten?

The core shorten branch logic is duplicated in those functions.  Could it be factored out or is there too much local state?

Why was this needed?

*** 2182,2192 ****
--- 2383,2393 ----
        (op != Op_Node &&         // Not an unused antidepedence node and
         // not an unallocated boxlock
         (OptoReg::is_valid(_regalloc->get_reg_first(n)) || op != Op_BoxLock)) ) {
  
      // Push any trailing projections
!     if( bb->_nodes[bb->_nodes.size()-1] != n ) {
!     if( bb->_nodes[_bb_end-1] != n ) {
        for (DUIterator_Fast imax, i = n->fast_outs(imax); i < imax; i++) {
          Node *foi = n->fast_out(i);
          if( foi->is_Proj() )
            _scheduled.push(foi);
        }

That code is complicated enough that I can't reason about it's correctness from a webrev.  Is this because of the trailing NOPs?

Can you add this comment to the that last anti_do_def piece I added:

// kill projections on a branch should appear to occur on the
// branch, not afterwards, so grab the masks from the projections
// and process them.

tom


On Aug 4, 2011, at 6:19 PM, Vladimir Kozlov wrote:

> http://cr.openjdk.java.net/~kvn/7063629/webrev
> 
> 7063629: use cbcond in C2 generated code on T4
> 
> The code is finally shaped as I want and it passed CTW, regression, nsk tests on T4 and x86.
> 
> Added new fused compare and branch instructions into sparc.ad and corresponding short versions which use cbcond instruction. Added new flag avoid_back_to_back to avoid generation of cbcond back to back.
> 
> Split shorten_branches() into 2 methods. First method conservatively estimates code size and branches location and does few rounds of branch shortening. It is executed before ScheduleAndBundle(). Step 3 is moved to new method shorten_branches_final() called after ScheduleAndBundle(). It does final paddings, alignment and final branch replacement. Method fill_buffer() does verification instead of padding.
> 
> Labels are binded now only during code generation in fill_buffer(). As result they are not available when forward branches are emitted. To fix that MacroAssembler branch instructions are used now in x86 .ad files. I replaced unused rtype parameter with maybe_short flag to force using only long branches in .ad long branch instructions.
> 
> Added check to adlc to verify that short version of a branch instructions has the same declaration in .ad file.
> 
> Added assert to verify that the size of emitted instruction matches the value returned by MachNode::size(). Found that MachBreakpointNode::size() returned incorrect value on x64.
> 
> Fixed loop alignment for Sparc (min alignment should be instruction size which is 4 bytes instead of 1 byte).
> 
> The prototype was done by Tom and I took some of his additional fixes. The block changes go with some code in output to put opto assembly style block comments in the PrintNMethods output. There's also snippet in there that deals with the fact kill projections on branches make it appear the kill occurs after the branch instead of being part of it.