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

[Tom Rodriguez]

On Aug 18, 2010, at 5:06 PM, Vladimir Kozlov wrote:

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

So the umask at a call is generally the debug info mask which is pretty much stack only so if you execute this code:

                // Both are either up or down, and there is overlap, No Split
                n->set_req(inpidx, def);

you force the def down.  If you insert a split at the use before the call you keep the call from forcing the def down which is what the new code is doing.  I suspect the new code should replace the code above instead of skipping the mem-mem case that follows but I'd like to leave it as for now.  I'm going to go back and evaluate this change and the coalesce change later.  I updated the code with this comment:

            if (UseFPUForSpilling && n->is_Call() && !uup && !dup ) {
              // The use at the call can force the def down so insert                                                                                 
              // a split before the use to allow the def more freedom.                                                                               
              maxlrg = split_USE(def,b,n,inpidx,maxlrg,dup,false, splits,slidx);

Part of the problem here is that up and down are slightly strange notions.  up means register only and down means not register only, so down can actually include some registers.  RegMask::is_AllStack covers the case where it's stack only.  Adding FPU registers to the spill mask makes this distinction some more illogical which is what motivated the tweaks I think.

tom

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