diff --git a/llvm/docs/CommandGuide/dsymutil.rst b/llvm/docs/CommandGuide/dsymutil.rst --- a/llvm/docs/CommandGuide/dsymutil.rst +++ b/llvm/docs/CommandGuide/dsymutil.rst @@ -107,6 +107,10 @@ output stream. When enabled warnings are embedded in the linked DWARF debug information. +.. option:: --remarks-drop-without-debug + + Drop remarks without valid debug locations. Without this flags, all remarks are kept. + .. option:: --remarks-output-format Specify the format to be used when serializing the linked remarks. diff --git a/llvm/include/llvm/Remarks/RemarkLinker.h b/llvm/include/llvm/Remarks/RemarkLinker.h --- a/llvm/include/llvm/Remarks/RemarkLinker.h +++ b/llvm/include/llvm/Remarks/RemarkLinker.h @@ -54,13 +54,26 @@ /// A path to append before the external file path found in remark metadata. std::optional PrependPath; + /// If true, keep all remarks, otherwise only keep remarks with valid debug + /// locations. + bool KeepAllRemarks = true; + /// Keep this remark. If it's already in the set, discard it. Remark &keep(std::unique_ptr Remark); + /// Returns true if \p R should be kept. If KeepAllRemarks is false, only + /// return true if \p R has a valid debug location. + bool shouldKeepRemark(const Remark &R) { + return KeepAllRemarks ? true : R.Loc.has_value(); + } + public: /// Set a path to prepend to the external file path. void setExternalFilePrependPath(StringRef PrependPath); + /// Set KeepAllRemarks to \p B. + void setKeepAllRemarks(bool B) { KeepAllRemarks = B; } + /// Link the remarks found in \p Buffer. /// If \p RemarkFormat is not provided, try to deduce it from the metadata in /// \p Buffer. diff --git a/llvm/lib/Remarks/RemarkLinker.cpp b/llvm/lib/Remarks/RemarkLinker.cpp --- a/llvm/lib/Remarks/RemarkLinker.cpp +++ b/llvm/lib/Remarks/RemarkLinker.cpp @@ -66,9 +66,6 @@ PrependPath = std::string(PrependPathIn); } -// Discard remarks with no source location. -static bool shouldKeepRemark(const Remark &R) { return R.Loc.has_value(); } - Error RemarkLinker::link(StringRef Buffer, std::optional RemarkFormat) { if (!RemarkFormat) { Expected ParserFormat = magicToFormat(Buffer); diff --git a/llvm/test/tools/dsymutil/cmdline.test b/llvm/test/tools/dsymutil/cmdline.test --- a/llvm/test/tools/dsymutil/cmdline.test +++ b/llvm/test/tools/dsymutil/cmdline.test @@ -22,6 +22,7 @@ CHECK: -out CHECK: {{-o }} CHECK: -papertrail +CHECK: -remarks-drop-without-debug CHECK: -remarks-output-format CHECK: -remarks-prepend-path CHECK: -reproducer diff --git a/llvm/tools/dsymutil/DwarfLinkerForBinary.cpp b/llvm/tools/dsymutil/DwarfLinkerForBinary.cpp --- a/llvm/tools/dsymutil/DwarfLinkerForBinary.cpp +++ b/llvm/tools/dsymutil/DwarfLinkerForBinary.cpp @@ -571,6 +571,7 @@ remarks::RemarkLinker RL; if (!Options.RemarksPrependPath.empty()) RL.setExternalFilePrependPath(Options.RemarksPrependPath); + RL.setKeepAllRemarks(Options.RemarksKeepAll); GeneralLinker.setObjectPrefixMap(&Options.ObjectPrefixMap); std::function TranslationLambda = [&](StringRef Input) { diff --git a/llvm/tools/dsymutil/LinkUtils.h b/llvm/tools/dsymutil/LinkUtils.h --- a/llvm/tools/dsymutil/LinkUtils.h +++ b/llvm/tools/dsymutil/LinkUtils.h @@ -98,6 +98,9 @@ /// The output format of the remarks. remarks::Format RemarksFormat = remarks::Format::Bitstream; + /// Whether all remarks should be kept or only remarks with valid debug + /// locations. + bool RemarksKeepAll = true; /// @} LinkOptions() = default; diff --git a/llvm/tools/dsymutil/Options.td b/llvm/tools/dsymutil/Options.td --- a/llvm/tools/dsymutil/Options.td +++ b/llvm/tools/dsymutil/Options.td @@ -194,3 +194,8 @@ HelpText<"Specify the format to be used when serializing the linked remarks.">, Group; def: Joined<["--", "-"], "remarks-output-format=">, Alias; + +def remarks_drop_without_debug: Flag<["--", "-"], "remarks-drop-without-debug">, + HelpText<"Drop remarks without valid debug locations. Without this flags, " + "all remarks are kept.">, + Group; diff --git a/llvm/tools/dsymutil/dsymutil.cpp b/llvm/tools/dsymutil/dsymutil.cpp --- a/llvm/tools/dsymutil/dsymutil.cpp +++ b/llvm/tools/dsymutil/dsymutil.cpp @@ -387,6 +387,9 @@ return FormatOrErr.takeError(); } + Options.LinkOpts.RemarksKeepAll = + !Args.hasArg(OPT_remarks_drop_without_debug); + if (Error E = verifyOptions(Options)) return std::move(E); return Options; diff --git a/llvm/unittests/Remarks/RemarksLinkingTest.cpp b/llvm/unittests/Remarks/RemarksLinkingTest.cpp --- a/llvm/unittests/Remarks/RemarksLinkingTest.cpp +++ b/llvm/unittests/Remarks/RemarksLinkingTest.cpp @@ -41,8 +41,11 @@ } static void check(remarks::Format InputFormat, StringRef Input, - remarks::Format OutputFormat, StringRef ExpectedOutput) { + remarks::Format OutputFormat, StringRef ExpectedOutput, + std::optional KeepAllRemarks = {}) { remarks::RemarkLinker RL; + if (KeepAllRemarks) + RL.setKeepAllRemarks(*KeepAllRemarks); EXPECT_FALSE(RL.link(Input, InputFormat)); serializeAndCheck(RL, OutputFormat, ExpectedOutput); } @@ -73,14 +76,28 @@ "Function: foo\n" "...\n"); - // Check that we don't keep remarks without debug locations. + // Check that we don't keep remarks without debug locations, unless + // KeepAllRemarks is set. + check(remarks::Format::YAML, + "--- !Missed\n" + "Pass: inline\n" + "Name: NoDefinition\n" + "Function: foo\n" + "...\n", + remarks::Format::YAML, "", + /*KeepAllRemarks=*/false); check(remarks::Format::YAML, "--- !Missed\n" "Pass: inline\n" "Name: NoDefinition\n" "Function: foo\n" "...\n", - remarks::Format::YAML, ""); + remarks::Format::YAML, + "--- !Missed\n" + "Pass: inline\n" + "Name: NoDefinition\n" + "Function: foo\n" + "...\n"); // Check that we deduplicate remarks. check(remarks::Format::YAML, @@ -127,6 +144,25 @@ " \n" "\n"); + // Check that we keep remarks without debug info. + check(remarks::Format::YAML, + "--- !Missed\n" + "Pass: inline\n" + "Name: NoDefinition\n" + "Function: foo\n" + "...\n", + remarks::Format::Bitstream, + "\n" + "\n" + " \n" + " \n" + " blob data = " + "'inline\\x00NoDefinition\\x00foo\\x00'\n" + "\n" + "\n" + " \n" + "\n"); + // Check that we deduplicate remarks. check(remarks::Format::YAML, "--- !Missed\n"