Skip to content

Commit

Permalink
Cherry-pick 259548.47@safari-7615-branch (0f2c121). https://bugs.webk…
Browse files Browse the repository at this point in the history
…it.org/show_bug.cgi?id=251640

    [JSC] FTL arguments elimination should ensure that replacement sites can access to original stack slots
    https://bugs.webkit.org/show_bug.cgi?id=251640
    rdar://99273500

    Reviewed by Mark Lam.

    FTL arguments elimination does analysis and attempts to eliminate arguments allocation if it is not escaped.
    We emit stack access at `arguments[0]` site for example, and remove `arguments` allocations.
    But important thing is that stack slots used for the `arguments` need to be available at `arguments[0]` access site.
    Since we are using stack slots for different purpose when inlining different functions, it is possible that the given
    stack slot is no longer available when using `arguments[0]`. For example,

        function a() { return arguments; }
        function b() { do-something }

        var arg = a()
        b();
        arg[0];         // If both "a" and "b" are inlined, stack slots used for inlined "a" can be used for the other purpose for "b"
                        // As a result, it is possible that the slot is not available at `arg[0]` access point.

    We were doing stack slot interference analysis to avoid the above problem[1]. However, it was not complete solution since it is only
    checking block-local status. So if we have branch between a() and arg[0], this analysis didn't work. Attached test
    "arguments-elimination-should-happen-only-when-stack-slot-is-available-at-replacement-site.js" is literally doing this.

        function empty() {}

        function bar2(...a0) {
          return a0;
        }

        function foo() {
          let xs = bar2(undefined);
          '' == 1 && 0;
          return empty(...xs, undefined);
        }

    Between bar2 and `...xs` site, we have branch due to &&. And at "...xs" site, the stack slot were no longer available.

    In this patch, we replace our existing interference analysis with the revised fix. We use OSR availability which can describe the
    state of each stack slot. For all arguments, initially, it is flushed state with a node. Then, when slot gets unavailable or overridden,
    we can see the availability change, which no longer points at the same node.
    We first do this OSR availability analysis and capture availability map of each candidates. And then, we analyze whether replacement sites
    are still seeing the same availability for arguments. And if it becomes different, we remove the candidate from optimization target. This change
    simplifies our analysis significantly, and make it procedure global (previous one was block local).

    [1]: https://commits.webkit.org/212536@main

    * JSTests/stress/arguments-elimination-should-happen-only-when-stack-slot-is-available-at-replacement-site.js: Added.
    (empty):
    (bar2):
    (foo):
    (main):
    * Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:

    Canonical link: https://commits.webkit.org/259548.47@safari-7615-branch
  • Loading branch information
Constellation authored and aperezdc committed Apr 3, 2023
1 parent bf3fe6d commit 7c80400
Show file tree
Hide file tree
Showing 2 changed files with 277 additions and 176 deletions.
@@ -0,0 +1,19 @@
//@ runDefault("--jitPolicyScale=0")
function empty() {}

function bar2(...a0) {
return a0;
}

function foo() {
let xs = bar2(undefined);
'' == 1 && 0;
return empty(...xs, undefined);
}
function main () {
for (let i = 0; i < 1_000_000; i++) {
let a = foo();
Array.isArray(a);
}
}
main();

0 comments on commit 7c80400

Please sign in to comment.