[webkit-changes] [284103] trunk/Source/JavaScriptCore

Wed, 13 Oct 2021 10:50:03 -0700

Title: [284103] trunk/Source/_javascript_Core
Revision
284103
Author
[email protected]
Date
2021-10-13 10:49:13 -0700 (Wed, 13 Oct 2021)

Log Message

[JSC][32bit] Fix wrong branchAdd32 assembly for ARMv7
https://bugs.webkit.org/show_bug.cgi?id=231362

Patch by Mikhail R. Gadelha <[email protected]> on 2021-10-13
Reviewed by Yusuke Suzuki.

After the unlinked baseline jit was merged, a new branchAdd32 method
needed to be implemented for ARMv7, however, the first version
submitted by me used the add32 method to perform the add before
branching.

In this patch, we fix the call to add32 by adding a new private method
add32Impl with an optional parameter that selects either add or adds,
make branchAdd32 call the adds version.

(Patch co-authored with Geza Lore)

* assembler/MacroAssemblerARMv7.h:
(JSC::MacroAssemblerARMv7::branchAdd32):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (284102 => 284103)


--- trunk/Source/_javascript_Core/ChangeLog	2021-10-13 17:28:28 UTC (rev 284102)
+++ trunk/Source/_javascript_Core/ChangeLog	2021-10-13 17:49:13 UTC (rev 284103)
@@ -1,3 +1,24 @@
+2021-10-13  Mikhail R. Gadelha  <[email protected]>
+
+        [JSC][32bit] Fix wrong branchAdd32 assembly for ARMv7
+        https://bugs.webkit.org/show_bug.cgi?id=231362
+
+        Reviewed by Yusuke Suzuki.
+
+        After the unlinked baseline jit was merged, a new branchAdd32 method
+        needed to be implemented for ARMv7, however, the first version
+        submitted by me used the add32 method to perform the add before
+        branching.
+
+        In this patch, we fix the call to add32 by adding a new private method
+        add32Impl with an optional parameter that selects either add or adds,
+        make branchAdd32 call the adds version.
+
+        (Patch co-authored with Geza Lore)
+
+        * assembler/MacroAssemblerARMv7.h:
+        (JSC::MacroAssemblerARMv7::branchAdd32):
+
 2021-10-12  Alexey Proskuryakov  <[email protected]>
 
         Invoke build scripts with python3 explicitly

Modified: trunk/Source/_javascript_Core/assembler/MacroAssemblerARMv7.h (284102 => 284103)


--- trunk/Source/_javascript_Core/assembler/MacroAssemblerARMv7.h	2021-10-13 17:28:28 UTC (rev 284102)
+++ trunk/Source/_javascript_Core/assembler/MacroAssemblerARMv7.h	2021-10-13 17:49:13 UTC (rev 284103)
@@ -189,7 +189,6 @@
         }
 
         ARMThumbImmediate armImm = ARMThumbImmediate::makeUInt12OrEncodedImm(imm.m_value);
-
         if (armImm.isValid())
             m_assembler.add(dest, src, armImm);
         else {
@@ -200,19 +199,8 @@
 
     void add32(TrustedImm32 imm, Address address)
     {
-        load32(address, dataTempRegister);
-
-        ARMThumbImmediate armImm = ARMThumbImmediate::makeUInt12OrEncodedImm(imm.m_value);
-        if (armImm.isValid())
-            m_assembler.add(dataTempRegister, dataTempRegister, armImm);
-        else {
-            // Hrrrm, since dataTempRegister holds the data loaded,
-            // use addressTempRegister to hold the immediate.
-            move(imm, addressTempRegister);
-            m_assembler.add(dataTempRegister, dataTempRegister, addressTempRegister);
-        }
-
-        store32(dataTempRegister, address);
+        constexpr bool updateFlags = false;
+        add32Impl(imm, address, updateFlags);
     }
 
     void add32(Address src, RegisterID dest)
@@ -223,19 +211,8 @@
 
     void add32(TrustedImm32 imm, AbsoluteAddress address)
     {
-        load32(address.m_ptr, dataTempRegister);
-
-        ARMThumbImmediate armImm = ARMThumbImmediate::makeUInt12OrEncodedImm(imm.m_value);
-        if (armImm.isValid())
-            m_assembler.add(dataTempRegister, dataTempRegister, armImm);
-        else {
-            // Hrrrm, since dataTempRegister holds the data loaded,
-            // use addressTempRegister to hold the immediate.
-            move(imm, addressTempRegister);
-            m_assembler.add(dataTempRegister, dataTempRegister, addressTempRegister);
-        }
-
-        store32(dataTempRegister, address.m_ptr);
+        constexpr bool updateFlags = false;
+        add32Impl(imm, address, updateFlags);
     }
 
     void getEffectiveAddress(BaseIndex address, RegisterID dest)
@@ -1577,6 +1554,52 @@
         }
     }
 
+    void add32Impl(TrustedImm32 imm, Address address, bool updateFlags = false)
+    {
+        load32(address, dataTempRegister);
+
+        ARMThumbImmediate armImm = ARMThumbImmediate::makeEncodedImm(imm.m_value);
+        if (armImm.isValid()) {
+            if (updateFlags)
+                m_assembler.add_S(dataTempRegister, dataTempRegister, armImm);
+            else
+                m_assembler.add(dataTempRegister, dataTempRegister, armImm);
+        } else {
+            // Hrrrm, since dataTempRegister holds the data loaded,
+            // use addressTempRegister to hold the immediate.
+            move(imm, addressTempRegister);
+            if (updateFlags)
+                m_assembler.add_S(dataTempRegister, dataTempRegister, addressTempRegister);
+            else
+                m_assembler.add(dataTempRegister, dataTempRegister, addressTempRegister);
+        }
+
+        store32(dataTempRegister, address);
+    }
+
+    void add32Impl(TrustedImm32 imm, AbsoluteAddress address, bool updateFlags = false)
+    {
+        load32(address.m_ptr, dataTempRegister);
+
+        ARMThumbImmediate armImm = ARMThumbImmediate::makeEncodedImm(imm.m_value);
+        if (armImm.isValid()) {
+            if (updateFlags)
+                m_assembler.add_S(dataTempRegister, dataTempRegister, armImm);
+            else
+                m_assembler.add(dataTempRegister, dataTempRegister, armImm);
+        } else {
+            // Hrrrm, since dataTempRegister holds the data loaded,
+            // use addressTempRegister to hold the immediate.
+            move(imm, addressTempRegister);
+            if (updateFlags)
+                m_assembler.add_S(dataTempRegister, dataTempRegister, addressTempRegister);
+            else
+                m_assembler.add(dataTempRegister, dataTempRegister, addressTempRegister);
+        }
+
+        store32(dataTempRegister, address.m_ptr);
+    }
+
 public:
     void test32(RegisterID reg, TrustedImm32 mask = TrustedImm32(-1))
     {
@@ -1847,13 +1870,15 @@
 
     Jump branchAdd32(ResultCondition cond, TrustedImm32 imm, AbsoluteAddress dest)
     {
-        add32(imm, dest);
+        constexpr bool updateFlags = true;
+        add32Impl(imm, dest, updateFlags);
         return Jump(makeBranch(cond));
     }
 
     Jump branchAdd32(ResultCondition cond, TrustedImm32 imm, Address dest)
     {
-        add32(imm, dest);
+        constexpr bool updateFlags = true;
+        add32Impl(imm, dest, updateFlags);
         return Jump(makeBranch(cond));
     }
 

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to