[Mlir-commits] [mlir] 54cdc03 - [mlir:Parser] Always splice parsed operations to the end of the parsed block

[River Riddle]

Author: River Riddle
Date: 2022-10-05T13:11:38-07:00
New Revision: 54cdc03dfadfe35a6a98a5970a5b850868176642

URL: https://github.com/llvm/llvm-project/commit/54cdc03dfadfe35a6a98a5970a5b850868176642
DIFF: https://github.com/llvm/llvm-project/commit/54cdc03dfadfe35a6a98a5970a5b850868176642.diff

LOG: [mlir:Parser] Always splice parsed operations to the end of the parsed block

The current splicing behavior dates back to when all blocks had terminators,
so we would "helpfully" splice before the terminator. This doesn't make sense
anymore, and leads to somewhat unexpected results when parsing multiple
pieces of IR into the same block.

Differential Revision: https://reviews.llvm.org/D135096

Added: 
    

Modified: 
    mlir/lib/AsmParser/Parser.cpp
    mlir/lib/Bytecode/Reader/BytecodeReader.cpp
    mlir/unittests/Parser/ParserTest.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/lib/AsmParser/Parser.cpp b/mlir/lib/AsmParser/Parser.cpp
index e4da76a79546b..b92d27b37dc2a 100644
--- a/mlir/lib/AsmParser/Parser.cpp
+++ b/mlir/lib/AsmParser/Parser.cpp
@@ -2593,8 +2593,8 @@ ParseResult TopLevelOperationParser::parse(Block *topLevelBlock,
       // top-level block.
       auto &parsedOps = topLevelOp->getBody()->getOperations();
       auto &destOps = topLevelBlock->getOperations();
-      destOps.splice(destOps.empty() ? destOps.end() : std::prev(destOps.end()),
-                     parsedOps, parsedOps.begin(), parsedOps.end());
+      destOps.splice(destOps.end(), parsedOps, parsedOps.begin(),
+                     parsedOps.end());
       return success();
     }
 

diff  --git a/mlir/lib/Bytecode/Reader/BytecodeReader.cpp b/mlir/lib/Bytecode/Reader/BytecodeReader.cpp
index d61a8acc1b676..8224729b6bd13 100644
--- a/mlir/lib/Bytecode/Reader/BytecodeReader.cpp
+++ b/mlir/lib/Bytecode/Reader/BytecodeReader.cpp
@@ -1414,8 +1414,7 @@ LogicalResult BytecodeReader::parseIRSection(ArrayRef<uint8_t> sectionData,
   // Splice the parsed operations over to the provided top-level block.
   auto &parsedOps = moduleOp->getBody()->getOperations();
   auto &destOps = block->getOperations();
-  destOps.splice(destOps.empty() ? destOps.end() : std::prev(destOps.end()),
-                 parsedOps, parsedOps.begin(), parsedOps.end());
+  destOps.splice(destOps.end(), parsedOps, parsedOps.begin(), parsedOps.end());
   return success();
 }
 

diff  --git a/mlir/unittests/Parser/ParserTest.cpp b/mlir/unittests/Parser/ParserTest.cpp
index df78174efc996..ef951102cc121 100644
--- a/mlir/unittests/Parser/ParserTest.cpp
+++ b/mlir/unittests/Parser/ParserTest.cpp
@@ -28,4 +28,31 @@ TEST(MLIRParser, ParseInvalidIR) {
   ASSERT_TRUE(module);
   ASSERT_TRUE(failed(verify(*module)));
 }
+
+TEST(MLIRParser, ParseAtEnd) {
+  std::string firstModuleStr = R"mlir(
+    "test.first"() : () -> ()
+  )mlir";
+  std::string secondModuleStr = R"mlir(
+    "test.second"() : () -> ()
+  )mlir";
+
+  MLIRContext context;
+  context.allowUnregisteredDialects();
+  Block block;
+
+  // Parse the first module string.
+  LogicalResult firstParse =
+      parseSourceString(firstModuleStr, &block, &context);
+  EXPECT_TRUE(succeeded(firstParse));
+
+  // Parse the second module string.
+  LogicalResult secondParse =
+      parseSourceString(secondModuleStr, &block, &context);
+  EXPECT_TRUE(succeeded(secondParse));
+
+  // Check the we parse at the end.
+  EXPECT_EQ(block.front().getName().getStringRef(), "test.first");
+  EXPECT_EQ(block.back().getName().getStringRef(), "test.second");
+}
 } // namespace