[Mlir-commits] [mlir] ffc80de - [mlir] Nits on uses of llvm::raw_string_ostream (NFC)

llvmlistbot at llvm.org llvmlistbot at llvm.org
Sun Sep 15 18:08:13 PDT 2024


Author: JOE1994
Date: 2024-09-15T21:06:32-04:00
New Revision: ffc80de8643969ffa0dbbd377c5b33e3a7488f5e

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

LOG: [mlir] Nits on uses of llvm::raw_string_ostream (NFC)

* Don't call raw_string_ostream::flush(), which is essentially a no-op
* Strip unneeded calls to raw_string_ostream::str(), to avoid excess indirection.

Added: 
    

Modified: 
    mlir/tools/mlir-linalg-ods-gen/mlir-linalg-ods-yaml-gen.cpp
    mlir/tools/mlir-pdll/mlir-pdll.cpp
    mlir/tools/mlir-src-sharder/mlir-src-sharder.cpp
    mlir/tools/mlir-tblgen/OpDocGen.cpp
    mlir/unittests/Bytecode/BytecodeTest.cpp
    mlir/unittests/IR/AttributeTest.cpp
    mlir/unittests/IR/OpPropertiesTest.cpp
    mlir/unittests/Support/IndentedOstreamTest.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/tools/mlir-linalg-ods-gen/mlir-linalg-ods-yaml-gen.cpp b/mlir/tools/mlir-linalg-ods-gen/mlir-linalg-ods-yaml-gen.cpp
index 7d42c03469dc98..aa5a52a21f1251 100644
--- a/mlir/tools/mlir-linalg-ods-gen/mlir-linalg-ods-yaml-gen.cpp
+++ b/mlir/tools/mlir-linalg-ods-gen/mlir-linalg-ods-yaml-gen.cpp
@@ -378,7 +378,6 @@ static std::string generateCppExpression(SerializedAffineMap self,
   std::string printedStr;
   llvm::raw_string_ostream printedSs(printedStr);
   self.affineMapAttr.print(printedSs);
-  printedSs.flush();
 
   static const char exprFormat[] =
       R"FMT(llvm::cast<AffineMapAttr>(mlir::parseAttribute("{0}", {1})).getValue())FMT";
@@ -391,7 +390,6 @@ static std::string interleaveToString(Container &container,
   std::string result;
   llvm::raw_string_ostream ss(result);
   llvm::interleave(container, ss, separator);
-  ss.flush();
   return result;
 }
 
@@ -827,7 +825,6 @@ generateNamedGenericOpDefns(LinalgOpConfig &opConfig,
                               break;
                             }
                           });
-    ss.flush();
     os << llvm::formatv(structuredOpIteratorTypesFormat, className,
                         iteratorsStr);
   } else {
@@ -892,7 +889,6 @@ exprs.push_back(getAffineConstantExpr(cst{1}, context));
         std::string symbolBindingsStr;
         llvm::raw_string_ostream symbolBindingsSs(symbolBindingsStr);
         llvm::interleave(symbolBindings, symbolBindingsSs, "\n");
-        symbolBindingsSs.flush();
 
         os << llvm::formatv(structuredOpSymbolBindingsFormat, className,
                             symbolBindingsStr);
@@ -913,7 +909,6 @@ exprs.push_back(getAffineConstantExpr(cst{1}, context));
         llvm::raw_string_ostream dimIdentsSs(dimIdentsStr);
         llvm::interleaveComma(dimIndices, dimIdentsSs,
                               [&](unsigned i) { dimIdentsSs << "d" << i; });
-        dimIdentsSs.flush();
 
         // Statements to add and simplify each affine map.
         SmallVector<std::string> stmts;

diff  --git a/mlir/tools/mlir-pdll/mlir-pdll.cpp b/mlir/tools/mlir-pdll/mlir-pdll.cpp
index c6ad6c361e9946..0fcf8d1b12d60f 100644
--- a/mlir/tools/mlir-pdll/mlir-pdll.cpp
+++ b/mlir/tools/mlir-pdll/mlir-pdll.cpp
@@ -207,7 +207,7 @@ int main(int argc, char **argv) {
     // any.
     if (auto existingOrErr =
             llvm::MemoryBuffer::getFile(outputFilename, /*IsText=*/true))
-      if (std::move(existingOrErr.get())->getBuffer() == outputStrOS.str())
+      if (std::move(existingOrErr.get())->getBuffer() == outputStr)
         shouldWriteOutput = false;
   }
 
@@ -219,7 +219,7 @@ int main(int argc, char **argv) {
       llvm::errs() << errorMessage << "\n";
       return 1;
     }
-    outputFile->os() << outputStrOS.str();
+    outputFile->os() << outputStr;
     outputFile->keep();
   }
 

