Make typing-asm match spec more closely around load/store, add more tests.

Shifts of integer values are in some contexts collapsed by the parser into single literal AST nodes, rather than a direct representation of the parse tree. Confirming this behavior in tests.

Integer TypedArrays are assumed to load and store "intish" values rather than more fine-grained type information. Reducing the precision of the typing information to match the spec and simplify the wasm generator.

The asm spec requires load and store values of various "float?", "floatish", "double?" and "intish" types to ensure undefined values are not visible and that float32 rounding occurs at the right time. More closely matching this.

Adding additional testing around unsigned / signed comparisons, loads and stores.

Adding addition debug mode printing when asserting about types fail.

BUG= https://code.google.com/p/v8/issues/detail?id=4203
TEST=test-asm-validator, wasm side tests
R=titzer@chromium.org,aseemgarg@chromium.org
LOG=N

Review URL: https://codereview.chromium.org/1471073003

Cr-Commit-Position: refs/heads/master@{#32419}

src/type-cache.h

@@ -58,6 +58,9 @@ class TypeCache final {
   Type* const kSafeInteger = CreateRange(-kMaxSafeInteger, kMaxSafeInteger);
   Type* const kPositiveSafeInteger = CreateRange(0.0, kMaxSafeInteger);
 
+  Type* const kUntaggedUndefined =
+      Type::Intersect(Type::Undefined(), Type::Untagged(), zone());
+
   // Asm.js related types.
   Type* const kAsmSigned = kInt32;
   Type* const kAsmUnsigned = kUint32;
@@ -65,6 +68,12 @@ class TypeCache final {
   Type* const kAsmFixnum = Type::Intersect(kAsmSigned, kAsmUnsigned, zone());
   Type* const kAsmFloat = kFloat32;
   Type* const kAsmDouble = kFloat64;
+  Type* const kAsmFloatQ = Type::Union(kAsmFloat, kUntaggedUndefined, zone());
+  Type* const kAsmDoubleQ = Type::Union(kAsmDouble, kUntaggedUndefined, zone());
+  // Not part of the Asm.js type hierarchy, but represents a part of what
+  // intish encompasses.
+  Type* const kAsmIntQ = Type::Union(kAsmInt, kUntaggedUndefined, zone());
+  Type* const kAsmFloatDoubleQ = Type::Union(kAsmFloatQ, kAsmDoubleQ, zone());
   // Asm.js size unions.
   Type* const kAsmSize8 = Type::Union(kInt8, kUint8, zone());
   Type* const kAsmSize16 = Type::Union(kInt16, kUint16, zone());
@@ -77,6 +86,11 @@ class TypeCache final {
       Type::Union(kAsmUnsigned, Type::Union(kAsmDouble, kAsmFloat, zone()),
                   zone()),
       zone());
+  Type* const kAsmIntArrayElement =
+      Type::Union(Type::Union(kInt8, kUint8, zone()),
+                  Type::Union(Type::Union(kInt16, kUint16, zone()),
+                              Type::Union(kInt32, kUint32, zone()), zone()),
+                  zone());
 
   // The FixedArray::length property always containts a smi in the range
   // [0, FixedArray::kMaxLength].

src/typing-asm.cc

@@ -41,6 +41,7 @@ AsmTyper::AsmTyper(Isolate* isolate, Zone* zone, Script* script,
       script_(script),
       root_(root),
       valid_(true),
+      intish_(0),
       stdlib_types_(zone),
       stdlib_heap_types_(zone),
       stdlib_math_types_(zone),
@@ -155,7 +156,7 @@ void AsmTyper::VisitFunctionAnnotation(FunctionLiteral* fun) {
       if (literal) {
         RECURSE(VisitLiteral(literal, true));
       } else {
-        RECURSE(VisitExpressionAnnotation(stmt->expression(), true));
+        RECURSE(VisitExpressionAnnotation(stmt->expression(), NULL, true));
       }
       expected_type_ = old_expected;
       result_type = computed_type_;
@@ -179,7 +180,7 @@ void AsmTyper::VisitFunctionAnnotation(FunctionLiteral* fun) {
     Variable* var = proxy->var();
     if (var->location() != VariableLocation::PARAMETER || var->index() != i)
       break;
-    RECURSE(VisitExpressionAnnotation(expr->value(), false));
+    RECURSE(VisitExpressionAnnotation(expr->value(), var, false));
     SetType(var, computed_type_);
     type->InitParameter(i, computed_type_);
     good = true;
@@ -190,10 +191,20 @@ void AsmTyper::VisitFunctionAnnotation(FunctionLiteral* fun) {
 }
 
 
-void AsmTyper::VisitExpressionAnnotation(Expression* expr, bool is_return) {
+void AsmTyper::VisitExpressionAnnotation(Expression* expr, Variable* var,
+                                         bool is_return) {
   // Normal +x or x|0 annotations.
   BinaryOperation* bin = expr->AsBinaryOperation();
   if (bin != NULL) {
+    if (var != NULL) {
+      VariableProxy* left = bin->left()->AsVariableProxy();
+      if (!left) {
+        FAIL(expr, "variable name expected in type annotation");
+      }
+      if (left->var() != var) {
+        FAIL(left, "variable type annotation references other variable");
+      }
+    }
     Literal* right = bin->right()->AsLiteral();
     if (right != NULL) {
       switch (bin->op()) {
@@ -593,19 +604,23 @@ void AsmTyper::VisitAssignment(Assignment* expr) {
   Type* type = expected_type_;
   RECURSE(VisitWithExpectation(
       expr->value(), type, "assignment value expected to match surrounding"));
+  Type* target_type = StorageType(computed_type_);
   if (intish_ != 0) {
-    FAIL(expr, "value still an intish");
-  }
-  Type* target_type = computed_type_;
-  if (target_type->Is(cache_.kAsmInt)) {
-    target_type = cache_.kAsmInt;
-  }
-  RECURSE(VisitWithExpectation(expr->target(), target_type,
-                               "assignment target expected to match value"));
-  if (intish_ != 0) {
-    FAIL(expr, "value still an intish");
+    FAIL(expr, "intish or floatish assignment");
+  }
+  if (expr->target()->IsVariableProxy()) {
+    RECURSE(VisitWithExpectation(expr->target(), target_type,
+                                 "assignment target expected to match value"));
+  } else if (expr->target()->IsProperty()) {
+    Property* property = expr->target()->AsProperty();
+    RECURSE(VisitWithExpectation(property->obj(), Type::Any(),
+                                 "bad propety object"));
+    if (!computed_type_->IsArray()) {
+      FAIL(property->obj(), "array expected");
+    }
+    VisitHeapAccess(property, true, target_type);
   }
-  IntersectResult(expr, computed_type_);
+  IntersectResult(expr, target_type);
 }
 
 
@@ -637,11 +652,15 @@ Type* AsmTyper::StorageType(Type* type) {
 }
 
 
-void AsmTyper::VisitHeapAccess(Property* expr) {
+void AsmTyper::VisitHeapAccess(Property* expr, bool assigning,
+                               Type* assignment_type) {
   Type::ArrayType* array_type = computed_type_->AsArray();
   size_t size = array_size_;
   Type* type = array_type->AsArray()->Element();
   if (type->IsFunction()) {
+    if (assigning) {
+      FAIL(expr, "assigning to function table is illegal");
+    }
     BinaryOperation* bin = expr->key()->AsBinaryOperation();
     if (bin == NULL || bin->op() != Token::BIT_AND) {
       FAIL(expr->key(), "expected & in call");
@@ -658,6 +677,7 @@ void AsmTyper::VisitHeapAccess(Property* expr) {
       FAIL(right, "call mask must match function table");
     }
     bin->set_bounds(Bounds(cache_.kAsmSigned));
+    IntersectResult(expr, type);
   } else {
     Literal* literal = expr->key()->AsLiteral();
     if (literal) {
@@ -683,8 +703,39 @@ void AsmTyper::VisitHeapAccess(Property* expr) {
       }
       bin->set_bounds(Bounds(cache_.kAsmSigned));
     }
+    Type* result_type;
+    if (type->Is(cache_.kAsmIntArrayElement)) {
+      result_type = cache_.kAsmIntQ;
+      intish_ = kMaxUncombinedAdditiveSteps;
+    } else if (type->Is(cache_.kAsmFloat)) {
+      if (assigning) {
+        result_type = cache_.kAsmFloatDoubleQ;
+      } else {
+        result_type = cache_.kAsmFloatQ;
+      }
+      intish_ = 0;
+    } else if (type->Is(cache_.kAsmDouble)) {
+      if (assigning) {
+        result_type = cache_.kAsmFloatDoubleQ;
+        if (intish_ != 0) {
+          FAIL(expr, "Assignment of floatish to Float64Array");
+        }
+      } else {
+        result_type = cache_.kAsmDoubleQ;
+      }
+      intish_ = 0;
+    } else {
+      UNREACHABLE();
+    }
+    if (assigning) {
+      if (!assignment_type->Is(result_type)) {
+        FAIL(expr, "illegal type in assignment");
+      }
+    } else {
+      IntersectResult(expr, expected_type_);
+      IntersectResult(expr, result_type);
+    }
   }
-  IntersectResult(expr, type);
 }
 
 
@@ -720,12 +771,11 @@ void AsmTyper::VisitProperty(Property* expr) {
 
   // Only recurse at this point so that we avoid needing
   // stdlib.Math to have a real type.
-  RECURSE(VisitWithExpectation(expr->obj(), Type::Any(),
-                               "property holder expected to be object"));
+  RECURSE(VisitWithExpectation(expr->obj(), Type::Any(), "bad propety object"));
 
   // For heap view or function table access.
   if (computed_type_->IsArray()) {
-    VisitHeapAccess(expr);
+    VisitHeapAccess(expr, false, NULL);
     return;
   }
 
@@ -780,6 +830,7 @@ void AsmTyper::VisitCall(Call* expr) {
           arg, fun_type->Parameter(i),
           "call argument expected to match callee parameter"));
     }
+    intish_ = 0;
     IntersectResult(expr, fun_type->Result());
   } else if (computed_type_->Is(Type::Any())) {
     // For foreign calls.
@@ -789,6 +840,7 @@ void AsmTyper::VisitCall(Call* expr) {
       RECURSE(VisitWithExpectation(arg, Type::Any(),
                                    "foreign call argument expected to be any"));
     }
+    intish_ = kMaxUncombinedAdditiveSteps;
     IntersectResult(expr, Type::Number());
   } else {
     FAIL(expr, "invalid callee");
@@ -994,13 +1046,17 @@ void AsmTyper::VisitBinaryOperation(BinaryOperation* expr) {
           IntersectResult(expr, cache_.kAsmInt);
           return;
         }
-      } else if (expr->op() == Token::MUL && left_type->Is(cache_.kAsmInt) &&
+      } else if (expr->op() == Token::MUL && expr->right()->IsLiteral() &&
                  right_type->Is(cache_.kAsmDouble)) {
         // For unary +, expressed as x * 1.0
         IntersectResult(expr, cache_.kAsmDouble);
         return;
       } else if (type->Is(cache_.kAsmFloat) && expr->op() != Token::MOD) {
+        if (left_intish != 0 || right_intish != 0) {
+          FAIL(expr, "float operation before required fround");
+        }
         IntersectResult(expr, cache_.kAsmFloat);
+        intish_ = 1;
         return;
       } else if (type->Is(cache_.kAsmDouble)) {
         IntersectResult(expr, cache_.kAsmDouble);

src/typing-asm.h

@@ -65,11 +65,11 @@ class AsmTyper : public AstVisitor {
   void VisitDeclarations(ZoneList<Declaration*>* d) override;
   void VisitStatements(ZoneList<Statement*>* s) override;
 
-  void VisitExpressionAnnotation(Expression* e, bool is_return);
+  void VisitExpressionAnnotation(Expression* e, Variable* var, bool is_return);
   void VisitFunctionAnnotation(FunctionLiteral* f);
   void VisitAsmModule(FunctionLiteral* f);
 
-  void VisitHeapAccess(Property* expr);
+  void VisitHeapAccess(Property* expr, bool assigning, Type* assignment_type);
 
   int ElementShiftSize(Type* type);
   Type* StorageType(Type* type);

test/cctest/expression-type-collector-macros.h

@@ -14,11 +14,30 @@
   CHECK_EQ(index, types.size()); \
   }
 
+#ifdef DEBUG
+#define CHECK_TYPE(type)                    \
+  if (!types[index].bounds.Narrows(type)) { \
+    fprintf(stderr, "Expected:\n");         \
+    fprintf(stderr, "  lower: ");           \
+    type.lower->Print();                    \
+    fprintf(stderr, "  upper: ");           \
+    type.upper->Print();                    \
+    fprintf(stderr, "Actual:\n");           \
+    fprintf(stderr, "  lower: ");           \
+    types[index].bounds.lower->Print();     \
+    fprintf(stderr, "  upper: ");           \
+    types[index].bounds.upper->Print();     \
+  }                                         \
+  CHECK(types[index].bounds.Narrows(type));
+#else
+#define CHECK_TYPE(type) CHECK(types[index].bounds.Narrows(type));
+#endif
+
 #define CHECK_EXPR(ekind, type)                  \
   CHECK_LT(index, types.size());                 \
   CHECK(strcmp(#ekind, types[index].kind) == 0); \
   CHECK_EQ(depth, types[index].depth);           \
-  CHECK(types[index].bounds.Narrows(type));      \
+  CHECK_TYPE(type);                              \
   for (int j = (++depth, ++index, 0); j < 1 ? 1 : (--depth, 0); ++j)
 
 #define CHECK_VAR(vname, type)                                     \