Skip to content

[llvm-remarkutil] Auto-detect serializer format based on file extension #160545

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 11, 2025
+34 −7
Merged

[llvm-remarkutil] Auto-detect serializer format based on file extension #160545

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
01135cd
[spr] initial version
tobias-stadler Sep 24, 2025
8d96f50
Rebase
tobias-stadler Oct 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions llvm/test/tools/llvm-remarkutil/filter.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,19 @@ RUN: llvm-remarkutil filter --remark-type=analysis %p/Inputs/filter.yaml | FileC

RUN: llvm-remarkutil yaml2bitstream -o %t.opt.bitstream %p/Inputs/filter.yaml
RUN: llvm-remarkutil filter --function=func1 %t.opt.bitstream | FileCheck %s --strict-whitespace --check-prefix=REMARK1
RUN: llvm-remarkutil filter --function=func1 %t.opt.bitstream -o %t.r1.yamL
RUN: cat %t.r1.yamL | FileCheck %s --strict-whitespace --check-prefix=REMARK1
RUN: llvm-remarkutil filter --function=func1 %t.opt.bitstream -o %t.r1.yMl
RUN: cat %t.r1.yMl | FileCheck %s --strict-whitespace --check-prefix=REMARK1
RUN: llvm-remarkutil filter --function=func1 %t.opt.bitstream --serializer=yaml -o %t.r1.fake.opt.bitstream
RUN: cat %t.r1.fake.opt.bitstream | FileCheck %s --strict-whitespace --check-prefix=REMARK1

RUN: llvm-remarkutil filter --function=func1 %t.opt.bitstream -o %t.r1.opt.bitstream
RUN: llvm-remarkutil bitstream2yaml %t.r1.opt.bitstream | FileCheck %s --strict-whitespace --check-prefix=REMARK1
RUN: llvm-remarkutil filter --function=func1 %t.opt.bitstream -o %t.r1
RUN: llvm-remarkutil bitstream2yaml %t.r1 | FileCheck %s --strict-whitespace --check-prefix=REMARK1
RUN: llvm-remarkutil filter --function=func1 %p/Inputs/filter.yaml --serializer=bitstream -o %t.r1.fake.yaml
RUN: llvm-remarkutil bitstream2yaml %t.r1.fake.yaml | FileCheck %s --strict-whitespace --check-prefix=REMARK1

RUN: llvm-remarkutil filter --function=func %p/Inputs/filter.yaml | FileCheck %s --allow-empty --strict-whitespace --check-prefix=EMPTY

Expand Down
8 changes: 2 additions & 6 deletions llvm/tools/llvm-remarkutil/RemarkFilter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,8 @@ static Error tryFilter() {
return MaybeParser.takeError();
auto &Parser = **MaybeParser;

Format SerializerFormat = OutputFormat;
if (SerializerFormat == Format::Auto) {
SerializerFormat = Parser.ParserFormat;
if (OutputFileName.empty() || OutputFileName == "-")
SerializerFormat = Format::YAML;
}
Format SerializerFormat =
getSerializerFormat(OutputFileName, OutputFormat, Parser.ParserFormat);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WDYT about making the default be bitstream (unless the input was yaml), and then adding a warning/error message here if the output is a tty, kind of like opt does when you try to write bitcode to the terminal? I think our "north star" should be to converge on "bitstream for everything except LIT tests".

Copy link
Contributor Author

@tobias-stadler tobias-stadler Sep 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a difficult trade-off imo. Working with these tools interactively in a tty, I personally like YAML as the default. I find that most often I call e.g. filter on huge bitstream files and pipe the result straight into vim/less for inspection. So if we do this, maybe instead of emitting the warning we can auto-switch to YAML if we detect tty and also add sth like -S as a shorthand for --serializer=yaml to make piping into text based tools easier?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that!

auto MaybeOF = getOutputFileForRemarks(OutputFileName, SerializerFormat);
if (!MaybeOF)
Expand Down
14 changes: 14 additions & 0 deletions llvm/tools/llvm-remarkutil/RemarkUtilHelpers.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,20 @@ getOutputFileForRemarks(StringRef OutputFileName, Format OutputFormat) {
: sys::fs::OF_None);
}

Format getSerializerFormat(StringRef OutputFileName, Format SelectedFormat,
Format DefaultFormat) {
if (SelectedFormat != Format::Auto)
return SelectedFormat;
SelectedFormat = DefaultFormat;
if (OutputFileName.empty() || OutputFileName == "-" ||
OutputFileName.ends_with_insensitive(".yaml") ||
OutputFileName.ends_with_insensitive(".yml"))
SelectedFormat = Format::YAML;
if (OutputFileName.ends_with_insensitive(".bitstream"))
SelectedFormat = Format::Bitstream;
return SelectedFormat;
}

Expected<FilterMatcher>
FilterMatcher::createRE(const llvm::cl::opt<std::string> &Arg) {
return createRE(Arg.ArgStr, Arg);
Expand Down
9 changes: 8 additions & 1 deletion llvm/tools/llvm-remarkutil/RemarkUtilHelpers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@
"serializer", cl::init(Format::Auto), \
cl::desc("Output remark format to serialize"), \
cl::values(clEnumValN(Format::Auto, "auto", \
"Follow the parser format (default)"), \
"Automatic detection based on output file " \
"extension or parser format (default)"), \
clEnumValN(Format::YAML, "yaml", "YAML"), \
clEnumValN(Format::Bitstream, "bitstream", "Bitstream")), \
cl::sub(SUBOPT));
Expand Down Expand Up @@ -151,6 +152,12 @@ getOutputFileWithFlags(StringRef OutputFileName, sys::fs::OpenFlags Flags);
Expected<std::unique_ptr<ToolOutputFile>>
getOutputFileForRemarks(StringRef OutputFileName, Format OutputFormat);

/// Choose the serializer format. If \p SelectedFormat is Format::Auto, try to
/// detect the format based on the extension of \p OutputFileName or fall back
/// to \p DefaultFormat.
Format getSerializerFormat(StringRef OutputFileName, Format SelectedFormat,
Format DefaultFormat);

/// Filter object which can be either a string or a regex to match with the
/// remark properties.
class FilterMatcher {
Expand Down