[Interpreter] Add support for loading literals from the constant pool.

test/cctest/interpreter/test-bytecode-generator.cc

Index: test/cctest/interpreter/test-bytecode-generator.cc
diff --git a/test/cctest/interpreter/test-bytecode-generator.cc b/test/cctest/interpreter/test-bytecode-generator.cc
index 16dc3eea2b16bfd54434d083ec5bbff5c3bfbf5a..a9eb58aa3aa84c300a3c4d47ac57251bf7f4c7bf 100644
--- a/test/cctest/interpreter/test-bytecode-generator.cc
+++ b/test/cctest/interpreter/test-bytecode-generator.cc
@@ -50,12 +50,15 @@ class BytecodeGeneratorHelper {
 
 
 // Structure for containing expected bytecode snippets.
+template<typename T>
 struct ExpectedSnippet {
   const char* body;
   int frame_size;
   int parameter_count;
   int bytecode_length;
   const uint8_t bytecode[16];
+  int constant_count;
+  T constants[16];
 };
 
 
@@ -69,16 +72,16 @@ TEST(PrimitiveReturnStatements) {
   InitializedHandleScope handle_scope;
   BytecodeGeneratorHelper helper;
 
-  ExpectedSnippet snippets[] = {
-      {"return;", 0, 1, 2, {B(LdaUndefined), B(Return)}},
-      {"return null;", 0, 1, 2, {B(LdaNull), B(Return)}},
-      {"return true;", 0, 1, 2, {B(LdaTrue), B(Return)}},
-      {"return false;", 0, 1, 2, {B(LdaFalse), B(Return)}},
-      {"return 0;", 0, 1, 2, {B(LdaZero), B(Return)}},
-      {"return +1;", 0, 1, 3, {B(LdaSmi8), U8(1), B(Return)}},
-      {"return -1;", 0, 1, 3, {B(LdaSmi8), U8(-1), B(Return)}},
-      {"return +127;", 0, 1, 3, {B(LdaSmi8), U8(127), B(Return)}},
-      {"return -128;", 0, 1, 3, {B(LdaSmi8), U8(-128), B(Return)}},
+  ExpectedSnippet<void*> snippets[] = {
+      {"return;", 0, 1, 2, {B(LdaUndefined), B(Return)}, 0},
+      {"return null;", 0, 1, 2, {B(LdaNull), B(Return)}, 0},
+      {"return true;", 0, 1, 2, {B(LdaTrue), B(Return)}, 0},
+      {"return false;", 0, 1, 2, {B(LdaFalse), B(Return)}, 0},
+      {"return 0;", 0, 1, 2, {B(LdaZero), B(Return)}, 0},
+      {"return +1;", 0, 1, 3, {B(LdaSmi8), U8(1), B(Return)}, 0},
+      {"return -1;", 0, 1, 3, {B(LdaSmi8), U8(-1), B(Return)}, 0},
+      {"return +127;", 0, 1, 3, {B(LdaSmi8), U8(127), B(Return)}, 0},
+      {"return -128;", 0, 1, 3, {B(LdaSmi8), U8(-128), B(Return)}, 0},
   };
 
   size_t num_snippets = sizeof(snippets) / sizeof(snippets[0]);
@@ -90,6 +93,7 @@ TEST(PrimitiveReturnStatements) {
     CHECK_EQ(ba->length(), snippets[i].bytecode_length);
     CHECK(!memcmp(ba->GetFirstBytecodeAddress(), snippets[i].bytecode,
                   ba->length()));
+    CHECK_EQ(ba->constant_pool(), CcTest::heap()->empty_fixed_array());
   }
 }
 
@@ -98,7 +102,7 @@ TEST(PrimitiveExpressions) {
   InitializedHandleScope handle_scope;
   BytecodeGeneratorHelper helper;
 
-  ExpectedSnippet snippets[] = {
+  ExpectedSnippet<void*> snippets[] = {
       {"var x = 0; return x;",
        kPointerSize,
        1,
@@ -108,7 +112,9 @@ TEST(PrimitiveExpressions) {
            B(Star), R(0),  //
            B(Ldar), R(0),  //
            B(Return)       //
-       }},
+       },
+       0
+      },
       {"var x = 0; return x + 3;",
        2 * kPointerSize,
        1,
@@ -121,7 +127,9 @@ TEST(PrimitiveExpressions) {
            B(LdaSmi8), U8(3),  //
            B(Add), R(1),       //
            B(Return)           //
-       }}};
+       },
+       0
+     }};
 
   size_t num_snippets = sizeof(snippets) / sizeof(snippets[0]);
   for (size_t i = 0; i < num_snippets; i++) {
@@ -132,6 +140,7 @@ TEST(PrimitiveExpressions) {
     CHECK_EQ(ba->length(), snippets[i].bytecode_length);
     CHECK(!memcmp(ba->GetFirstBytecodeAddress(), snippets[i].bytecode,
                   ba->length()));
+    CHECK_EQ(ba->constant_pool(), CcTest::heap()->empty_fixed_array());
   }
 }
 
