Bug 1178772 - Add check_macroassembler_style.py: Verify that each MacroAssembler declaration maps to all its definitions. r=h4writer
authorNicolas B. Pierron <nicolas.b.pierron@mozilla.com>
Thu, 09 Jul 2015 14:35:29 +0200
changeset 283789 31435f8615d7f8e6e4f3c35b8e812e9f798ecbf7
parent 283788 8bd08e459f24da8eee680163c453ed51a539d24b
child 283790 3bfb920c798aac3730d70a4079f260c2f055cefc
push id5067
push userraliiev@mozilla.com
push dateMon, 21 Sep 2015 14:04:52 +0000
treeherdermozilla-beta@14221ffe5b2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersh4writer
bugs1178772
milestone42.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1178772 - Add check_macroassembler_style.py: Verify that each MacroAssembler declaration maps to all its definitions. r=h4writer
config/check_macroassembler_style.py
config/check_spidermonkey_style.py
config/check_utils.py
js/src/Makefile.in
js/src/jit/MacroAssembler-inl.h
js/src/jit/MacroAssembler.cpp
js/src/jit/MacroAssembler.h
js/src/jit/arm/MacroAssembler-arm.cpp
js/src/jit/arm64/MacroAssembler-arm64.cpp
js/src/jit/mips/MacroAssembler-mips.cpp
js/src/jit/none/Trampoline-none.cpp
js/src/jit/x64/MacroAssembler-x64.cpp
js/src/jit/x86-shared/MacroAssembler-x86-shared.cpp
js/src/jit/x86/MacroAssembler-x86.cpp
testing/mach_commands.py
new file mode 100644
--- /dev/null
+++ b/config/check_macroassembler_style.py
@@ -0,0 +1,264 @@
+# vim: set ts=8 sts=4 et sw=4 tw=99:
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+#----------------------------------------------------------------------------
+# This script checks that SpiderMonkey MacroAssembler methods are properly
+# annotated.
+#
+# The MacroAssembler has one interface for all platforms, but it might have one
+# definition per platform. The code of the MacroAssembler use a macro to
+# annotate the method declarations, in order to delete the function if it is not
+# present on the current platform, and also to locate the files in which the
+# methods are defined.
+#
+# This script scans the MacroAssembler.h header, for method declarations.
+# It also scans MacroAssembler-/arch/.cpp, MacroAssembler-/arch/-inl.h, and
+# MacroAssembler-inl.h for method definitions. The result of both scans are
+# uniformized, and compared, to determine if the MacroAssembler.h header as
+# proper methods annotations.
+#----------------------------------------------------------------------------
+
+from __future__ import print_function
+
+import difflib
+import os
+import re
+import subprocess
+import sys
+from check_utils import get_all_toplevel_filenames
+
+architecture_independent = set([ 'generic' ])
+all_architecture_names = set([ 'x86', 'x64', 'arm', 'arm64', 'mips' ])
+all_shared_architecture_names = set([ 'x86_shared', 'arm', 'arm64', 'mips' ])
+
+def get_normalized_signatures(signature, fileAnnot = None):
+    # Remove semicolon.
+    signature = signature.replace(';', ' ')
+    # Normalize spaces.
+    signature = re.sub(r'\s+', ' ', signature).strip()
+    # Remove argument names.
+    signature = re.sub(r'(?P<type>(?:[(]|,\s)[\w\s:*&]+)(?P<name>\s\w+)(?=[,)])', '\g<type>', signature)
+    # Remove class name
+    signature = signature.replace('MacroAssembler::', '')
+
+    # Extract list of architectures
+    archs = ['generic']
+    if fileAnnot:
+        archs = [fileAnnot['arch']]
+
+    if 'DEFINED_ON(' in signature:
+        archs = re.sub(r'.*DEFINED_ON\((?P<archs>[^()]*)\).*', '\g<archs>', signature).split(',')
+        archs = [a.strip() for a in archs]
+        signature = re.sub(r'\s+DEFINED_ON\([^()]*\)', '', signature)
+
+    elif 'PER_ARCH' in signature:
+        archs = all_architecture_names
+        signature = re.sub(r'\s+PER_ARCH', '', signature)
+
+    elif 'PER_SHARED_ARCH' in signature:
+        archs = all_shared_architecture_names
+        signature = re.sub(r'\s+PER_SHARED_ARCH', '', signature)
+
+    else:
+        # No signature annotation, the list of architectures remains unchanged.
+        pass
+
+    # Extract inline annotation
+    inline = False
+    if fileAnnot:
+        inline = fileAnnot['inline']
+
+    if 'inline ' in signature:
+        signature = re.sub(r'inline\s+', '', signature)
+        inline = True
+
+    return [
+        { 'arch': a, 'sig': 'inline ' + signature }
+        for a in archs
+    ]
+
+file_suffixes = set([
+    a.replace('_', '-') for a in
+    all_architecture_names.union(all_shared_architecture_names)
+])
+def get_file_annotation(filename):
+    origFilename = filename
+    filename = filename.split('/')[-1]
+
+    inline = False
+    if filename.endswith('.cpp'):
+        filename = filename[:-len('.cpp')]
+    elif filename.endswith('-inl.h'):
+        inline = True
+        filename = filename[:-len('-inl.h')]
+    else:
+        raise Exception('unknown file name', origFilename)
+
+    arch = 'generic'
+    for suffix in file_suffixes:
+        if filename == 'MacroAssembler-' + suffix:
+            arch = suffix
+            break
+
+    return {
+        'inline': inline,
+        'arch': arch.replace('-', '_')
+    }
+
+def get_macroassembler_definitions(filename):
+    try:
+        fileAnnot = get_file_annotation(filename)
+    except:
+        return []
+
+    style_section = False
+    code_section = False
+    lines = ''
+    signatures = []
+    with open(os.path.join('../..', filename)) as f:
+        for line in f:
+            if '//{{{ check_macroassembler_style' in line:
+                style_section = True
+            elif '//}}} check_macroassembler_style' in line:
+                style_section = False
+            if not style_section:
+                continue
+
+            line = re.sub(r'//.*', '', line)
+            if line.startswith('{'):
+                if 'MacroAssembler::' in lines:
+                    signatures.extend(get_normalized_signatures(lines, fileAnnot))
+                code_section = True
+                continue
+            if line.startswith('}'):
+                code_section = False
+                lines = ''
+                continue
+            if code_section:
+                continue
+
+            if len(line.strip()) == 0:
+                lines = ''
+                continue
+            lines = lines + line
+            # Continue until we have a complete declaration
+            if '{' not in lines:
+                continue
+            # Skip variable declarations
+            if ')' not in lines:
+                lines = ''
+                continue
+
+    return signatures
+
+def get_macroassembler_declaration(filename):
+    style_section = False
+    lines = ''
+    signatures = []
+    with open(os.path.join('../..', filename)) as f:
+        for line in f:
+            if '//{{{ check_macroassembler_style' in line:
+                style_section = True
+            elif '//}}} check_macroassembler_style' in line:
+                style_section = False
+            if not style_section:
+                continue
+
+            line = re.sub(r'//.*', '', line)
+            if len(line.strip()) == 0:
+                lines = ''
+                continue
+            lines = lines + line
+            # Continue until we have a complete declaration
+            if ';' not in lines:
+                continue
+            # Skip variable declarations
+            if ')' not in lines:
+                lines = ''
+                continue
+
+            signatures.extend(get_normalized_signatures(lines))
+            lines = ''
+
+    return signatures
+
+def append_signatures(d, sigs):
+    for s in sigs:
+        if s['sig'] not in d:
+            d[s['sig']] = []
+        d[s['sig']].append(s['arch']);
+    return d
+
+def generate_file_content(signatures):
+    output = []
+    for s in sorted(signatures.keys()):
+        archs = set(signatures[s])
+        if len(archs.symmetric_difference(architecture_independent)) == 0:
+            output.append(s + ';\n')
+            if s.startswith('inline'):
+                output.append('    is defined in MacroAssembler-inl.h\n')
+            else:
+                output.append('    is defined in MacroAssembler.cpp\n')
+        else:
+            if len(archs.symmetric_difference(all_architecture_names)) == 0:
+                output.append(s + ' PER_ARCH;\n')
+            elif len(archs.symmetric_difference(all_shared_architecture_names)) == 0:
+                output.append(s + ' PER_SHARED_ARCH;\n')
+            else:
+                output.append(s + ' DEFINED_ON(' + ', '.join(archs) + ');\n')
+            for a in archs:
+                a = a.replace('_', '-')
+                masm = '%s/MacroAssembler-%s' % (a, a)
+                if s.startswith('inline'):
+                    output.append('    is defined in %s-inl.h\n' % masm)
+                else:
+                    output.append('    is defined in %s.cpp\n' % masm)
+    return output
+
+def check_style():
+    # We read from the header file the signature of each function.
+    decls = dict()      # type: dict(signature => ['x86', 'x64'])
+
+    # We infer from each file the signature of each MacroAssembler function.
+    defs = dict()       # type: dict(signature => ['x86', 'x64'])
+
+    # Select the appropriate files.
+    for filename in get_all_toplevel_filenames():
+        if not filename.startswith('js/src/jit/'):
+            continue
+        if 'MacroAssembler' not in filename:
+            continue
+
+        if filename.endswith('MacroAssembler.h'):
+            decls = append_signatures(decls, get_macroassembler_declaration(filename))
+        else:
+            defs = append_signatures(defs, get_macroassembler_definitions(filename))
+
+    # Compare declarations and definitions output.
+    difflines = difflib.unified_diff(generate_file_content(decls),
+                                     generate_file_content(defs),
+                                     fromfile='check_macroassembler_style.py declared syntax',
+                                     tofile='check_macroassembler_style.py found definitions')
+    ok = True
+    for diffline in difflines:
+        ok = False
+        print(diffline, end='')
+
+    return ok
+
+
+def main():
+    ok = check_style()
+
+    if ok:
+        print('TEST-PASS | check_macroassembler_style.py | ok')
+    else:
+        print('TEST-UNEXPECTED-FAIL | check_macroassembler_style.py | actual output does not match expected output;  diff is above')
+
+    sys.exit(0 if ok else 1)
+
+
+if __name__ == '__main__':
+    main()
--- a/config/check_spidermonkey_style.py
+++ b/config/check_spidermonkey_style.py
@@ -38,16 +38,17 @@
 from __future__ import print_function
 
 import difflib
 import os
 import re
 import subprocess
 import sys
 import traceback
