RFR(S) 8032656: Tag the MachSpillCopies with purpose information

[Vladimir Kozlov]

Looks fine. NOTE: our repo is closed. Please, hold with the push.

Thanks,
Vlaidmir

On 1/29/14 2:27 PM, Niclas Adlertz wrote:
> Hi Vladimir,
>
> Thank you.
> Here is the new
> webrev:http://cr.openjdk.java.net/~adlertz/JDK-8032656/webrev02/
> I also included machnode.hpp in chaitin.hpp since we need the definition
> of MachSpillCopyNode::SpillType. (Build failed on Solaris otherwise).
>
> Kind Regards,
> Niclas Adlertz
>
> On 2014-01-29 19:46, Vladimir Kozlov wrote:
>> Niclas,
>>
>> Changes are fine but names should be adjusted. I think we should use
>> original "SpillCopy" suffix instead of "SpillNode". "RegToMem" and
>> "MemToReg" are missing the suffux.
>>
>> Thanks,
>> Vladimir
>>
>> On 1/29/14 3:14 AM, Niclas Adlertz wrote:
>>> Hi Vladimir K,
>>>
>>> Thank you for your response.
>>>
>>> Here is an updated webrev:
>>> http://cr.openjdk.java.net/~adlertz/JDK-8032656/webrev01/
>>>
>>> Kind Regards,
>>> Niclas Adlertz
>>>
>>>
>>> On 2014-01-28 18:21, Vladimir Kozlov wrote:
>>>> On 1/28/14 3:18 AM, Niclas Adlertz wrote:
>>>>> Hi Vladimir,
>>>>>
>>>>> Thank you for your response.
>>>>> That is a possibility as well.
>>>>>
>>>>> By using an enum I still have the problem of deciding what to return
>>>>> in Name().
>>>>> I could have either
>>>>> * an array of const char* with the names and index into it by using
>>>>> the enum when returning a name
>>>>> * a switch-case on the enums in the Name() method.
>>>>
>>>> Switch by enum in Name().
>>>>
>>>>>
>>>>> However, I think sub-classing the MachSpillCopyNode looks cleaner.
>>>>> The sub-classing approach could also be of use in a product build
>>>>> when debugging a crash and we want to check what type
>>>>> of node we are at.
>>>>
>>>> You can determine type by enum value too.
>>>>
>>>> Thanks,
>>>> Vladimir
>>>>
>>>>>
>>>>> If it's ok with you, I'll wait and see what the others think. If more
>>>>> people think the enum approach is better I can use
>>>>> enums instead.
>>>>>
>>>>> Kind Regards,
>>>>> Niclas Adlertz
>>>>>
>>>>> On 2014-01-28 11:50, Vladimir Ivanov wrote:
>>>>>> Niclas,
>>>>>>
>>>>>> Why didn't you introduce an enum instead and pass the reason to
>>>>>> constructor? Introducing 13 subclasses just to
>>>>>> overload Name() method in debug builds looks like an overkill to me.
>>>>>>
>>>>>> Best regards,
>>>>>> Vladimir Ivanov
>>>>>>
>>>>>> On 1/28/14 2:07 PM, Niclas Adlertz wrote:
>>>>>>> Hi all,
>>>>>>>
>>>>>>> When debugging or visualizing spills/splits/moves in C2 it's hard to
>>>>>>> know what caused an insertion of a certain MachSpillCopy. I've made
>>>>>>> this
>>>>>>> easier by creating subclasses of the MachSpillCopyNode.
>>>>>>>
>>>>>>> webrev: http://cr.openjdk.java.net/~adlertz/JDK-8032656/webrev00/
>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8032656
>>>>>>>
>>>>>>> Kind Regards,
>>>>>>> Niclas Adlertz
>>>>>
>>>
>