MacroAssemblerX86 should be happy with shift(cx, cx)
authorfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 11 Feb 2016 21:22:02 +0000 (21:22 +0000)
committerfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 11 Feb 2016 21:22:02 +0000 (21:22 +0000)
https://bugs.webkit.org/show_bug.cgi?id=154124

Reviewed by Saam Barati.

Prior to this change the assembler asserted that shift_amount and dest cannot be the same.
That's a good assertion for when shift_amount is not in cx. But if it's in cx already then
it's OK for them to be the same. Air will sometimes do shift(cx, cx) if you do "x << x" and
the coalescing got particularly clever.

* assembler/MacroAssemblerX86Common.h:
(JSC::MacroAssemblerX86Common::lshift32):
(JSC::MacroAssemblerX86Common::rshift32):
(JSC::MacroAssemblerX86Common::urshift32):
* assembler/MacroAssemblerX86_64.h:
(JSC::MacroAssemblerX86_64::lshift64):
(JSC::MacroAssemblerX86_64::rshift64):
(JSC::MacroAssemblerX86_64::urshift64):
* b3/testb3.cpp:
(JSC::B3::testLShiftSelf32):
(JSC::B3::testRShiftSelf32):
(JSC::B3::testURShiftSelf32):
(JSC::B3::testLShiftSelf64):
(JSC::B3::testRShiftSelf64):
(JSC::B3::testURShiftSelf64):
(JSC::B3::run):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@196433 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h
Source/JavaScriptCore/assembler/MacroAssemblerX86_64.h
Source/JavaScriptCore/b3/testb3.cpp

index f0e8009..51d35c9 100644 (file)
@@ -1,3 +1,32 @@
+2016-02-11  Filip Pizlo  <fpizlo@apple.com>
+
+        MacroAssemblerX86 should be happy with shift(cx, cx)
+        https://bugs.webkit.org/show_bug.cgi?id=154124
+
+        Reviewed by Saam Barati.
+
+        Prior to this change the assembler asserted that shift_amount and dest cannot be the same.
+        That's a good assertion for when shift_amount is not in cx. But if it's in cx already then
+        it's OK for them to be the same. Air will sometimes do shift(cx, cx) if you do "x << x" and
+        the coalescing got particularly clever.
+
+        * assembler/MacroAssemblerX86Common.h:
+        (JSC::MacroAssemblerX86Common::lshift32):
+        (JSC::MacroAssemblerX86Common::rshift32):
+        (JSC::MacroAssemblerX86Common::urshift32):
+        * assembler/MacroAssemblerX86_64.h:
+        (JSC::MacroAssemblerX86_64::lshift64):
+        (JSC::MacroAssemblerX86_64::rshift64):
+        (JSC::MacroAssemblerX86_64::urshift64):
+        * b3/testb3.cpp:
+        (JSC::B3::testLShiftSelf32):
+        (JSC::B3::testRShiftSelf32):
+        (JSC::B3::testURShiftSelf32):
+        (JSC::B3::testLShiftSelf64):
+        (JSC::B3::testRShiftSelf64):
+        (JSC::B3::testURShiftSelf64):
+        (JSC::B3::run):
+
 2016-02-11  Saam barati  <sbarati@apple.com>
 
         The sampling profiler's stack walker methods should be marked with SUPPRESS_ASAN