diff  --git a/mlir/tools/mlir-src-sharder/mlir-src-sharder.cpp b/mlir/tools/mlir-src-sharder/mlir-src-sharder.cpp
index 2f92ff2b26c150..9ec9b721502076 100644
--- a/mlir/tools/mlir-src-sharder/mlir-src-sharder.cpp
+++ b/mlir/tools/mlir-src-sharder/mlir-src-sharder.cpp
@@ -98,7 +98,7 @@ int main(int argc, char **argv) {
     // any.
     if (auto existingOrErr =
             llvm::MemoryBuffer::getFile(outputFilename, /*IsText=*/true))
-      if (std::move(existingOrErr.get())->getBuffer() == os.str())
+      if (std::move(existingOrErr.get())->getBuffer() == outputStr)
         shouldWriteOutput = false;
   }
 

diff  --git a/mlir/tools/mlir-tblgen/OpDocGen.cpp b/mlir/tools/mlir-tblgen/OpDocGen.cpp
index d60eda0a8b958b..bf759572d25013 100644
--- a/mlir/tools/mlir-tblgen/OpDocGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpDocGen.cpp
@@ -162,7 +162,7 @@ static void emitOpTraitsDoc(const Operator &op, raw_ostream &os) {
           os << effect << " on " << rec->getValueAsString("resource");
         });
         os << "}";
-        effects.insert(backticks(os.str()));
+        effects.insert(backticks(effectStr));
         name.append(llvm::formatv(" ({0})", traitName).str());
       }
       interfaces.insert(backticks(name));
@@ -433,7 +433,7 @@ static void maybeNest(bool nest, llvm::function_ref<void(raw_ostream &os)> fn,
   std::string str;
   llvm::raw_string_ostream ss(str);
   fn(ss);
-  for (StringRef x : llvm::split(ss.str(), "\n")) {
+  for (StringRef x : llvm::split(str, "\n")) {
     if (nest && x.starts_with("#"))
       os << "#";
     os << x << "\n";

diff  --git a/mlir/unittests/Bytecode/BytecodeTest.cpp b/mlir/unittests/Bytecode/BytecodeTest.cpp
index a37a2afc226453..0342f294f38d6d 100644
--- a/mlir/unittests/Bytecode/BytecodeTest.cpp
+++ b/mlir/unittests/Bytecode/BytecodeTest.cpp
@@ -49,7 +49,6 @@ TEST(Bytecode, MultiModuleWithResource) {
   std::string buffer;
   llvm::raw_string_ostream ostream(buffer);
   ASSERT_TRUE(succeeded(writeBytecodeToFile(module.get(), ostream)));
-  ostream.flush();
 
   // Create copy of buffer which is aligned to requested resource alignment.
   constexpr size_t kAlignment = 0x20;
@@ -139,7 +138,7 @@ TEST(Bytecode, OpWithoutProperties) {
   ASSERT_TRUE(succeeded(writeBytecodeToFile(op.get(), os)));
   std::unique_ptr<Block> block = std::make_unique<Block>();
   ASSERT_TRUE(succeeded(readBytecodeFile(
-      llvm::MemoryBufferRef(os.str(), "string-buffer"), block.get(), config)));
+      llvm::MemoryBufferRef(bytecode, "string-buffer"), block.get(), config)));
   Operation *roundtripped = &block->front();
   EXPECT_EQ(roundtripped->getAttrs().size(), 2u);
   EXPECT_TRUE(roundtripped->getInherentAttr("inherent_attr") != std::nullopt);

diff  --git a/mlir/unittests/IR/AttributeTest.cpp b/mlir/unittests/IR/AttributeTest.cpp
index e72bfe9d82e7cf..981e919289c306 100644
--- a/mlir/unittests/IR/AttributeTest.cpp
+++ b/mlir/unittests/IR/AttributeTest.cpp
@@ -498,7 +498,7 @@ TEST(CopyCountAttr, PrintStripped) {
   os << "|" << res << "|";
   res.printStripped(os << "[");
   os << "]";
-  EXPECT_EQ(os.str(), "|#test.copy_count<hello>|[copy_count<hello>]");
+  EXPECT_EQ(str, "|#test.copy_count<hello>|[copy_count<hello>]");
 }
 
 } // namespace

diff  --git a/mlir/unittests/IR/OpPropertiesTest.cpp b/mlir/unittests/IR/OpPropertiesTest.cpp
index 365775d541ec3d..b4a633a2c62e68 100644
--- a/mlir/unittests/IR/OpPropertiesTest.cpp
+++ b/mlir/unittests/IR/OpPropertiesTest.cpp
@@ -191,7 +191,7 @@ TEST(OpPropertiesTest, Properties) {
                  "array = array<i64: 40, 41>, "
                  "b = -4.200000e+01 : f32, "
                  "label = \"bar foo\"}> : () -> ()\n",
-                 os.str().c_str());
+                 output.c_str());
   }
   // Get a mutable reference to the properties for this operation and modify it
   // in place one member at a time.
