Skip to content

Commit

Permalink
[snippy]: Color diagnostic output when possible
Browse files Browse the repository at this point in the history
This patch rips out most of the incorrect diagnostics that write
to errs() directly and replaces them with proper `snippy::warn()` calls.

Now snippy::{fatal,warn,notice,remark} color their output for visibility.
We already had this behavior for YAML diagnostics, since those use SMDiagnostic,
which uses the same `WithColor` mechanism for coloring the output diagnostic severity.
  • Loading branch information
sizn-sc authored and asi-sc committed Nov 15, 2024
1 parent 977e1da commit 6734530
Show file tree
Hide file tree
Showing 7 changed files with 117 additions and 32 deletions.
6 changes: 3 additions & 3 deletions llvm/test/tools/llvm-snippy/memory-scheme/ranges-diag.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,6 @@ access-ranges:
first-offset: 5
last-offset: 5

# CHECK: Warning: first offset 3 > last offset 1
# CHECK: Warning: last offset 5 >= stride 5
# CHECK: Cannot sample memory access for instruction
# CHECK: warning: Invalid memory access range: 'first-offset' (3) > 'last-offset' (1)
# CHECK: warning: Invalid memory access range: 'last-offset' (5) >= 'stride' (5)
# CHECK: error: Cannot sample memory access for instruction
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,4 @@
# TO-STDOUT-NEXT: - addr: 0x80000000
# TO-STDOUT-NEXT: - addr: 0x80000300

# NO-MEM: warning: cannot dump memory accesses as no accesses were generated. File won't be created.
# NO-MEM: warning: Cannot dump memory accesses: No accesses were generated, file won't be created.
11 changes: 10 additions & 1 deletion llvm/tools/llvm-snippy/include/snippy/Support/DiagnosticInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ enum class WarningName {
TooFarMaxPCDist,
ModelException,
UnusedSection,
GenPlanVerification
GenPlanVerification,
SeedNotSpecified,
};

