Move Type-related functions into Type class (NFC)

aheejin · aheejin · commit 944ae899b7a6 · 2019-12-24T21:55:28.000-08:00
Several type-related functions currently exist outside of `Type`
class and thus in the `wasm`, effectively global, namespace. This moves
these functions into `Type` class, making them either member functions
or static functions.

Also this renames `getSize` to `getByteSize` to make it not to be
confused with `size`, which returns the number of types in multiple
types. This also reorders the order of functions in `wasm-type.cpp` to
match that of `wasm-type.h`.
diff --git a/src/asm2wasm.h b/src/asm2wasm.h
@@ -2008,7 +2008,7 @@ Function* Asm2WasmBuilder::processFunction(Ref ast) {
       ret->offset = 0;
       ret->align = view.bytes;
       ret->ptr = processUnshifted(ast[2], view.bytes);
-      ret->type = getType(view.bytes, !view.integer);
+      ret->type = Type::getType(view.bytes, !view.integer);
       return ret;
     } else if (what == UNARY_PREFIX) {
       if (ast[1] == PLUS) {
diff --git a/src/ir/ExpressionAnalyzer.cpp b/src/ir/ExpressionAnalyzer.cpp
@@ -151,7 +151,7 @@ template<typename T> void visitImmediates(Expression* curr, T& visitor) {
     }
     void visitLoad(Load* curr) {
       visitor.visitInt(curr->bytes);
-      if (curr->type != unreachable && curr->bytes < getTypeSize(curr->type)) {
+      if (curr->type != unreachable && curr->bytes < curr->type.getByteSize()) {
         visitor.visitInt(curr->signed_);
       }
       visitor.visitAddress(curr->offset);
diff --git a/src/ir/load-utils.h b/src/ir/load-utils.h
@@ -31,7 +31,7 @@ inline bool isSignRelevant(Load* load) {
   if (load->type == unreachable) {
     return false;
   }
-  return !type.isFloat() && load->bytes < getTypeSize(type);
+  return !type.isFloat() && load->bytes < type.getByteSize();
 }
 
 // check if a load can be signed (which some opts want to do)
diff --git a/src/passes/Asyncify.cpp b/src/passes/Asyncify.cpp
@@ -1090,7 +1090,7 @@ struct AsyncifyLocals : public WalkerPass<PostWalker<AsyncifyLocals>> {
     Index total = 0;
     for (Index i = 0; i < numPreservableLocals; i++) {
       auto type = func->getLocalType(i);
-      auto size = getTypeSize(type);
+      auto size = type.getByteSize();
       total += size;
     }
     auto* block = builder->makeBlock();
@@ -1101,7 +1101,7 @@ struct AsyncifyLocals : public WalkerPass<PostWalker<AsyncifyLocals>> {
     Index offset = 0;
     for (Index i = 0; i < numPreservableLocals; i++) {
       auto type = func->getLocalType(i);
-      auto size = getTypeSize(type);
+      auto size = type.getByteSize();
       assert(size % STACK_ALIGN == 0);
       // TODO: higher alignment?
       block->list.push_back(builder->makeLocalSet(
@@ -1130,7 +1130,7 @@ struct AsyncifyLocals : public WalkerPass<PostWalker<AsyncifyLocals>> {
     Index offset = 0;
     for (Index i = 0; i < numPreservableLocals; i++) {
       auto type = func->getLocalType(i);
-      auto size = getTypeSize(type);
+      auto size = type.getByteSize();
       assert(size % STACK_ALIGN == 0);
       // TODO: higher alignment?
       block->list.push_back(
diff --git a/src/passes/AvoidReinterprets.cpp b/src/passes/AvoidReinterprets.cpp
@@ -32,7 +32,7 @@ static bool canReplaceWithReinterpret(Load* load) {
   // a reinterpret of the same address. A partial load would see
   // more bytes and possibly invalid data, and an unreachable
   // pointer is just not interesting to handle.
-  return load->type != unreachable && load->bytes == getTypeSize(load->type);
+  return load->type != unreachable && load->bytes == load->type.getByteSize();
 }
 
 static Load* getSingleLoad(LocalGraph* localGraph, LocalGet* get) {
@@ -116,7 +116,7 @@ struct AvoidReinterprets : public WalkerPass<PostWalker<AvoidReinterprets>> {
         // We should use another load here, to avoid reinterprets.
         info.ptrLocal = Builder::addVar(func, i32);
         info.reinterpretedLocal =
-          Builder::addVar(func, reinterpretType(load->type));
+          Builder::addVar(func, load->type.reinterpretType());
       } else {
         unoptimizables.insert(load);
       }
@@ -151,7 +151,7 @@ struct AvoidReinterprets : public WalkerPass<PostWalker<AvoidReinterprets>> {
                 // A reinterpret of a get of a load - use the new local.
                 Builder builder(*module);
                 replaceCurrent(builder.makeLocalGet(
-                  info.reinterpretedLocal, reinterpretType(load->type)));
+                  info.reinterpretedLocal, load->type.reinterpretType()));
               }
             }
           }
@@ -185,7 +185,7 @@ struct AvoidReinterprets : public WalkerPass<PostWalker<AvoidReinterprets>> {
                                 load->offset,
                                 load->align,
                                 ptr,
-                                reinterpretType(load->type));
+                                load->type.reinterpretType());
       }
     } finalOptimizer(infos, localGraph, getModule());
 
diff --git a/src/passes/ConstHoisting.cpp b/src/passes/ConstHoisting.cpp
@@ -88,7 +88,7 @@ struct ConstHoisting : public WalkerPass<PostWalker<ConstHoisting>> {
       }
       case f32:
       case f64: {
-        size = getTypeSize(value.type);
+        size = value.type.getByteSize();
         break;
       }
       case v128:     // v128 not implemented yet
diff --git a/src/passes/Print.cpp b/src/passes/Print.cpp
@@ -186,7 +186,7 @@ struct PrintExpressionContents
       o << ".atomic";
     }
     o << ".load";
-    if (curr->type != unreachable && curr->bytes < getTypeSize(curr->type)) {
+    if (curr->type != unreachable && curr->bytes < curr->type.getByteSize()) {
       if (curr->bytes == 1) {
         o << '8';
       } else if (curr->bytes == 2) {
@@ -233,7 +233,7 @@ struct PrintExpressionContents
   }
   static void printRMWSize(std::ostream& o, Type type, uint8_t bytes) {
     prepareColor(o) << forceConcrete(type) << ".atomic.rmw";
-    if (type != unreachable && bytes != getTypeSize(type)) {
+    if (type != unreachable && bytes != type.getByteSize()) {
       if (bytes == 1) {
         o << '8';
       } else if (bytes == 2) {
@@ -269,7 +269,7 @@ struct PrintExpressionContents
         o << "xchg";
         break;
     }
-    if (curr->type != unreachable && curr->bytes != getTypeSize(curr->type)) {
+    if (curr->type != unreachable && curr->bytes != curr->type.getByteSize()) {
       o << "_u";
     }
     restoreNormalColor(o);
@@ -281,7 +281,7 @@ struct PrintExpressionContents
     prepareColor(o);
     printRMWSize(o, curr->type, curr->bytes);
     o << "cmpxchg";
-    if (curr->type != unreachable && curr->bytes != getTypeSize(curr->type)) {
+    if (curr->type != unreachable && curr->bytes != curr->type.getByteSize()) {
       o << "_u";
     }
     restoreNormalColor(o);
diff --git a/src/passes/SafeHeap.cpp b/src/passes/SafeHeap.cpp
@@ -176,7 +176,7 @@ struct SafeHeap : public Pass {
       load.type = type;
       for (Index bytes : {1, 2, 4, 8, 16}) {
         load.bytes = bytes;
-        if (bytes > getTypeSize(type) || (type == f32 && bytes != 4) ||
+        if (bytes > type.getByteSize() || (type == f32 && bytes != 4) ||
             (type == f64 && bytes != 8) || (type == v128 && bytes != 16)) {
           continue;
         }
@@ -212,7 +212,7 @@ struct SafeHeap : public Pass {
       store.type = none;
       for (Index bytes : {1, 2, 4, 8, 16}) {
         store.bytes = bytes;
-        if (bytes > getTypeSize(valueType) ||
+        if (bytes > valueType.getByteSize() ||
             (valueType == f32 && bytes != 4) ||
             (valueType == f64 && bytes != 8) ||
             (valueType == v128 && bytes != 16)) {
diff --git a/src/passes/SpillPointers.cpp b/src/passes/SpillPointers.cpp
@@ -78,7 +78,7 @@ struct SpillPointers
     PointerMap pointerMap;
     for (Index i = 0; i < func->getNumLocals(); i++) {
       if (func->getLocalType(i) == ABI::PointerType) {
-        auto offset = pointerMap.size() * getTypeSize(ABI::PointerType);
+        auto offset = pointerMap.size() * ABI::PointerType.getByteSize();
         pointerMap[i] = offset;
       }
     }
@@ -140,7 +140,7 @@ struct SpillPointers
       // get the stack space, and set the local to it
       ABI::getStackSpace(spillLocal,
                          func,
-                         getTypeSize(ABI::PointerType) * pointerMap.size(),
+                         ABI::PointerType.getByteSize() * pointerMap.size(),
                          *getModule());
     }
   }
@@ -184,9 +184,9 @@ struct SpillPointers
     // add the spills
     for (auto index : toSpill) {
       block->list.push_back(
-        builder.makeStore(getTypeSize(ABI::PointerType),
+        builder.makeStore(ABI::PointerType.getByteSize(),
                           pointerMap[index],
-                          getTypeSize(ABI::PointerType),
+                          ABI::PointerType.getByteSize(),
                           builder.makeLocalGet(spillLocal, ABI::PointerType),
                           builder.makeLocalGet(index, ABI::PointerType),
                           ABI::PointerType));
diff --git a/src/wasm-interpreter.h b/src/wasm-interpreter.h
@@ -1719,7 +1719,7 @@ template<typename GlobalManager, typename SubType> class ModuleInstanceBase {
       if (timeout.breaking()) {
         return timeout;
       }
-      auto bytes = getTypeSize(curr->expectedType);
+      auto bytes = curr->expectedType.getByteSize();
       auto addr = instance.getFinalAddress(ptr.value, bytes);
       auto loaded = instance.doAtomicLoad(addr, bytes, curr->expectedType);
       NOTE_EVAL1(loaded);
diff --git a/src/wasm-type.h b/src/wasm-type.h
@@ -79,6 +79,20 @@ class Type {
 
   // Allows for using Types in switch statements
   constexpr operator uint32_t() const { return id; }
+
+  // Returns the type size in bytes. Only single types are supported.
+  unsigned getByteSize() const;
+
+  // Reinterpret an integer type to a float type with the same size and vice
+  // versa. Only single integer and float types are supported.
+  Type reinterpretType() const;
+
+  // Returns the feature set required to use this type.
+  FeatureSet getFeatures() const;
+
+  // Returns a type based on its size in bytes and whether it is a float type.
+  static Type getType(unsigned byteSize, bool float_);
+
   std::string toString() const;
 };
 
@@ -123,11 +137,6 @@ constexpr Type anyref = Type::anyref;
 constexpr Type exnref = Type::exnref;
 constexpr Type unreachable = Type::unreachable;
 
-unsigned getTypeSize(Type type);
-FeatureSet getFeatures(Type type);
-Type getType(unsigned size, bool float_);
-Type reinterpretType(Type type);
-
 } // namespace wasm
 
 template<> class std::hash<wasm::Signature> {
diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp
@@ -2974,7 +2974,7 @@ bool WasmBinaryBuilder::maybeVisitAtomicWait(Expression*& out, uint8_t code) {
   curr->ptr = popNonVoidExpression();
   Address readAlign;
   readMemoryAccess(readAlign, curr->offset);
-  if (readAlign != getTypeSize(curr->expectedType)) {
+  if (readAlign != curr->expectedType.getByteSize()) {
     throwError("Align of AtomicWait must match size");
   }
   curr->finalize();
@@ -2994,7 +2994,7 @@ bool WasmBinaryBuilder::maybeVisitAtomicNotify(Expression*& out, uint8_t code) {
   curr->ptr = popNonVoidExpression();
   Address readAlign;
   readMemoryAccess(readAlign, curr->offset);
-  if (readAlign != getTypeSize(curr->type)) {
+  if (readAlign != curr->type.getByteSize()) {
     throwError("Align of AtomicNotify must match size");
   }
   curr->finalize();
diff --git a/src/wasm/wasm-s-parser.cpp b/src/wasm/wasm-s-parser.cpp
@@ -1268,7 +1268,7 @@ SExpressionWasmBuilder::makeLoad(Element& s, Type type, bool isAtomic) {
   auto* ret = allocator.alloc<Load>();
   ret->isAtomic = isAtomic;
   ret->type = type;
-  ret->bytes = parseMemBytes(extra, getTypeSize(type));
+  ret->bytes = parseMemBytes(extra, type.getByteSize());
   ret->signed_ = extra[0] && extra[1] == 's';
   size_t i = parseMemAttributes(s, &ret->offset, &ret->align, ret->bytes);
   ret->ptr = parseExpression(s[i]);
@@ -1282,7 +1282,7 @@ SExpressionWasmBuilder::makeStore(Element& s, Type type, bool isAtomic) {
   auto ret = allocator.alloc<Store>();
   ret->isAtomic = isAtomic;
   ret->valueType = type;
-  ret->bytes = parseMemBytes(extra, getTypeSize(type));
+  ret->bytes = parseMemBytes(extra, type.getByteSize());
   size_t i = parseMemAttributes(s, &ret->offset, &ret->align, ret->bytes);
   ret->ptr = parseExpression(s[i]);
   ret->value = parseExpression(s[i + 1]);
@@ -1294,7 +1294,7 @@ Expression* SExpressionWasmBuilder::makeAtomicRMWOrCmpxchg(Element& s,
                                                            Type type) {
   const char* extra = findMemExtra(
     *s[0], 11 /* after "type.atomic.rmw" */, /* isAtomic = */ false);
-  auto bytes = parseMemBytes(extra, getTypeSize(type));
+  auto bytes = parseMemBytes(extra, type.getByteSize());
   extra = strchr(extra, '.'); // after the optional '_u' and before the opcode
   if (!extra) {
     throw ParseException("malformed atomic rmw instruction");
diff --git a/src/wasm/wasm-type.cpp b/src/wasm/wasm-type.cpp
diff --git a/src/wasm/wasm-validator.cpp b/src/wasm/wasm-validator.cpp

Original file line number	Diff line number	Diff line change
`@@ -151,7 +151,7 @@ template<typename T> void visitImmediates(Expression* curr, T& visitor) {`
`151`	`151`	`}`
`152`	`152`	`void visitLoad(Load* curr) {`
`153`	`153`	`visitor.visitInt(curr->bytes);`
`154`		`- if (curr->type != unreachable && curr->bytes < getTypeSize(curr->type)) {`
	`154`	`+ if (curr->type != unreachable && curr->bytes < curr->type.getByteSize()) {`
`155`	`155`	`visitor.visitInt(curr->signed_);`
`156`	`156`	`}`
`157`	`157`	`visitor.visitAddress(curr->offset);`
Original file line number	Diff line number	Diff line change
`@@ -31,7 +31,7 @@ inline bool isSignRelevant(Load* load) {`
`31`	`31`	`if (load->type == unreachable) {`
`32`	`32`	`return false;`
`33`	`33`	`}`
`34`		`- return !type.isFloat() && load->bytes < getTypeSize(type);`
	`34`	`+ return !type.isFloat() && load->bytes < type.getByteSize();`
`35`	`35`	`}`
`36`	`36`
`37`	`37`	`// check if a load can be signed (which some opts want to do)`
Original file line number	Diff line number	Diff line change
`@@ -32,7 +32,7 @@ static bool canReplaceWithReinterpret(Load* load) {`
`32`	`32`	`// a reinterpret of the same address. A partial load would see`
`33`	`33`	`// more bytes and possibly invalid data, and an unreachable`
`34`	`34`	`// pointer is just not interesting to handle.`
`35`		`- return load->type != unreachable && load->bytes == getTypeSize(load->type);`
	`35`	`+ return load->type != unreachable && load->bytes == load->type.getByteSize();`
`36`	`36`	`}`
`37`	`37`
`38`	`38`	`static Load* getSingleLoad(LocalGraph* localGraph, LocalGet* get) {`
`@@ -116,7 +116,7 @@ struct AvoidReinterprets : public WalkerPass<PostWalker<AvoidReinterprets>> {`
`116`	`116`	`// We should use another load here, to avoid reinterprets.`
`117`	`117`	`info.ptrLocal = Builder::addVar(func, i32);`
`118`	`118`	`info.reinterpretedLocal =`
`119`		`- Builder::addVar(func, reinterpretType(load->type));`
	`119`	`+ Builder::addVar(func, load->type.reinterpretType());`
`120`	`120`	`} else {`
`121`	`121`	`unoptimizables.insert(load);`
`122`	`122`	`}`
`@@ -151,7 +151,7 @@ struct AvoidReinterprets : public WalkerPass<PostWalker<AvoidReinterprets>> {`
`151`	`151`	`// A reinterpret of a get of a load - use the new local.`
`152`	`152`	`Builder builder(*module);`
`153`	`153`	`replaceCurrent(builder.makeLocalGet(`
`154`		`- info.reinterpretedLocal, reinterpretType(load->type)));`
	`154`	`+ info.reinterpretedLocal, load->type.reinterpretType()));`
`155`	`155`	`}`
`156`	`156`	`}`
`157`	`157`	`}`
`@@ -185,7 +185,7 @@ struct AvoidReinterprets : public WalkerPass<PostWalker<AvoidReinterprets>> {`
`185`	`185`	`load->offset,`
`186`	`186`	`load->align,`
`187`	`187`	`ptr,`
`188`		`- reinterpretType(load->type));`
	`188`	`+ load->type.reinterpretType());`
`189`	`189`	`}`
`190`	`190`	`} finalOptimizer(infos, localGraph, getModule());`
`191`	`191`
Original file line number	Diff line number	Diff line change
`@@ -88,7 +88,7 @@ struct ConstHoisting : public WalkerPass<PostWalker<ConstHoisting>> {`
`88`	`88`	`}`
`89`	`89`	`case f32:`
`90`	`90`	`case f64: {`
`91`		`- size = getTypeSize(value.type);`
	`91`	`+ size = value.type.getByteSize();`
`92`	`92`	`break;`
`93`	`93`	`}`
`94`	`94`	`case v128: // v128 not implemented yet`
Original file line number	Diff line number	Diff line change
`@@ -1719,7 +1719,7 @@ template<typename GlobalManager, typename SubType> class ModuleInstanceBase {`
`1719`	`1719`	`if (timeout.breaking()) {`
`1720`	`1720`	`return timeout;`
`1721`	`1721`	`}`
`1722`		`- auto bytes = getTypeSize(curr->expectedType);`
	`1722`	`+ auto bytes = curr->expectedType.getByteSize();`
`1723`	`1723`	`auto addr = instance.getFinalAddress(ptr.value, bytes);`
`1724`	`1724`	`auto loaded = instance.doAtomicLoad(addr, bytes, curr->expectedType);`
`1725`	`1725`	`NOTE_EVAL1(loaded);`