Workaround for Cortex-A53 erratum 835769
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 4 Nov 2014 07:36:15 +0000 (07:36 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 4 Nov 2014 07:36:15 +0000 (07:36 +0000)
https://bugs.webkit.org/show_bug.cgi?id=138315

Patch by Akos Kiss <akiss@inf.u-szeged.hu> on 2014-11-03
Reviewed by Filip Pizlo.

This patch introduces CMake variable and preprocessor macro
WTF_CPU_ARM64_CORTEXA53 with the aim of enabling Cortex-A53-specific
.:

code paths, if set true.

* Source/cmake/OptionsCommon.cmake:
Add -mfix-cortex-a53-835769 to the compiler flags if compiler supports
it.
* Source/cmakeconfig.h.cmake:
#cmakedefine01 for WTF_CPU_ARM64_CORTEXA53

Source/JavaScriptCore:

code paths, if set true. The patch also implements one case where such
code paths are needed: the workaround for Cortex-A53 erratum 835769. If
WTF_CPU_ARM64_CORTEXA53 is set then:
- CMake checks whether the compiler already has support for a workaround
  and adds -mfix-cortex-a53-835769 to the compiler flags if so,
- the ARM64 backend of offlineasm inserts a nop between memory and
  multiply-accumulate instructions, and
- the ARM64 assembler also inserts a nop between memory and (64-bit)
  multiply-accumulate instructions.

* assembler/ARM64Assembler.h:
(JSC::ARM64Assembler::madd):
Call nopCortexA53Fix835769() to insert a nop if CPU(ARM64_CORTEXA53) and
if necessary.
(JSC::ARM64Assembler::msub): Likewise.
(JSC::ARM64Assembler::smaddl): Likewise.
(JSC::ARM64Assembler::smsubl): Likewise.
(JSC::ARM64Assembler::umaddl): Likewise.
(JSC::ARM64Assembler::umsubl): Likewise.
(JSC::ARM64Assembler::nopCortexA53Fix835769):
Added. Insert a nop if the previously emitted instruction was a load, a
store, or a prefetch, and if the current instruction is 64-bit.
* offlineasm/arm64.rb:
Add the arm64CortexA53Fix835769 phase and call it from
getModifiedListARM64 to insert nopCortexA53Fix835769 between appropriate
macro instructions. Also, lower nopCortexA53Fix835769 to nop if
CPU(ARM64_CORTEXA53), to nothing otherwise.
* offlineasm/instructions.rb:
Define macro instruction nopFixCortexA53Err835769.

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

ChangeLog
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/assembler/ARM64Assembler.h
Source/JavaScriptCore/offlineasm/arm64.rb
Source/JavaScriptCore/offlineasm/instructions.rb
Source/cmake/OptionsCommon.cmake
Source/cmakeconfig.h.cmake

index ad6da81..587eae7 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,20 @@
+2014-11-03  Akos Kiss  <akiss@inf.u-szeged.hu>
+
+        Workaround for Cortex-A53 erratum 835769
+        https://bugs.webkit.org/show_bug.cgi?id=138315
+
+        Reviewed by Filip Pizlo.
+
+        This patch introduces CMake variable and preprocessor macro
+        WTF_CPU_ARM64_CORTEXA53 with the aim of enabling Cortex-A53-specific
+        code paths, if set true.
+
+        * Source/cmake/OptionsCommon.cmake:
+        Add -mfix-cortex-a53-835769 to the compiler flags if compiler supports
+        it.
+        * Source/cmakeconfig.h.cmake:
+        #cmakedefine01 for WTF_CPU_ARM64_CORTEXA53
+
 2014-11-02  Akos Kiss  <akiss@inf.u-szeged.hu>
 
         [GTK] Fix the build of FTL JIT
index 1933176..824d00e 100644 (file)
@@ -1,3 +1,42 @@
+2014-11-03  Akos Kiss  <akiss@inf.u-szeged.hu>
+
+        Workaround for Cortex-A53 erratum 835769
+        https://bugs.webkit.org/show_bug.cgi?id=138315
+
+        Reviewed by Filip Pizlo.
+
+        This patch introduces CMake variable and preprocessor macro
+        WTF_CPU_ARM64_CORTEXA53 with the aim of enabling Cortex-A53-specific
+        code paths, if set true. The patch also implements one case where such
+        code paths are needed: the workaround for Cortex-A53 erratum 835769. If
+        WTF_CPU_ARM64_CORTEXA53 is set then:
+        - CMake checks whether the compiler already has support for a workaround
+          and adds -mfix-cortex-a53-835769 to the compiler flags if so,
+        - the ARM64 backend of offlineasm inserts a nop between memory and
+          multiply-accumulate instructions, and
+        - the ARM64 assembler also inserts a nop between memory and (64-bit) 
+          multiply-accumulate instructions.
+
+        * assembler/ARM64Assembler.h:
+        (JSC::ARM64Assembler::madd):
+        Call nopCortexA53Fix835769() to insert a nop if CPU(ARM64_CORTEXA53) and
+        if necessary.
+        (JSC::ARM64Assembler::msub): Likewise.
+        (JSC::ARM64Assembler::smaddl): Likewise.
+        (JSC::ARM64Assembler::smsubl): Likewise.
+        (JSC::ARM64Assembler::umaddl): Likewise.
+        (JSC::ARM64Assembler::umsubl): Likewise.
+        (JSC::ARM64Assembler::nopCortexA53Fix835769):
+        Added. Insert a nop if the previously emitted instruction was a load, a
+        store, or a prefetch, and if the current instruction is 64-bit.
+        * offlineasm/arm64.rb:
+        Add the arm64CortexA53Fix835769 phase and call it from
+        getModifiedListARM64 to insert nopCortexA53Fix835769 between appropriate
+        macro instructions. Also, lower nopCortexA53Fix835769 to nop if
+        CPU(ARM64_CORTEXA53), to nothing otherwise.
+        * offlineasm/instructions.rb:
+        Define macro instruction nopFixCortexA53Err835769.
+
 2014-11-03  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r175509.
index 7a273aa..0d22e81 100644 (file)
@@ -1568,6 +1568,7 @@ public:
     ALWAYS_INLINE void madd(RegisterID rd, RegisterID rn, RegisterID rm, RegisterID ra)
     {
         CHECK_DATASIZE();
+        nopCortexA53Fix835769<datasize>();
         insn(dataProcessing3Source(DATASIZE, DataOp_MADD, rm, ra, rn, rd));
     }
 
@@ -1620,6 +1621,7 @@ public:
     ALWAYS_INLINE void msub(RegisterID rd, RegisterID rn, RegisterID rm, RegisterID ra)
     {
         CHECK_DATASIZE();
+        nopCortexA53Fix835769<datasize>();
         insn(dataProcessing3Source(DATASIZE, DataOp_MSUB, rm, ra, rn, rd));
     }
 
@@ -1806,6 +1808,7 @@ public:
 
     ALWAYS_INLINE void smaddl(RegisterID rd, RegisterID rn, RegisterID rm, RegisterID ra)
     {
+        nopCortexA53Fix835769<64>();
         insn(dataProcessing3Source(Datasize_64, DataOp_SMADDL, rm, ra, rn, rd));
     }
 
@@ -1816,6 +1819,7 @@ public:
 
     ALWAYS_INLINE void smsubl(RegisterID rd, RegisterID rn, RegisterID rm, RegisterID ra)
     {
+        nopCortexA53Fix835769<64>();
         insn(dataProcessing3Source(Datasize_64, DataOp_SMSUBL, rm, ra, rn, rd));
     }
 
@@ -2059,6 +2063,7 @@ public:
 
     ALWAYS_INLINE void umaddl(RegisterID rd, RegisterID rn, RegisterID rm, RegisterID ra)
     {
+        nopCortexA53Fix835769<64>();
         insn(dataProcessing3Source(Datasize_64, DataOp_UMADDL, rm, ra, rn, rd));
     }
 
@@ -2069,6 +2074,7 @@ public:
 
     ALWAYS_INLINE void umsubl(RegisterID rd, RegisterID rn, RegisterID rm, RegisterID ra)
     {
+        nopCortexA53Fix835769<64>();
         insn(dataProcessing3Source(Datasize_64, DataOp_UMSUBL, rm, ra, rn, rd));
     }
 
@@ -3629,6 +3635,26 @@ private:
         return (0xd6000000 | opc << 21 | op2 << 16 | op3 << 10 | xOrZr(rn) << 5 | op4);
     }
 
