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

Vladimir Kozlov vladimir.kozlov at
Tue Aug 16 11:14:36 PDT 2011

Thank you, Tom


Tom Rodriguez wrote:
> That looks good.
> tom
> On Aug 15, 2011, at 5:20 PM, Vladimir Kozlov wrote:
>> 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.
>> 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
>>>> 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:
>>>>>>>> 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.

More information about the hotspot-compiler-dev mailing list