index 0e8ed1d..806c317 100644 (file)
@@ -295,11 +295,10 @@ public:
 
     void lshift32(RegisterID shift_amount, RegisterID dest)
     {
-        ASSERT(shift_amount != dest);
-
         if (shift_amount == X86Registers::ecx)
             m_assembler.shll_CLr(dest);
         else {
+            ASSERT(shift_amount != dest);
             // On x86 we can only shift by ecx; if asked to shift by another register we'll
             // need rejig the shift amount into ecx first, and restore the registers afterwards.
             // If we dest is ecx, then shift the swapped register!
@@ -424,11 +423,11 @@ public:
 
     void rshift32(RegisterID shift_amount, RegisterID dest)
     {
-        ASSERT(shift_amount != dest);
-
         if (shift_amount == X86Registers::ecx)
             m_assembler.sarl_CLr(dest);
         else {
+            ASSERT(shift_amount != dest);
+            
             // On x86 we can only shift by ecx; if asked to shift by another register we'll
             // need rejig the shift amount into ecx first, and restore the registers afterwards.
             // If we dest is ecx, then shift the swapped register!
@@ -461,11 +460,11 @@ public:
     
     void urshift32(RegisterID shift_amount, RegisterID dest)
     {
-        ASSERT(shift_amount != dest);
-
         if (shift_amount == X86Registers::ecx)
             m_assembler.shrl_CLr(dest);
         else {
+            ASSERT(shift_amount != dest);
+        
             // On x86 we can only shift by ecx; if asked to shift by another register we'll
             // need rejig the shift amount into ecx first, and restore the registers afterwards.
             // If we dest is ecx, then shift the swapped register!
index 7dd7e1e..9db3aa3 100644 (file)
@@ -376,11 +376,11 @@ public:
     
     void lshift64(RegisterID src, RegisterID dest)
     {
-        ASSERT(src != dest);
-        
         if (src == X86Registers::ecx)
             m_assembler.shlq_CLr(dest);
         else {
+            ASSERT(src != dest);
+            
             // Can only shift by ecx, so we do some swapping if we see anything else.
             swap(src, X86Registers::ecx);
             m_assembler.shlq_CLr(dest);
@@ -395,11 +395,11 @@ public:
 
     void rshift64(RegisterID src, RegisterID dest)
     {
-        ASSERT(src != dest);
-
         if (src == X86Registers::ecx)
             m_assembler.sarq_CLr(dest);
         else {
+            ASSERT(src != dest);
+            
             // Can only shift by ecx, so we do some swapping if we see anything else.
             swap(src, X86Registers::ecx);
             m_assembler.sarq_CLr(dest);
@@ -414,11 +414,11 @@ public:
 
     void urshift64(RegisterID src, RegisterID dest)
     {
-        ASSERT(src != dest);
-
         if (src == X86Registers::ecx)
             m_assembler.shrq_CLr(dest);
         else {
+            ASSERT(src != dest);
+            
             // Can only shift by ecx, so we do some swapping if we see anything else.
             swap(src, X86Registers::ecx);
             m_assembler.shrq_CLr(dest);
index 30eaf70..f792c00 100644 (file)
@@ -10065,6 +10065,147 @@ void testFoldPathEqual()
     CHECK(invoke<intptr_t>(*code, 42) == 0);
 }
 
+void testLShiftSelf32()
+{
+    Procedure proc;
+    BasicBlock* root = proc.addBlock();
+    Value* arg = root->appendNew<Value>(
+        proc, Trunc, Origin(),
+        root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR0));
+    root->appendNew<ControlValue>(
+        proc, Return, Origin(),
+        root->appendNew<Value>(proc, Shl, Origin(), arg, arg));
+
+    auto code = compile(proc);
+
+    auto check = [&] (int32_t value) {
+        CHECK(invoke<int32_t>(*code, value) == value << (value & 31));
+    };
+
+    check(0);
+    check(1);
+    check(31);
+    check(32);
+}
+
+void testRShiftSelf32()
+{
+    Procedure proc;
+    BasicBlock* root = proc.addBlock();
+    Value* arg = root->appendNew<Value>(
+        proc, Trunc, Origin(),
+        root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR0));
+    root->appendNew<ControlValue>(
+        proc, Return, Origin(),
+        root->appendNew<Value>(proc, SShr, Origin(), arg, arg));
+
+    auto code = compile(proc);
+
+    auto check = [&] (int32_t value) {
+        CHECK(invoke<int32_t>(*code, value) == value >> (value & 31));
+    };
+
+    check(0);
+    check(1);
+    check(31);
+    check(32);
+}
+
+void testURShiftSelf32()
+{
+    Procedure proc;
+    BasicBlock* root = proc.addBlock();
+    Value* arg = root->appendNew<Value>(
+        proc, Trunc, Origin(),
+        root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR0));
+    root->appendNew<ControlValue>(
+        proc, Return, Origin(),
+        root->appendNew<Value>(proc, ZShr, Origin(), arg, arg));
+
+    auto code = compile(proc);
+
+    auto check = [&] (uint32_t value) {
+        CHECK(invoke<uint32_t>(*code, value) == value >> (value & 31));
+    };
+
+    check(0);
+    check(1);
+    check(31);
+    check(32);
+}
+
+void testLShiftSelf64()
+{
+    Procedure proc;
+    BasicBlock* root = proc.addBlock();
+    Value* arg = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR0);
+    root->appendNew<ControlValue>(
+        proc, Return, Origin(),
+        root->appendNew<Value>(
+            proc, Shl, Origin(), arg, root->appendNew<Value>(proc, Trunc, Origin(), arg)));
+
+    auto code = compile(proc);
+
+    auto check = [&] (int64_t value) {
+        CHECK(invoke<int64_t>(*code, value) == value << (value & 63));
+    };
+
+    check(0);
+    check(1);
+    check(31);
+    check(32);
+    check(63);
+    check(64);
+}
+
+void testRShiftSelf64()
+{
+    Procedure proc;
+    BasicBlock* root = proc.addBlock();
+    Value* arg = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR0);
+    root->appendNew<ControlValue>(
+        proc, Return, Origin(),
+        root->appendNew<Value>(
+            proc, SShr, Origin(), arg, root->appendNew<Value>(proc, Trunc, Origin(), arg)));
+
+    auto code = compile(proc);
+
+    auto check = [&] (int64_t value) {
+        CHECK(invoke<int64_t>(*code, value) == value >> (value & 63));
+    };
+
+    check(0);
+    check(1);
+    check(31);
+    check(32);
+    check(63);
+    check(64);
+}
+
+void testURShiftSelf64()
+{
+    Procedure proc;
+    BasicBlock* root = proc.addBlock();
+    Value* arg = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR0);
+    root->appendNew<ControlValue>(
+        proc, Return, Origin(),
+        root->appendNew<Value>(
+            proc, ZShr, Origin(), arg, root->appendNew<Value>(proc, Trunc, Origin(), arg)));
+
+    auto code = compile(proc);
+
+    auto check = [&] (uint64_t value) {
+        CHECK(invoke<uint64_t>(*code, value) == value >> (value & 63));
+    };
+
+    check(0);
+    check(1);
+    check(31);
+    check(32);
+    check(63);
+    check(64);
+}
+
 // Make sure the compiler does not try to optimize anything out.
 NEVER_INLINE double zero()
 {
@@ -11466,6 +11607,13 @@ void run(const char* filter)
     RUN(testComputeDivisionMagic<int32_t>(2, -2147483647, 0));
     RUN(testTrivialInfiniteLoop());
     RUN(testFoldPathEqual());
+    
+    RUN(testRShiftSelf32());
+    RUN(testURShiftSelf32());
+    RUN(testLShiftSelf32());
+    RUN(testRShiftSelf64());
+    RUN(testURShiftSelf64());
+    RUN(testLShiftSelf64());
 
     if (tasks.isEmpty())
         usage();