+    // Workaround for Cortex-A53 erratum (835769). Emit an extra nop if the
+    // last instruction in the buffer is a load, store or prefetch. Needed
+    // before 64-bit multiply-accumulate instructions.
+    template<int datasize>
+    ALWAYS_INLINE void nopCortexA53Fix835769()
+    {
+#if CPU(ARM64_CORTEXA53)
+        CHECK_DATASIZE();
+        if (datasize == 64) {
+            if (LIKELY(m_buffer.codeSize() >= sizeof(int32_t))) {
+                // From ARMv8 Reference Manual, Section C4.1: the encoding of the
+                // instructions in the Loads and stores instruction group is:
+                // ---- 1-0- ---- ---- ---- ---- ---- ----
+                if (UNLIKELY((*reinterpret_cast_ptr<int32_t*>(reinterpret_cast_ptr<char*>(m_buffer.data()) + m_buffer.codeSize() - sizeof(int32_t)) & 0x0a000000) == 0x08000000))
+                    nop();
+            }
+        }
+#endif
+    }
+
     AssemblerBuffer m_buffer;
     Vector<LinkRecord, 0, UnsafeVectorOverflow> m_jumpsToLink;
     int m_indexOfLastWatchpoint;
index 1940d7d..3a0d786 100644 (file)
@@ -228,6 +228,34 @@ def arm64LowerMalformedLoadStoreAddresses(list)
     newList
 end
 
