isValidForm for SimpleAddr should use ptr() instead of tmp()
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 12 May 2017 06:04:07 +0000 (06:04 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 12 May 2017 06:04:07 +0000 (06:04 +0000)
https://bugs.webkit.org/show_bug.cgi?id=171992

Reviewed by Filip Pizlo.

Arg::tmp() asserts that its kind is Tmp. Inst::isValidForm for
SimpleAddr was using Arg::tmp() instead of ptr() to check
if the address Tmp isGP(). It should be using Arg::ptr() instead
of Arg::tmp() since Arg::ptr() is designed for SimpleAddr.

This patch also fixes an incorrect assertion in the ARM64
macro assembler. We were asserting various atomic ops were
only over 32/64 bit operations. However, the code was properly handling
8/16/32/64 bit ops. I changed the assertion to reflect what is
actually going on.

* assembler/ARM64Assembler.h:
(JSC::ARM64Assembler::ldar):
(JSC::ARM64Assembler::ldxr):
(JSC::ARM64Assembler::ldaxr):
(JSC::ARM64Assembler::stxr):
(JSC::ARM64Assembler::stlr):
(JSC::ARM64Assembler::stlxr):
* b3/air/opcode_generator.rb:
* b3/testb3.cpp:
(JSC::B3::testLoadAcq42):
(JSC::B3::testStoreRelAddLoadAcq32):
(JSC::B3::testStoreRelAddLoadAcq8):
(JSC::B3::testStoreRelAddFenceLoadAcq8):
(JSC::B3::testStoreRelAddLoadAcq16):
(JSC::B3::testStoreRelAddLoadAcq64):
(JSC::B3::testAtomicWeakCAS):
(JSC::B3::testAtomicStrongCAS):
(JSC::B3::testAtomicXchg):

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/assembler/ARM64Assembler.h
Source/JavaScriptCore/b3/air/opcode_generator.rb
Source/JavaScriptCore/b3/testb3.cpp

index 05e063d..f1079f2 100644 (file)
@@ -1,3 +1,40 @@
+2017-05-11  Saam Barati  <sbarati@apple.com>
+
+        isValidForm for SimpleAddr should use ptr() instead of tmp()
+        https://bugs.webkit.org/show_bug.cgi?id=171992
+
+        Reviewed by Filip Pizlo.
+
+        Arg::tmp() asserts that its kind is Tmp. Inst::isValidForm for
+        SimpleAddr was using Arg::tmp() instead of ptr() to check
+        if the address Tmp isGP(). It should be using Arg::ptr() instead
+        of Arg::tmp() since Arg::ptr() is designed for SimpleAddr.
+        
+        This patch also fixes an incorrect assertion in the ARM64
+        macro assembler. We were asserting various atomic ops were
+        only over 32/64 bit operations. However, the code was properly handling
+        8/16/32/64 bit ops. I changed the assertion to reflect what is
+        actually going on.
+
+        * assembler/ARM64Assembler.h:
+        (JSC::ARM64Assembler::ldar):
+        (JSC::ARM64Assembler::ldxr):
+        (JSC::ARM64Assembler::ldaxr):
+        (JSC::ARM64Assembler::stxr):
+        (JSC::ARM64Assembler::stlr):
+        (JSC::ARM64Assembler::stlxr):
+        * b3/air/opcode_generator.rb:
+        * b3/testb3.cpp:
+        (JSC::B3::testLoadAcq42):
+        (JSC::B3::testStoreRelAddLoadAcq32):
+        (JSC::B3::testStoreRelAddLoadAcq8):
+        (JSC::B3::testStoreRelAddFenceLoadAcq8):
+        (JSC::B3::testStoreRelAddLoadAcq16):
+        (JSC::B3::testStoreRelAddLoadAcq64):
+        (JSC::B3::testAtomicWeakCAS):
+        (JSC::B3::testAtomicStrongCAS):
+        (JSC::B3::testAtomicXchg):
+
 2017-05-11  Matt Lewis  <jlewis3@apple.com>
 
         Unreviewed, rolling out r216677.
index 095550d..6ad46de 100644 (file)
 #include <stdint.h>
 
 #define CHECK_DATASIZE_OF(datasize) ASSERT(datasize == 32 || datasize == 64)
+#define CHECK_MEMOPSIZE_OF(size) ASSERT(size == 8 || size == 16 || size == 32 || size == 64 || size == 128);
 #define DATASIZE_OF(datasize) ((datasize == 64) ? Datasize_64 : Datasize_32)
 #define MEMOPSIZE_OF(datasize) ((datasize == 8 || datasize == 128) ? MemOpSize_8_or_128 : (datasize == 16) ? MemOpSize_16 : (datasize == 32) ? MemOpSize_32 : MemOpSize_64)
 #define CHECK_DATASIZE() CHECK_DATASIZE_OF(datasize)
+#define CHECK_MEMOPSIZE() CHECK_MEMOPSIZE_OF(datasize)
 #define CHECK_VECTOR_DATASIZE() ASSERT(datasize == 64 || datasize == 128)
 #define DATASIZE DATASIZE_OF(datasize)
 #define MEMOPSIZE MEMOPSIZE_OF(datasize)
@@ -1550,42 +1552,42 @@ public:
     template<int datasize>
     void ldar(RegisterID dst, RegisterID src)
     {
-        CHECK_DATASIZE();
+        CHECK_MEMOPSIZE();
         insn(exoticLoad(MEMOPSIZE, ExoticLoadFence_Acquire, ExoticLoadAtomic_None, dst, src));
     }
 
     template<int datasize>
     void ldxr(RegisterID dst, RegisterID src)
     {
-        CHECK_DATASIZE();
+        CHECK_MEMOPSIZE();
         insn(exoticLoad(MEMOPSIZE, ExoticLoadFence_None, ExoticLoadAtomic_Link, dst, src));
     }
 
     template<int datasize>
     void ldaxr(RegisterID dst, RegisterID src)
     {
-        CHECK_DATASIZE();
+        CHECK_MEMOPSIZE();
         insn(exoticLoad(MEMOPSIZE, ExoticLoadFence_Acquire, ExoticLoadAtomic_Link, dst, src));
     }
     
     template<int datasize>
     void stxr(RegisterID result, RegisterID src, RegisterID dst)
     {
-        CHECK_DATASIZE();
+        CHECK_MEMOPSIZE();
         insn(exoticStore(MEMOPSIZE, ExoticStoreFence_None, result, src, dst));
     }
 
     template<int datasize>
     void stlr(RegisterID src, RegisterID dst)
     {
-        CHECK_DATASIZE();
+        CHECK_MEMOPSIZE();
         insn(storeRelease(MEMOPSIZE, src, dst));
     }
 
     template<int datasize>
     void stlxr(RegisterID result, RegisterID src, RegisterID dst)
     {
-        CHECK_DATASIZE();
+        CHECK_MEMOPSIZE();
         insn(exoticStore(MEMOPSIZE, ExoticStoreFence_Release, result, src, dst));
     }
     
index 4feb050..b953618 100644 (file)
@@ -901,7 +901,7 @@ writeH("OpcodeGenerated") {
                     outp.puts "if (!Arg::isValidBitImm64Form(args[#{index}].value()))"
                     outp.puts "OPGEN_RETURN(false);"
                 when "SimpleAddr"
-                    outp.puts "if (!args[#{index}].tmp().isGP())"
+                    outp.puts "if (!args[#{index}].ptr().isGP())"
                     outp.puts "OPGEN_RETURN(false);"
                 when "Addr"
                     if arg.role == "UA"
index 4c2a75c..10428ff 100644 (file)
@@ -300,13 +300,6 @@ void testLoad42()
 
 void testLoadAcq42()
 {
-#ifndef NDEBUG
-    if (isARM64()) {
-        // FIXME: https://bugs.webkit.org/show_bug.cgi?id=171826
-        return;
-    }
-#endif
-
     Procedure proc;
     BasicBlock* root = proc.addBlock();
     int x = 42;
@@ -5777,13 +5770,6 @@ void testStoreAddLoad32(int amount)
 
 void testStoreRelAddLoadAcq32(int amount)
 {
-#ifndef NDEBUG
-    if (isARM64()) {
-        // FIXME: https://bugs.webkit.org/show_bug.cgi?id=171826
-        return;
-    }
-#endif
-
     Procedure proc;
     BasicBlock* root = proc.addBlock();
     int slot = 37;
@@ -5857,13 +5843,6 @@ void testStoreAddLoad8(int amount, B3::Opcode loadOpcode)
 
 void testStoreRelAddLoadAcq8(int amount, B3::Opcode loadOpcode)
 {
-#ifndef NDEBUG
-    if (isARM64()) {
-        // FIXME: https://bugs.webkit.org/show_bug.cgi?id=171826
-        return;
-    }
-#endif
-
     Procedure proc;
     BasicBlock* root = proc.addBlock();
     int8_t slot = 37;
@@ -5893,13 +5872,6 @@ void testStoreRelAddLoadAcq8(int amount, B3::Opcode loadOpcode)
 
 void testStoreRelAddFenceLoadAcq8(int amount, B3::Opcode loadOpcode)
 {
-#ifndef NDEBUG
-    if (isARM64()) {
-        // FIXME: https://bugs.webkit.org/show_bug.cgi?id=171826
-        return;
-    }
-#endif
-
     Procedure proc;
     BasicBlock* root = proc.addBlock();
     int8_t slot = 37;
@@ -5983,13 +5955,6 @@ void testStoreAddLoad16(int amount, B3::Opcode loadOpcode)
 
 void testStoreRelAddLoadAcq16(int amount, B3::Opcode loadOpcode)
 {
-#ifndef NDEBUG
-    if (isARM64()) {
-        // FIXME: https://bugs.webkit.org/show_bug.cgi?id=171826
-        return;
-    }
-#endif
-
     Procedure proc;
     BasicBlock* root = proc.addBlock();
     int16_t slot = 37;
@@ -6061,13 +6026,6 @@ void testStoreAddLoad64(int amount)
 
 void testStoreRelAddLoadAcq64(int amount)
 {
-#ifndef NDEBUG
-    if (isARM64()) {
-        // FIXME: https://bugs.webkit.org/show_bug.cgi?id=171826
-        return;
-    }
-#endif
-
     Procedure proc;
     BasicBlock* root = proc.addBlock();
     int64_t slot = 37000000000ll;
@@ -14696,12 +14654,6 @@ void testOptimizeMaterialization()
 template<typename T>
 void testAtomicWeakCAS()
 {
-#ifndef NDEBUG
-    if (isARM64()) {
-        // FIXME: https://bugs.webkit.org/show_bug.cgi?id=171826
-        return;
-    }
-#endif
     Type type = NativeTraits<T>::type;
     Width width = NativeTraits<T>::width;
     
@@ -14949,12 +14901,6 @@ void testAtomicWeakCAS()
 template<typename T>
 void testAtomicStrongCAS()
 {
-#ifndef NDEBUG
-    if (isARM64()) {
-        // FIXME: https://bugs.webkit.org/show_bug.cgi?id=171826
-        return;
-    }
-#endif
     Type type = NativeTraits<T>::type;
     Width width = NativeTraits<T>::width;
     
@@ -15224,12 +15170,6 @@ void testAtomicStrongCAS()
 template<typename T>
 void testAtomicXchg(B3::Opcode opcode)
 {
-#ifndef NDEBUG
-    if (isARM64()) {
-        // FIXME: https://bugs.webkit.org/show_bug.cgi?id=171826
-        return;
-    }
-#endif
     Type type = NativeTraits<T>::type;
     Width width = NativeTraits<T>::width;