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