Hi hfinkel,
Hi,
Here is a patch that propagates the backend diagnostics into the frontend
diagnostics.
** Context **
In r197508, we added in the LLVMContext a diagnostic handler hook to report
errors, warnings, and notes that are issued by the backend
(http://llvm-reviews.chandlerc.com/D2376).
Currently, there are three kind of diagnostic:
- Inline asm: The existing inline asm errors, that have been extended to
warnings and notes.
- Stack size: The existing backend warning that used to wildly print on stderr
a warning when the stack size exceeds a given size.
- Plugin: A generic kind that allows arbitrary diagnostic reporting.
** Overall Design **
With this new hook, the backend passes the diagnostic information via
DiagnosticInfo class.
The base class gives two basic information:
- The severity.
- The kind.
Based on these information (Severity, Kind), the frontend can decide to either:
- Use the related sub class (identify by Kind) and build its own diagnostic.
or
- Use the default print method that is a fall back in case the frontend does
not know how to deal with this specific diagnostic.
The print method takes a DiagnosticPrinter class as argument. This helper class
can be tuned in the frontend to format appropriately some important cases. A
default implementation using raw_ostream is available.
Note: Backend diagnostics are supposed to be rarely added. Thus, the patch does
not feature any support for adding a new backend diagnostic. This remains a
manual task.
** Proposed Patch **
- Add a diagnostic handler matching the new hook in BackendConsumer
(BackendConsumer::DiagnosticHandler).
- Register this handler in the LLVMContext.
- Add a new frontend diagnostic kind (backend-<kind>) for each backend kind
(inline-asm, stack-size, plugin).
- Format and propagate the backend diagnostics to the newly added frontend
diagnostics (BackendConsumer::InlineAsmDiagHandler and
BackendConsumer::StackSizeHandler).
- Fall back to the print method for plugin diagnostics (as part of
BackendConsumer::DiagnosticHandler2).
** What Is Missing? **
- Dedicated DiagnosticPrinter.
This patch does not roll a dedicated DiagnosticPrinter. I thought a frontend
guy may be better suited for this task if we want to supply this support.
- Demangling of function name.
It would be nice to demangle the function name as illustrated by the fix-me in
BackendConsumer::StackSizeHandler. I have not idea how I should do that.
- Warning with argument.
The stack size warning requires that we specify the limit that will trigger the
diagnostic. Currently, this limit is still set using -llvm
-warn-stack-size=<limit>. As far as I know, we do not have a way to specify an
option to a warning, e.g., -WmyWarning=<limit>. Therefore, this weirdness is
not corrected by this patch.
The bottom line is, to turn a stack size warning into an error, one has to set
-mlvm -warn-stack-size=<limit> -Werror=backend-stack-size (or -Werror).
Thanks for your feedbacks.
Cheers,
-Quentin
http://llvm-reviews.chandlerc.com/D2430
Files:
include/clang/Basic/DiagnosticCategories.td
include/clang/Basic/DiagnosticFrontendKinds.td
include/clang/Basic/DiagnosticGroups.td
lib/CodeGen/CodeGenAction.cpp
test/Frontend/backend-diagnostic.c
Index: include/clang/Basic/DiagnosticCategories.td
===================================================================
--- include/clang/Basic/DiagnosticCategories.td
+++ include/clang/Basic/DiagnosticCategories.td
@@ -8,3 +8,4 @@
//===----------------------------------------------------------------------===//
class CatInlineAsm : DiagCategory<"Inline Assembly Issue">;
+class CatBackend : DiagCategory<"Backend Issue">;
Index: include/clang/Basic/DiagnosticFrontendKinds.td
===================================================================
--- include/clang/Basic/DiagnosticFrontendKinds.td
+++ include/clang/Basic/DiagnosticFrontendKinds.td
@@ -16,11 +16,19 @@
// Error generated by the backend.
def err_fe_inline_asm : Error<"%0">, CatInlineAsm;
+def warn_fe_inline_asm : Warning<"%0">, CatInlineAsm, InGroup<BackendInlineAsm>;
+def note_fe_inline_asm : Note<"%0">, CatInlineAsm;
def note_fe_inline_asm_here : Note<"instantiated into assembly here">;
def err_fe_cannot_link_module : Error<"cannot link module '%0': %1">,
DefaultFatal;
-
+def warn_fe_backend_stack_size: Warning<"stack size exceeded (%0) in %1">, CatBackend, InGroup<BackendStackSize>;
+def err_fe_backend_stack_size: Error<"%0">, CatBackend;
+def note_fe_backend_stack_size: Note<"%0">, CatBackend;
+
+def warn_fe_backend_plugin: Warning<"%0">, CatBackend, InGroup<BackendPlugin>;
+def err_fe_backend_plugin: Error<"%0">, CatBackend;
+def note_fe_backend_plugin: Note<"%0">, CatBackend;
def err_fe_invalid_code_complete_file : Error<
"cannot locate code-completion file %0">, DefaultFatal;
Index: include/clang/Basic/DiagnosticGroups.td
===================================================================
--- include/clang/Basic/DiagnosticGroups.td
+++ include/clang/Basic/DiagnosticGroups.td
@@ -618,3 +618,8 @@
// OpenMP warnings.
def SourceUsesOpenMP : DiagGroup<"source-uses-openmp">;
+
+// Backend warnings.
+def BackendInlineAsm : DiagGroup<"backend-inline-asm">;
+def BackendStackSize : DiagGroup<"backend-stack-size">;
+def BackendPlugin : DiagGroup<"backend-plugin">;
Index: lib/CodeGen/CodeGenAction.cpp
===================================================================
--- lib/CodeGen/CodeGenAction.cpp
+++ lib/CodeGen/CodeGenAction.cpp
@@ -21,6 +21,8 @@
#include "llvm/ADT/OwningPtr.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/Bitcode/ReaderWriter.h"
+#include "llvm/IR/DiagnosticInfo.h"
+#include "llvm/IR/DiagnosticPrinter.h"
#include "llvm/IR/LLVMContext.h"
#include "llvm/IR/Module.h"
#include "llvm/IRReader/IRReader.h"
@@ -158,10 +160,17 @@
void *OldContext = Ctx.getInlineAsmDiagnosticContext();
Ctx.setInlineAsmDiagnosticHandler(InlineAsmDiagHandler, this);
+ LLVMContext::DiagnosticHandlerTy OldDiagnosticHandler =
+ Ctx.getDiagnosticHandler();
+ void *OldDiagnosticContext = Ctx.getDiagnosticContext();
+ Ctx.setDiagnosticHandler(DiagnosticHandler, this);
+
EmitBackendOutput(Diags, CodeGenOpts, TargetOpts, LangOpts,
TheModule.get(), Action, AsmOutStream);
Ctx.setInlineAsmDiagnosticHandler(OldHandler, OldContext);
+
+ Ctx.setDiagnosticHandler(OldDiagnosticHandler, OldDiagnosticContext);
}
virtual void HandleTagDeclDefinition(TagDecl *D) {
@@ -202,8 +211,23 @@
((BackendConsumer*)Context)->InlineAsmDiagHandler2(SM, Loc);
}
+ static void DiagnosticHandler(const llvm::DiagnosticInfo &DI,
+ void *Context) {
+ ((BackendConsumer *)Context)->DiagnosticHandler2(DI);
+ }
+
void InlineAsmDiagHandler2(const llvm::SMDiagnostic &,
SourceLocation LocCookie);
+
+ void DiagnosticHandler2(const llvm::DiagnosticInfo &DI);
+ /// \brief Specialized handler for InlineAsm diagnostic.
+ /// \return True if the diagnostic has been successfully reported, false
+ /// otherwise.
+ bool InlineAsmDiagHandler(const llvm::DiagnosticInfoInlineAsm &D);
+ /// \brief Specialized handler for StackSize diagnostic.
+ /// \return True if the diagnostic has been successfully reported, false
+ /// otherwise.
+ bool StackSizeDiagHandler(const llvm::DiagnosticInfoStackSize &D);
};
void BackendConsumer::anchor() {}
@@ -282,6 +306,93 @@
Diags.Report(Loc, diag::err_fe_inline_asm).AddString(Message);
}
+#define ComputeDiagID(Severity, GroupName, DiagID) \
+ do { \
+ switch (Severity) { \
+ case llvm::DS_Error: \
+ DiagID = diag::err_fe_##GroupName; \
+ break; \
+ case llvm::DS_Warning: \
+ DiagID = diag::warn_fe_##GroupName; \
+ break; \
+ case llvm::DS_Note: \
+ DiagID = diag::note_fe_##GroupName; \
+ break; \
+ } \
+ } while (false)
+
+bool
+BackendConsumer::InlineAsmDiagHandler(const llvm::DiagnosticInfoInlineAsm &D) {
+ unsigned DiagID;
+ ComputeDiagID(D.getSeverity(), inline_asm, DiagID);
+ std::string Message = D.getMsgStr().str();
+
+ // If this problem has clang-level source location information, report the
+ // issue as being a prbolem in the source with a note showing the instantiated
+ // code.
+ SourceLocation LocCookie =
+ SourceLocation::getFromRawEncoding(D.getLocCookie());
+ if (LocCookie.isValid())
+ Diags.Report(LocCookie, DiagID).AddString(Message);
+ else {
+ // Otherwise, report the backend diagnostic as occurring in the generated
+ // .s file.
+ // If Loc is invalid, we still need to report the diagnostic, it just gets
+ // no location info.
+ FullSourceLoc Loc;
+ Diags.Report(Loc, DiagID).AddString(Message);
+ }
+ // We handled all the possible severities.
+ return true;
+}
+
+bool
+BackendConsumer::StackSizeDiagHandler(const llvm::DiagnosticInfoStackSize &D) {
+ if (D.getSeverity() != llvm::DS_Warning)
+ // For now, the only support we have for StackSize diagnostic is warning.
+ // We do not know how to format other severities.
+ return false;
+
+ // FIXME: We should demangle the function name.
+ // FIXME: Is there a way to get a location for that function?
+ FullSourceLoc Loc;
+ Diags.Report(Loc, diag::warn_fe_backend_stack_size)
+ << D.getStackSize() << D.getFunction().getName();
+ return true;
+}
+
+/// \brief This function is invoked when the backend needs
+/// to report something to the user.
+void BackendConsumer::DiagnosticHandler2(const DiagnosticInfo &DI) {
+ FullSourceLoc Loc;
+ unsigned DiagID = diag::err_fe_inline_asm;
+ llvm::DiagnosticSeverity Severity = DI.getSeverity();
+ // Get the diagnostic ID based.
+ switch (DI.getKind()) {
+ case llvm::DK_InlineAsm:
+ if (InlineAsmDiagHandler(cast<DiagnosticInfoInlineAsm>(DI)))
+ return;
+ ComputeDiagID(Severity, inline_asm, DiagID);
+ break;
+ case llvm::DK_StackSize:
+ if (StackSizeDiagHandler(cast<DiagnosticInfoStackSize>(DI)))
+ return;
+ ComputeDiagID(Severity, backend_stack_size, DiagID);
+ break;
+ default:
+ // Plugin IDs are not bound to any value as they are set dynamically.
+ ComputeDiagID(Severity, backend_plugin, DiagID);
+ break;
+ }
+ std::string MsgStorage;
+ raw_string_ostream Stream(MsgStorage);
+ DiagnosticPrinterRawOStream DP(Stream);
+ DI.print(DP);
+ Stream.flush();
+
+ // Report the backend message using the usual diagnostic mechanism.
+ Diags.Report(Loc, DiagID).AddString(MsgStorage);
+}
//
CodeGenAction::CodeGenAction(unsigned _Act, LLVMContext *_VMContext)
Index: test/Frontend/backend-diagnostic.c
===================================================================
--- test/Frontend/backend-diagnostic.c
+++ test/Frontend/backend-diagnostic.c
@@ -0,0 +1,29 @@
+// REQUIRES: x86-registered-target
+// Play around with backend reporting:
+// _REGULAR_: Regular behavior, no warning switch enabled.
+// _PROMOTE_: Promote warning to error.
+// _IGNORE_: Drop backend warning.
+//
+// RUN: not %clang_cc1 %s -mllvm -warn-stack-size=0 -S -o - -triple=i386-apple-darwin 2> %t.err
+// RUN: FileCheck < %t.err %s --check-prefix=REGULAR --check-prefix=ASM
+// RUN: not %clang_cc1 %s -mllvm -warn-stack-size=0 -S -o - -triple=i386-apple-darwin -Werror=backend-stack-size 2> %t.err
+// RUN: FileCheck < %t.err %s --check-prefix=PROMOTE --check-prefix=ASM
+// RUN: not %clang_cc1 %s -mllvm -warn-stack-size=0 -S -o - -triple=i386-apple-darwin -Wno-backend-stack-size 2> %t.err
+// RUN: FileCheck < %t.err %s --check-prefix=IGNORE --check-prefix=ASM
+
+extern void doIt(char *);
+
+// REGULAR: warning: stack size exceeded ({{[0-9]+}}) in stackSizeWarning
+// PROMOTE: error: stack size exceeded ({{[0-9]+}}) in stackSizeWarning
+// IGNORE-NOT: stack size exceeded ({{[0-9]+}}) in stackSizeWarning
+void stackSizeWarning() {
+ char buffer[80];
+ doIt(buffer);
+}
+
+// ASM: inline assembly requires more registers than available
+void inlineAsmError(int x0, int x1, int x2, int x3, int x4,
+ int x5, int x6, int x7, int x8, int x9) {
+ __asm__("hello world": : "r" (x0),"r" (x1),"r" (x2),"r" (x3),
+ "r" (x4),"r" (x5),"r" (x6),"r" (x7),"r" (x8),"r" (x9));
+}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits