[PATCH] Add warning capabilities in LLVM (backend part)
Quentin Colombet
qcolombet at apple.com
Fri Nov 15 10:46:18 PST 2013
On Nov 15, 2013, at 10:34 AM, Quentin Colombet <qcolombet at apple.com> wrote:
> On Nov 15, 2013, at 10:28 AM, Tom Stellard <tom at stellard.net> wrote:
>
>> On Fri, Nov 15, 2013 at 10:17:11AM -0800, Quentin Colombet wrote:
>>> Hi dblaikie, echristo, rengolin, chandlerc,
>>>
>>> Hi,
>>>
>>> This patch implements the proposal discussed a few months ago (http://lists.cs.uiuc.edu/pipermail/llvmdev/2013-July/063845.html) regarding adding warning capabilities in LLVM.
>>>
>>> //// Overview ////
>>>
>>> The patch adds a new LLVMContext::report that can be used to communicate to the front-end, if any, that something of interest happened.
>>> The report contains the following information:
>>> - The kind of the report: What this is this about.
>>> - The severity of the report: How bad this is.
>>> - The default message to print: What should be reported.
>>> - A data field: Some meta information.
>>>
>>> The data field should be used to communicate all sort of data to the front-end. Ultimately, this field should provide enough information to the front-end so that it could build its own diagnostic if it does not want to print the default message.
>>> This means that both the back-end and the front-end should agree on the structure of this field based on the pair kind/severity.
>>> This feature is not stressed by this patch.
>>>
>>> This patch introduces a new ReportHandlerTy and a new ReportContext that should be set by the front-end to be able to map these reports in its own diagnostics system.
>>
>> I like this feature, thanks for working on it. Could it be used as
>> a replacement for report_fatal_error() ?
> Thanks for asking, this is a great point.
>
> I thought about that and I believe it can, but not everywhere.
> Indeed, report_fatal_error does not require a LLVMContext, whereas this approaches does.
> Moreover, report_fatal_error does not return, whereas LLVMContext::report may return based on the ferror-limit set in the front-end.
Note: for this specific point, we could add a new severity: RS_FatalError.
>
> Therefore, we should look into each case to see if it is suitable to switch to this report.
>
> -Quentin
>>
>> -Tom
>>
>>>
>>> //// Next Steps ////
>>>
>>> - Send the patch for clang (I have it on hold).
>>> - Switch to this reporting for all warnings printing happening in the backend.
>>> - Switch the InlineAsmDiagHandler to the new reporting mechanism. This involves a bit of work, especially regarding the meta data.
>>>
>>> Thanks for your reviews.
>>>
>>> Cheers,
>>> Quentin
>>>
>>> PS: Sorry for the delay, it took my ages to get to this point in my todolist.
>>>
>>> http://llvm-reviews.chandlerc.com/D2190
>>>
>>> Files:
>>> include/llvm/IR/LLVMContext.h
>>> lib/CodeGen/PrologEpilogInserter.cpp
>>> lib/IR/LLVMContext.cpp
>>> lib/IR/LLVMContextImpl.cpp
>>> lib/IR/LLVMContextImpl.h
>>
>>> Index: include/llvm/IR/LLVMContext.h
>>> ===================================================================
>>> --- include/llvm/IR/LLVMContext.h
>>> +++ include/llvm/IR/LLVMContext.h
>>> @@ -52,6 +52,22 @@
>>> MD_invariant_load = 6 // "invariant.load"
>>> };
>>>
>>> + /// Defines the different supported kind of report.
>>> + /// \see LLVMContext::report.
>>> + enum ReportKind {
>>> + RK_InlineAsm,
>>> + RK_StackSize,
>>> + RK_Other
>>> + };
>>> +
>>> + /// Defines the different supported severity of a report.
>>> + /// \see LLVMContext::report.
>>> + enum ReportSeverity {
>>> + RS_Error,
>>> + RS_Warning,
>>> + RS_Note
>>> + };
>>> +
>>> /// getMDKindID - Return a unique non-zero ID for the specified metadata kind.
>>> /// This ID is uniqued across modules in the current LLVMContext.
>>> unsigned getMDKindID(StringRef Name) const;
>>> @@ -64,6 +80,14 @@
>>> typedef void (*InlineAsmDiagHandlerTy)(const SMDiagnostic&, void *Context,
>>> unsigned LocCookie);
>>>
>>> + /// Defines the type of a report handler.
>>> + /// \see LLVMContext::setReportHandler.
>>> + /// \see LLVMContext::report.
>>> + typedef void (*ReportHandlerTy)(void *Context, ReportKind Kind,
>>> + StringRef StringData,
>>> + ReportSeverity Severity,
>>> + StringRef Message);
>>> +
>>> /// setInlineAsmDiagnosticHandler - This method sets a handler that is invoked
>>> /// when problems with inline asm are detected by the backend. The first
>>> /// argument is a function pointer and the second is a context pointer that
>>> @@ -82,6 +106,40 @@
>>> /// setInlineAsmDiagnosticHandler.
>>> void *getInlineAsmDiagnosticContext() const;
>>>
>>> + /// setReportHandler - This method sets a handler that is invoked
>>> + /// when the backend needs to report anything to the user. The first
>>> + /// argument is a function pointer and the second is a context pointer that
>>> + /// gets passed into the ReportHandler.
>>> + ///
>>> + /// LLVMContext doesn't take ownership or interpret either of these
>>> + /// pointers.
>>> + void setReportHandler(ReportHandlerTy ReportHandler, void *ReportContext = 0);
>>> +
>>> + /// getReportHandler - Return the diagnostic handler set by
>>> + /// setReportHandler.
>>> + ReportHandlerTy getReportHandler() const;
>>> +
>>> + /// getReportContext - Return the diagnostic context set by
>>> + /// setReportContext.
>>> + void *getReportContext() const;
>>> +
>>> + /// report - Report a message to the currently installed report handler.
>>> + /// This function returns, in particular in the case of error reporting
>>> + /// (Severity == RS_Error), so code should be prepared to drop the
>>> + /// erroneous construct on the floor and "not crash".
>>> + /// The generated code need not be correct.
>>> + /// \p Kind is used to precise the kind of report this is about.
>>> + /// \p StringData can be used to pass specific information to the report
>>> + /// handler, so that it can create its own specific message.
>>> + /// \p Severity gives the severity of the report.
>>> + /// \p Message is the default message to be printed. It is used when the
>>> + /// report handler is not capable to take advantages of \p StringData.
>>> + /// The reported message will be implicitly prefixed with the severity
>>> + /// keyword, i.e., "error: " for RS_Error, "warning: " for RS_Warning,
>>> + /// and "note: " for RS_Note.
>>> + /// The message should not end with a ".".
>>> + void report(ReportKind Kind, StringRef StringData,
>>> + ReportSeverity Severity, StringRef Message);
>>>
>>> /// emitError - Emit an error message to the currently installed error handler
>>> /// with optional location information. This function returns, so code should
>>> Index: lib/CodeGen/PrologEpilogInserter.cpp
>>> ===================================================================
>>> --- lib/CodeGen/PrologEpilogInserter.cpp
>>> +++ lib/CodeGen/PrologEpilogInserter.cpp
>>> @@ -30,6 +30,7 @@
>>> #include "llvm/CodeGen/MachineRegisterInfo.h"
>>> #include "llvm/CodeGen/RegisterScavenging.h"
>>> #include "llvm/IR/InlineAsm.h"
>>> +#include "llvm/IR/LLVMContext.h"
>>> #include "llvm/Support/CommandLine.h"
>>> #include "llvm/Support/Compiler.h"
>>> #include "llvm/Support/Debug.h"
>>> @@ -160,10 +161,19 @@
>>>
>>> // Warn on stack size when we exceeds the given limit.
>>> MachineFrameInfo *MFI = Fn.getFrameInfo();
>>> + uint64_t StackSize = MFI->getStackSize();
>>> if (WarnStackSize.getNumOccurrences() > 0 &&
>>> - WarnStackSize < MFI->getStackSize())
>>> - errs() << "warning: Stack size limit exceeded (" << MFI->getStackSize()
>>> - << ") in " << Fn.getName() << ".\n";
>>> + WarnStackSize < StackSize) {
>>> + std::string DataStorage;
>>> + raw_string_ostream Data(DataStorage);
>>> + Data << StackSize;
>>> +
>>> + std::string MsgStorage("Stack size limit exceeded (");
>>> + raw_string_ostream Msg(MsgStorage);
>>> + Msg << StackSize << ") in " << Fn.getName();
>>> + F->getContext().report(LLVMContext::RK_StackSize, Data.str(),
>>> + LLVMContext::RS_Warning, Msg.str());
>>> + }
>>>
>>> delete RS;
>>> ReturnBlocks.clear();
>>> Index: lib/IR/LLVMContext.cpp
>>> ===================================================================
>>> --- lib/IR/LLVMContext.cpp
>>> +++ lib/IR/LLVMContext.cpp
>>> @@ -98,6 +98,22 @@
>>> return pImpl->InlineAsmDiagContext;
>>> }
>>>
>>> +void LLVMContext::setReportHandler(ReportHandlerTy ReportHandler,
>>> + void *ReportContext) {
>>> + pImpl->ReportHandler = ReportHandler;
>>> + pImpl->ReportContext = ReportContext;
>>> +}
>>> +
>>> +/// getReportHandler - Return the diagnostic handler set by
>>> +/// setReportHandler.
>>> +LLVMContext::ReportHandlerTy LLVMContext::getReportHandler() const {
>>> + return pImpl->ReportHandler;
>>> +}
>>> +
>>> +/// getReportContext - Return the diagnostic context set by
>>> +/// setReportContext.
>>> +void *LLVMContext::getReportContext() const { return pImpl->ReportContext; }
>>> +
>>> void LLVMContext::emitError(const Twine &ErrorStr) {
>>> emitError(0U, ErrorStr);
>>> }
>>> @@ -112,6 +128,29 @@
>>> return emitError(LocCookie, ErrorStr);
>>> }
>>>
>>> +void LLVMContext::report(ReportKind Kind, StringRef StringData,
>>> + ReportSeverity Severity,
>>> + StringRef Message) {
>>> + // If there is a report handler, use it.
>>> + if (pImpl->ReportHandler != 0) {
>>> + pImpl->ReportHandler(pImpl->ReportContext, Kind, StringData, Severity,
>>> + Message);
>>> + return;
>>> + }
>>> + // Otherwise, print the message with a prefix based on the severity.
>>> + switch (Severity) {
>>> + case RS_Error:
>>> + errs() << "error: " << Message << "\n";
>>> + exit(1);
>>> + case RS_Warning:
>>> + errs() << "warning: " << Message << "\n";
>>> + break;
>>> + case RS_Note:
>>> + errs() << "note: " << Message << "\n";
>>> + break;
>>> + }
>>> +}
>>> +
>>> void LLVMContext::emitError(unsigned LocCookie, const Twine &ErrorStr) {
>>> // If there is no error handler installed, just print the error and exit.
>>> if (pImpl->InlineAsmDiagHandler == 0) {
>>> Index: lib/IR/LLVMContextImpl.cpp
>>> ===================================================================
>>> --- lib/IR/LLVMContextImpl.cpp
>>> +++ lib/IR/LLVMContextImpl.cpp
>>> @@ -37,6 +37,8 @@
>>> Int64Ty(C, 64) {
>>> InlineAsmDiagHandler = 0;
>>> InlineAsmDiagContext = 0;
>>> + ReportHandler = 0;
>>> + ReportContext = 0;
>>> NamedStructTypesUniqueID = 0;
>>> }
>>>
>>> Index: lib/IR/LLVMContextImpl.h
>>> ===================================================================
>>> --- lib/IR/LLVMContextImpl.h
>>> +++ lib/IR/LLVMContextImpl.h
>>> @@ -238,8 +238,11 @@
>>>
>>> LLVMContext::InlineAsmDiagHandlerTy InlineAsmDiagHandler;
>>> void *InlineAsmDiagContext;
>>> -
>>> - typedef DenseMap<DenseMapAPIntKeyInfo::KeyTy, ConstantInt*,
>>> +
>>> + LLVMContext::ReportHandlerTy ReportHandler;
>>> + void *ReportContext;
>>> +
>>> + typedef DenseMap<DenseMapAPIntKeyInfo::KeyTy, ConstantInt *,
>>> DenseMapAPIntKeyInfo> IntMapTy;
>>> IntMapTy IntConstants;
>>>
>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131115/b530ecde/attachment.html>
More information about the llvm-commits
mailing list