review (S) for 6978249: spill between cpu and fpu registers when those moves are fast

[Vladimir Kozlov]

On 8/18/10 5:01 PM, Tom Rodriguez wrote:
>> coalesce.cpp:
>> How it is related to FPU spilling?
>
> It's not directly related but there were some spilling tweaks done as part of the change.  I need to go back and evaluate them more carefully but I want to include them for now which is why I left them guarded.

OK.

>> matcher.cpp:
>> this code is x86 specific, for sparc it would be different. Need comment so we will not forget to fix it for sparc.
>
> I think it might have to become AD specific because the sparc to instructions aren't symmetric.  I added some comments explaining the logic.

Good.

>> Why you added spill mask for F/D to I/L? Are these changes allow to spill FPU regs to GPU regs?
>
> Yes, it allows spilling in both directions.  If they are fast enough for one why not allow the other?

OK.

>> reg_split.cpp:
>> The next comment does not explain why we need to split for Calls:
>> // These need a Split regardless of overlap or pressure
>
> I thought I understood this one but reading it again I'm not sure.  I'll look at it some more.

Thanks,
Vladimir

>
> tom
>
>>
>>
>> On 8/18/10 3:19 PM, Tom Rodriguez wrote:
>>> http://cr.openjdk.java.net/~never/6978249
>>>
>>> 6978249: spill between cpu and fpu registers when those moves are fast
>>> Reviewed-by:
>>>
>>> On some architectures moves between CPU and FPU registers are fast so
>>> they can be used for spilling instead of the stack.  This change adds
>>> a new flag UseFPUForSpilling and sets up the spill reg masks to allow
>>> this when the flag is on.  Currently for Nehalem class chips it seems
>>> to be a uniform win but we'll keep it under AggressiveOpts for now.
>>> There are some minor changes to spilling logic that are currently
>>> guarded until we determine that they are generally a good idea.  I
>>> also moved the logic for PrintFlagsFinal since the initialization of
>>> several subsystems may change some flag values which will be missed by
>>> the current location.  Tested with scimark, ctw and the nsk tests on
>>> 32 and 64 bit.
>