Request for reviews (XS): 7079317: Incorrect branch's destination block in PrintoOptoAssembly output

[Vladimir Kozlov]

Tom,

You should not give me these ideas since I can't back out now :) . Here is 
implementation using MachBranchNode. The only problem was JumpX mach node which 
is subclass of MachConstantNode. But it is fine since it does not have label, 
short version or delay slot (the sparc instruction has delay slot but we use 
ialu_reg_reg pipe_class). It needs only one additional check in output.cpp where 
Kill projections are processed.

http://cr.openjdk.java.net/~kvn/7079317/webrev

Thanks,
Vladimir

Tom Rodriguez wrote:
> On Aug 15, 2011, at 12:04 PM, Vladimir Kozlov wrote:
> 
>> Tom Rodriguez wrote:
>>> On Aug 15, 2011, at 10:50 AM, Vladimir Kozlov wrote:
>>>> Node::size() for branches calls code in scratch_emit_size() which resets label and block. An other solution for this problem would be save/restore label and block in scratch_emit_size() but it would require a lot more code changes.
>>> Ah.  Fixing scratch_emit_size seems better since it's kind of a surprising behaviour.  It's not that much code is it?
>> It needs a virtual method in MachNode which increase vtable of all Mach nodes. Here is webrev:
> 
> If we're really concerned about vtable size, all of those subtype specific setter/getters could probably be elsewhere down in the hierarchy.  The only meaningful implementations of label_set are in subclasses of MachGotoNode and MachIfNode so it seems like it could be moved into a new superclass of them.
> 
> I guess alternatively you could have a single virtual which returns the labelOper and implement label_set and save_label non-virtually in terms of that, though that probably doesn't play well with MachNullCheck which is_Branch but doesn't have a label.  The whole labelOper machinery looks ridiculously complicated...
> 
> Anyway, your change is ok with me as is.
> 
> tom
> 
>> http://cr.openjdk.java.net/~kvn/7079317/webrev
>>
>> Vladimir
>>
>>> tom
>>>> Vladimir
>>>>
>>>> Tom Rodriguez wrote:
>>>>> I don't understand how calling insts_size and Node::size causes a bug.  What am I missing?
>>>>> tom
>>>>> On Aug 15, 2011, at 8:58 AM, Vladimir Kozlov wrote:
>>>>>> http://cr.openjdk.java.net/~kvn/7079317/webrev
>>>>>>
>>>>>> 7079317: Incorrect branch's destination block in PrintoOptoAssembly output
>>>>>>
>>>>>> After changes for 7063629 PrintoOptoAssembly output shows all branches have B0 as destination block.
>>>>>> Remove unneeded debug verification code which overwrites label and block information for branches. There are other checks there which verify that code size was not changed.
>