From 07f967d94dd7eb9666e8c2d5f6e0f4e8a14919cd Mon Sep 17 00:00:00 2001 From: Francis Visoiu Mistrih Date: Thu, 5 Sep 2019 18:30:20 +0000 Subject: [PATCH] [Remarks] Don't serialize metadata if a string table is not used For YAML remarks with no string table, the mode should not affect the output. llvm-svn: 371106 --- llvm/include/llvm/Remarks/YAMLRemarkSerializer.h | 11 +++- llvm/lib/Remarks/YAMLRemarkSerializer.cpp | 23 ++++--- .../Remarks/YAMLRemarksSerializerTest.cpp | 70 ++++++++++++++++++---- 3 files changed, 82 insertions(+), 22 deletions(-) diff --git a/llvm/include/llvm/Remarks/YAMLRemarkSerializer.h b/llvm/include/llvm/Remarks/YAMLRemarkSerializer.h index f57ae4c..77b7838 100644 --- a/llvm/include/llvm/Remarks/YAMLRemarkSerializer.h +++ b/llvm/include/llvm/Remarks/YAMLRemarkSerializer.h @@ -40,9 +40,6 @@ struct YAMLRemarkSerializer : public RemarkSerializer { std::unique_ptr metaSerializer(raw_ostream &OS, Optional ExternalFilename = None) override; - -protected: - bool DidEmitMeta = false; }; struct YAMLMetaSerializer : public MetaSerializer { @@ -58,6 +55,10 @@ struct YAMLMetaSerializer : public MetaSerializer { /// like the regular YAML remark but instead of string entries it's using /// numbers that map to an index in the string table. struct YAMLStrTabRemarkSerializer : public YAMLRemarkSerializer { + /// Wether we already emitted the metadata in standalone mode. + /// This should be set to true after the first invocation of `emit`. + bool DidEmitMeta = false; + YAMLStrTabRemarkSerializer(raw_ostream &OS, SerializerMode Mode) : YAMLRemarkSerializer(OS, Mode) { // Having a string table set up enables the serializer to use it. @@ -68,6 +69,10 @@ struct YAMLStrTabRemarkSerializer : public YAMLRemarkSerializer { : YAMLRemarkSerializer(OS, Mode) { StrTab = std::move(StrTabIn); } + + /// Override to emit the metadata if necessary. + void emit(const Remark &Remark) override; + std::unique_ptr metaSerializer(raw_ostream &OS, Optional ExternalFilename = None) override; diff --git a/llvm/lib/Remarks/YAMLRemarkSerializer.cpp b/llvm/lib/Remarks/YAMLRemarkSerializer.cpp index 28f9150..a4d67cc 100644 --- a/llvm/lib/Remarks/YAMLRemarkSerializer.cpp +++ b/llvm/lib/Remarks/YAMLRemarkSerializer.cpp @@ -153,15 +153,6 @@ YAMLRemarkSerializer::YAMLRemarkSerializer(raw_ostream &OS, SerializerMode Mode) : RemarkSerializer(OS, Mode), YAMLOutput(OS, reinterpret_cast(this)) {} void YAMLRemarkSerializer::emit(const Remark &Remark) { - // In standalone mode, emit the metadata first and set DidEmitMeta to avoid - // emitting it again. - if (Mode == SerializerMode::Standalone) { - std::unique_ptr MetaSerializer = - metaSerializer(OS, /*ExternalFilename=*/None); - MetaSerializer->emit(); - DidEmitMeta = true; - } - // Again, YAMLTraits expect a non-const object for inputting, but we're not // using that here. auto R = const_cast(&Remark); @@ -174,6 +165,20 @@ YAMLRemarkSerializer::metaSerializer(raw_ostream &OS, return std::make_unique(OS, ExternalFilename); } +void YAMLStrTabRemarkSerializer::emit(const Remark &Remark) { + // In standalone mode, for the serializer with a string table, emit the + // metadata first and set DidEmitMeta to avoid emitting it again. + if (Mode == SerializerMode::Standalone && !DidEmitMeta) { + std::unique_ptr MetaSerializer = + metaSerializer(OS, /*ExternalFilename=*/None); + MetaSerializer->emit(); + DidEmitMeta = true; + } + + // Then do the usual remark emission. + YAMLRemarkSerializer::emit(Remark); +} + std::unique_ptr YAMLStrTabRemarkSerializer::metaSerializer( raw_ostream &OS, Optional ExternalFilename) { assert(StrTab); diff --git a/llvm/unittests/Remarks/YAMLRemarksSerializerTest.cpp b/llvm/unittests/Remarks/YAMLRemarksSerializerTest.cpp index 761a46f..1ce932d 100644 --- a/llvm/unittests/Remarks/YAMLRemarksSerializerTest.cpp +++ b/llvm/unittests/Remarks/YAMLRemarksSerializerTest.cpp @@ -21,7 +21,7 @@ using namespace llvm; -static void check(remarks::SerializerMode Mode, const remarks::Remark &R, +static void check(remarks::SerializerMode Mode, ArrayRef Rs, StringRef ExpectedR, Optional ExpectedMeta, bool UseStrTab = false, Optional StrTab = None) { @@ -40,7 +40,8 @@ static void check(remarks::SerializerMode Mode, const remarks::Remark &R, EXPECT_FALSE(errorToBool(MaybeS.takeError())); std::unique_ptr S = std::move(*MaybeS); - S->emit(R); + for (const remarks::Remark &R : Rs) + S->emit(R); EXPECT_EQ(OS.str(), ExpectedR); if (ExpectedMeta) { @@ -55,14 +56,14 @@ static void check(remarks::SerializerMode Mode, const remarks::Remark &R, static void check(const remarks::Remark &R, StringRef ExpectedR, StringRef ExpectedMeta, bool UseStrTab = false, Optional StrTab = None) { - return check(remarks::SerializerMode::Separate, R, ExpectedR, ExpectedMeta, + return check(remarks::SerializerMode::Separate, makeArrayRef(&R, &R + 1), ExpectedR, ExpectedMeta, UseStrTab, std::move(StrTab)); } static void checkStandalone(const remarks::Remark &R, StringRef ExpectedR, Optional StrTab = None) { bool UseStrTab = StrTab.hasValue(); - return check(remarks::SerializerMode::Standalone, R, ExpectedR, + return check(remarks::SerializerMode::Standalone, makeArrayRef(&R, &R +1), ExpectedR, /*ExpectedMeta=*/None, UseStrTab, std::move(StrTab)); } @@ -117,10 +118,7 @@ TEST(YAMLRemarks, SerializerRemarkStandalone) { R.Args.back().Loc = remarks::RemarkLocation{"argpath", 6, 7}; checkStandalone( R, - StringRef("REMARKS\0" - "\0\0\0\0\0\0\0\0" - "\0\0\0\0\0\0\0\0" - "--- !Missed\n" + StringRef("--- !Missed\n" "Pass: pass\n" "Name: name\n" "DebugLoc: { File: path, Line: 3, Column: 4 }\n" @@ -130,8 +128,7 @@ TEST(YAMLRemarks, SerializerRemarkStandalone) { " - key: value\n" " - keydebug: valuedebug\n" " DebugLoc: { File: argpath, Line: 6, Column: 7 }\n" - "...\n", - 301)); + "...\n")); } TEST(YAMLRemarks, SerializerRemarkStrTab) { @@ -246,3 +243,56 @@ TEST(YAMLRemarks, SerializerRemarkParsedStrTabStandalone) { 315), std::move(PreFilledStrTab)); } + +TEST(YAMLRemarks, SerializerRemarkParsedStrTabStandaloneMultipleRemarks) { + StringRef StrTab("pass\0name\0func\0path\0value\0valuedebug\0argpath\0", 45); + remarks::ParsedStringTable ParsedStrTab(StrTab); + remarks::StringTable PreFilledStrTab(ParsedStrTab); + SmallVector Rs; + remarks::Remark R; + R.RemarkType = remarks::Type::Missed; + R.PassName = "pass"; + R.RemarkName = "name"; + R.FunctionName = "func"; + R.Loc = remarks::RemarkLocation{"path", 3, 4}; + R.Hotness = 5; + R.Args.emplace_back(); + R.Args.back().Key = "key"; + R.Args.back().Val = "value"; + R.Args.emplace_back(); + R.Args.back().Key = "keydebug"; + R.Args.back().Val = "valuedebug"; + R.Args.back().Loc = remarks::RemarkLocation{"argpath", 6, 7}; + Rs.emplace_back(R.clone()); + Rs.emplace_back(std::move(R)); + check(remarks::SerializerMode::Standalone, Rs, + StringRef("REMARKS\0" + "\0\0\0\0\0\0\0\0" + "\x2d\0\0\0\0\0\0\0" + "pass\0name\0func\0path\0value\0valuedebug\0argpath\0" + "--- !Missed\n" + "Pass: 0\n" + "Name: 1\n" + "DebugLoc: { File: 3, Line: 3, Column: 4 }\n" + "Function: 2\n" + "Hotness: 5\n" + "Args:\n" + " - key: 4\n" + " - keydebug: 5\n" + " DebugLoc: { File: 6, Line: 6, Column: 7 }\n" + "...\n" + "--- !Missed\n" + "Pass: 0\n" + "Name: 1\n" + "DebugLoc: { File: 3, Line: 3, Column: 4 }\n" + "Function: 2\n" + "Hotness: 5\n" + "Args:\n" + " - key: 4\n" + " - keydebug: 5\n" + " DebugLoc: { File: 6, Line: 6, Column: 7 }\n" + "...\n", + 561), + /*ExpectedMeta=*/None, + /*UseStrTab=*/true, std::move(PreFilledStrTab)); +} -- 2.7.4