only look at the |signed| field of loads if it is relevant (#1235)

Author: kripken

src/ast/ExpressionAnalyzer.cpp

@@ -14,9 +14,9 @@
  * limitations under the License.
  */
 
-#include "ast_utils.h"
 #include "support/hash.h"
-
+#include "ast_utils.h"
+#include "ast/load-utils.h"
 
 namespace wasm {
 // Given a stack of expressions, checks if the topmost is used as a result.
@@ -208,7 +208,10 @@ bool ExpressionAnalyzer::flexibleEqual(Expression* left, Expression* right, Expr
       }
       case Expression::Id::LoadId: {
         CHECK(Load, bytes);
-        CHECK(Load, signed_);
+        if (LoadUtils::isSignRelevant(left->cast<Load>()) &&
+            LoadUtils::isSignRelevant(right->cast<Load>())) {
+          CHECK(Load, signed_);
+        }
         CHECK(Load, offset);
         CHECK(Load, align);
         PUSH(Load, ptr);
@@ -455,7 +458,9 @@ uint32_t ExpressionAnalyzer::hash(Expression* curr) {
       }
       case Expression::Id::LoadId: {
         HASH(Load, bytes);
-        HASH(Load, signed_);
+        if (LoadUtils::isSignRelevant(curr->cast<Load>())) {
+          HASH(Load, signed_);
+        }
         HASH(Load, offset);
         HASH(Load, align);
         PUSH(Load, ptr);

src/ast/load-utils.h

@@ -0,0 +1,40 @@
+/*
+ * Copyright 2017 WebAssembly Community Group participants
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef wasm_ast_load_h
+#define wasm_ast_load_h
+
+#include "wasm.h"
+
+namespace wasm {
+
+namespace LoadUtils {
+
+// checks if the sign of a load matters, which is when an integer
+// load is of fewer bytes than the size of the type (so we must
+// fill in bits either signed or unsigned wise)
+inline bool isSignRelevant(Load* load) {
+  auto type = load->type;
+  if (load->type == unreachable) return false;
+  return !isWasmTypeFloat(type) && load->bytes < getWasmTypeSize(type);
+}
+
+} // namespace LoadUtils
+
+} // namespace wasm
+
+#endif // wasm_ast_load_h
+

src/passes/OptimizeInstructions.cpp

@@ -30,6 +30,7 @@
 #include <ast/manipulation.h>
 #include <ast/properties.h>
 #include <ast/literal-utils.h>
+#include <ast/load-utils.h>
 
 // TODO: Use the new sign-extension opcodes where appropriate. This needs to be conditionalized on the availability of atomics.
 
@@ -243,7 +244,8 @@ Index getMaxBits(Expression* curr, LocalInfoProvider* localInfoProvider) {
     return localInfoProvider->getMaxBitsForLocal(get);
   } else if (auto* load = curr->dynCast<Load>()) {
     // if signed, then the sign-extension might fill all the bits
-    if (!load->signed_) {
+    // if unsigned, then we have a limit
+    if (LoadUtils::isSignRelevant(load) && !load->signed_) {
       return 8 * load->bytes;
     }
   }
@@ -320,7 +322,7 @@ struct LocalScanner : PostWalker<LocalScanner> {
     if (Properties::getSignExtValue(value)) {
       signExtBits = Properties::getSignExtBits(value);
     } else if (auto* load = value->dynCast<Load>()) {
-      if (load->signed_) {
+      if (LoadUtils::isSignRelevant(load) && load->signed_) {
         signExtBits = load->bytes * 8;
       }
     }