[llvm] [llvm-remarkutil] Make invalid states un-representable in the count tool (PR #140829)
Jon Roelofs via llvm-commits
llvm-commits at lists.llvm.org
Wed May 28 08:49:34 PDT 2025
https://github.com/jroelofs updated https://github.com/llvm/llvm-project/pull/140829
>From 28a34df08f2fe95029012304e547a526fa43d4a8 Mon Sep 17 00:00:00 2001
From: Jon Roelofs <jonathan_roelofs at apple.com>
Date: Mon, 19 May 2025 12:18:12 -0700
Subject: [PATCH 1/8] [llvm-remarkutil] Make invalid states un-representable in
the count tool.
This consolidates some of the error handling around regex arguments to the
tool, and sets up the APIs such that errors must be handled before their usage.
---
llvm/tools/llvm-remarkutil/RemarkCounter.cpp | 71 ++++++++++---------
llvm/tools/llvm-remarkutil/RemarkCounter.h | 56 +--------------
.../tools/llvm-remarkutil/RemarkUtilHelpers.h | 33 +++++++++
3 files changed, 73 insertions(+), 87 deletions(-)
diff --git a/llvm/tools/llvm-remarkutil/RemarkCounter.cpp b/llvm/tools/llvm-remarkutil/RemarkCounter.cpp
index 2d9432e41d9c0..c9806b2ba8a17 100644
--- a/llvm/tools/llvm-remarkutil/RemarkCounter.cpp
+++ b/llvm/tools/llvm-remarkutil/RemarkCounter.cpp
@@ -111,19 +111,6 @@ static unsigned getValForKey(StringRef Key, const Remark &Remark) {
return *RemarkArg->getValAsInt();
}
-Error Filters::regexArgumentsValid() {
- if (RemarkNameFilter && RemarkNameFilter->IsRegex)
- if (auto E = checkRegex(RemarkNameFilter->FilterRE))
- return E;
- if (PassNameFilter && PassNameFilter->IsRegex)
- if (auto E = checkRegex(PassNameFilter->FilterRE))
- return E;
- if (ArgFilter && ArgFilter->IsRegex)
- if (auto E = checkRegex(ArgFilter->FilterRE))
- return E;
- return Error::success();
-}
-
bool Filters::filterRemark(const Remark &Remark) {
if (RemarkNameFilter && !RemarkNameFilter->match(Remark.RemarkName))
return false;
@@ -250,27 +237,42 @@ Error RemarkCounter::print(StringRef OutputFileName) {
Expected<Filters> getRemarkFilter() {
// Create Filter properties.
std::optional<FilterMatcher> RemarkNameFilter;
+ if (!RemarkNameOpt.empty())
+ RemarkNameFilter = FilterMatcher::createExact(RemarkNameOpt);
+ else if (!RemarkNameOptRE.empty()) {
+ auto FM = FilterMatcher::createRE(RemarkNameOptRE, "rremark-name");
+ if (!FM)
+ return FM.takeError();
+ RemarkNameFilter = std::move(*FM);
+ }
+
std::optional<FilterMatcher> PassNameFilter;
+ if (!PassNameOpt.empty())
+ PassNameFilter = FilterMatcher::createExact(PassNameOpt);
+ else if (!PassNameOptRE.empty()) {
+ auto FM = FilterMatcher::createRE(PassNameOptRE, "rpass-name");
+ if (!FM)
+ return FM.takeError();
+ PassNameFilter = std::move(*FM);
+ }
+
std::optional<FilterMatcher> RemarkArgFilter;
+ if (!RemarkFilterArgByOpt.empty())
+ RemarkArgFilter = FilterMatcher::createExact(RemarkFilterArgByOpt);
+ else if (!RemarkArgFilterOptRE.empty()) {
+ auto FM = FilterMatcher::createRE(RemarkArgFilterOptRE, "rfilter-arg-by");
+ if (!FM)
+ return FM.takeError();
+ RemarkArgFilter = std::move(*FM);
+ }
+
std::optional<Type> RemarkType;
- if (!RemarkNameOpt.empty())
- RemarkNameFilter = {RemarkNameOpt, false};
- else if (!RemarkNameOptRE.empty())
- RemarkNameFilter = {RemarkNameOptRE, true};
- if (!PassNameOpt.empty())
- PassNameFilter = {PassNameOpt, false};
- else if (!PassNameOptRE.empty())
- PassNameFilter = {PassNameOptRE, true};
if (RemarkTypeOpt != Type::Failure)
RemarkType = RemarkTypeOpt;
- if (!RemarkFilterArgByOpt.empty())
- RemarkArgFilter = {RemarkFilterArgByOpt, false};
- else if (!RemarkArgFilterOptRE.empty())
- RemarkArgFilter = {RemarkArgFilterOptRE, true};
+
// Create RemarkFilter.
- return Filters::createRemarkFilter(std::move(RemarkNameFilter),
- std::move(PassNameFilter),
- std::move(RemarkArgFilter), RemarkType);
+ return Filters{std::move(RemarkNameFilter), std::move(PassNameFilter),
+ std::move(RemarkArgFilter), RemarkType};
}
Error useCollectRemark(StringRef Buffer, Counter &Counter, Filters &Filter) {
@@ -313,12 +315,17 @@ static Error collectRemarks() {
SmallVector<FilterMatcher, 4> ArgumentsVector;
if (!Keys.empty()) {
for (auto &Key : Keys)
- ArgumentsVector.push_back({Key, false});
+ ArgumentsVector.push_back(FilterMatcher::createExact(Key));
} else if (!RKeys.empty())
- for (auto Key : RKeys)
- ArgumentsVector.push_back({Key, true});
+ for (auto Key : RKeys) {
+ auto FM = FilterMatcher::createRE(Key, "count-by");
+ if (!FM)
+ return FM.takeError();
+ ArgumentsVector.push_back(std::move(*FM));
+ }
else
- ArgumentsVector.push_back({".*", true});
+ ArgumentsVector.push_back(
+ cantFail(FilterMatcher::createRE(".*", "count-by")));
Expected<ArgumentCounter> AC = ArgumentCounter::createArgumentCounter(
GroupByOpt, ArgumentsVector, Buffer, Filter);
diff --git a/llvm/tools/llvm-remarkutil/RemarkCounter.h b/llvm/tools/llvm-remarkutil/RemarkCounter.h
index 34d5bff774055..3b977791d87c2 100644
--- a/llvm/tools/llvm-remarkutil/RemarkCounter.h
+++ b/llvm/tools/llvm-remarkutil/RemarkCounter.h
@@ -45,26 +45,6 @@ inline std::string groupByToStr(GroupBy GroupBy) {
}
}
-/// Filter object which can be either a string or a regex to match with the
-/// remark properties.
-struct FilterMatcher {
- Regex FilterRE;
- std::string FilterStr;
- bool IsRegex;
- FilterMatcher(std::string Filter, bool IsRegex) : IsRegex(IsRegex) {
- if (IsRegex)
- FilterRE = Regex(Filter);
- else
- FilterStr = Filter;
- }
-
- bool match(StringRef StringToMatch) const {
- if (IsRegex)
- return FilterRE.match(StringToMatch);
- return FilterStr == StringToMatch.trim().str();
- }
-};
-
/// Filter out remarks based on remark properties based on name, pass name,
/// argument and type.
struct Filters {
@@ -72,39 +52,11 @@ struct Filters {
std::optional<FilterMatcher> PassNameFilter;
std::optional<FilterMatcher> ArgFilter;
std::optional<Type> RemarkTypeFilter;
- /// Returns a filter object if all the arguments provided are valid regex
- /// types otherwise return an error.
- static Expected<Filters>
- createRemarkFilter(std::optional<FilterMatcher> RemarkNameFilter,
- std::optional<FilterMatcher> PassNameFilter,
- std::optional<FilterMatcher> ArgFilter,
- std::optional<Type> RemarkTypeFilter) {
- Filters Filter;
- Filter.RemarkNameFilter = std::move(RemarkNameFilter);
- Filter.PassNameFilter = std::move(PassNameFilter);
- Filter.ArgFilter = std::move(ArgFilter);
- Filter.RemarkTypeFilter = std::move(RemarkTypeFilter);
- if (auto E = Filter.regexArgumentsValid())
- return std::move(E);
- return std::move(Filter);
- }
+
/// Returns true if \p Remark satisfies all the provided filters.
bool filterRemark(const Remark &Remark);
-
-private:
- /// Check if arguments can be parsed as valid regex types.
- Error regexArgumentsValid();
};
-/// Convert Regex string error to an error object.
-inline Error checkRegex(const Regex &Regex) {
- std::string Error;
- if (!Regex.isValid(Error))
- return createStringError(make_error_code(std::errc::invalid_argument),
- Twine("Regex: ", Error));
- return Error::success();
-}
-
/// Abstract counter class used to define the general required methods for
/// counting a remark.
struct Counter {
@@ -160,12 +112,6 @@ struct ArgumentCounter : Counter {
StringRef Buffer, Filters &Filter) {
ArgumentCounter AC;
AC.Group = Group;
- for (auto &Arg : Arguments) {
- if (Arg.IsRegex) {
- if (auto E = checkRegex(Arg.FilterRE))
- return std::move(E);
- }
- }
if (auto E = AC.getAllMatchingArgumentsInRemark(Buffer, Arguments, Filter))
return std::move(E);
return AC;
diff --git a/llvm/tools/llvm-remarkutil/RemarkUtilHelpers.h b/llvm/tools/llvm-remarkutil/RemarkUtilHelpers.h
index 5d2335224d4c2..31a64ce668ed1 100644
--- a/llvm/tools/llvm-remarkutil/RemarkUtilHelpers.h
+++ b/llvm/tools/llvm-remarkutil/RemarkUtilHelpers.h
@@ -18,6 +18,7 @@
#include "llvm/Support/Error.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Regex.h"
#include "llvm/Support/ToolOutputFile.h"
// Keep input + output help + names consistent across the various modes via a
@@ -55,5 +56,37 @@ Expected<std::unique_ptr<ToolOutputFile>>
getOutputFileWithFlags(StringRef OutputFileName, sys::fs::OpenFlags Flags);
Expected<std::unique_ptr<ToolOutputFile>>
getOutputFileForRemarks(StringRef OutputFileName, Format OutputFormat);
+
+/// Filter object which can be either a string or a regex to match with the
+/// remark properties.
+class FilterMatcher {
+ Regex FilterRE;
+ std::string FilterStr;
+ bool IsRegex;
+
+ FilterMatcher(StringRef Filter, bool IsRegex)
+ : FilterRE(Filter), FilterStr(Filter), IsRegex(IsRegex) {}
+
+public:
+ static FilterMatcher createExact(StringRef Filter) { return {Filter, false}; }
+
+ static Expected<FilterMatcher> createRE(StringRef Filter,
+ StringRef Argument) {
+ FilterMatcher FM(Filter, true);
+ std::string Error;
+ if (!FM.FilterRE.isValid(Error))
+ return createStringError(make_error_code(std::errc::invalid_argument),
+ "invalid argument '--" + Argument + "=" +
+ Filter + "': " + Error);
+ return std::move(FM);
+ }
+
+ bool match(StringRef StringToMatch) const {
+ if (IsRegex)
+ return FilterRE.match(StringToMatch);
+ return FilterStr == StringToMatch.trim().str();
+ }
+};
+
} // namespace remarks
} // namespace llvm
>From e5acd0489e9f6f5fc4b5a8c71d765568c180db88 Mon Sep 17 00:00:00 2001
From: Jon Roelofs <jonathan_roelofs at apple.com>
Date: Tue, 20 May 2025 19:07:20 -0700
Subject: [PATCH 2/8] add tests for count error handling
---
llvm/test/tools/llvm-remarkutil/instruction-count.test | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/llvm/test/tools/llvm-remarkutil/instruction-count.test b/llvm/test/tools/llvm-remarkutil/instruction-count.test
index c4bc3e5780e3f..09a9dab026ec2 100644
--- a/llvm/test/tools/llvm-remarkutil/instruction-count.test
+++ b/llvm/test/tools/llvm-remarkutil/instruction-count.test
@@ -2,6 +2,9 @@ RUN: llvm-remarkutil instruction-count --parser=yaml %p/Inputs/instruction-count
RUN: llvm-remarkutil yaml2bitstream %p/Inputs/instruction-count.yaml | llvm-remarkutil instruction-count --parser=bitstream | FileCheck %s
RUN: llvm-remarkutil count --parser=yaml --count-by=arg --group-by=function --remark-name="InstructionCount" %p/Inputs/instruction-count.yaml | FileCheck %s --check-prefix=COUNT-CHECK
RUN: llvm-remarkutil yaml2bitstream %p/Inputs/instruction-count.yaml | llvm-remarkutil count --parser=bitstream --count-by=arg --group-by=function --remark-name="InstructionCount" | FileCheck %s --check-prefix=COUNT-CHECK
+RUN: not llvm-remarkutil count --parser=yaml --count-by=arg --group-by=function --rremark-name=* %p/Inputs/instruction-count.yaml 2>&1 | FileCheck %s --check-prefix=ERROR-REMARK-NAME
+RUN: not llvm-remarkutil count --parser=yaml --count-by=arg --group-by=function --rpass-name=* %p/Inputs/instruction-count.yaml 2>&1 | FileCheck %s --check-prefix=ERROR-PASS-NAME
+RUN: not llvm-remarkutil count --parser=yaml --count-by=arg --group-by=function --rfilter-arg-by=* %p/Inputs/instruction-count.yaml 2>&1 | FileCheck %s --check-prefix=ERROR-FILTER-ARG-BY
; CHECK-LABEL: Function,InstructionCount
; CHECK: func1,1
@@ -12,3 +15,7 @@ RUN: llvm-remarkutil yaml2bitstream %p/Inputs/instruction-count.yaml | llvm-rem
; COUNT-CHECK: func1,1
; COUNT-CHECK: func2,2
; COUNT-CHECK: func3,3
+
+; ERROR-REMARK-NAME: error: invalid argument '--rremark-name=*': repetition-operator operand invalid
+; ERROR-PASS-NAME: error: invalid argument '--rpass-name=*': repetition-operator operand invalid
+; ERROR-FILTER-ARG-BY: error: invalid argument '--rfilter-arg-by=*': repetition-operator operand invalid
\ No newline at end of file
>From 35e835261d789ad778ae5ebdf9e2b435728e16a7 Mon Sep 17 00:00:00 2001
From: Jon Roelofs <jonathan_roelofs at apple.com>
Date: Tue, 20 May 2025 19:11:46 -0700
Subject: [PATCH 3/8] simplify new test
---
llvm/test/tools/llvm-remarkutil/instruction-count.test | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/llvm/test/tools/llvm-remarkutil/instruction-count.test b/llvm/test/tools/llvm-remarkutil/instruction-count.test
index 09a9dab026ec2..f521e69a4d5df 100644
--- a/llvm/test/tools/llvm-remarkutil/instruction-count.test
+++ b/llvm/test/tools/llvm-remarkutil/instruction-count.test
@@ -2,9 +2,9 @@ RUN: llvm-remarkutil instruction-count --parser=yaml %p/Inputs/instruction-count
RUN: llvm-remarkutil yaml2bitstream %p/Inputs/instruction-count.yaml | llvm-remarkutil instruction-count --parser=bitstream | FileCheck %s
RUN: llvm-remarkutil count --parser=yaml --count-by=arg --group-by=function --remark-name="InstructionCount" %p/Inputs/instruction-count.yaml | FileCheck %s --check-prefix=COUNT-CHECK
RUN: llvm-remarkutil yaml2bitstream %p/Inputs/instruction-count.yaml | llvm-remarkutil count --parser=bitstream --count-by=arg --group-by=function --remark-name="InstructionCount" | FileCheck %s --check-prefix=COUNT-CHECK
-RUN: not llvm-remarkutil count --parser=yaml --count-by=arg --group-by=function --rremark-name=* %p/Inputs/instruction-count.yaml 2>&1 | FileCheck %s --check-prefix=ERROR-REMARK-NAME
-RUN: not llvm-remarkutil count --parser=yaml --count-by=arg --group-by=function --rpass-name=* %p/Inputs/instruction-count.yaml 2>&1 | FileCheck %s --check-prefix=ERROR-PASS-NAME
-RUN: not llvm-remarkutil count --parser=yaml --count-by=arg --group-by=function --rfilter-arg-by=* %p/Inputs/instruction-count.yaml 2>&1 | FileCheck %s --check-prefix=ERROR-FILTER-ARG-BY
+RUN: not llvm-remarkutil count --parser=yaml --count-by=arg --group-by=function --rremark-name=* %p/Inputs/instruction-count.yaml 2>&1 | FileCheck %s --check-prefix=ERROR -DARG=rremark-name
+RUN: not llvm-remarkutil count --parser=yaml --count-by=arg --group-by=function --rpass-name=* %p/Inputs/instruction-count.yaml 2>&1 | FileCheck %s --check-prefix=ERROR -DARG=rpass-name
+RUN: not llvm-remarkutil count --parser=yaml --count-by=arg --group-by=function --rfilter-arg-by=* %p/Inputs/instruction-count.yaml 2>&1 | FileCheck %s --check-prefix=ERROR -DARG=rfilter-arg-by
; CHECK-LABEL: Function,InstructionCount
; CHECK: func1,1
@@ -16,6 +16,4 @@ RUN: not llvm-remarkutil count --parser=yaml --count-by=arg --group-by=function
; COUNT-CHECK: func2,2
; COUNT-CHECK: func3,3
-; ERROR-REMARK-NAME: error: invalid argument '--rremark-name=*': repetition-operator operand invalid
-; ERROR-PASS-NAME: error: invalid argument '--rpass-name=*': repetition-operator operand invalid
-; ERROR-FILTER-ARG-BY: error: invalid argument '--rfilter-arg-by=*': repetition-operator operand invalid
\ No newline at end of file
+; ERROR: error: invalid argument '--[[ARG]]=*': repetition-operator operand invalid
\ No newline at end of file
>From 17bd09b2b408abe7254f7c8b450ee53ee167b4ee Mon Sep 17 00:00:00 2001
From: Jon Roelofs <jonathan_roelofs at apple.com>
Date: Tue, 20 May 2025 19:42:59 -0700
Subject: [PATCH 4/8] make it harder to mention the wrong arg name in an error
---
llvm/tools/llvm-remarkutil/RemarkCounter.cpp | 11 ++++-----
.../llvm-remarkutil/RemarkUtilHelpers.cpp | 23 +++++++++++++++++++
.../tools/llvm-remarkutil/RemarkUtilHelpers.h | 20 ++++++++--------
3 files changed, 38 insertions(+), 16 deletions(-)
diff --git a/llvm/tools/llvm-remarkutil/RemarkCounter.cpp b/llvm/tools/llvm-remarkutil/RemarkCounter.cpp
index c9806b2ba8a17..b3a86e359d1ef 100644
--- a/llvm/tools/llvm-remarkutil/RemarkCounter.cpp
+++ b/llvm/tools/llvm-remarkutil/RemarkCounter.cpp
@@ -240,7 +240,7 @@ Expected<Filters> getRemarkFilter() {
if (!RemarkNameOpt.empty())
RemarkNameFilter = FilterMatcher::createExact(RemarkNameOpt);
else if (!RemarkNameOptRE.empty()) {
- auto FM = FilterMatcher::createRE(RemarkNameOptRE, "rremark-name");
+ auto FM = FilterMatcher::createRE(RemarkNameOptRE);
if (!FM)
return FM.takeError();
RemarkNameFilter = std::move(*FM);
@@ -250,7 +250,7 @@ Expected<Filters> getRemarkFilter() {
if (!PassNameOpt.empty())
PassNameFilter = FilterMatcher::createExact(PassNameOpt);
else if (!PassNameOptRE.empty()) {
- auto FM = FilterMatcher::createRE(PassNameOptRE, "rpass-name");
+ auto FM = FilterMatcher::createRE(PassNameOptRE);
if (!FM)
return FM.takeError();
PassNameFilter = std::move(*FM);
@@ -260,7 +260,7 @@ Expected<Filters> getRemarkFilter() {
if (!RemarkFilterArgByOpt.empty())
RemarkArgFilter = FilterMatcher::createExact(RemarkFilterArgByOpt);
else if (!RemarkArgFilterOptRE.empty()) {
- auto FM = FilterMatcher::createRE(RemarkArgFilterOptRE, "rfilter-arg-by");
+ auto FM = FilterMatcher::createRE(RemarkArgFilterOptRE);
if (!FM)
return FM.takeError();
RemarkArgFilter = std::move(*FM);
@@ -318,14 +318,13 @@ static Error collectRemarks() {
ArgumentsVector.push_back(FilterMatcher::createExact(Key));
} else if (!RKeys.empty())
for (auto Key : RKeys) {
- auto FM = FilterMatcher::createRE(Key, "count-by");
+ auto FM = FilterMatcher::createRE(Key, RKeys);
if (!FM)
return FM.takeError();
ArgumentsVector.push_back(std::move(*FM));
}
else
- ArgumentsVector.push_back(
- cantFail(FilterMatcher::createRE(".*", "count-by")));
+ ArgumentsVector.push_back(FilterMatcher::createAny());
Expected<ArgumentCounter> AC = ArgumentCounter::createArgumentCounter(
GroupByOpt, ArgumentsVector, Buffer, Filter);
diff --git a/llvm/tools/llvm-remarkutil/RemarkUtilHelpers.cpp b/llvm/tools/llvm-remarkutil/RemarkUtilHelpers.cpp
index c0357161c4f98..153cbd6ca2d91 100644
--- a/llvm/tools/llvm-remarkutil/RemarkUtilHelpers.cpp
+++ b/llvm/tools/llvm-remarkutil/RemarkUtilHelpers.cpp
@@ -53,5 +53,28 @@ getOutputFileForRemarks(StringRef OutputFileName, Format OutputFormat) {
? sys::fs::OF_TextWithCRLF
: sys::fs::OF_None);
}
+
+Expected<FilterMatcher>
+FilterMatcher::createRE(const llvm::cl::opt<std::string> &Arg) {
+ return createRE(Arg.ArgStr, Arg);
+}
+
+
+Expected<FilterMatcher>
+FilterMatcher::createRE(StringRef Filter, const cl::list<std::string> &Arg) {
+ return createRE(Arg.ArgStr, Filter);
+}
+
+Expected<FilterMatcher> FilterMatcher::createRE(StringRef Arg,
+ StringRef Value) {
+ FilterMatcher FM(Value, true);
+ std::string Error;
+ if (!FM.FilterRE.isValid(Error))
+ return createStringError(make_error_code(std::errc::invalid_argument),
+ "invalid argument '--" + Arg + "=" + Value +
+ "': " + Error);
+ return std::move(FM);
+}
+
} // namespace remarks
} // namespace llvm
diff --git a/llvm/tools/llvm-remarkutil/RemarkUtilHelpers.h b/llvm/tools/llvm-remarkutil/RemarkUtilHelpers.h
index 31a64ce668ed1..847afbbfa7142 100644
--- a/llvm/tools/llvm-remarkutil/RemarkUtilHelpers.h
+++ b/llvm/tools/llvm-remarkutil/RemarkUtilHelpers.h
@@ -15,6 +15,7 @@
#include "llvm/Remarks/RemarkFormat.h"
#include "llvm/Remarks/RemarkParser.h"
#include "llvm/Remarks/YAMLRemarkSerializer.h"
+#include "llvm/Support/CommandLine.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/MemoryBuffer.h"
@@ -67,19 +68,18 @@ class FilterMatcher {
FilterMatcher(StringRef Filter, bool IsRegex)
: FilterRE(Filter), FilterStr(Filter), IsRegex(IsRegex) {}
+ static Expected<FilterMatcher> createRE(StringRef Arg, StringRef Value);
+
public:
static FilterMatcher createExact(StringRef Filter) { return {Filter, false}; }
- static Expected<FilterMatcher> createRE(StringRef Filter,
- StringRef Argument) {
- FilterMatcher FM(Filter, true);
- std::string Error;
- if (!FM.FilterRE.isValid(Error))
- return createStringError(make_error_code(std::errc::invalid_argument),
- "invalid argument '--" + Argument + "=" +
- Filter + "': " + Error);
- return std::move(FM);
- }
+ static Expected<FilterMatcher>
+ createRE(const llvm::cl::opt<std::string> &Arg);
+
+ static Expected<FilterMatcher>
+ createRE(StringRef Filter, const cl::list<std::string> &Arg);
+
+ static FilterMatcher createAny() { return {".*", true}; }
bool match(StringRef StringToMatch) const {
if (IsRegex)
>From c6041736ec2bed64bf88a345d9a599f676843430 Mon Sep 17 00:00:00 2001
From: Jon Roelofs <jonathan_roelofs at apple.com>
Date: Tue, 20 May 2025 19:43:23 -0700
Subject: [PATCH 5/8] rm stray whitespace
---
llvm/tools/llvm-remarkutil/RemarkUtilHelpers.cpp | 1 -
1 file changed, 1 deletion(-)
diff --git a/llvm/tools/llvm-remarkutil/RemarkUtilHelpers.cpp b/llvm/tools/llvm-remarkutil/RemarkUtilHelpers.cpp
index 153cbd6ca2d91..72c5735296fcb 100644
--- a/llvm/tools/llvm-remarkutil/RemarkUtilHelpers.cpp
+++ b/llvm/tools/llvm-remarkutil/RemarkUtilHelpers.cpp
@@ -59,7 +59,6 @@ FilterMatcher::createRE(const llvm::cl::opt<std::string> &Arg) {
return createRE(Arg.ArgStr, Arg);
}
-
Expected<FilterMatcher>
FilterMatcher::createRE(StringRef Filter, const cl::list<std::string> &Arg) {
return createRE(Arg.ArgStr, Filter);
>From abe8f96eb6498e40167383189d794e32b6b2d318 Mon Sep 17 00:00:00 2001
From: Jon Roelofs <jonathan_roelofs at apple.com>
Date: Wed, 21 May 2025 08:46:48 -0700
Subject: [PATCH 6/8] clang-format
---
llvm/tools/llvm-remarkutil/RemarkUtilHelpers.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/llvm/tools/llvm-remarkutil/RemarkUtilHelpers.h b/llvm/tools/llvm-remarkutil/RemarkUtilHelpers.h
index 847afbbfa7142..45c9ed054f8cd 100644
--- a/llvm/tools/llvm-remarkutil/RemarkUtilHelpers.h
+++ b/llvm/tools/llvm-remarkutil/RemarkUtilHelpers.h
@@ -76,8 +76,8 @@ class FilterMatcher {
static Expected<FilterMatcher>
createRE(const llvm::cl::opt<std::string> &Arg);
- static Expected<FilterMatcher>
- createRE(StringRef Filter, const cl::list<std::string> &Arg);
+ static Expected<FilterMatcher> createRE(StringRef Filter,
+ const cl::list<std::string> &Arg);
static FilterMatcher createAny() { return {".*", true}; }
>From 3a8444dc75668b4a7c2be484a04d294a63a6a9d0 Mon Sep 17 00:00:00 2001
From: Jon Roelofs <jonathan_roelofs at apple.com>
Date: Wed, 28 May 2025 08:33:04 -0700
Subject: [PATCH 7/8] createExactOrRE
---
.../llvm-remarkutil/instruction-count.test | 10 +++--
llvm/tools/llvm-remarkutil/RemarkCounter.cpp | 40 +++++--------------
.../llvm-remarkutil/RemarkUtilHelpers.cpp | 14 +++++++
.../tools/llvm-remarkutil/RemarkUtilHelpers.h | 3 ++
4 files changed, 34 insertions(+), 33 deletions(-)
diff --git a/llvm/test/tools/llvm-remarkutil/instruction-count.test b/llvm/test/tools/llvm-remarkutil/instruction-count.test
index f521e69a4d5df..d94f4f94cc1df 100644
--- a/llvm/test/tools/llvm-remarkutil/instruction-count.test
+++ b/llvm/test/tools/llvm-remarkutil/instruction-count.test
@@ -2,9 +2,10 @@ RUN: llvm-remarkutil instruction-count --parser=yaml %p/Inputs/instruction-count
RUN: llvm-remarkutil yaml2bitstream %p/Inputs/instruction-count.yaml | llvm-remarkutil instruction-count --parser=bitstream | FileCheck %s
RUN: llvm-remarkutil count --parser=yaml --count-by=arg --group-by=function --remark-name="InstructionCount" %p/Inputs/instruction-count.yaml | FileCheck %s --check-prefix=COUNT-CHECK
RUN: llvm-remarkutil yaml2bitstream %p/Inputs/instruction-count.yaml | llvm-remarkutil count --parser=bitstream --count-by=arg --group-by=function --remark-name="InstructionCount" | FileCheck %s --check-prefix=COUNT-CHECK
-RUN: not llvm-remarkutil count --parser=yaml --count-by=arg --group-by=function --rremark-name=* %p/Inputs/instruction-count.yaml 2>&1 | FileCheck %s --check-prefix=ERROR -DARG=rremark-name
-RUN: not llvm-remarkutil count --parser=yaml --count-by=arg --group-by=function --rpass-name=* %p/Inputs/instruction-count.yaml 2>&1 | FileCheck %s --check-prefix=ERROR -DARG=rpass-name
-RUN: not llvm-remarkutil count --parser=yaml --count-by=arg --group-by=function --rfilter-arg-by=* %p/Inputs/instruction-count.yaml 2>&1 | FileCheck %s --check-prefix=ERROR -DARG=rfilter-arg-by
+RUN: not llvm-remarkutil count --parser=yaml --count-by=arg --group-by=function --rremark-name=* %p/Inputs/instruction-count.yaml 2>&1 | FileCheck %s --check-prefix=ERROR-REPOPERATOR -DARG=rremark-name
+RUN: not llvm-remarkutil count --parser=yaml --count-by=arg --group-by=function --rpass-name=* %p/Inputs/instruction-count.yaml 2>&1 | FileCheck %s --check-prefix=ERROR-REPOPERATOR -DARG=rpass-name
+RUN: not llvm-remarkutil count --parser=yaml --count-by=arg --group-by=function --rfilter-arg-by=* %p/Inputs/instruction-count.yaml 2>&1 | FileCheck %s --check-prefix=ERROR-REPOPERATOR -DARG=rfilter-arg-by
+RUN: not llvm-remarkutil count --parser=yaml --count-by=arg --group-by=function --rremark-name=InstCombine --remark-name=InstCombine %p/Inputs/instruction-count.yaml 2>&1 | FileCheck %s --check-prefix=ERROR-BOTHFILTERS -DARG=rremark-name
; CHECK-LABEL: Function,InstructionCount
; CHECK: func1,1
@@ -16,4 +17,5 @@ RUN: not llvm-remarkutil count --parser=yaml --count-by=arg --group-by=function
; COUNT-CHECK: func2,2
; COUNT-CHECK: func3,3
-; ERROR: error: invalid argument '--[[ARG]]=*': repetition-operator operand invalid
\ No newline at end of file
+; ERROR-REPOPERATOR: error: invalid argument '--[[ARG]]=*': repetition-operator operand invalid
+; ERROR-BOTHFILTERS: error: conflicting arguments: --remark-name and --rremark-name
\ No newline at end of file
diff --git a/llvm/tools/llvm-remarkutil/RemarkCounter.cpp b/llvm/tools/llvm-remarkutil/RemarkCounter.cpp
index b3a86e359d1ef..0b2cfe47ec096 100644
--- a/llvm/tools/llvm-remarkutil/RemarkCounter.cpp
+++ b/llvm/tools/llvm-remarkutil/RemarkCounter.cpp
@@ -236,43 +236,25 @@ Error RemarkCounter::print(StringRef OutputFileName) {
Expected<Filters> getRemarkFilter() {
// Create Filter properties.
- std::optional<FilterMatcher> RemarkNameFilter;
- if (!RemarkNameOpt.empty())
- RemarkNameFilter = FilterMatcher::createExact(RemarkNameOpt);
- else if (!RemarkNameOptRE.empty()) {
- auto FM = FilterMatcher::createRE(RemarkNameOptRE);
- if (!FM)
- return FM.takeError();
- RemarkNameFilter = std::move(*FM);
- }
+ auto MaybeRemarkNameFilter = FilterMatcher::createExactOrRE(RemarkNameOpt, RemarkNameOptRE);
+ if (!MaybeRemarkNameFilter)
+ return MaybeRemarkNameFilter.takeError();
- std::optional<FilterMatcher> PassNameFilter;
- if (!PassNameOpt.empty())
- PassNameFilter = FilterMatcher::createExact(PassNameOpt);
- else if (!PassNameOptRE.empty()) {
- auto FM = FilterMatcher::createRE(PassNameOptRE);
- if (!FM)
- return FM.takeError();
- PassNameFilter = std::move(*FM);
- }
+ auto MaybePassNameFilter = FilterMatcher::createExactOrRE(PassNameOpt, PassNameOptRE);
+ if (!MaybePassNameFilter)
+ return MaybePassNameFilter.takeError();
- std::optional<FilterMatcher> RemarkArgFilter;
- if (!RemarkFilterArgByOpt.empty())
- RemarkArgFilter = FilterMatcher::createExact(RemarkFilterArgByOpt);
- else if (!RemarkArgFilterOptRE.empty()) {
- auto FM = FilterMatcher::createRE(RemarkArgFilterOptRE);
- if (!FM)
- return FM.takeError();
- RemarkArgFilter = std::move(*FM);
- }
+ auto MaybeRemarkArgFilter = FilterMatcher::createExactOrRE(RemarkFilterArgByOpt, RemarkArgFilterOptRE);
+ if (!MaybeRemarkArgFilter)
+ return MaybeRemarkArgFilter.takeError();
std::optional<Type> RemarkType;
if (RemarkTypeOpt != Type::Failure)
RemarkType = RemarkTypeOpt;
// Create RemarkFilter.
- return Filters{std::move(RemarkNameFilter), std::move(PassNameFilter),
- std::move(RemarkArgFilter), RemarkType};
+ return Filters{std::move(*MaybeRemarkNameFilter), std::move(*MaybePassNameFilter),
+ std::move(*MaybeRemarkArgFilter), RemarkType};
}
Error useCollectRemark(StringRef Buffer, Counter &Counter, Filters &Filter) {
diff --git a/llvm/tools/llvm-remarkutil/RemarkUtilHelpers.cpp b/llvm/tools/llvm-remarkutil/RemarkUtilHelpers.cpp
index 72c5735296fcb..bdbf538eb21a7 100644
--- a/llvm/tools/llvm-remarkutil/RemarkUtilHelpers.cpp
+++ b/llvm/tools/llvm-remarkutil/RemarkUtilHelpers.cpp
@@ -75,5 +75,19 @@ Expected<FilterMatcher> FilterMatcher::createRE(StringRef Arg,
return std::move(FM);
}
+Expected<std::optional<FilterMatcher>> FilterMatcher::createExactOrRE(const llvm::cl::opt<std::string> &ExactArg,
+ const llvm::cl::opt<std::string> &REArg) {
+ if (!ExactArg.empty() && !REArg.empty())
+ return createStringError(make_error_code(std::errc::invalid_argument), "conflicting arguments: --" + ExactArg.ArgStr + " and --" + REArg.ArgStr);
+
+ if (!ExactArg.empty())
+ return createExact(ExactArg);
+
+ if (!REArg.empty())
+ return createRE(REArg);
+
+ return std::nullopt;
+}
+
} // namespace remarks
} // namespace llvm
diff --git a/llvm/tools/llvm-remarkutil/RemarkUtilHelpers.h b/llvm/tools/llvm-remarkutil/RemarkUtilHelpers.h
index 45c9ed054f8cd..bad070ba47dc0 100644
--- a/llvm/tools/llvm-remarkutil/RemarkUtilHelpers.h
+++ b/llvm/tools/llvm-remarkutil/RemarkUtilHelpers.h
@@ -79,6 +79,9 @@ class FilterMatcher {
static Expected<FilterMatcher> createRE(StringRef Filter,
const cl::list<std::string> &Arg);
+ static Expected<std::optional<FilterMatcher>> createExactOrRE(const llvm::cl::opt<std::string> &ExactArg,
+ const llvm::cl::opt<std::string> &REArg);
+
static FilterMatcher createAny() { return {".*", true}; }
bool match(StringRef StringToMatch) const {
>From 65812f130e40a0ad25fbe7b58a57d23d9a82d747 Mon Sep 17 00:00:00 2001
From: Jon Roelofs <jonathan_roelofs at apple.com>
Date: Wed, 28 May 2025 08:49:15 -0700
Subject: [PATCH 8/8] clang-format
---
llvm/tools/llvm-remarkutil/RemarkCounter.cpp | 12 ++++++++----
llvm/tools/llvm-remarkutil/RemarkUtilHelpers.cpp | 9 ++++++---
llvm/tools/llvm-remarkutil/RemarkUtilHelpers.h | 5 +++--
3 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/llvm/tools/llvm-remarkutil/RemarkCounter.cpp b/llvm/tools/llvm-remarkutil/RemarkCounter.cpp
index 0b2cfe47ec096..0e6198cbb5203 100644
--- a/llvm/tools/llvm-remarkutil/RemarkCounter.cpp
+++ b/llvm/tools/llvm-remarkutil/RemarkCounter.cpp
@@ -236,15 +236,18 @@ Error RemarkCounter::print(StringRef OutputFileName) {
Expected<Filters> getRemarkFilter() {
// Create Filter properties.
- auto MaybeRemarkNameFilter = FilterMatcher::createExactOrRE(RemarkNameOpt, RemarkNameOptRE);
+ auto MaybeRemarkNameFilter =
+ FilterMatcher::createExactOrRE(RemarkNameOpt, RemarkNameOptRE);
if (!MaybeRemarkNameFilter)
return MaybeRemarkNameFilter.takeError();
- auto MaybePassNameFilter = FilterMatcher::createExactOrRE(PassNameOpt, PassNameOptRE);
+ auto MaybePassNameFilter =
+ FilterMatcher::createExactOrRE(PassNameOpt, PassNameOptRE);
if (!MaybePassNameFilter)
return MaybePassNameFilter.takeError();
- auto MaybeRemarkArgFilter = FilterMatcher::createExactOrRE(RemarkFilterArgByOpt, RemarkArgFilterOptRE);
+ auto MaybeRemarkArgFilter = FilterMatcher::createExactOrRE(
+ RemarkFilterArgByOpt, RemarkArgFilterOptRE);
if (!MaybeRemarkArgFilter)
return MaybeRemarkArgFilter.takeError();
@@ -253,7 +256,8 @@ Expected<Filters> getRemarkFilter() {
RemarkType = RemarkTypeOpt;
// Create RemarkFilter.
- return Filters{std::move(*MaybeRemarkNameFilter), std::move(*MaybePassNameFilter),
+ return Filters{std::move(*MaybeRemarkNameFilter),
+ std::move(*MaybePassNameFilter),
std::move(*MaybeRemarkArgFilter), RemarkType};
}
diff --git a/llvm/tools/llvm-remarkutil/RemarkUtilHelpers.cpp b/llvm/tools/llvm-remarkutil/RemarkUtilHelpers.cpp
index bdbf538eb21a7..ad6c46eceb8f2 100644
--- a/llvm/tools/llvm-remarkutil/RemarkUtilHelpers.cpp
+++ b/llvm/tools/llvm-remarkutil/RemarkUtilHelpers.cpp
@@ -75,10 +75,13 @@ Expected<FilterMatcher> FilterMatcher::createRE(StringRef Arg,
return std::move(FM);
}
-Expected<std::optional<FilterMatcher>> FilterMatcher::createExactOrRE(const llvm::cl::opt<std::string> &ExactArg,
- const llvm::cl::opt<std::string> &REArg) {
+Expected<std::optional<FilterMatcher>>
+FilterMatcher::createExactOrRE(const llvm::cl::opt<std::string> &ExactArg,
+ const llvm::cl::opt<std::string> &REArg) {
if (!ExactArg.empty() && !REArg.empty())
- return createStringError(make_error_code(std::errc::invalid_argument), "conflicting arguments: --" + ExactArg.ArgStr + " and --" + REArg.ArgStr);
+ return createStringError(make_error_code(std::errc::invalid_argument),
+ "conflicting arguments: --" + ExactArg.ArgStr +
+ " and --" + REArg.ArgStr);
if (!ExactArg.empty())
return createExact(ExactArg);
diff --git a/llvm/tools/llvm-remarkutil/RemarkUtilHelpers.h b/llvm/tools/llvm-remarkutil/RemarkUtilHelpers.h
index bad070ba47dc0..eb393bc3e3046 100644
--- a/llvm/tools/llvm-remarkutil/RemarkUtilHelpers.h
+++ b/llvm/tools/llvm-remarkutil/RemarkUtilHelpers.h
@@ -79,8 +79,9 @@ class FilterMatcher {
static Expected<FilterMatcher> createRE(StringRef Filter,
const cl::list<std::string> &Arg);
- static Expected<std::optional<FilterMatcher>> createExactOrRE(const llvm::cl::opt<std::string> &ExactArg,
- const llvm::cl::opt<std::string> &REArg);
+ static Expected<std::optional<FilterMatcher>>
+ createExactOrRE(const llvm::cl::opt<std::string> &ExactArg,
+ const llvm::cl::opt<std::string> &REArg);
static FilterMatcher createAny() { return {".*", true}; }
More information about the llvm-commits
mailing list