Title: [291601] trunk/Source/_javascript_Core
Revision
291601
Author
sbar...@apple.com
Date
2022-03-21 20:52:50 -0700 (Mon, 21 Mar 2022)

Log Message

AirFixObviousSpills needs to consider a PreIndex and PostIndex as clobbering the Reg used for indexing
https://bugs.webkit.org/show_bug.cgi?id=238178
<rdar://87345895>

Reviewed by Mark Lam.

Inside AirFixObviousSpills, we run a basic alias analysis for StackSlots and
registers. For example, when we overwrite a register, we clear anything
it's aliased with. However, the way we were doing this was by looking at
each Arg that was Defd. However, this iteration was missing that
PostIndex/PreIndex mutate the register that feeds into the address Arg.
This patch fixes the issue by walking the instruction in such a way that
we visit all the Defs we care about, both Regs and StackSlots.

* b3/air/AirFixObviousSpills.cpp:

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (291600 => 291601)


--- trunk/Source/_javascript_Core/ChangeLog	2022-03-22 03:49:48 UTC (rev 291600)
+++ trunk/Source/_javascript_Core/ChangeLog	2022-03-22 03:52:50 UTC (rev 291601)
@@ -1,3 +1,21 @@
+2022-03-21  Saam Barati  <sbar...@apple.com>
+
+        AirFixObviousSpills needs to consider a PreIndex and PostIndex as clobbering the Reg used for indexing
+        https://bugs.webkit.org/show_bug.cgi?id=238178
+        <rdar://87345895>
+
+        Reviewed by Mark Lam.
+
+        Inside AirFixObviousSpills, we run a basic alias analysis for StackSlots and
+        registers. For example, when we overwrite a register, we clear anything
+        it's aliased with. However, the way we were doing this was by looking at
+        each Arg that was Defd. However, this iteration was missing that
+        PostIndex/PreIndex mutate the register that feeds into the address Arg.
+        This patch fixes the issue by walking the instruction in such a way that
+        we visit all the Defs we care about, both Regs and StackSlots.
+
+        * b3/air/AirFixObviousSpills.cpp:
+
 2022-03-21  Yusuke Suzuki  <ysuz...@apple.com>
 
         [JSC] ReferenceError when using extra parens in class fields

Modified: trunk/Source/_javascript_Core/b3/air/AirFixObviousSpills.cpp (291600 => 291601)


--- trunk/Source/_javascript_Core/b3/air/AirFixObviousSpills.cpp	2022-03-22 03:49:48 UTC (rev 291600)
+++ trunk/Source/_javascript_Core/b3/air/AirFixObviousSpills.cpp	2022-03-22 03:52:50 UTC (rev 291601)
@@ -188,14 +188,20 @@
         if (AirFixObviousSpillsInternal::verbose)
             dataLog("    Executing ", inst, ": ", m_state, "\n");
 
-        Inst::forEachDefWithExtraClobberedRegs<Arg>(
-            &inst, &inst,
-            [&] (const Arg& arg, Arg::Role, Bank, Width) {
+        Inst::forEachDefWithExtraClobberedRegs<Reg>(&inst, &inst,
+            [&] (const Reg& reg, Arg::Role, Bank, Width) {
                 if (AirFixObviousSpillsInternal::verbose)
-                    dataLog("        Clobbering ", arg, "\n");
-                m_state.clobber(arg);
+                    dataLog("        Clobbering ", reg, "\n");
+                m_state.clobber(reg);
             });
-        
+
+        Inst::forEachDef<StackSlot*>(&inst, &inst,
+            [&] (StackSlot* slot, Arg::Role, Bank, Width) {
+                if (AirFixObviousSpillsInternal::verbose)
+                    dataLog("        Clobbering ", *slot, "\n");
+                m_state.clobber(slot);
+            });
+
         forAllAliases(
             [&] (const auto& alias) {
                 m_state.addAlias(alias);
@@ -558,31 +564,30 @@
             return std::nullopt;
         }
 
-        void clobber(const Arg& arg)
+        void clobber(const Reg& reg)
         {
-            if (arg.isReg()) {
-                regConst.removeAllMatching(
-                    [&] (const RegConst& alias) -> bool {
-                        return alias.reg == arg.reg();
-                    });
-                regSlot.removeAllMatching(
-                    [&] (const RegSlot& alias) -> bool {
-                        return alias.reg == arg.reg();
-                    });
-                return;
-            }
-            if (arg.isStack()) {
-                slotConst.removeAllMatching(
-                    [&] (const SlotConst& alias) -> bool {
-                        return alias.slot == arg.stackSlot();
-                    });
-                regSlot.removeAllMatching(
-                    [&] (const RegSlot& alias) -> bool {
-                        return alias.slot == arg.stackSlot();
-                    });
-            }
+            regConst.removeAllMatching(
+                [&] (const RegConst& alias) -> bool {
+                    return alias.reg == reg;
+                });
+            regSlot.removeAllMatching(
+                [&] (const RegSlot& alias) -> bool {
+                    return alias.reg == reg;
+                });
         }
 
+        void clobber(StackSlot* slot)
+        {
+            slotConst.removeAllMatching(
+                [&] (const SlotConst& alias) -> bool {
+                    return alias.slot == slot;
+                });
+            regSlot.removeAllMatching(
+                [&] (const RegSlot& alias) -> bool {
+                    return alias.slot == slot;
+                });
+        }
+
         void sort()
         {
             std::sort(regConst.begin(), regConst.end(), [] (const RegConst& a, const RegConst& b) {
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to