Fix Subzero binary instruction to allow optional flags argument.

unittest/IceParseInstsTest.cpp

Index: unittest/IceParseInstsTest.cpp
diff --git a/unittest/IceParseInstsTest.cpp b/unittest/IceParseInstsTest.cpp
index 8b452d3e2b3480d60b31b7166cfb39e8cad86ad8..0c5d57ce001b3c5519858bb806c3b46279fc620a 100644
--- a/unittest/IceParseInstsTest.cpp
+++ b/unittest/IceParseInstsTest.cpp
@@ -401,4 +401,66 @@ TEST(NaClParseInstsTests, StoreAlignment) {
       DumpMunger.getLinesWithSubstring("store"));
 }
 
+// Test that we handle the optional FLAGs argument to binary instructions.

[Jim Stichnoth, 2016/03/24 17:46:32]

Would it be possible to (in addition or instead) have a lit test, in the style
of the parse_errs *.tbc files?

[Karl, 2016/03/24 19:28:36]

Replacing with tbc test.

+TEST(IceParseInstsTest, IgnoredBinopFlags) {
+  const uint64_t BitcodeRecords[] = {
+    naclbitc::ENTER_SUBBLOCK, naclbitc::BLK_CODE_ENTER,
+      naclbitc::MODULE_BLOCK_ID, 2, Terminator,
+    naclbitc::ENTER_SUBBLOCK, naclbitc::BLK_CODE_ENTER,
+      naclbitc::TYPE_BLOCK_ID_NEW, 2, Terminator,
+    naclbitc::UNABBREV_RECORD, naclbitc::TYPE_CODE_NUMENTRY, 3, Terminator,
+    naclbitc::UNABBREV_RECORD, naclbitc::TYPE_CODE_INTEGER, 32, Terminator,
+    naclbitc::UNABBREV_RECORD, naclbitc::TYPE_CODE_FUNCTION,
+      0, 0, 0, 0, Terminator,
+    naclbitc::UNABBREV_RECORD, naclbitc::TYPE_CODE_VOID, Terminator,
+    naclbitc::END_BLOCK, naclbitc::BLK_CODE_EXIT, Terminator,
+    naclbitc::UNABBREV_RECORD, naclbitc::MODULE_CODE_FUNCTION,
+      1, 0, 0, 3, Terminator,
+    naclbitc::ENTER_SUBBLOCK, naclbitc::BLK_CODE_ENTER,
+      naclbitc::FUNCTION_BLOCK_ID, 2, Terminator,
+    naclbitc::UNABBREV_RECORD, naclbitc::FUNC_CODE_DECLAREBLOCKS, 1, Terminator,
+    naclbitc::UNABBREV_RECORD, naclbitc::FUNC_CODE_INST_BINOP,
+      2, 1, 0, Terminator,
+    naclbitc::UNABBREV_RECORD, naclbitc::FUNC_CODE_INST_RET, 1, Terminator,
+    naclbitc::END_BLOCK, naclbitc::BLK_CODE_EXIT, Terminator,
+    naclbitc::END_BLOCK, naclbitc::BLK_CODE_EXIT, Terminator
+  };
+
+  // Show bitcode objdump for BitcodeRecorsd.

[Jim Stichnoth, 2016/03/24 17:46:32]

BitcodeRecords

[Karl, 2016/03/24 19:28:36]

Ignoring, replacing with tbc test

+  NaClObjDumpMunger DumpMunger(ARRAY_TERM(BitcodeRecords));
+  EXPECT_TRUE(DumpMunger.runTestForAssembly());
+  EXPECT_EQ(
+      "module {  // BlockID = 8\n"
+      "  types {  // BlockID = 17\n"
+      "    count 3;\n"
+      "    @t0 = i32;\n"
+      "    @t1 = i32 (i32, i32);\n"
+      "    @t2 = void;\n"
+      "  }\n"
+      "  define internal i32 @f0(i32, i32);\n"
+      "  function i32 @f0(i32 %p0, i32 %p1) {  // BlockID = 12\n"
+      "    blocks 1;\n"
+      "  %b0:\n"
+      "    %v0 = add i32 %p0, %p1;\n"
+      "    ret i32 %v0;\n"
+      "  }\n"
+      "}\n",
+      DumpMunger.getTestResults());
+
+  // Show that Subzero parses the input.
+  IceTest::SubzeroBitcodeMunger Munger(ARRAY_TERM(BitcodeRecords));
+  EXPECT_TRUE(Munger.runTest());
+
+  // Show that adding a 4th "flags" field to the BINOP instruction is
+  // accepted.
+  const uint64_t ReplaceIndex = 10;

[Jim Stichnoth, 2016/03/24 17:46:32]

These could be constexpr instead of const... but then you'd probably want to
make the same change to all the ReplaceIndex above for consistency... so your
call.

[Karl, 2016/03/24 19:28:36]

Ignoring, replacing with tbc test.

+  const uint64_t FLAGS = 5981; // Value ignored.
+  const uint64_t Edit[] = {
+    ReplaceIndex, NaClMungedBitcode::Replace,
+    naclbitc::UNABBREV_RECORD, naclbitc::FUNC_CODE_INST_BINOP,
+      2, 1, 0, FLAGS, Terminator
+  };
+  EXPECT_TRUE(Munger.runTest(ARRAY(Edit)));
+}
+
 } // end of anonymous namespace