Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
43 changes: 41 additions & 2 deletions llvm/lib/IR/LLVMContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,30 @@

using namespace llvm;

namespace opts {

enum class HaltOnFirstDiagErrorAction {
Exit,
Abort,
None,
};

static cl::opt<HaltOnFirstDiagErrorAction> HaltOnFirstDiagErrorOpt(
"halt-on-first-diag-error",
cl::desc(
"Halt action to take on the first unhandled diagnostic error reported"),
cl::values(
clEnumValN(
HaltOnFirstDiagErrorAction::Exit, "exit",
"Exit with error code 1 on first diagnostic with error severity"),
clEnumValN(HaltOnFirstDiagErrorAction::Abort, "abort",
"Abort with a stacktrace immediately on first diagnostic "
"with error severity"),
clEnumValN(HaltOnFirstDiagErrorAction::None, "none",
"Do not halt on first diagnostic with error severity")),
cl::init(HaltOnFirstDiagErrorAction::Exit), cl::Hidden);
} // namespace opts

static StringRef knownBundleName(unsigned BundleTagID) {
switch (BundleTagID) {
case LLVMContext::OB_deopt:
Expand Down Expand Up @@ -242,6 +266,19 @@ LLVMContext::getDiagnosticMessagePrefix(DiagnosticSeverity Severity) {
llvm_unreachable("Unknown DiagnosticSeverity");
}

static void handleHaltOnFirstDiagError() {
switch (opts::HaltOnFirstDiagErrorOpt) {
case opts::HaltOnFirstDiagErrorAction::Exit:
std::exit(1);
break;
case opts::HaltOnFirstDiagErrorAction::Abort:
std::abort();
break;
default:
break;
}
}

void LLVMContext::diagnose(const DiagnosticInfo &DI) {
if (auto *OptDiagBase = dyn_cast<DiagnosticInfoOptimizationBase>(&DI))
if (LLVMRemarkStreamer *RS = getLLVMRemarkStreamer())
Expand All @@ -264,8 +301,10 @@ void LLVMContext::diagnose(const DiagnosticInfo &DI) {
errs() << getDiagnosticMessagePrefix(DI.getSeverity()) << ": ";
DI.print(DP);
errs() << "\n";
if (DI.getSeverity() == DS_Error)
exit(1);

if (DI.getSeverity() == DS_Error) {
handleHaltOnFirstDiagError();
}
}

//===----------------------------------------------------------------------===//
Expand Down
25 changes: 25 additions & 0 deletions llvm/test/tools/llc/no-diagnostic-handler.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
; COM: Test that the default behavior persists (the llc-specific handler prints all errors).
; RUN: not llc -mtriple=amdgcn -verify-machineinstrs=0 -global-isel=false < %s 2>&1 | FileCheck -check-prefix=ALL-ERRORS %s
; COM: Do not halt on the first error when the llc-specific handler is not loaded.
; RUN: not llc -mtriple=amdgcn -verify-machineinstrs=0 -global-isel=false -no-diag-handler -halt-on-first-diag-error=none < %s 2>&1 | FileCheck -check-prefix=ALL-ERRORS %s

; COM: Now halt on the first error by disabling the llc-specific handler and test the different halt actions
; RUN: not llc -mtriple=amdgcn -verify-machineinstrs=0 -global-isel=false -no-diag-handler -halt-on-first-diag-error=exit < %s 2>&1 | FileCheck -check-prefix=FIRST-ERROR %s
; COM: Same error message as in -halt-on-first-diag-error=exit but with a crash.
; RUN: not --crash llc -mtriple=amdgcn -verify-machineinstrs=0 -global-isel=false -no-diag-handler -halt-on-first-diag-error=abort < %s 2>&1 | FileCheck -check-prefix=FIRST-ERROR %s

; ALL-ERRORS: error: <unknown>:0:0: in function illegal_vgpr_to_sgpr_copy_i32 void (): illegal VGPR to SGPR copy
; FIRST-ERROR: error: <unknown>:0:0: in function illegal_vgpr_to_sgpr_copy_i32 void (): illegal VGPR to SGPR copy
define amdgpu_kernel void @illegal_vgpr_to_sgpr_copy_i32() #0 {
%vgpr = call i32 asm sideeffect "; def $0", "=${v1}"()
call void asm sideeffect "; use $0", "${s9}"(i32 %vgpr)
ret void
}

; ALL-ERRORS: error: <unknown>:0:0: in function illegal_vgpr_to_sgpr_copy_v2i32 void (): illegal VGPR to SGPR copy
; FIRST-ERROR-NOT: error: <unknown>:0:0: in function illegal_vgpr_to_sgpr_copy_v2i32 void (): illegal VGPR to SGPR copy
define amdgpu_kernel void @illegal_vgpr_to_sgpr_copy_v2i32() #0 {
%vgpr = call <2 x i32> asm sideeffect "; def $0", "=${v[0:1]}"()
call void asm sideeffect "; use $0", "${s[10:11]}"(<2 x i32> %vgpr)
ret void
}
11 changes: 9 additions & 2 deletions llvm/tools/llc/llc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,11 @@ static cl::opt<std::string> PassPipeline(
static cl::alias PassPipeline2("p", cl::aliasopt(PassPipeline),
cl::desc("Alias for -passes"));

static cl::opt<bool>
NoDiagHandler("no-diag-handler",
cl::desc("Do not load the llc-specific diagnostic handler."),
cl::init(false), cl::Hidden);

namespace {

std::vector<std::string> &getRunPassNames() {
Expand Down Expand Up @@ -384,8 +389,10 @@ int main(int argc, char **argv) {
LLVMContext Context;
Context.setDiscardValueNames(DiscardValueNames);

// Set a diagnostic handler that doesn't exit on the first error
Context.setDiagnosticHandler(std::make_unique<LLCDiagnosticHandler>());
if (!NoDiagHandler) {
// Set a diagnostic handler that doesn't exit on the first error
Context.setDiagnosticHandler(std::make_unique<LLCDiagnosticHandler>());
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you tried setting a DiagHandlerCallback ?

Your current proposal adds 2 options: one for everyone linking with LLVMContext, and another llc specific.

You could just keep one that is llc-specific (like, abort-on-error or something like that).
The idea is to keep the LLCDiagnosticHandler, but call:

if(AbortOnError) {
  Context.setDiagnosticHandlerCallBack(
    [](const DiagnosticInfo*, void*) { abort(1); },
    nullptr);
}

When an error is diagnosed, it would go through LLCDiagnosticHandler::handleDiagnostics which calls DiagnosticHandler::handleDiagnostics, which calls DiagnosticHandler::DiagHandlerCallback if it is setted, which in our case will call abort(1).

That would reduce the surface of the change to achieve the behavior you want.

Copy link
Contributor

Choose a reason for hiding this comment

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

Unrelated to this, it's weird that LLCDiagnosticHandler::handleDiagnostics calls DiagnosticHandler::handleDiagnostics but ignores its result (if the diagnostic was handled by the callback), which is always true if the callback is set.


Expected<std::unique_ptr<ToolOutputFile>> RemarksFileOrErr =
setupLLVMOptimizationRemarks(Context, RemarksFilename, RemarksPasses,
Expand Down