+# Workaround for Cortex-A53 erratum (835769)
+def arm64CortexA53Fix835769(list)
+    newList = []
+    lastOpcodeUnsafe = false
+
+    list.each {
+        | node |
+        if node.is_a? Instruction
+            case node.opcode
+            when /^store/, /^load/
+                # List all macro instructions that can be lowered to a load, store or prefetch ARM64 assembly instruction
+                lastOpcodeUnsafe = true
+            when  "muli", "mulp", "mulq", "smulli"
+                # List all macro instructions that can be lowered to a 64-bit multiply-accumulate ARM64 assembly instruction
+                # (defined as one of MADD, MSUB, SMADDL, SMSUBL, UMADDL or UMSUBL).
+                if lastOpcodeUnsafe
+                    newList << Instruction.new(node.codeOrigin, "nopCortexA53Fix835769", [])
+                end
+                lastOpcodeUnsafe = false
+            else
+                lastOpcodeUnsafe = false
+            end
+        end
+        newList << node
+    }
+    newList
+end
+
 class Sequence
     def getModifiedListARM64
         result = @list
@@ -284,6 +312,7 @@ class Sequence
         result = riscLowerTest(result)
         result = assignRegistersToTemporaries(result, :gpr, ARM64_EXTRA_GPRS)
         result = assignRegistersToTemporaries(result, :fpr, ARM64_EXTRA_FPRS)
+        result = arm64CortexA53Fix835769(result)
         return result
     end
 end
@@ -837,7 +866,11 @@ class Instruction
         when "memfence"
             $asm.puts "dmb sy"
         when "pcrtoaddr"
-          $asm.puts "adr #{operands[1].arm64Operand(:ptr)}, #{operands[0].value}"
+            $asm.puts "adr #{operands[1].arm64Operand(:ptr)}, #{operands[0].value}"
+        when "nopCortexA53Fix835769"
+            $asm.putStr("#if CPU(ARM64_CORTEXA53)")
+            $asm.puts "nop"
+            $asm.putStr("#endif")
         else
             lowerDefault
         end
index 8728ee4..1d0d867 100644 (file)
@@ -266,7 +266,8 @@ ARM_INSTRUCTIONS =
 
 ARM64_INSTRUCTIONS =
     [
-     "pcrtoaddr"    # Address from PC relative offset - adr instruction
+     "pcrtoaddr",   # Address from PC relative offset - adr instruction
+     "nopFixCortexA53Err835769" # nop on Cortex-A53 (nothing otherwise)
     ]
 
 RISC_INSTRUCTIONS =
index ffcdfe5..ab2309b 100644 (file)
@@ -30,6 +30,20 @@ if (CMAKE_COMPILER_IS_GNUCXX OR "${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang")
     set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11")
 endif ()
 
+if (WTF_CPU_ARM64_CORTEXA53)
+    if (NOT WTF_CPU_ARM64)
+        message(FATAL_ERROR "WTF_CPU_ARM64_CORTEXA53 set without WTF_CPU_ARM64")
+    endif ()
+    message("Checking if compiler supports -mfix-cortex-a53-835769")
+    execute_process(COMMAND ${CMAKE_C_COMPILER} -mfix-cortex-a53-835769 ERROR_VARIABLE COMPILER_OUTPUT)
+    if ((NOT "${COMPILER_OUTPUT}" MATCHES "unrecognized command line option")
+        AND (NOT "${COMPILER_OUTPUT}" MATCHES "unknown argument"))
+        set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -mfix-cortex-a53-835769")
+        set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -mfix-cortex-a53-835769")
+        message("Enabling Cortex-A53 workaround for compiler")
+    endif ()
+endif ()
+
 option(DEBUG_FISSION "Use Debug Fission support")
 if (DEBUG_FISSION)
     execute_process(COMMAND ${CMAKE_C_COMPILER} -fuse-ld=gold -Wl,--version ERROR_QUIET OUTPUT_VARIABLE LD_VERSION)
index d1efe66..659c69f 100644 (file)
 #cmakedefine01 WTF_USE_TILED_BACKING_STORE
 #cmakedefine01 HAVE_LLVM
 #cmakedefine01 HAVE_GTK_UNIX_PRINTING
+#cmakedefine01 WTF_CPU_ARM64_CORTEXA53
 
 #if defined(BUILDING_GTK__) && !defined(GTK_API_VERSION_2)
 #define GDK_VERSION_MIN_REQUIRED @GDK_VERSION_MIN_REQUIRED@