@@ -142,17 +151,17 @@ TEST(Parameters) {
 
   int last_param_index =
       -InterpreterFrameConstants::kLastParamFromRegisterPointer / kPointerSize;
-  ExpectedSnippet snippets[] = {
+  ExpectedSnippet<void*> snippets[] = {
       {"function f() { return this; }",
-       0, 1, 3, {B(Ldar), R(last_param_index), B(Return)}},
+       0, 1, 3, {B(Ldar), R(last_param_index), B(Return)}, 0},
       {"function f(arg1) { return arg1; }",
-       0, 2, 3, {B(Ldar), R(last_param_index), B(Return)}},
+       0, 2, 3, {B(Ldar), R(last_param_index), B(Return)}, 0},
       {"function f(arg1) { return this; }",
-       0, 2, 3, {B(Ldar), R(last_param_index - 1), B(Return)}},
+       0, 2, 3, {B(Ldar), R(last_param_index - 1), B(Return)}, 0},
       {"function f(arg1, arg2, arg3, arg4, arg5, arg6, arg7) { return arg4; }",
-       0, 8, 3, {B(Ldar), R(last_param_index - 3), B(Return)}},
+       0, 8, 3, {B(Ldar), R(last_param_index - 3), B(Return)}, 0},
       {"function f(arg1, arg2, arg3, arg4, arg5, arg6, arg7) { return this; }",
-       0, 8, 3, {B(Ldar), R(last_param_index - 7), B(Return)}}
+       0, 8, 3, {B(Ldar), R(last_param_index - 7), B(Return)}, 0}
   };
 
   size_t num_snippets = sizeof(snippets) / sizeof(snippets[0]);
@@ -163,9 +172,160 @@ TEST(Parameters) {
     CHECK_EQ(ba->length(), snippets[i].bytecode_length);
     CHECK(!memcmp(ba->GetFirstBytecodeAddress(), snippets[i].bytecode,
                   ba->length()));
+    CHECK_EQ(ba->constant_pool(), CcTest::heap()->empty_fixed_array());
   }
 }
 