struct WarningCounters {
Expand Down Expand Up @@ -83,9 +84,17 @@ class SnippyDiagnosticInfo : public llvm::DiagnosticInfo {
void notice(WarningName WN, llvm::LLVMContext &Ctx, const llvm::Twine &Prefix,
const llvm::Twine &Desc);

void notice(const llvm::Twine &Prefix, const llvm::Twine &Desc,
WarningName WN = WarningName::NotAWarning);

void notice(llvm::LLVMContext &Ctx, const llvm::Twine &Prefix,
const llvm::Twine &Desc, WarningName WN = WarningName::NotAWarning);

void warn(WarningName WN, llvm::LLVMContext &Ctx, const llvm::Twine &Prefix,
const llvm::Twine &Desc);

void warn(WarningName WN, const llvm::Twine &Prefix, const llvm::Twine &Desc);

[[noreturn]] void fatal(llvm::LLVMContext &Ctx, const llvm::Twine &Prefix,
const llvm::Twine &Desc);

Expand Down
29 changes: 22 additions & 7 deletions llvm/tools/llvm-snippy/lib/Config/MemoryScheme.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -177,13 +177,28 @@ std::string yaml::MappingTraits<snippy::MemoryAccessRange>::validate(
yaml::IO &Io, snippy::MemoryAccessRange &Range) {
if (shouldSkipValidation(Io))
return "";
// TODO: Remove this garbage and replace with a proper diagnostic
if (Range.FirstOffset > Range.LastOffset)
errs() << "Warning: first offset " << Range.FirstOffset << " > last offset "
<< Range.LastOffset << "\n";
if (Range.LastOffset >= Range.Stride)
errs() << "Warning: last offset " << Range.LastOffset << " >= stride "
<< Range.Stride << "\n";

if (Range.FirstOffset > Range.LastOffset) {
// TODO: Maybe this should become a hard error with the next breaking
// release?
snippy::warn(snippy::WarningName::MemoryAccess,
"Invalid memory access range",
Twine("'first-offset' (")
.concat(Twine(Range.FirstOffset))
.concat(") > 'last-offset' (")
.concat(Twine(Range.LastOffset).concat(")")));
}

if (Range.LastOffset >= Range.Stride) {
// TODO: Maybe this should become a hard error with the next breaking
// release?
snippy::warn(snippy::WarningName::MemoryAccess,
"Invalid memory access range",
Twine("'last-offset' (")
.concat(Twine(Range.LastOffset))
.concat(") >= 'stride' (")
.concat(Twine(Range.Stride).concat(")")));
}

if (Range.Stride == 0)
return "Stride cannot be equal to 0";
Expand Down
4 changes: 2 additions & 2 deletions llvm/tools/llvm-snippy/lib/MemAccessDumperPass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,8 @@ void dumpMemAccesses(StringRef Filename, const PlainAccessesType &Plain,
const BurstGroupAccessesType &BurstRanges,
const PlainAccessesType &BurstPlain, bool Restricted) {
if (Plain.empty() && BurstRanges.empty() && BurstPlain.empty()) {
errs() << "warning: cannot dump memory accesses as no accesses were "
"generated. File won't be created.\n";
snippy::warn(WarningName::MemoryAccess, "Cannot dump memory accesses",
"No accesses were generated, file won't be created.");
return;
}

Expand Down
91 changes: 76 additions & 15 deletions llvm/tools/llvm-snippy/lib/Support/DiagnosticInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,13 @@
//
//===----------------------------------------------------------------------===//

#include "llvm/IR/DiagnosticPrinter.h"
#include "llvm/Support/CommandLine.h"

#include "snippy/Support/DiagnosticInfo.h"
#include "snippy/Support/Options.h"

#include "llvm/IR/DiagnosticPrinter.h"
#include "llvm/Support/CommandLine.h"
#include "llvm/Support/WithColor.h"

namespace llvm {

// The unique option because it used not only in snippy
Expand Down Expand Up @@ -61,58 +62,118 @@ void SnippyDiagnosticInfo::print(llvm::DiagnosticPrinter &DP) const {
++WarningIt->second.EncounteredTotal;
}

void handleDiagnostic(LLVMContext &Ctx, const SnippyDiagnosticInfo &Diag) {
auto OldHandlerCallback = Ctx.getDiagnosticHandlerCallBack();

Ctx.setDiagnosticHandlerCallBack([](const DiagnosticInfo &Info, void *) {
HighlightColor Color = [&]() {
switch (Info.getSeverity()) {
case DS_Error:
return HighlightColor::Error;
case DS_Remark:
return HighlightColor::Remark;
case DS_Warning:
return HighlightColor::Warning;
case DS_Note:
return HighlightColor::Note;
}
}();

WithColor(errs(), Color)
<< LLVMContext::getDiagnosticMessagePrefix(Info.getSeverity());

errs() << ": ";

DiagnosticPrinterRawOStream DP(errs());
Info.print(DP);
errs() << "\n";
if (Info.getSeverity() == DS_Error)
exit(1);
});

Ctx.diagnose(Diag);
Ctx.setDiagnosticHandlerCallBack(OldHandlerCallback);
}

void handleDiagnostic(const SnippyDiagnosticInfo &Diag) {
LLVMContext Ctx;
handleDiagnostic(Ctx, Diag);
}

void notice(const llvm::Twine &Prefix, const llvm::Twine &Desc,
WarningName WN) {
SnippyDiagnosticInfo Diag(Prefix, Desc, llvm::DS_Remark,
WarningName::NotAWarning);
handleDiagnostic(Diag);
}

void notice(llvm::LLVMContext &Ctx, const llvm::Twine &Prefix,
const llvm::Twine &Desc, WarningName WN) {
SnippyDiagnosticInfo Diag(Prefix, Desc, llvm::DS_Remark,
WarningName::NotAWarning);
handleDiagnostic(Ctx, Diag);
}

void notice(WarningName WN, llvm::LLVMContext &Ctx, const llvm::Twine &Prefix,
const llvm::Twine &Desc) {
SnippyDiagnosticInfo Diag(Prefix, Desc, llvm::DS_Remark, WN);
Ctx.diagnose(Diag);
handleDiagnostic(Ctx, Diag);
}

void warn(WarningName WN, llvm::LLVMContext &Ctx, const llvm::Twine &Prefix,
const llvm::Twine &Desc) {
auto MsgCategory = WError ? llvm::DS_Error : llvm::DS_Warning;
SnippyDiagnosticInfo Diag(Prefix, Desc, MsgCategory, WN);
Ctx.diagnose(Diag);
handleDiagnostic(Ctx, Diag);
}

void warn(WarningName WN, const llvm::Twine &Prefix, const llvm::Twine &Desc) {
auto MsgCategory = WError ? llvm::DS_Error : llvm::DS_Warning;
SnippyDiagnosticInfo Diag(Prefix, Desc, MsgCategory, WN);
handleDiagnostic(Diag);
}

[[noreturn]] void fatal(llvm::LLVMContext &Ctx, const llvm::Twine &Prefix,
const llvm::Twine &Desc) {
SnippyDiagnosticInfo Diag(Prefix, Desc, llvm::DS_Error,
WarningName::NotAWarning);
Ctx.diagnose(Diag);
handleDiagnostic(Ctx, Diag);
llvm_unreachable("snippy::fatal should never return");
}

[[noreturn]] void fatal(llvm::LLVMContext &Ctx, const llvm::Twine &Prefix,
Error E) {
SnippyDiagnosticInfo Diag(Prefix, toString(std::move(E)), llvm::DS_Error,
WarningName::NotAWarning);
Ctx.diagnose(Diag);
handleDiagnostic(Ctx, Diag);
llvm_unreachable("snippy::fatal should never return");
}

[[noreturn]] void fatal(Error E) {
llvm::LLVMContext Ctx;
SnippyDiagnosticInfo Diag(toString(std::move(E)), llvm::DS_Error,
WarningName::NotAWarning);
Ctx.diagnose(Diag);
handleDiagnostic(Diag);
llvm_unreachable("snippy::fatal should never return");
}

[[noreturn]] void fatal(const llvm::Twine &Prefix, const llvm::Twine &Desc) {
llvm::LLVMContext Ctx;
SnippyDiagnosticInfo Diag(Prefix, Desc, llvm::DS_Error,
WarningName::NotAWarning);
Ctx.diagnose(Diag);
handleDiagnostic(Diag);
llvm_unreachable("snippy::fatal should never return");
}

[[noreturn]] void fatal(const llvm::Twine &Desc) {
llvm::LLVMContext Ctx;
SnippyDiagnosticInfo Diag(Desc, llvm::DS_Error, WarningName::NotAWarning);
Ctx.diagnose(Diag);
handleDiagnostic(Diag);
llvm_unreachable("snippy::fatal should never return");
}

[[noreturn]] void fatal(const llvm::Twine &Prefix, Error E) {
llvm::LLVMContext Ctx;
SnippyDiagnosticInfo Diag(Prefix, toString(std::move(E)), llvm::DS_Error,
WarningName::NotAWarning);
Ctx.diagnose(Diag);
handleDiagnostic(Diag);
llvm_unreachable("snippy::fatal should never return");
}

} // namespace snippy
Expand Down
6 changes: 3 additions & 3 deletions llvm/tools/llvm-snippy/llvm-snippy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -455,12 +455,12 @@ static bool isParsingWithPluginEnabled() {

static unsigned long long
seedOptToValue(StringRef SeedStr, StringRef SeedType = "instructions seed",
StringRef Warning = "no instructions seed specified,"
" using auto-generated one: ") {
StringRef Warning =
"no instructions seed specified, using auto-generated one") {
if (SeedStr.empty()) {
auto SeedValue =
std::chrono::system_clock::now().time_since_epoch().count();
llvm::errs() << "warning: " << Warning << SeedValue << "\n";
snippy::warn(WarningName::SeedNotSpecified, Warning, Twine(SeedValue));
return SeedValue;
}

Expand Down

0 comments on commit 6734530

Please sign in to comment.