@@ -201,40 +201,44 @@ TEST(OpPropertiesTest, Properties) {
     std::string output;
     llvm::raw_string_ostream os(output);
     opWithProp.print(os);
-    EXPECT_TRUE(StringRef(os.str()).contains("a = 42"));
-    EXPECT_TRUE(StringRef(os.str()).contains("b = -4.200000e+01"));
-    EXPECT_TRUE(StringRef(os.str()).contains("array = array<i64: 40, 41>"));
-    EXPECT_TRUE(StringRef(os.str()).contains("label = \"bar foo\""));
+    StringRef view(output);
+    EXPECT_TRUE(view.contains("a = 42"));
+    EXPECT_TRUE(view.contains("b = -4.200000e+01"));
+    EXPECT_TRUE(view.contains("array = array<i64: 40, 41>"));
+    EXPECT_TRUE(view.contains("label = \"bar foo\""));
   }
   prop.b = 42.;
   {
     std::string output;
     llvm::raw_string_ostream os(output);
     opWithProp.print(os);
-    EXPECT_TRUE(StringRef(os.str()).contains("a = 42"));
-    EXPECT_TRUE(StringRef(os.str()).contains("b = 4.200000e+01"));
-    EXPECT_TRUE(StringRef(os.str()).contains("array = array<i64: 40, 41>"));
-    EXPECT_TRUE(StringRef(os.str()).contains("label = \"bar foo\""));
+    StringRef view(output);
+    EXPECT_TRUE(view.contains("a = 42"));
+    EXPECT_TRUE(view.contains("b = 4.200000e+01"));
+    EXPECT_TRUE(view.contains("array = array<i64: 40, 41>"));
+    EXPECT_TRUE(view.contains("label = \"bar foo\""));
   }
   prop.array.push_back(42);
   {
     std::string output;
     llvm::raw_string_ostream os(output);
     opWithProp.print(os);
-    EXPECT_TRUE(StringRef(os.str()).contains("a = 42"));
-    EXPECT_TRUE(StringRef(os.str()).contains("b = 4.200000e+01"));
-    EXPECT_TRUE(StringRef(os.str()).contains("array = array<i64: 40, 41, 42>"));
-    EXPECT_TRUE(StringRef(os.str()).contains("label = \"bar foo\""));
+    StringRef view(output);
+    EXPECT_TRUE(view.contains("a = 42"));
+    EXPECT_TRUE(view.contains("b = 4.200000e+01"));
+    EXPECT_TRUE(view.contains("array = array<i64: 40, 41, 42>"));
+    EXPECT_TRUE(view.contains("label = \"bar foo\""));
   }
   prop.label = std::make_shared<std::string>("foo bar");
   {
     std::string output;
     llvm::raw_string_ostream os(output);
     opWithProp.print(os);
-    EXPECT_TRUE(StringRef(os.str()).contains("a = 42"));
-    EXPECT_TRUE(StringRef(os.str()).contains("b = 4.200000e+01"));
-    EXPECT_TRUE(StringRef(os.str()).contains("array = array<i64: 40, 41, 42>"));
-    EXPECT_TRUE(StringRef(os.str()).contains("label = \"foo bar\""));
+    StringRef view(output);
+    EXPECT_TRUE(view.contains("a = 42"));
+    EXPECT_TRUE(view.contains("b = 4.200000e+01"));
+    EXPECT_TRUE(view.contains("array = array<i64: 40, 41, 42>"));
+    EXPECT_TRUE(view.contains("label = \"foo bar\""));
   }
 }
 
