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

[Niclas Adlertz]

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
>>>>
>>