Introduce llvmremark util diff command #85007
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add relevant documentation for the `llvm-remarkutil diff` subcommand
|
@llvm/pr-subscribers-llvm-binary-utilities Author: Zain Jaffal (zjaffal) ChangesThis tool is a generic tool to compare between two remark files and give the summary of the difference between two remark versions. The tool organises the remarks by debug location and displays the difference in arguments and remark type for each remark with the same header i.e remark name, function name and pass. This is useful for cases where remark type difference or argument change indicate a regression like in the vectorize remarks. Patch is 54.60 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/85007.diff 18 Files Affected:
diff --git a/llvm/docs/CommandGuide/llvm-remarkutil.rst b/llvm/docs/CommandGuide/llvm-remarkutil.rst
index af7d8eb31c0181..857aa5b394cd69 100644
--- a/llvm/docs/CommandGuide/llvm-remarkutil.rst
+++ b/llvm/docs/CommandGuide/llvm-remarkutil.rst
@@ -21,7 +21,9 @@ Subcommands
* :ref:`yaml2bitstream_subcommand` - Reserialize YAML remarks to bitstream.
* :ref:`instruction-count_subcommand` - Output function instruction counts.
* :ref:`annotation-count_subcommand` - Output remark type count from annotation remarks.
+ * :ref:`count_subcommand` - Output generic remark count.
* :ref:`size-diff_subcommand` - Compute diff in size remarks.
+ * :ref:`diff_subcommand` - Compute diff between remarks
.. _bitstream2yaml_subcommand:
@@ -425,3 +427,144 @@ EXIT STATUS
:program:`llvm-remarkutil size-diff` returns 0 on success, and a non-zero value
otherwise.
+
+.. _diff_subcommand:
+
+diff
+~~~~~
+
+.. program:: llvm-remarkutil diff
+
+
+USAGE: :program:`llvm-remarkutil diff` [*options*] <remarka_file> <remarkb_file>
+
+Summary
+^^^^^^^
+
+:program:`llvm-remarkutil diff` hilights the difference between two versions of `remarks <https://llvm.org/docs/Remarks.html>`_ based on specified properties.
+The tool will organise remarks based on the debug location and highlight the differences between remarks with the same header i.e remark name, pass name and function name. The tool by default highlights the differences in arguments between two remarks and the difference in remark type.
+The tool contains utilities to filter the remark diff based on remark name, pass name, argument value and remark type.
+
+
+Example
+^^^^^^^
+
+``remarks-passed.yaml``
+
+::
+
+ --- !Passed
+ Pass: Pass
+ Name: RemarkName
+ DebugLoc: { File: path/to/anno.c, Line: 1, Column: 2 }
+ Function: func0
+ Args:
+ - String: '2'
+ - String: 'Info2'
+
+
+``remarks-missed.yaml``
+
+::
+
+ --- !Missed
+ Pass: Pass
+ Name: RemarkName
+ DebugLoc: { File: path/to/anno.c, Line: 1, Column: 2 }
+ Function: func0
+ Args:
+ - String: '2'
+ - String: 'Info2'
+
+
+Output
+::
+
+ File A: remarks-passed.yaml
+ File B: remarks-missed.yaml
+ ----------
+ path/to/anno.c:func0 Ln: 1 Col: 2
+ --- Has the same header ---
+ Name: RemarkName
+ FunctionName: func0
+ PassName: Pass
+ Only at A >>>>
+ Type: Passed
+ =====
+ Only at B <<<<
+ Type: Missed
+ =====
+
+Notes
+^^^^^
+
+* Duplicate remarks in each remark file are discared.
+* If a remark doesn't contain a debug location then it won't be taken into account when caluclating diffs.
+
+Options
+^^^^^^^
+
+Options are similar to the ones for :ref:`count_subcommand` in terms of filtering remarks based on properties.
+Additional options include the following.
+
+.. option:: --only-show-a
+
+ Show remarks that are only in A.
+
+.. option:: --only-show-b
+
+ Show remarks that are only in B.
+
+.. option:: --only-show-common-remarks
+
+ Show intersecting remarks between A and B.
+
+.. option:: --only-show-different-remarks
+
+ Show all the remarks that are unique to A and B.
+
+.. option:: --report_style=<value>
+
+ * human Human-readable format
+ * json JSON format
+.. option:: -show-arg-diff-only
+
+ Show only the remarks that have the same header and differ in arguments
+
+.. option:: --show-remark-type-diff-only
+
+ Only show diff if remarks have the same header but different typ
+
+JSON format
+^^^^^^^^^^^
+
+Given the above example the corresponding ``json`` format is
+
+.. code-block:: json
+
+ {
+ "Files": {
+ "A": "remarks-passed.yaml",
+ "B": "remarks-missed.yaml"
+ },
+ "path/to/anno.c": {
+ "func0": {
+ "Ln: 1 Col: 2": {
+ "HasSameHeaderObj": [
+ {
+ "Diff": {
+ "RemarkTypeA": "Passed",
+ "RemarkTypeB": "Missed"
+ },
+ "FunctionName": "func0",
+ "PassName": "Pass",
+ "RemarkName": "RemarkName"
+ }
+ ],
+ "OnlyA": [],
+ "OnlyB": []
+ }
+ }
+ }
+ }
+
diff --git a/llvm/test/tools/llvm-remarkutil/diff/1-loc-2-args.test b/llvm/test/tools/llvm-remarkutil/diff/1-loc-2-args.test
new file mode 100644
index 00000000000000..b81e7ffed65758
--- /dev/null
+++ b/llvm/test/tools/llvm-remarkutil/diff/1-loc-2-args.test
@@ -0,0 +1,15 @@
+RUN: llvm-remarkutil diff %p/Inputs/1-loc-2-args.yaml %p/Inputs/1-loc-2-args-2.yaml --parser=yaml | FileCheck %s
+
+; CHECK-LABEL: File A: {{.*}}/Inputs/1-loc-2-args.yaml
+; CHECK: File B: {{.*}}/Inputs/1-loc-2-args-2.yaml
+; CHECK: path/to/anno.c:func0 Ln: 1 Col: 2
+; CHECK: --- Has the same header ---
+; CHECK: Name: RemarkName
+; CHECK: FunctionName: func0
+; CHECK: PassName: Pass
+; CHECK: Only at A >>>>
+; CHECK: String: 1
+; CHECK: =====
+; CHECK: Only at B <<<<
+; CHECK: String: 2
+; CHECK: =====
\ No newline at end of file
diff --git a/llvm/test/tools/llvm-remarkutil/diff/Inputs/1-loc-2-args-2.yaml b/llvm/test/tools/llvm-remarkutil/diff/Inputs/1-loc-2-args-2.yaml
new file mode 100644
index 00000000000000..dc90c257a51e14
--- /dev/null
+++ b/llvm/test/tools/llvm-remarkutil/diff/Inputs/1-loc-2-args-2.yaml
@@ -0,0 +1,8 @@
+--- !Analysis
+Pass: Pass
+Name: RemarkName
+DebugLoc: { File: path/to/anno.c, Line: 1, Column: 2 }
+Function: func0
+Args:
+ - String: '2'
+ - String: 'Info'
\ No newline at end of file
diff --git a/llvm/test/tools/llvm-remarkutil/diff/Inputs/1-loc-2-args-3.yaml b/llvm/test/tools/llvm-remarkutil/diff/Inputs/1-loc-2-args-3.yaml
new file mode 100644
index 00000000000000..e737623927fa8d
--- /dev/null
+++ b/llvm/test/tools/llvm-remarkutil/diff/Inputs/1-loc-2-args-3.yaml
@@ -0,0 +1,8 @@
+--- !Analysis
+Pass: Pass
+Name: RemarkName
+DebugLoc: { File: path/to/anno2.c, Line: 1, Column: 2 }
+Function: func0
+Args:
+ - String: '2'
+ - String: 'Info'
\ No newline at end of file
diff --git a/llvm/test/tools/llvm-remarkutil/diff/Inputs/1-loc-2-args.yaml b/llvm/test/tools/llvm-remarkutil/diff/Inputs/1-loc-2-args.yaml
new file mode 100644
index 00000000000000..23b618289a7c5a
--- /dev/null
+++ b/llvm/test/tools/llvm-remarkutil/diff/Inputs/1-loc-2-args.yaml
@@ -0,0 +1,8 @@
+--- !Analysis
+Pass: Pass
+Name: RemarkName
+DebugLoc: { File: path/to/anno.c, Line: 1, Column: 2 }
+Function: func0
+Args:
+ - String: '1'
+ - String: 'Info'
\ No newline at end of file
diff --git a/llvm/test/tools/llvm-remarkutil/diff/Inputs/empty-file.yaml b/llvm/test/tools/llvm-remarkutil/diff/Inputs/empty-file.yaml
new file mode 100644
index 00000000000000..e69de29bb2d1d6
diff --git a/llvm/test/tools/llvm-remarkutil/diff/Inputs/remark-no-debug-loc.yaml b/llvm/test/tools/llvm-remarkutil/diff/Inputs/remark-no-debug-loc.yaml
new file mode 100644
index 00000000000000..e69de29bb2d1d6
diff --git a/llvm/test/tools/llvm-remarkutil/diff/Inputs/remarks-missed.yaml b/llvm/test/tools/llvm-remarkutil/diff/Inputs/remarks-missed.yaml
new file mode 100644
index 00000000000000..b7f5d5870ec6d2
--- /dev/null
+++ b/llvm/test/tools/llvm-remarkutil/diff/Inputs/remarks-missed.yaml
@@ -0,0 +1,8 @@
+--- !Missed
+Pass: Pass
+Name: RemarkName
+DebugLoc: { File: path/to/anno.c, Line: 1, Column: 2 }
+Function: func0
+Args:
+ - String: '2'
+ - String: 'Info2'
\ No newline at end of file
diff --git a/llvm/test/tools/llvm-remarkutil/diff/Inputs/remarks-passed.yaml b/llvm/test/tools/llvm-remarkutil/diff/Inputs/remarks-passed.yaml
new file mode 100644
index 00000000000000..818e97f3290ce8
--- /dev/null
+++ b/llvm/test/tools/llvm-remarkutil/diff/Inputs/remarks-passed.yaml
@@ -0,0 +1,8 @@
+--- !Passed
+Pass: Pass
+Name: RemarkName
+DebugLoc: { File: path/to/anno.c, Line: 1, Column: 2 }
+Function: func0
+Args:
+ - String: '2'
+ - String: 'Info2'
\ No newline at end of file
diff --git a/llvm/test/tools/llvm-remarkutil/diff/disjoined-remarks.test b/llvm/test/tools/llvm-remarkutil/diff/disjoined-remarks.test
new file mode 100644
index 00000000000000..48e55f3eedcad2
--- /dev/null
+++ b/llvm/test/tools/llvm-remarkutil/diff/disjoined-remarks.test
@@ -0,0 +1,26 @@
+RUN: llvm-remarkutil diff %p/Inputs/1-loc-2-args.yaml %p/Inputs/1-loc-2-args-3.yaml --parser=yaml | FileCheck %s
+
+; CHECK-LABEL: File A: {{.*}}/Inputs/1-loc-2-args.yaml
+; CHECK: File B: {{.*}}/Inputs/1-loc-2-args-3.yaml
+; CHECK: ----------
+; CHECK: path/to/anno.c:func0 Ln: 1 Col: 2
+; CHECK: Only at A >>>>
+; CHECK: Name: RemarkName
+; CHECK: FunctionName: func0
+; CHECK: PassName: Pass
+; CHECK: Type: Analysis
+; CHECK: Args:
+; CHECK: String: 1
+; CHECK: String: Info
+; CHECK: =====
+; CHECK: ----------
+; CHECK: path/to/anno2.c:func0 Ln: 1 Col: 2
+; CHECK: Only at B <<<<
+; CHECK: Name: RemarkName
+; CHECK: FunctionName: func0
+; CHECK: PassName: Pass
+; CHECK: Type: Analysis
+; CHECK: Args:
+; CHECK: String: 2
+; CHECK: String: Info
+; CHECK: =====
diff --git a/llvm/test/tools/llvm-remarkutil/diff/remark-type-diff.test b/llvm/test/tools/llvm-remarkutil/diff/remark-type-diff.test
new file mode 100644
index 00000000000000..074d264ad1b01b
--- /dev/null
+++ b/llvm/test/tools/llvm-remarkutil/diff/remark-type-diff.test
@@ -0,0 +1,16 @@
+bin/llvm-remarkutil diff %p/Inputs/remarks-passed.yaml %p/Inputs/remarks-missed.yaml | FileCheck %s
+
+;CHECK-LABEL: File A: {{.*}}/Inputs/remarks-passed.yaml
+;CHECK: File B: {{.*}}/Inputs/remarks-missed.yaml
+;CHECK: ----------
+;CHECK: path/to/anno.c:func0 Ln: 1 Col: 2
+;CHECK: --- Has the same header ---
+;CHECK: Name: RemarkName
+;CHECK: FunctionName: func0
+;CHECK: PassName: Pass
+;CHECK: Only at A >>>>
+;CHECK: Type: Passed
+;CHECK: =====
+;CHECK: Only at B <<<<
+;CHECK: Type: Missed
+;CHECK: =====
\ No newline at end of file
diff --git a/llvm/tools/llvm-remarkutil/CMakeLists.txt b/llvm/tools/llvm-remarkutil/CMakeLists.txt
index 48aeb9397cda16..f0dc7ea1b2c8c6 100644
--- a/llvm/tools/llvm-remarkutil/CMakeLists.txt
+++ b/llvm/tools/llvm-remarkutil/CMakeLists.txt
@@ -8,6 +8,7 @@ add_llvm_tool(llvm-remarkutil
RemarkConvert.cpp
RemarkCount.cpp
RemarkCounter.cpp
+ RemarkDiff.cpp
RemarkSizeDiff.cpp
RemarkUtil.cpp
RemarkUtilHelpers.cpp
diff --git a/llvm/tools/llvm-remarkutil/RemarkCounter.cpp b/llvm/tools/llvm-remarkutil/RemarkCounter.cpp
index dc0685f342886a..87692437c29674 100644
--- a/llvm/tools/llvm-remarkutil/RemarkCounter.cpp
+++ b/llvm/tools/llvm-remarkutil/RemarkCounter.cpp
@@ -24,6 +24,7 @@ static cl::SubCommand CountSub("count",
INPUT_FORMAT_COMMAND_LINE_OPTIONS(CountSub)
INPUT_OUTPUT_COMMAND_LINE_OPTIONS(CountSub)
+FILTER_COMMAND_LINE_OPTIONS(CountSub)
static cl::list<std::string>
Keys("args", cl::desc("Specify remark argument/s to count by."),
@@ -33,45 +34,6 @@ static cl::list<std::string> RKeys(
cl::desc(
"Specify remark argument/s to count (accepts regular expressions)."),
cl::value_desc("arguments"), cl::sub(CountSub), cl::ValueOptional);
-static cl::opt<std::string>
- RemarkNameOpt("remark-name",
- cl::desc("Optional remark name to filter collection by."),
- cl::ValueOptional, cl::sub(CountSub));
-static cl::opt<std::string>
- PassNameOpt("pass-name", cl::ValueOptional,
- cl::desc("Optional remark pass name to filter collection by."),
- cl::sub(CountSub));
-
-static cl::opt<std::string> RemarkFilterArgByOpt(
- "filter-arg-by", cl::desc("Optional remark arg to filter collection by."),
- cl::ValueOptional, cl::sub(CountSub));
-static cl::opt<std::string>
- RemarkNameOptRE("rremark-name",
- cl::desc("Optional remark name to filter collection by "
- "(accepts regular expressions)."),
- cl::ValueOptional, cl::sub(CountSub));
-static cl::opt<std::string>
- RemarkArgFilterOptRE("rfilter-arg-by",
- cl::desc("Optional remark arg to filter collection by "
- "(accepts regular expressions)."),
- cl::sub(CountSub), cl::ValueOptional);
-static cl::opt<std::string>
- PassNameOptRE("rpass-name", cl::ValueOptional,
- cl::desc("Optional remark pass name to filter collection "
- "by (accepts regular expressions)."),
- cl::sub(CountSub));
-static cl::opt<Type> RemarkTypeOpt(
- "remark-type", cl::desc("Optional remark type to filter collection by."),
- cl::values(clEnumValN(Type::Unknown, "unknown", "UNKOWN"),
- clEnumValN(Type::Passed, "passed", "PASSED"),
- clEnumValN(Type::Missed, "missed", "MISSED"),
- clEnumValN(Type::Analysis, "analysis", "ANALYSIS"),
- clEnumValN(Type::AnalysisFPCommute, "analysis-fp-commute",
- "ANALYSIS_FP_COMMUTE"),
- clEnumValN(Type::AnalysisAliasing, "analysis-aliasing",
- "ANALYSIS_ALIASING"),
- clEnumValN(Type::Failure, "failure", "FAILURE")),
- cl::init(Type::Failure), cl::sub(CountSub));
static cl::opt<CountBy> CountByOpt(
"count-by", cl::desc("Specify the property to collect remarks by."),
cl::values(
@@ -111,34 +73,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;
- if (PassNameFilter && !PassNameFilter->match(Remark.PassName))
- return false;
- if (RemarkTypeFilter)
- return *RemarkTypeFilter == Remark.RemarkType;
- if (ArgFilter) {
- if (!any_of(Remark.Args,
- [this](Argument Arg) { return ArgFilter->match(Arg.Val); }))
- return false;
- }
- return true;
-}
-
Error ArgumentCounter::getAllMatchingArgumentsInRemark(
StringRef Buffer, ArrayRef<FilterMatcher> Arguments, Filters &Filter) {
auto MaybeParser = createRemarkParser(InputFormat, Buffer);
diff --git a/llvm/tools/llvm-remarkutil/RemarkCounter.h b/llvm/tools/llvm-remarkutil/RemarkCounter.h
index 34d5bff7740556..3568ba6b88bb78 100644
--- a/llvm/tools/llvm-remarkutil/RemarkCounter.h
+++ b/llvm/tools/llvm-remarkutil/RemarkCounter.h
@@ -45,66 +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 {
- std::optional<FilterMatcher> RemarkNameFilter;
- 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 {
diff --git a/llvm/tools/llvm-remarkutil/RemarkDiff.cpp b/llvm/tools/llvm-remarkutil/RemarkDiff.cpp
new file mode 100644
index 00000000000000..d40f63b2df8a98
--- /dev/null
+++ b/llvm/tools/llvm-remarkutil/RemarkDiff.cpp
@@ -0,0 +1,445 @@
+//===-------------- RemarkDiff.cpp ----------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// Diffs remarks between two remark files.
+/// The tool offers different modes for comparing two versions of remarks.
+/// 1. Look through common remarks between two files.
+/// 2. Compare the remark type. This is useful to check if an optimzation
+/// changed from passing to failing.
+/// 3. Compare remark arguments. This is useful to check if a remark argument
+/// changed after some compiler change.
+///
+/// The results are presented as a json file.
+///
+//===----------------------------------------------------------------------===//
+
+#include "RemarkDiff.h"
+#include "llvm/Support/JSON.h"
+#include <utility>
+
+using namespace llvm;
+using namespace remarks;
+using namespace llvm::remarkutil;
+
+static cl::SubCommand DiffSub("diff",
+ "diff remarks based on specified properties.");
+static cl::opt<std::string> RemarkFileA(cl::Positional,
+ cl::desc("<remarka_file>"),
+ cl::Required, cl::sub(DiffSub));
+static cl::opt<std::string> RemarkFileB(cl::Positional,
+ cl::desc("<remarkb_file>"),
+ cl::Required, cl::sub(DiffSub));
+
+static cl::opt<bool> Verbose(
+ "v", cl::init(false),
+ cl::desc("Output detailed difference for remarks. By default the tool will "
+ "only show the remark name, type and location. If the flag is "
+ "added we display the arguments that are different."),
+ cl::sub(DiffSub));
+static cl::opt<bool>
+ ShowArgDiffOnly("show-arg-diff-only", cl::init(false),
+ cl::desc("Show only the remarks that have the same header "
+ "and differ in arguments"),
+ cl::sub(DiffSub));
+static cl::opt<bool> OnlyShowCommonRemarks(
+ "only-show-common-remarks", cl::init(false),
+ cl::desc("Ignore any remarks that don't exist in both <remarka_file> and "
+ "<remarkb_file>."),
+ cl::sub(DiffSub));
+static cl::opt<bool> ShowOnlyDifferentRemarks(
+ "only-show-different-remarks", cl::init(false),
+ cl::desc("Show remarks that are exclusively at either A or B"),
+ cl::sub(DiffSub));
+static cl::opt<bool> ShowOnlyA("only-show-a", cl::init(false),
+ cl::desc("Show remarks that are only in A"),
+ cl::sub(DiffSub));
+
+static cl::opt<bool> ShowOnlyB("only-show-b", cl::init(false),
+ cl::desc("Show remarks that are only in B"),
+ ...
[truncated]
|
|
Ping |
ornata
reviewed
Mar 25, 2024
| Summary | ||
| ^^^^^^^ | ||
|
|
||
| :program:`llvm-remarkutil diff` hilights the difference between two versions of `remarks <https://llvm.org/docs/Remarks.html>`_ based on specified properties. |
ornata
reviewed
Mar 25, 2024
| ^^^^^^^ | ||
|
|
||
| :program:`llvm-remarkutil diff` hilights the difference between two versions of `remarks <https://llvm.org/docs/Remarks.html>`_ based on specified properties. | ||
| The tool will organise remarks based on the debug location and highlight the differences between remarks with the same header i.e remark name, pass name and function name. The tool by default highlights the differences in arguments between two remarks and the difference in remark type. |
There was a problem hiding this comment.
The tool will organise remarks based on the debug location and highlight the differences between remarks with the same header i.e remark name, pass name and function name
How about
The tool will highlight differences in remarks with the same remark name, pass name, and function name. Remarks are sorted by debug location.
ornata
reviewed
Mar 25, 2024
| RA.Args.size() > RB.Args.size() ? RA.Args : RB.Args; | ||
| bool IsARemaining = RA.Args.size() > RB.Args.size() ? true : false; | ||
| for (; ArgIdx < RemainingArgs.size(); ArgIdx++) | ||
| if (IsARemaining) |
There was a problem hiding this comment.
Is it possible to do this?
auto &RemainingRemarks = RA.Args.size() ? RB.Args.size() ? RA : RB;
There was a problem hiding this comment.
The code I have copies the rest of the remaining remarks into OnlyA or OnlyB. I guess I can move the check of IsARemaining outside the loop
ornata
reviewed
Mar 25, 2024
| } | ||
|
|
||
| // Compare remark type between RA and RB. | ||
| if (DiffConfig.ShowRemarkTypeDiff && RA.RemarkType != RB.RemarkType) { |
There was a problem hiding this comment.
also IMO the "type diff" part should be in a function so it is easier to extend
e.g.
if (DiffConfig.ShowRemarkTypeDiff)
showRemarkTypeDiff(RA, RB);
ornata
reviewed
Mar 25, 2024
| /// Check if the remark has the same name, function name and pass name as \p | ||
| /// RHS | ||
| bool hasSameHeader(const RemarkInfo &RHS) const { | ||
| return RemarkName == RHS.RemarkName && FunctionName == RHS.FunctionName && |
There was a problem hiding this comment.
Should RemarkHeader be a struct/class with an overloaded == ?
Then RemarkInfo can contain a RemarkHeader.
ornata
reviewed
Mar 25, 2024
|
|
||
| /// A wrapper for Remark class that can be used for generating remark diff. | ||
| struct RemarkInfo { | ||
| std::string RemarkName; |
ornata
reviewed
Mar 25, 2024
| enum ReportStyleOptions { human_output, json_output }; | ||
| /// copy of Argument class using std::string instead of StringRef. | ||
| struct RemarkArgInfo { | ||
| std::string Key; |
Make remark header a seperate struct
move functions to be part of each class
|
@fhahn ping |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This tool is a generic tool to compare between two remark files and give the summary of the difference between two remark versions.
The tool organises the remarks by debug location and displays the difference in arguments and remark type for each remark with the same header i.e remark name, function name and pass.
This is useful for cases where remark type difference or argument change indicate a regression like in the vectorize remarks.