Bug 198520 – Argument elimination should check transitive dependents for interference

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Tadeu Zagallo]

<rdar://problem/50863343>

[Tadeu Zagallo]

Created attachment 371256 [details]
Patch

[Saam Barati]

Comment on attachment 371256 [details]
Patch

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

LGTM too, just one question

> Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:609
> +                            if (node == candidate && inlineCallFrame == candidate->origin.semantic.inlineCallFrame())

Why this? Why not just break unconditionally when finding the candidate like before?

[Tadeu Zagallo]

Comment on attachment 371256 [details]
Patch

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

>> Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:609
>> +                            if (node == candidate && inlineCallFrame == candidate->origin.semantic.inlineCallFrame())
> 
> Why this? Why not just break unconditionally when finding the candidate like before?

Because the current candidate can interfere with the transitive candidate, which actually happens with the test case included.

[Saam Barati]

That seems weird. I thought the KillStack was the issue?

Also, it’s weird we’re running clobberize on the pre-transformed node. Presumably they may have different effects?

[Tadeu Zagallo]

(In reply to Saam Barati from comment #4)
> That seems weird. I thought the KillStack was the issue?

You're right, that's not necessary.

> Also, it’s weird we’re running clobberize on the pre-transformed node.
> Presumably they may have different effects?

Do you mean prior to transformation? Nothing has been transformed yet at this point.
The tricky thing is that transform has to happen top-dowm and interference is computed bottom-up.

[Tadeu Zagallo]

Created attachment 371309 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 371309 [details]
Patch for landing

Clearing flags on attachment: 371309

Committed r246075: <https://trac.webkit.org/changeset/246075>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Yusuke Suzuki]

*** Bug 197956 has been marked as a duplicate of this bug. ***