+
+TEST(Constants) {
+  InitializedHandleScope handle_scope;
+  BytecodeGeneratorHelper helper;
+
+  // Check large SMIs.
+  {
+    ExpectedSnippet<int> snippets[] = {
+        {"return 12345678;", 0, 1, 3,
+         {
+            B(LdaConstant), U8(0),
+            B(Return)
+         }, 1, { 12345678 }
+        },
+        {"var a = 1234; return 5678;", 1 * kPointerSize, 1, 7,
+         {
+            B(LdaConstant), U8(0),
+            B(Star), R(0),
+            B(LdaConstant), U8(1),
+            B(Return)
+         }, 2, { 1234, 5678 }
+        },
+        {"var a = 1234; return 1234;",
+         1 * kPointerSize, 1, 7,
+         {
+            B(LdaConstant), U8(0),
+            B(Star), R(0),
+            B(LdaConstant), U8(0),
+            B(Return)
+         }, 1, { 1234 }
+        }
+    };
+
+    size_t num_snippets = sizeof(snippets) / sizeof(snippets[0]);
+    for (size_t i = 0; i < num_snippets; i++) {
+      Handle<BytecodeArray> ba =
+          helper.MakeBytecodeForFunctionBody(snippets[i].body);
+      CHECK_EQ(ba->frame_size(), snippets[i].frame_size);
+      CHECK_EQ(ba->parameter_count(), snippets[i].parameter_count);
+      CHECK_EQ(ba->length(), snippets[i].bytecode_length);
+      CHECK(!memcmp(ba->GetFirstBytecodeAddress(), snippets[i].bytecode,
+                    ba->length()));
+      CHECK_EQ(ba->constant_pool()->length(), snippets[i].constant_count);
+      for (int j = 0; j < snippets[i].constant_count; j++) {
+        CHECK_EQ(Smi::cast(ba->constant_pool()->get(j))->value(),
+                 snippets[i].constants[j]);
+      }
+    }
+  }
+
+  // Check heap number double constants
+  {
+    ExpectedSnippet<double> snippets[] = {
+        {"return 1.2;",
+         0, 1, 3,
+         {
+            B(LdaConstant), U8(0),
+            B(Return)
+         }, 1, { 1.2 }
+        },
+        {"var a = 1.2; return 2.6;", 1 * kPointerSize, 1, 7,
+         {
+            B(LdaConstant), U8(0),
+            B(Star), R(0),
+            B(LdaConstant), U8(1),
+            B(Return)
+         }, 2, { 1.2, 2.6 }
+        },
+        {"var a = 3.14; return 3.14;", 1 * kPointerSize, 1, 7,
+         {
+            B(LdaConstant), U8(0),
+            B(Star), R(0),
+            B(LdaConstant), U8(1),
+            B(Return)
+         }, 2,
+         // TODO(rmcilroy): Currently multiple identical double literals end up
+         // being allocated as new HeapNumbers and so require multiple constant
+         // pool entries. De-dup identical values.
+         { 3.14, 3.14 }

[rmcilroy, 2015/08/27 16:34:07]

Michi - looks like double literals aren't de-duped by the parser (we get a new
HeapNumber each time). Is this done elsewhere or have we always had a new
HeapNumber for each double literal?

[Michael Starzinger, 2015/08/27 19:36:29]

Not sure I understand your comment correctly. How would the parser de-dupe
literals? That would make the AST degenerate into a DAG if we would do that. Or
am I missing something here?

Compilers can de-dupe the values. One mechanism of de-duping would be GVN which
makes sure that identical nodes in the graph are coalesced. Both Hydrogen and
TurboFan do that.

[Michael Starzinger, 2015/08/27 19:55:36]

Ah, I think now I understand where you are coming from, I think your question
was whether AstValue::Internalize should canonicalize HeapNumbers or not. Maeh,
the data-structure for canonicalizing them would probably end up eating more
memory than the duped HeapNumbers, I don't think that would buy us anything
besides complexity.

Rather the question is, why do we "internalize" double literals in the first
place. One could argue that it should be the responsibility of the compiler to
decide whether and where to allocate HeapNumbers. But that's opening a can of
worms. I suggest we don't go there.

+        }
+    };
+
+    size_t num_snippets = sizeof(snippets) / sizeof(snippets[0]);
+    for (size_t i = 0; i < num_snippets; i++) {
+      Handle<BytecodeArray> ba =
+          helper.MakeBytecodeForFunctionBody(snippets[i].body);
+      CHECK_EQ(ba->frame_size(), snippets[i].frame_size);
+      CHECK_EQ(ba->parameter_count(), snippets[i].parameter_count);
+      CHECK_EQ(ba->length(), snippets[i].bytecode_length);
+      CHECK(!memcmp(ba->GetFirstBytecodeAddress(), snippets[i].bytecode,
+                    ba->length()));
+      CHECK_EQ(ba->constant_pool()->length(), snippets[i].constant_count);
+      for (int j = 0; j < snippets[i].constant_count; j++) {
+        CHECK_EQ(HeapNumber::cast(ba->constant_pool()->get(j))->value(),
+                 snippets[i].constants[j]);
+      }
+    }
+  }
+
+  // Check string literals
+  {
+    ExpectedSnippet<const char*> snippets[] = {
+        {"return \"This is a string\";", 0, 1, 3,
+         {
+            B(LdaConstant), U8(0),
+            B(Return)
+         }, 1,
+         { "This is a string" }
+        },
+        {"var a = \"First string\"; return \"Second string\";",
+         1 * kPointerSize, 1, 7,
+         {
+            B(LdaConstant), U8(0),
+            B(Star), R(0),
+            B(LdaConstant), U8(1),
+            B(Return)
+         }, 2, { "First string", "Second string"}
+        },
+        {"var a = \"Same string\"; return \"Same string\";",
+         1 * kPointerSize, 1, 7,
+         {
+            B(LdaConstant), U8(0),
+            B(Star), R(0),
+            B(LdaConstant), U8(0),
+            B(Return)
+         }, 1, { "Same string" }
+        }
+    };
+
+    size_t num_snippets = sizeof(snippets) / sizeof(snippets[0]);
+    for (size_t i = 0; i < num_snippets; i++) {
+      Handle<BytecodeArray> ba =
+          helper.MakeBytecodeForFunctionBody(snippets[i].body);
+      CHECK_EQ(ba->frame_size(), snippets[i].frame_size);
+      CHECK_EQ(ba->parameter_count(), snippets[i].parameter_count);
+      CHECK_EQ(ba->length(), snippets[i].bytecode_length);
+      CHECK(!memcmp(ba->GetFirstBytecodeAddress(), snippets[i].bytecode,
+                    ba->length()));
+      CHECK_EQ(ba->constant_pool()->length(), snippets[i].constant_count);
+      for (int j = 0; j < snippets[i].constant_count; j++) {
+        Handle<String> expected =
+            CcTest::i_isolate()->factory()->NewStringFromAsciiChecked(
+                snippets[i].constants[j]);
+        CHECK(String::cast(ba->constant_pool()->get(j))->Equals(*expected));
+      }
+    }
+  }
+}
+
+
 }  // namespace interpreter
 }  // namespace internal
 }  // namespance v8