Changeset 204009 in webkit

Timestamp:

Aug 1, 2016 10:02:27 PM (8 years ago)

Author:

commit-queue@webkit.org

Message:

[JSC][ARM64] Fix branchTest32/64 taking an immediate as mask
​https://bugs.webkit.org/show_bug.cgi?id=160439

Patch by Benjamin Poulain <​bpoulain@apple.com> on 2016-08-01
Reviewed by Filip Pizlo.

Source/JavaScriptCore:

• assembler/MacroAssemblerARM64.h:

(JSC::MacroAssemblerARM64::branchTest64):

• b3/air/AirOpcode.opcodes:

Fix the ARM64 codegen to lower BitImm64 without using a scratch register.

Source/WTF:

• wtf/MathExtras.h:

(getLSBSet):
This was not working at all for MacroAssembler.
Since TrustedImm32/64 are signed integers, the arithmetic shift would
never get rid of the top bit and we get an infinite loop.

Location:

trunk/Source

Files:

• JavaScriptCore/ChangeLog (modified) (1 diff)
• JavaScriptCore/assembler/MacroAssemblerARM64.h (modified) (1 diff)
• JavaScriptCore/b3/air/AirOpcode.opcodes (modified) (2 diffs)
• WTF/ChangeLog (modified) (1 diff)
• WTF/wtf/MathExtras.h (modified) (1 diff)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2016-08-01  Benjamin Poulain  <bpoulain@apple.com>
+
+        [JSC][ARM64] Fix branchTest32/64 taking an immediate as mask
+        https://bugs.webkit.org/show_bug.cgi?id=160439
+
+        Reviewed by Filip Pizlo.
+
+        * assembler/MacroAssemblerARM64.h:
+        (JSC::MacroAssemblerARM64::branchTest64):
+        * b3/air/AirOpcode.opcodes:
+        Fix the ARM64 codegen to lower BitImm64 without using a scratch register.
+
 2016-07-22  Filip Pizlo  <fpizlo@apple.com>
 

trunk/Source/JavaScriptCore/assembler/MacroAssemblerARM64.h

@@ -2521 +2521 @@
     Jump branchTest64(ResultCondition cond, RegisterID reg, TrustedImm64 mask)
     {
-        move(mask, getCachedDataTempRegisterIDAndInvalidate());
-        return branchTest64(cond, reg, dataTempRegister);
+        if (mask.m_value == -1) {
+            if ((cond == Zero) || (cond == NonZero))
+                return Jump(makeCompareAndBranch<64>(static_cast<ZeroCondition>(cond), reg));
+            m_assembler.tst<64>(reg, reg);
+        } else if (hasOneBitSet(mask.m_value) && ((cond == Zero) || (cond == NonZero)))
+            return Jump(makeTestBitAndBranch(reg, getLSBSet(mask.m_value), static_cast<ZeroCondition>(cond)));
+        else {
+            LogicalImmediate logicalImm = LogicalImmediate::create64(mask.m_value);
+
+            if (logicalImm.isValid()) {
+                m_assembler.tst<64>(reg, logicalImm);
+                return Jump(makeBranch(cond));
+            }
+
+            move(mask, getCachedDataTempRegisterIDAndInvalidate());
+            m_assembler.tst<64>(reg, dataTempRegister);
+        }
+        return Jump(makeBranch(cond));
     }
 

trunk/Source/JavaScriptCore/b3/air/AirOpcode.opcodes

@@ -681 +681 @@
 BranchTest32 U:G:32, U:G:32, U:G:32 /branch
     ResCond, Tmp, Tmp
-    x86: ResCond, Tmp, BitImm
+    ResCond, Tmp, BitImm
     x86: ResCond, Addr, BitImm
     x86: ResCond, Index, BitImm
@@ -689 +689 @@
 64: BranchTest64 U:G:32, U:G:64, U:G:64 /branch
     ResCond, Tmp, Tmp
+    arm64: ResCond, Tmp, BitImm64
     x86: ResCond, Tmp, BitImm
     x86: ResCond, Addr, BitImm

trunk/Source/WTF/ChangeLog

@@ +1 @@
+2016-08-01  Benjamin Poulain  <bpoulain@apple.com>
+
+        [JSC][ARM64] Fix branchTest32/64 taking an immediate as mask
+        https://bugs.webkit.org/show_bug.cgi?id=160439
+
+        Reviewed by Filip Pizlo.
+
+        * wtf/MathExtras.h:
+        (getLSBSet):
+        This was not working at all for MacroAssembler.
+        Since TrustedImm32/64 are signed integers, the arithmetic shift would
+        never get rid of the top bit and we get an infinite loop.
+
 2016-07-29  Mark Lam  <mark.lam@apple.com>
 

trunk/Source/WTF/wtf/MathExtras.h

@@ -216 +216 @@
 template <typename T> inline unsigned getLSBSet(T value)
 {
+    typedef typename std::make_unsigned<T>::type UnsignedT;
     unsigned result = 0;
 
-    while (value >>= 1)
+    UnsignedT unsignedValue = static_cast<UnsignedT>(value);
+    while (unsignedValue >>= 1)
         ++result;