+from check_utils import get_all_toplevel_filenames
 
 # We don't bother checking files in these directories, because they're (a) auxiliary or (b)
 # imported code that doesn't follow our coding style.
 ignored_js_src_dirs = [
    'js/src/config/',            # auxiliary stuff
    'js/src/ctypes/libffi/',     # imported code
    'js/src/devtools/',          # auxiliary stuff
    'js/src/editline/',          # imported code
@@ -212,47 +213,33 @@ class FileKind(object):
             return FileKind.TBL
 
         if filename.endswith('.msg'):
             return FileKind.MSG
 
         error(filename, None, 'unknown file kind')
 
 
-def get_all_filenames():
-    '''Get a list of all the files in the (Mercurial or Git) repository.'''
-    cmds = [['hg', 'manifest', '-q'], ['git', 'ls-files', '--full-name', '../..']]
-    for cmd in cmds:
-        try:
-            all_filenames = subprocess.check_output(cmd, universal_newlines=True,
-                                                    stderr=subprocess.PIPE).split('\n')
-            return all_filenames
-        except:
-            continue
-    else:
-        raise Exception('failed to run any of the repo manifest commands', cmds)
-
-
 def check_style():
     # We deal with two kinds of name.
     # - A "filename" is a full path to a file from the repository root.
     # - An "inclname" is how a file is referred to in a #include statement.
     #
     # Examples (filename -> inclname)
     # - "mfbt/Attributes.h"     -> "mozilla/Attributes.h"
     # - "mfbt/decimal/Decimal.h -> "mozilla/Decimal.h"
     # - "js/public/Vector.h"    -> "js/Vector.h"
     # - "js/src/vm/String.h"    -> "vm/String.h"
 
     mfbt_inclnames = set()      # type: set(inclname)
     mozalloc_inclnames = set()  # type: set(inclname)
     js_names = dict()           # type: dict(filename, inclname)
 
     # Select the appropriate files.
-    for filename in get_all_filenames():
+    for filename in get_all_toplevel_filenames():
         if filename.startswith('mfbt/') and filename.endswith('.h'):
             inclname = 'mozilla/' + filename.split('/')[-1]
             mfbt_inclnames.add(inclname)
 
         if filename.startswith('memory/mozalloc/') and filename.endswith('.h'):
             inclname = 'mozilla/' + filename.split('/')[-1]
             mozalloc_inclnames.add(inclname)
 
new file mode 100644
--- /dev/null
+++ b/config/check_utils.py
@@ -0,0 +1,30 @@
+# vim: set ts=8 sts=4 et sw=4 tw=99:
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+import subprocess
+
+def get_all_toplevel_filenames():
+    '''Get a list of all the files in the (Mercurial or Git) repository.'''
+    try:
+        cmd = ['hg', 'manifest', '-q']
+        all_filenames = subprocess.check_output(cmd, universal_newlines=True,
+                                                stderr=subprocess.PIPE).split('\n')
+        return all_filenames
+    except:
+        pass
+
+    try:
+        # Get the relative path to the top-level directory.
+        cmd = ['git', 'rev-parse', '--show-cdup']
+        top_level = subprocess.check_output(cmd, universal_newlines=True,
+                                                stderr=subprocess.PIPE).split('\n')[0]
+        cmd = ['git', 'ls-files', '--full-name', top_level]
+        all_filenames = subprocess.check_output(cmd, universal_newlines=True,
+                                                stderr=subprocess.PIPE).split('\n')
+        return all_filenames
+    except:
+        pass
+
+    raise Exception('failed to run any of the repo manifest commands', cmds)
--- a/js/src/Makefile.in
+++ b/js/src/Makefile.in
@@ -122,24 +122,27 @@ ifneq ($(LLVM_SYMBOLIZER),)
 # Use the LLVM symbolizer when running jit-tests under ASan, if available
 JITTEST_ASAN_ENV=ASAN_SYMBOLIZER_PATH='$(LLVM_SYMBOLIZER)'
 endif
 endif
 
 check-style::
 	(cd $(srcdir) && $(PYTHON) $(topsrcdir)/config/check_spidermonkey_style.py);
 
+check-masm::
+	(cd $(srcdir) && $(PYTHON) $(topsrcdir)/config/check_macroassembler_style.py);
+
 check-jit-test::
 	$(JITTEST_ASAN_ENV) $(wildcard $(RUN_TEST_PROGRAM)) $(PYTHON) -u $(srcdir)/jit-test/jit_test.py \
 	        --no-slow --no-progress --format=automation --jitflags=all \
 			$(JITTEST_VALGRIND_FLAG) \
 			$(JITTEST_EXTRA_ARGS) \
 	        $(DIST)/bin/$(JS_SHELL_NAME)$(BIN_SUFFIX)
 
-check:: check-style
+check:: check-style check-masm
 
 check-jstests:
 	$(wildcard $(RUN_TEST_PROGRAM)) $(PYTHON) -u $(srcdir)/tests/jstests.py \
 		--no-progress --format=automation --timeout 300 \
 		$(JSTESTS_EXTRA_ARGS) \
 		$(DIST)/bin/$(JS_SHELL_NAME)$(BIN_SUFFIX)
 
 # FIXME:
--- a/js/src/jit/MacroAssembler-inl.h
+++ b/js/src/jit/MacroAssembler-inl.h
@@ -7,16 +7,17 @@
 #ifndef jit_MacroAssembler_inl_h
 #define jit_MacroAssembler_inl_h
 
 #include "jit/MacroAssembler.h"
 
 namespace js {
 namespace jit {
 
+//{{{ check_macroassembler_style
 // ===============================================================
 // Frame manipulation functions.
 
 uint32_t
 MacroAssembler::framePushed() const
 {
     return framePushed_;
 }
@@ -70,16 +71,17 @@ MacroAssembler::call(const CallSiteDesc&
 
 void
 MacroAssembler::call(const CallSiteDesc& desc, Label* label)
 {
     call(label);
     append(desc, currentOffset(), framePushed());
 }
 
+//}}} check_macroassembler_style
 // ===============================================================
 
 void
 MacroAssembler::PushStubCode()
 {
     exitCodePatch_ = PushWithPatch(ImmWord(-1));
 }
 
--- a/js/src/jit/MacroAssembler.cpp
+++ b/js/src/jit/MacroAssembler.cpp
@@ -2549,16 +2549,17 @@ MacroAssembler::icBuildOOLFakeExitFrame(
 void
 MacroAssembler::icRestoreLive(LiveRegisterSet& liveRegs, AfterICSaveLive& aic)
 {
     restoreFrameAlignmentForICArguments(aic);
     MOZ_ASSERT(framePushed() == aic.initialStack);
     PopRegsInMask(liveRegs);
 }
 
+//{{{ check_macroassembler_style
 // ===============================================================
 // Stack manipulation functions.
 
 void
 MacroAssembler::PushRegsInMask(LiveGeneralRegisterSet set)
 {
     PushRegsInMask(LiveRegisterSet(set.set(), FloatRegisterSet()));
 }
@@ -2713,8 +2714,10 @@ MacroAssembler::freeStack(uint32_t amoun
     framePushed_ -= amount;
 }
 
 void
 MacroAssembler::freeStack(Register amount)
 {
     addToStackPtr(amount);
 }
+
+//}}} check_macroassembler_style
--- a/js/src/jit/MacroAssembler.h
+++ b/js/src/jit/MacroAssembler.h
@@ -2,16 +2,17 @@
  * vim: set ts=8 sts=4 et sw=4 tw=99:
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef jit_MacroAssembler_h
 #define jit_MacroAssembler_h
 
+#include "mozilla/MacroForEach.h"
 #include "mozilla/MathAlgorithms.h"
 
 #include "jscompartment.h"
 
 #if defined(JS_CODEGEN_X86)
 # include "jit/x86/MacroAssembler-x86.h"
 #elif defined(JS_CODEGEN_X64)
 # include "jit/x64/MacroAssembler-x64.h"
@@ -29,38 +30,144 @@
 #include "jit/AtomicOp.h"
 #include "jit/IonInstrumentation.h"
 #include "jit/JitCompartment.h"
 #include "jit/VMFunctions.h"
 #include "vm/ProxyObject.h"
 #include "vm/Shape.h"
 #include "vm/UnboxedObject.h"
 
-// This Macro is only a hint for code readers that the function is defined in a
-// specific macro assembler, and not in the generic macro assembler.  Thus, when
-// looking for the implementation one might find if the function is implemented
-// and where it is supposed to be implemented.
-# define PER_ARCH
+// * How to read/write MacroAssembler method declarations:
+//
+// The following macros are made to avoid #ifdef around each method declarations
+// of the Macro Assembler, and they are also used as an hint on the location of
+// the implementations of each method.  For example, the following declaration
+//
+//   void Pop(FloatRegister t) DEFINED_ON(x86_shared, arm);
+//
+// suggests the MacroAssembler::Pop(FloatRegister) method is implemented in
+// x86-shared/MacroAssembler-x86-shared.h, and also in arm/MacroAssembler-arm.h.
+//
+// - If there is no annotation, then there is only one generic definition in
+//   MacroAssembler.cpp.
+//
+// - If the declaration is "inline", then the method definition(s) would be in
+//   the "-inl.h" variant of the same file(s).
+//
+// The script check_macroassembler_style.py (check-masm target of the Makefile)
+// is used to verify that method definitions are matching the annotation added
+// to the method declarations.  If there is any difference, then you either
+// forgot to define the method in one of the macro assembler, or you forgot to
+// update the annotation of the macro assembler declaration.
+//
+// Some convenient short-cuts are used to avoid repeating the same list of
+// architectures on each method declaration, such as PER_ARCH and
+// PER_SHARED_ARCH.
+
+# define ALL_ARCH mips, arm, arm64, x86, x64
+# define ALL_SHARED_ARCH mips, arm, arm64, x86_shared
 
+// * How this macro works:
+//
+// DEFINED_ON is a macro which check if, for the current architecture, the
+// method is defined on the macro assembler or not.
+//
+// For each architecutre, we have a macro named DEFINED_ON_arch.  This macro is
+// empty if this is not the current architecture.  Otherwise it must be either
+// set to "define" or "crash" (only use for the none target so-far).
+//
+// The DEFINED_ON macro maps the list of architecture names given as argument to
+// a list of macro names.  For example,
+//
+//   DEFINED_ON(arm, x86_shared)
+//
+// is expanded to
+//
+//   DEFINED_ON_none DEFINED_ON_arm DEFINED_ON_x86_shared
+//
+// which are later expanded on ARM, x86, x64 by DEFINED_ON_EXPAND_ARCH_RESULTS
+// to
+//
+//   define
+//
+// or if the JIT is disabled or set to no architecture to
+//
+//   crash
+//
+// or to nothing, if the current architecture is not lsited in the list of
+// arguments of DEFINED_ON.  Note, only one of the DEFINED_ON_arch macro
+// contributes to the non-empty result, which is the macro of the current
+// architecture if it is listed in the arguments of DEFINED_ON.
+//
+// This result is appended to DEFINED_ON_RESULT_ before expanding the macro,
+// which result is either no annotation, a MOZ_CRASH(), or a "= delete"
+// annotation on the method declaration.
+
+# define DEFINED_ON_x86
+# define DEFINED_ON_x64
+# define DEFINED_ON_x86_shared
+# define DEFINED_ON_arm
+# define DEFINED_ON_arm64
+# define DEFINED_ON_mips
+# define DEFINED_ON_none
+
+// Specialize for each architecture.
 #if defined(JS_CODEGEN_X86)
-# define ONLY_X86_X64
+# undef DEFINED_ON_x86
+# define DEFINED_ON_x86 define
+# undef DEFINED_ON_x86_shared
+# define DEFINED_ON_x86_shared define
 #elif defined(JS_CODEGEN_X64)
-# define ONLY_X86_X64
+# undef DEFINED_ON_x64
+# define DEFINED_ON_x64 define
+# undef DEFINED_ON_x86_shared
+# define DEFINED_ON_x86_shared define
 #elif defined(JS_CODEGEN_ARM)
-# define ONLY_X86_X64 = delete
+# undef DEFINED_ON_arm
+# define DEFINED_ON_arm define
 #elif defined(JS_CODEGEN_ARM64)
-# define ONLY_X86_X64 = delete
+# undef DEFINED_ON_arm64
+# define DEFINED_ON_arm64 define
 #elif defined(JS_CODEGEN_MIPS)
-# define ONLY_X86_X64 = delete
+# undef DEFINED_ON_mips
+# define DEFINED_ON_mips define
 #elif defined(JS_CODEGEN_NONE)
-# define ONLY_X86_X64 = delete
+# undef DEFINED_ON_none
+# define DEFINED_ON_none crash
 #else
 # error "Unknown architecture!"
 #endif
 
+# define DEFINED_ON_RESULT_crash   { MOZ_CRASH(); }
+# define DEFINED_ON_RESULT_define
+# define DEFINED_ON_RESULT_        = delete
+
+# define DEFINED_ON_DISPATCH_RESULT(Result)     \
+    DEFINED_ON_RESULT_ ## Result
+
+// We need to let the evaluation of MOZ_FOR_EACH terminates.
+# define DEFINED_ON_EXPAND_ARCH_RESULTS_3(ParenResult)  \
+    DEFINED_ON_DISPATCH_RESULT ParenResult
+# define DEFINED_ON_EXPAND_ARCH_RESULTS_2(ParenResult)  \
+    DEFINED_ON_EXPAND_ARCH_RESULTS_3 (ParenResult)
+# define DEFINED_ON_EXPAND_ARCH_RESULTS(ParenResult)    \
+    DEFINED_ON_EXPAND_ARCH_RESULTS_2 (ParenResult)
+
+# define DEFINED_ON_FWDARCH(Arch) DEFINED_ON_ ## Arch
+# define DEFINED_ON_MAP_ON_ARCHS(ArchList)              \
+    DEFINED_ON_EXPAND_ARCH_RESULTS(                     \
+      (MOZ_FOR_EACH(DEFINED_ON_FWDARCH, (), ArchList)))
+
+# define DEFINED_ON(...)                                \
+    DEFINED_ON_MAP_ON_ARCHS((none, __VA_ARGS__))
+
+# define PER_ARCH DEFINED_ON(ALL_ARCH)
+# define PER_SHARED_ARCH DEFINED_ON(ALL_SHARED_ARCH)
+
+
 #ifdef IS_LITTLE_ENDIAN
 #define IMM32_16ADJ(X) X << 16
 #else
 #define IMM32_16ADJ(X) X
 #endif
 
 namespace js {
 namespace jit {
@@ -280,16 +387,17 @@ class MacroAssembler : public MacroAssem
     MoveResolver& moveResolver() {
         return moveResolver_;
     }
 
     size_t instructionsSize() const {
         return size();
     }
 
+    //{{{ check_macroassembler_style
   public:
     // ===============================================================
     // Frame manipulation functions.
 
     inline uint32_t framePushed() const;
     inline void setFramePushed(uint32_t framePushed);
     inline void adjustFrame(int32_t value);
 
@@ -304,72 +412,73 @@ class MacroAssembler : public MacroAssem
     //
     // It is maintained by all stack manipulation functions below.
     uint32_t framePushed_;
 
   public:
     // ===============================================================
     // Stack manipulation functions.
 
-    void PushRegsInMask(LiveRegisterSet set) PER_ARCH;
+    void PushRegsInMask(LiveRegisterSet set) PER_SHARED_ARCH;
     void PushRegsInMask(LiveGeneralRegisterSet set);
 
     void PopRegsInMask(LiveRegisterSet set);
     void PopRegsInMask(LiveGeneralRegisterSet set);
-    void PopRegsInMaskIgnore(LiveRegisterSet set, LiveRegisterSet ignore) PER_ARCH;
+    void PopRegsInMaskIgnore(LiveRegisterSet set, LiveRegisterSet ignore) PER_SHARED_ARCH;
 
-    void Push(const Operand op) PER_ARCH ONLY_X86_X64;
-    void Push(Register reg) PER_ARCH;
-    void Push(const Imm32 imm) PER_ARCH;
-    void Push(const ImmWord imm) PER_ARCH;
-    void Push(const ImmPtr imm) PER_ARCH;
-    void Push(const ImmGCPtr ptr) PER_ARCH;
-    void Push(FloatRegister reg) PER_ARCH;
+    void Push(const Operand op) DEFINED_ON(x86_shared);
+    void Push(Register reg) PER_SHARED_ARCH;
+    void Push(const Imm32 imm) PER_SHARED_ARCH;
+    void Push(const ImmWord imm) PER_SHARED_ARCH;
+    void Push(const ImmPtr imm) PER_SHARED_ARCH;
+    void Push(const ImmGCPtr ptr) PER_SHARED_ARCH;
+    void Push(FloatRegister reg) PER_SHARED_ARCH;
     void Push(jsid id, Register scratchReg);
     void Push(TypedOrValueRegister v);
     void Push(ConstantOrRegister v);
     void Push(const ValueOperand& val);
     void Push(const Value& val);
     void Push(JSValueType type, Register reg);
     void PushValue(const Address& addr);
     void PushEmptyRooted(VMFunction::RootType rootType);
     inline CodeOffsetLabel PushWithPatch(ImmWord word);
     inline CodeOffsetLabel PushWithPatch(ImmPtr imm);
 
-    void Pop(const Operand op) PER_ARCH ONLY_X86_X64;
-    void Pop(Register reg) PER_ARCH;
-    void Pop(FloatRegister t) PER_ARCH ONLY_X86_X64;
-    void Pop(const ValueOperand& val) PER_ARCH;
+    void Pop(const Operand op) DEFINED_ON(x86_shared);
+    void Pop(Register reg) PER_SHARED_ARCH;
+    void Pop(FloatRegister t) DEFINED_ON(x86_shared);
+    void Pop(const ValueOperand& val) PER_SHARED_ARCH;
     void popRooted(VMFunction::RootType rootType, Register cellReg, const ValueOperand& valueReg);
 
     // Move the stack pointer based on the requested amount.
     void adjustStack(int amount);
     void reserveStack(uint32_t amount) PER_ARCH;
     void freeStack(uint32_t amount);
 
     // Warning: This method does not update the framePushed() counter.
     void freeStack(Register amount);
 
   public:
     // ===============================================================
     // Simple call functions.
 
-    void call(Register reg) PER_ARCH;
-    void call(const Address& addr) PER_ARCH ONLY_X86_X64;
-    void call(Label* label) PER_ARCH;
-    void call(ImmWord imm) PER_ARCH;
+    void call(Register reg) PER_SHARED_ARCH;
+    void call(const Address& addr) DEFINED_ON(x86_shared);
+    void call(Label* label) PER_SHARED_ARCH;
+    void call(ImmWord imm) PER_SHARED_ARCH;
     // Call a target native function, which is neither traceable nor movable.
-    void call(ImmPtr imm) PER_ARCH;
-    void call(AsmJSImmPtr imm) PER_ARCH;
+    void call(ImmPtr imm) PER_SHARED_ARCH;
+    void call(AsmJSImmPtr imm) PER_SHARED_ARCH;
     // Call a target JitCode, which must be traceable, and may be movable.
-    void call(JitCode* c) PER_ARCH;
+    void call(JitCode* c) PER_SHARED_ARCH;
 
     inline void call(const CallSiteDesc& desc, const Register reg);
     inline void call(const CallSiteDesc& desc, Label* label);
 
+    //}}} check_macroassembler_style
   public:
 
     // Emits a test of a value against all types in a TypeSet. A scratch
     // register is required.
     template <typename Source>
     void guardTypeSet(const Source& address, const TypeSet* types, BarrierKind kind, Register scratch, Label* miss);
 
     void guardObjectType(Register obj, const TypeSet* types, Register scratch, Label* miss);
--- a/js/src/jit/arm/MacroAssembler-arm.cpp
+++ b/js/src/jit/arm/MacroAssembler-arm.cpp
@@ -5105,16 +5105,17 @@ MacroAssemblerARMCompat::asMasm()
 }
 
 const MacroAssembler&
 MacroAssemblerARMCompat::asMasm() const
 {
     return *static_cast<const MacroAssembler*>(this);
 }
 
+//{{{ check_macroassembler_style
 // ===============================================================
 // Stack manipulation functions.
 
 void
 MacroAssembler::PushRegsInMask(LiveRegisterSet set)
 {
     int32_t diffF = set.fpus().getPushSizeInBytes();
     int32_t diffG = set.gprs().size() * sizeof(intptr_t);
@@ -5295,8 +5296,10 @@ MacroAssembler::call(JitCode* c)
     if (HasMOVWT())
         rs = L_MOVWT;
     else
         rs = L_LDR;
 
     ma_movPatchable(ImmPtr(c->raw()), ScratchRegister, Always, rs);
     ma_callJitHalfPush(ScratchRegister);
 }
+
+//}}} check_macroassembler_style
--- a/js/src/jit/arm64/MacroAssembler-arm64.cpp
+++ b/js/src/jit/arm64/MacroAssembler-arm64.cpp
@@ -547,29 +547,21 @@ MacroAssemblerCompat::callAndPushReturnA
 
 void
 MacroAssemblerCompat::breakpoint()
 {
     static int code = 0xA77;
     Brk((code++) & 0xffff);
 }
 
+//{{{ check_macroassembler_style
 // ===============================================================
 // Stack manipulation functions.
 
 void
-MacroAssembler::reserveStack(uint32_t amount)
-{
-    // TODO: This bumps |sp| every time we reserve using a second register.
-    // It would save some instructions if we had a fixed frame size.
-    vixl::MacroAssembler::Claim(Operand(amount));
-    adjustFrame(amount);
-}
-
-void
 MacroAssembler::PushRegsInMask(LiveRegisterSet set)
 {
     for (GeneralRegisterBackwardIterator iter(set.gprs()); iter.more(); ) {
         vixl::CPURegister src[4] = { vixl::NoCPUReg, vixl::NoCPUReg, vixl::NoCPUReg, vixl::NoCPUReg };
 
         for (size_t i = 0; i < 4 && iter.more(); i++) {
             src[i] = ARMRegister(*iter, 64);
             ++iter;
@@ -683,29 +675,38 @@ MacroAssembler::Push(const ImmGCPtr ptr)
 void
 MacroAssembler::Push(FloatRegister f)
 {
     push(f);
     adjustFrame(sizeof(double));
 }
 
 void
-MacroAssembler::Pop(const Register reg)
+MacroAssembler::Pop(Register reg)
 {
     pop(reg);
     adjustFrame(-1 * int64_t(sizeof(int64_t)));
 }
 
 void
 MacroAssembler::Pop(const ValueOperand& val)
 {
     pop(val);
     adjustFrame(-1 * int64_t(sizeof(int64_t)));
 }
 
+void
+MacroAssembler::reserveStack(uint32_t amount)
+{
+    // TODO: This bumps |sp| every time we reserve using a second register.
+    // It would save some instructions if we had a fixed frame size.
+    vixl::MacroAssembler::Claim(Operand(amount));
+    adjustFrame(amount);
+}
+
 // ===============================================================
 // Simple call functions.
 
 void
 MacroAssembler::call(Register reg)
 {
     syncStackPtr();
     Blr(ARMRegister(reg, 64));
@@ -748,10 +749,12 @@ MacroAssembler::call(JitCode* c)
     vixl::UseScratchRegisterScope temps(this);
     const ARMRegister scratch64 = temps.AcquireX();
     syncStackPtr();
     BufferOffset off = immPool64(scratch64, uint64_t(c->raw()));
     addPendingJump(off, ImmPtr(c->raw()), Relocation::JITCODE);
     blr(scratch64);
 }
 
+//}}} check_macroassembler_style
+
 } // namespace jit
 } // namespace js
--- a/js/src/jit/mips/MacroAssembler-mips.cpp
+++ b/js/src/jit/mips/MacroAssembler-mips.cpp
@@ -3646,16 +3646,17 @@ MacroAssemblerMIPSCompat::asMasm()
 }
 
 const MacroAssembler&
 MacroAssemblerMIPSCompat::asMasm() const
 {
     return *static_cast<const MacroAssembler*>(this);
 }
 
+//{{{ check_macroassembler_style
 // ===============================================================
 // Stack manipulation functions.
 
 void
 MacroAssembler::PushRegsInMask(LiveRegisterSet set)
 {
     int32_t diffF = set.fpus().getPushSizeInBytes();
     int32_t diffG = set.gprs().size() * sizeof(intptr_t);
@@ -3817,8 +3818,10 @@ MacroAssembler::call(ImmPtr target)
 void
 MacroAssembler::call(JitCode* c)
 {
     BufferOffset bo = m_buffer.nextOffset();
     addPendingJump(bo, ImmPtr(c->raw()), Relocation::JITCODE);
     ma_liPatchable(ScratchRegister, Imm32((uint32_t)c->raw()));
     ma_callJitHalfPush(ScratchRegister);
 }
+
+//}}} check_macroassembler_style
--- a/js/src/jit/none/Trampoline-none.cpp
+++ b/js/src/jit/none/Trampoline-none.cpp
@@ -48,31 +48,8 @@ BailoutFrameInfo::BailoutFrameInfo(const
     MOZ_CRASH();
 }
 
 bool ICCompare_Int32::Compiler::generateStubCode(MacroAssembler&) { MOZ_CRASH(); }
 bool ICCompare_Double::Compiler::generateStubCode(MacroAssembler&) { MOZ_CRASH(); }
 bool ICBinaryArith_Int32::Compiler::generateStubCode(MacroAssembler&) { MOZ_CRASH(); }
 bool ICUnaryArith_Int32::Compiler::generateStubCode(MacroAssembler&) { MOZ_CRASH(); }
 JitCode* JitRuntime::generateProfilerExitFrameTailStub(JSContext*) { MOZ_CRASH(); }
-
-void MacroAssembler::alignFrameForICArguments(AfterICSaveLive& aic) { MOZ_CRASH(); }
-void MacroAssembler::restoreFrameAlignmentForICArguments(AfterICSaveLive& aic) { MOZ_CRASH(); }
-
-void MacroAssembler::clampDoubleToUint8(FloatRegister input, Register output) { MOZ_CRASH(); }
-
-// ===============================================================
-// Stack manipulation functions.
-
-void MacroAssembler::PushRegsInMask(LiveRegisterSet) { MOZ_CRASH(); }
-void MacroAssembler::PopRegsInMaskIgnore(LiveRegisterSet, LiveRegisterSet) { MOZ_CRASH(); }
-
-void MacroAssembler::Push(Register reg) { MOZ_CRASH(); }
-void MacroAssembler::Push(const Imm32 imm) { MOZ_CRASH(); }
-void MacroAssembler::Push(const ImmWord imm) { MOZ_CRASH(); }
-void MacroAssembler::Push(const ImmPtr imm) { MOZ_CRASH(); }
-void MacroAssembler::Push(const ImmGCPtr ptr) { MOZ_CRASH(); }
-void MacroAssembler::Push(FloatRegister reg) { MOZ_CRASH(); }
-
-void MacroAssembler::Pop(Register reg) { MOZ_CRASH(); }
-void MacroAssembler::Pop(const ValueOperand& val) { MOZ_CRASH(); }
-
-void MacroAssembler::reserveStack(uint32_t amount) { MOZ_CRASH(); }
--- a/js/src/jit/x64/MacroAssembler-x64.cpp
+++ b/js/src/jit/x64/MacroAssembler-x64.cpp
@@ -573,16 +573,17 @@ MacroAssemblerX64::asMasm()
 }
 
 const MacroAssembler&
 MacroAssemblerX64::asMasm() const
 {
     return *static_cast<const MacroAssembler*>(this);
 }
 
+//{{{ check_macroassembler_style
 // ===============================================================
 // Stack manipulation functions.
 
 void
 MacroAssembler::reserveStack(uint32_t amount)
 {
     if (amount) {
         // On windows, we cannot skip very far down the stack without touching the
@@ -595,8 +596,10 @@ MacroAssembler::reserveStack(uint32_t am
             subq(Imm32(4096), StackPointer);
             store32(Imm32(0), Address(StackPointer, 0));
             amountLeft -= 4096;
         }
         subq(Imm32(amountLeft), StackPointer);
     }
     framePushed_ += amount;
 }
+
+//}}} check_macroassembler_style
--- a/js/src/jit/x86-shared/MacroAssembler-x86-shared.cpp
+++ b/js/src/jit/x86-shared/MacroAssembler-x86-shared.cpp
@@ -190,16 +190,17 @@ MacroAssemblerX86Shared::asMasm()
 }
 
 const MacroAssembler&
 MacroAssemblerX86Shared::asMasm() const
 {
     return *static_cast<const MacroAssembler*>(this);
 }
 
+//{{{ check_macroassembler_style
 // ===============================================================
 // Stack manipulation functions.
 
 void
 MacroAssembler::PushRegsInMask(LiveRegisterSet set)
 {
     FloatRegisterSet fpuSet(set.fpus().reduceSetForPush());
     unsigned numFpu = fpuSet.size();
@@ -410,8 +411,10 @@ MacroAssembler::call(ImmPtr target)
     call(ImmWord(uintptr_t(target.value)));
 }
 
 void
 MacroAssembler::call(JitCode* target)
 {
     Assembler::call(target);
 }
+
+//}}} check_macroassembler_style
--- a/js/src/jit/x86/MacroAssembler-x86.cpp
+++ b/js/src/jit/x86/MacroAssembler-x86.cpp
@@ -560,16 +560,17 @@ MacroAssemblerX86::asMasm()
 }
 
 const MacroAssembler&
 MacroAssemblerX86::asMasm() const
 {
     return *static_cast<const MacroAssembler*>(this);
 }
 
+//{{{ check_macroassembler_style
 // ===============================================================
 // Stack manipulation functions.
 
 void
 MacroAssembler::reserveStack(uint32_t amount)
 {
     if (amount) {
         // On windows, we cannot skip very far down the stack without touching the
@@ -582,8 +583,10 @@ MacroAssembler::reserveStack(uint32_t am
             subl(Imm32(4096), StackPointer);
             store32(Imm32(0), Address(StackPointer, 0));
             amountLeft -= 4096;
         }
         subl(Imm32(amountLeft), StackPointer);
     }
     framePushed_ += amount;
 }
+
+//}}} check_macroassembler_style
--- a/testing/mach_commands.py
+++ b/testing/mach_commands.py
@@ -347,17 +347,21 @@ class CheckSpiderMonkeyCommand(MachComma
         print('running jsapi-tests')
         jsapi_tests_cmd = [os.path.join(self.bindir, executable_name('jsapi-tests'))]
         jsapi_tests_result = subprocess.call(jsapi_tests_cmd)
 
         print('running check-style')
         check_style_cmd = [sys.executable, os.path.join(self.topsrcdir, 'config', 'check_spidermonkey_style.py')]
         check_style_result = subprocess.call(check_style_cmd, cwd=os.path.join(self.topsrcdir, 'js', 'src'))
 
-        all_passed = jittest_result and jstest_result and jsapi_tests_result and check_style_result
+        print('running check-masm')
+        check_masm_cmd = [sys.executable, os.path.join(self.topsrcdir, 'config', 'check_macroassembler_style.py')]
+        check_masm_result = subprocess.call(check_masm_cmd, cwd=os.path.join(self.topsrcdir, 'js', 'src'))
+
+        all_passed = jittest_result and jstest_result and jsapi_tests_result and check_style_result and check_masm_result
 
         return all_passed
 
 @CommandProvider
 class JsapiTestsCommand(MachCommandBase):
     @Command('jsapi-tests', category='testing', description='Run jsapi tests (JavaScript engine).')
     @CommandArgument('test_name', nargs='?', metavar='N',
         help='Test to run. Can be a prefix or omitted. If omitted, the entire ' \