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

Vladimir Kozlov vladimir.kozlov at oracle.com
Wed Jan 29 10:46:58 PST 2014


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


More information about the hotspot-compiler-dev mailing list