Bug 216110 – Finish comment describing the various *Stack SSA nodes in DFG

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Keith Miller]

Finish comment describing the various *Stack SSA nodes in DFG

[Keith Miller]

Created attachment 407839 [details]
Patch

[EWS]

Committed r266532: <https://trac.webkit.org/changeset/266532>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 407839 [details].

[Radar WebKit Bug Importer]

<rdar://problem/68288007>

[Saam Barati]

Comment on attachment 407839 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=407839&action=review

> Source/JavaScriptCore/dfg/DFGNodeType.h:79
> +    /* exits are rare this is preferable to representing availability directly in SSA. */\

why? Storing to the stack isn't free. If something is in a register, you don't also want to throw it onto the stack just for OSRs sake.

[Keith Miller]

Comment on attachment 407839 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=407839&action=review

>> Source/JavaScriptCore/dfg/DFGNodeType.h:79
>> +    /* exits are rare this is preferable to representing availability directly in SSA. */\
> 
> why? Storing to the stack isn't free. If something is in a register, you don't also want to throw it onto the stack just for OSRs sake.

I don't think that's what this comment says but maybe it could be more explicit.

[Saam Barati]

(In reply to Keith Miller from comment #5)
> Comment on attachment 407839 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=407839&action=review
> 
> >> Source/JavaScriptCore/dfg/DFGNodeType.h:79
> >> +    /* exits are rare this is preferable to representing availability directly in SSA. */\
> > 
> > why? Storing to the stack isn't free. If something is in a register, you don't also want to throw it onto the stack just for OSRs sake.
> 
> I don't think that's what this comment says but maybe it could be more
> explicit.

"this is preferable to representing availability directly in SSA" is not right.

[Saam Barati]

(In reply to Saam Barati from comment #6)
> (In reply to Keith Miller from comment #5)
> > Comment on attachment 407839 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=407839&action=review
> > 
> > >> Source/JavaScriptCore/dfg/DFGNodeType.h:79
> > >> +    /* exits are rare this is preferable to representing availability directly in SSA. */\
> > > 
> > > why? Storing to the stack isn't free. If something is in a register, you don't also want to throw it onto the stack just for OSRs sake.
> > 
> > I don't think that's what this comment says but maybe it could be more
> > explicit.
> 
> "this is preferable to representing availability directly in SSA" is not
> right.

Maybe you mean if both things were already the case? Like we happened to have a spill for another reason?

[Keith Miller]

Comment on attachment 407839 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=407839&action=review

>>>>> Source/JavaScriptCore/dfg/DFGNodeType.h:79
>>>>> +    /* exits are rare this is preferable to representing availability directly in SSA. */\
>>>> 
>>>> why? Storing to the stack isn't free. If something is in a register, you don't also want to throw it onto the stack just for OSRs sake.
>>> 
>>> I don't think that's what this comment says but maybe it could be more explicit.
>> 
>> "this is preferable to representing availability directly in SSA" is not right.
> 
> Maybe you mean if both things were already the case? Like we happened to have a spill for another reason?

Correct, I'm saying that the PutStacks are used to represent shadow data flow. We don't necessarily want B3 spend time register allocating that data if we could get it from the stack. That comment wasn't trying to say it's preferable to PutStack every OSR exit value.