@@ -297,9 +301,10 @@ TEST(OpPropertiesTest, DefaultValues) {
     std::string output;
     llvm::raw_string_ostream os(output);
     op->print(os);
-    EXPECT_TRUE(StringRef(os.str()).contains("a = -1"));
-    EXPECT_TRUE(StringRef(os.str()).contains("b = -1"));
-    EXPECT_TRUE(StringRef(os.str()).contains("array = array<i64: -33>"));
+    StringRef view(output);
+    EXPECT_TRUE(view.contains("a = -1"));
+    EXPECT_TRUE(view.contains("b = -1"));
+    EXPECT_TRUE(view.contains("array = array<i64: -33>"));
   }
   op->erase();
 }
@@ -371,9 +376,10 @@ TEST(OpPropertiesTest, getOrAddProperties) {
     std::string output;
     llvm::raw_string_ostream os(output);
     op->print(os);
-    EXPECT_TRUE(StringRef(os.str()).contains("a = 1"));
-    EXPECT_TRUE(StringRef(os.str()).contains("b = 2"));
-    EXPECT_TRUE(StringRef(os.str()).contains("array = array<i64: 3, 4, 5>"));
+    StringRef view(output);
+    EXPECT_TRUE(view.contains("a = 1"));
+    EXPECT_TRUE(view.contains("b = 2"));
+    EXPECT_TRUE(view.contains("array = array<i64: 3, 4, 5>"));
   }
   op->erase();
 }
@@ -400,8 +406,9 @@ TEST(OpPropertiesTest, withoutPropertiesDiscardableAttrs) {
   std::string output;
   llvm::raw_string_ostream os(output);
   op->print(os);
-  EXPECT_TRUE(StringRef(os.str()).contains("inherent_attr = 42"));
-  EXPECT_TRUE(StringRef(os.str()).contains("other_attr = 56"));
+  StringRef view(output);
+  EXPECT_TRUE(view.contains("inherent_attr = 42"));
+  EXPECT_TRUE(view.contains("other_attr = 56"));
 
   OwningOpRef<Operation *> reparsed = parseSourceString(os.str(), config);
   auto trivialHash = [](Value v) { return hash_value(v); };

diff  --git a/mlir/unittests/Support/IndentedOstreamTest.cpp b/mlir/unittests/Support/IndentedOstreamTest.cpp
index 08a4de533c1c34..804a7e475e096f 100644
--- a/mlir/unittests/Support/IndentedOstreamTest.cpp
+++ b/mlir/unittests/Support/IndentedOstreamTest.cpp
@@ -18,7 +18,7 @@ TEST(FormatTest, SingleLine) {
   raw_indented_ostream ros(os);
   ros << 10;
   ros.flush();
-  EXPECT_THAT(os.str(), StrEq("10"));
+  EXPECT_THAT(str, StrEq("10"));
 }
 
 TEST(FormatTest, SimpleMultiLine) {
@@ -31,7 +31,7 @@ TEST(FormatTest, SimpleMultiLine) {
   ros << "c";
   ros << "\n";
   ros.flush();
-  EXPECT_THAT(os.str(), StrEq("ab\nc\n"));
+  EXPECT_THAT(str, StrEq("ab\nc\n"));
 }
 
 TEST(FormatTest, SimpleMultiLineIndent) {
@@ -44,7 +44,7 @@ TEST(FormatTest, SimpleMultiLineIndent) {
   ros << "c";
   ros << "\n";
   ros.flush();
-  EXPECT_THAT(os.str(), StrEq("  a    b\n    c\n"));
+  EXPECT_THAT(str, StrEq("  a    b\n    c\n"));
 }
 
 TEST(FormatTest, SingleRegion) {
@@ -71,7 +71,7 @@ TEST(FormatTest, SingleRegion) {
     inner inner
   }
 after)";
-  EXPECT_THAT(os.str(), StrEq(expected));
+  EXPECT_THAT(str, StrEq(expected));
 
   // Repeat the above with inline form.
   str.clear();
@@ -106,7 +106,7 @@ TEST(FormatTest, Reindent) {
 
 
 )";
-  EXPECT_THAT(os.str(), StrEq(expected));
+  EXPECT_THAT(str, StrEq(expected));
 }
 
 TEST(FormatTest, ReindentLineEndings) {
@@ -122,5 +122,5 @@ TEST(FormatTest, ReindentLineEndings) {
   ros.printReindented(desc);
   ros.flush();
   const auto *expected = "First line\r\n        second line";
-  EXPECT_THAT(os.str(), StrEq(expected));
+  EXPECT_THAT(str, StrEq(expected));
 }


        


More information about the Mlir-commits mailing list