Bug 1632067 part 6 - Simplify arguments length code. r=iain

Define the lengths directly in the Python script. Make CacheIRWriter assert the
written arguments match this information to ensure everything stays in sync.

Differential Revision: https://phabricator.services.mozilla.com/D71904

UltraBlame original commit: 1a37730d1bd1fe8df7e613edf04d18db5ce75f6c

Author: marco-c

js/src/jit/BaselineInspector.cpp

@@ -676,7 +676,7 @@ static bool MaybeArgumentReader(ICStub* stub, CacheOp targetOp,
   CacheIRReader stubReader(GetCacheIRStubInfo(stub));
   while (stubReader.more()) {
     CacheOp op = stubReader.readOp();
-    uint32_t argLength = CacheIROpFormat::ArgLengths[uint8_t(op)];
+    uint32_t argLength = CacheIROpArgLengths[size_t(op)];
 
     if (op == targetOp) {
       MOZ_ASSERT(argReader.isNothing(),

js/src/jit/CacheIR.cpp

@@ -38,47 +38,18 @@ const char* const js::jit::CacheKindNames[] = {
 #undef DEFINE_KIND
 };
 
-const char* const js::jit::CacheIrOpNames[] = {
+const char* const js::jit::CacheIROpNames[] = {
 #define OPNAME(op, ...) #op,
     CACHE_IR_OPS(OPNAME)
 #undef OPNAME
 };
 
-
-
-
-namespace js::jit::CacheIROpFormat {
-
-static constexpr uint32_t CacheIRArgLength(ArgType arg) {
-  switch (arg) {
-    case None:
-      return 0;
-    case Id:
-      return sizeof(uint8_t);
-    case Field:
-      return sizeof(uint8_t);
-    case Byte:
-      return sizeof(uint8_t);
-    case Int32:
-    case UInt32:
-      return sizeof(uint32_t);
-    case Word:
-      return sizeof(uintptr_t);
-  }
-}
-template <typename... Args>
-static constexpr uint32_t CacheIRArgsLength(Args... args) {
-  return (CacheIRArgLength(args) + ...);
-}
-
-const uint32_t ArgLengths[] = {
-#define ARGLENGTH(op, ...) CacheIRArgsLength(__VA_ARGS__),
+const uint32_t js::jit::CacheIROpArgLengths[] = {
+#define ARGLENGTH(op, len) len,
     CACHE_IR_OPS(ARGLENGTH)
 #undef ARGLENGTH
 };
 
-}  
-
 void CacheIRWriter::assertSameCompartment(JSObject* obj) {
   cx_->debugOnlyCheck(obj);
 }

js/src/jit/CacheIR.h

@@ -181,31 +181,14 @@ enum class CacheKind : uint8_t {
 
 extern const char* const CacheKindNames[];
 
-
-
-
-
-namespace CacheIROpFormat {
-enum ArgType {
-  None,
-  Id,
-  Field,
-  Byte,
-  Int32,
-  UInt32,
-  Word,
-};
-
-extern const uint32_t ArgLengths[];
-}  
-
 enum class CacheOp {
 #define DEFINE_OP(op, ...) op,
   CACHE_IR_OPS(DEFINE_OP)
 #undef DEFINE_OP
 };
 
-extern const char* const CacheIrOpNames[];
+extern const char* const CacheIROpNames[];
+extern const uint32_t CacheIROpArgLengths[];
 
 class StubField {
  public:
@@ -481,12 +464,33 @@ class MOZ_RAII CacheIRWriter : public JS::CustomAutoRooter {
   mutable uint32_t lastOffset_;
   mutable uint32_t lastIndex_;
 
+#ifdef DEBUG
+  
+  mozilla::Maybe<CacheOp> currentOp_;
+  size_t currentOpArgsStart_ = 0;
+#endif
+
   void assertSameCompartment(JSObject*);
 
   void writeOp(CacheOp op) {
     MOZ_ASSERT(uint32_t(op) <= UINT8_MAX);
     buffer_.writeByte(uint32_t(op));
     nextInstructionId_++;
+#ifdef DEBUG
+    MOZ_ASSERT(currentOp_.isNothing(), "Missing call to assertLengthMatches?");
+    currentOp_.emplace(op);
+    currentOpArgsStart_ = buffer_.length();
+#endif
+  }
+
+  void assertLengthMatches() {
+#ifdef DEBUG
+    
+    size_t expectedLen = CacheIROpArgLengths[size_t(*currentOp_)];
+    MOZ_ASSERT_IF(!failed(),
+                  buffer_.length() - currentOpArgsStart_ == expectedLen);
+    currentOp_.reset();
+#endif
   }
 
   void writeOperandId(OperandId opId) {

js/src/jit/CacheIRSpewer.cpp

@@ -45,7 +45,7 @@ class MOZ_RAII CacheIROpsJitSpewer {
   CACHE_IR_SPEWER_GENERATED
 
   void spewOp(CacheOp op) {
-    const char* opName = CacheIrOpNames[size_t(op)];
+    const char* opName = CacheIROpNames[size_t(op)];
     out_.printf("%s%-30s", prefix_, opName);
   }
   void spewOpEnd() { out_.printf("\n"); }
@@ -183,7 +183,7 @@ class MOZ_RAII CacheIROpsJSONSpewer {
   CACHE_IR_SPEWER_GENERATED
 
   void spewOp(CacheOp op) {
-    const char* opName = CacheIrOpNames[size_t(op)];
+    const char* opName = CacheIROpNames[size_t(op)];
     j_.beginObject();
     j_.property("op", opName);
     j_.beginListProperty("args");

js/src/jit/GenerateCacheIRFiles.py

@@ -112,6 +112,10 @@ def gen_writer_method(name, args, custom_writer):
 
 
 
+    
+    
+    
+    
 
 
     method_name = name[0].lower() + name[1:]
@@ -138,6 +142,7 @@ def gen_writer_method(name, args, custom_writer):
     code += '{} {}({}) {{\\\n'.format(ret_type, method_name, ', '.join(method_args))
     code += '  writeOp(CacheOp::{});\\\n'.format(name)
     code += args_code
+    code += '  assertLengthMatches();\\\n'
     if ret_type != 'void':
         code += '  return result;\\\n'
     code += '}'
@@ -306,6 +311,53 @@ def gen_spewer_method(name, args):
     return code
 
 
+
+
+
+arg_length = {
+    'ValId': 1,
+    'ObjId': 1,
+    'StringId': 1,
+    'SymbolId': 1,
+    'Int32Id': 1,
+    'NumberId': 1,
+    'BigIntId': 1,
+    'ValueTagId': 1,
+    'RawId': 1,
+
+    'ShapeField': 1,
+    'GroupField': 1,
+    'ObjectField': 1,
+    'StringField': 1,
+    'AtomField': 1,
+    'PropertyNameField': 1,
+    'SymbolField': 1,
+    'RawWordField': 1,
+    'RawPointerField': 1,
+    'DOMExpandoGenerationField': 1,
+    'IdField': 1,
+    'ValueField': 1,
+
+    'ByteImm': 1,
+    'BoolImm': 1,
+    'CallFlagsImm': 1,
+    'TypedThingLayoutImm': 1,
+    'ReferenceTypeImm': 1,
+    'ScalarTypeImm': 1,
+    'MetaTwoByteKindImm': 1,
+    'JSOpImm': 1,
+    'ValueTypeImm': 1,
+    'GuardClassKindImm': 1,
+    'JSWhyMagicImm': 1,
+
+    'Int32Imm': 4,
+    'UInt32Imm': 4,
+
+    'JSNativeImm': 'sizeof(uintptr_t)',
+    'StaticStringImm': 'sizeof(uintptr_t)',
+}
+
+
 def generate_cacheirops_header(c_out, yaml_path):
     """Generate CacheIROpsGenerated.h from CacheIROps.yaml. The generated file
     contains a list of all CacheIR ops and generated source code for
@@ -315,51 +367,6 @@ def generate_cacheirops_header(c_out, yaml_path):
 
 
 
-    mapping = {
-        'ValId': 'Id',
-        'ObjId': 'Id',
-        'StringId': 'Id',
-        'SymbolId': 'Id',
-        'Int32Id': 'Id',
-        'NumberId': 'Id',
-        'BigIntId': 'Id',
-        'ValueTagId': 'Id',
-        'RawId': 'Id',
-
-        'ShapeField': 'Field',
-        'GroupField': 'Field',
-        'ObjectField': 'Field',
-        'StringField': 'Field',
-        'AtomField': 'Field',
-        'PropertyNameField': 'Field',
-        'SymbolField': 'Field',
-        'RawWordField': 'Field',
-        'RawPointerField': 'Field',
-        'DOMExpandoGenerationField': 'Field',
-        'IdField': 'Field',
-        'ValueField': 'Field',
-
-        'ByteImm': 'Byte',
-        'BoolImm': 'Byte',
-        'CallFlagsImm': 'Byte',
-        'TypedThingLayoutImm': 'Byte',
-        'ReferenceTypeImm': 'Byte',
-        'ScalarTypeImm': 'Byte',
-        'MetaTwoByteKindImm': 'Byte',
-        'JSOpImm': 'Byte',
-        'ValueTypeImm': 'Byte',
-        'GuardClassKindImm': 'Byte',
-        'JSWhyMagicImm': 'Byte',
-
-        'Int32Imm': 'Int32',
-
-        'UInt32Imm': 'UInt32',
-
-        'JSNativeImm': 'Word',
-        'StaticStringImm': 'Word',
-    }
-
-    
     ops_items = []
 
 
@@ -385,10 +392,10 @@ def generate_cacheirops_header(c_out, yaml_path):
         assert isinstance(custom_writer, bool)
 
         if args:
-            args_str = ', '.join([mapping[v] for v in args.values()])
+            args_length = ' + '.join([str(arg_length[v]) for v in args.values()])
         else:
-            args_str = 'None'
-        ops_items.append('_({}, {})'.format(name, args_str))
+            args_length = '0'
+        ops_items.append('_({}, {})'.format(name, args_length))
 
         writer_methods.append(gen_writer_method(name, args, custom_writer))
         if shared:

js/src/jit/WarpCacheIRTranspiler.cpp

@@ -98,7 +98,7 @@ bool WarpCacheIRTranspiler::transpile(const MDefinitionStackVector& inputs) {
 #undef DEFINE_OP
 
       default:
-        fprintf(stderr, "Unsupported op: %s\n", CacheIrOpNames[size_t(op)]);
+        fprintf(stderr, "Unsupported op: %s\n", CacheIROpNames[size_t(op)]);
         MOZ_CRASH("Unsupported op");
     }
   } while (reader.more());

js/src/jit/WarpOracle.cpp

@@ -658,7 +658,7 @@ AbortReasonOr<Ok> WarpOracle::maybeInlineIC(WarpOpSnapshotList& snapshots,
   CacheIRReader reader(stubInfo);
   while (reader.more()) {
     CacheOp op = reader.readOp();
-    uint32_t argLength = CacheIROpFormat::ArgLengths[uint8_t(op)];
+    uint32_t argLength = CacheIROpArgLengths[size_t(op)];
     reader.skip(argLength);
 
     switch (op) {
@@ -671,7 +671,7 @@ AbortReasonOr<Ok> WarpOracle::maybeInlineIC(WarpOpSnapshotList& snapshots,
       default:
 
         JitSpew(JitSpew_WarpTranspiler, "unsupported CacheIR opcode: %s",
-                CacheIrOpNames[uint8_t(op)]);
+                CacheIROpNames[size_t(op)]);
         return Ok();
     }
   }