<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>On Apr 4, 2014, at 11:18 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, Apr 04, 2014 at 10:59:11AM -0700, Eric Christopher wrote:<br><blockquote type="cite">I'd very much prefer not to expose the severity for now. This is a<br>fairly well thought out, but very new interface and we guarantee<br>stability of the C interface so we'd be locking the current severity<br>interface down.<br><br>I'm not, honestly, a huge fan of opening this up to the C API in<br>general - is there a good reason to do so?<br><br></blockquote><br>We would like to use this in Mesa so we can report backend compiler<br>errors to the graphics drivers. The most important use case is reporting<br>when the verifier detects an illegal instruction, because executing<br>illegal instructions has a good chance of locking up the system.<br><br>We'd also like to query the severity level, but we really only care about Errors<br>and not the others. In fact we probably could just use the severity and not<br>worry about the error message. What if we limited the C API to just:<br><br>LLVMContextSetDiagnosticHandler(LLVMContextRef C, LLVMDiagnosticHandler Handler,<br> void *DiagnosticContext);<br>/// @returns true if DiagnosticSeverity is DS_Error and false otherwise.<br>bool LLVMDiagInfoHasError(LLVMDiagnosticInfoRef DI);<br><br>Would this work?<br></div></blockquote><div><br></div><div>FWIW, the C API of lto has the severity thing.</div><div><br></div><div>The diagnostic handler is slightly different too and deal with string and severity:</div><div><div style="margin: 0px; font-size: 11px; font-family: Menlo;">/**</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;"> * Diagnostic severity.</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;"> *</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;"> * \since LTO_API_VERSION=7</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;"> */</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;">typedef enum {</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;"> LTO_DS_ERROR = 0,</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;"> LTO_DS_WARNING = 1,</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;"> LTO_DS_REMARK = 3, // Added in LTO_API_VERSION=10.</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;"> LTO_DS_NOTE = 2</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;">} lto_codegen_diagnostic_severity_t;</div><div style="margin: 0px; font-size: 11px; font-family: Menlo; min-height: 13px;"><br></div><div style="margin: 0px; font-size: 11px; font-family: Menlo;">/**</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;"> * Diagnostic handler type.</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;"> * \p severity defines the severity.</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;"> * \p diag is the actual diagnostic.</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;"> * The diagnostic is not prefixed by any of severity keyword, e.g., 'error: '.</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;"> * \p ctxt is used to pass the context set with the diagnostic handler.</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;"> *</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;"> * \since LTO_API_VERSION=7</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;"> */</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;">typedef void (*lto_diagnostic_handler_t)(</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;"> lto_codegen_diagnostic_severity_t severity, const char *diag, void *ctxt);</div><div style="margin: 0px;"><br></div><div style="margin: 0px;">-Quentin</div></div><br><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>On Fri, Apr 4, 2014 at 10:39 AM, Quentin Colombet <<a href="mailto:qcolombet@apple.com">qcolombet@apple.com</a>> wrote:<br><blockquote type="cite">Hi Darren,<br><br>This LGTM with two comments:<br>- LLVMPrintDiagInfoToString should state that the string has to be freed<br>(see inline comment).<br>- Should we provide an API to expose the severity as well? What would do the<br>users of LLVMPrintDiagInfoToString if we do not?<br><br>Note that I'm not super familiar with the C API, so you may want to wait for<br>another review.<br><br>Thanks,<br>-Quentin<br><br>On Apr 2, 2014, at 8:32 PM, Tom Stellard <<a href="mailto:tom@stellard.net">tom@stellard.net</a>> wrote:<br><br>From: Darren Powell <<a href="mailto:darren.powell@amd.com">darren.powell@amd.com</a>><br><br>---<br>include/llvm-c/Core.h | 20 ++++++++++++++++++++<br>include/llvm/IR/DiagnosticInfo.h | 3 +++<br>lib/IR/Core.cpp | 20 ++++++++++++++++++++<br>3 files changed, 43 insertions(+)<br><br>diff --git a/include/llvm-c/Core.h b/include/llvm-c/Core.h<br>index 50c5e3a..17367ff 100644<br>--- a/include/llvm-c/Core.h<br>+++ b/include/llvm-c/Core.h<br>@@ -124,6 +124,12 @@ typedef struct LLVMOpaquePassRegistry<br>*LLVMPassRegistryRef;<br>* @see llvm::Use */<br>typedef struct LLVMOpaqueUse *LLVMUseRef;<br><br>+<br>+/**<br>+ * @see llvm::DiagnosticInfo<br>+ */<br>+typedef struct LLVMOpaqueDiagnosticInfo *LLVMDiagnosticInfoRef;<br>+<br>typedef enum {<br> LLVMZExtAttribute = 1<<0,<br> LLVMSExtAttribute = 1<<1,<br>@@ -453,6 +459,8 @@ void LLVMEnablePrettyStackTrace(void);<br>* @{<br>*/<br><br>+typedef void (*LLVMDiagnosticHandler)(LLVMDiagnosticInfoRef, void *);<br>+<br>/**<br>* Create a new context.<br>*<br>@@ -467,6 +475,13 @@ LLVMContextRef LLVMContextCreate(void);<br>LLVMContextRef LLVMGetGlobalContext(void);<br><br>/**<br>+ * Set the diagnostic handler for this context.<br>+ */<br>+void LLVMContextSetDiagnosticHandler(LLVMContextRef C,<br>+ LLVMDiagnosticHandler Handler,<br>+ void *DiagnosticContext);<br>+<br>+/**<br>* Destroy a context instance.<br>*<br>* This should be called for every call to LLVMContextCreate() or memory<br>@@ -474,6 +489,11 @@ LLVMContextRef LLVMGetGlobalContext(void);<br>*/<br>void LLVMContextDispose(LLVMContextRef C);<br><br>+/**<br>+ * Extract string description from DiagnosticInfo.<br><br><br>Add a comment here saying that the string should be freed using<br>LLVMDisposeMessage.<br><br>+ */<br>+char *LLVMPrintDiagInfoToString(LLVMDiagnosticInfoRef DI);<br>+<br>unsigned LLVMGetMDKindIDInContext(LLVMContextRef C, const char* Name,<br> unsigned SLen);<br>unsigned LLVMGetMDKindID(const char* Name, unsigned SLen);<br>diff --git a/include/llvm/IR/DiagnosticInfo.h<br>b/include/llvm/IR/DiagnosticInfo.h<br>index de2a9d8..5fd6636 100644<br>--- a/include/llvm/IR/DiagnosticInfo.h<br>+++ b/include/llvm/IR/DiagnosticInfo.h<br>@@ -193,6 +193,9 @@ public:<br> }<br>};<br><br>+// Create wrappers for C Binding types (see CBindingWrapping.h).<br>+DEFINE_SIMPLE_CONVERSION_FUNCTIONS(DiagnosticInfo, LLVMDiagnosticInfoRef)<br>+<br>} // End namespace llvm<br><br>#endif<br>diff --git a/lib/IR/Core.cpp b/lib/IR/Core.cpp<br>index 4f3d8b1..e3b70c7 100644<br>--- a/lib/IR/Core.cpp<br>+++ b/lib/IR/Core.cpp<br>@@ -17,6 +17,8 @@<br>#include "llvm/IR/Attributes.h"<br>#include "llvm/IR/CallSite.h"<br>#include "llvm/IR/Constants.h"<br>+#include "llvm/IR/DiagnosticInfo.h"<br>+#include "llvm/IR/DiagnosticPrinter.h"<br>#include "llvm/IR/DerivedTypes.h"<br>#include "llvm/IR/GlobalAlias.h"<br>#include "llvm/IR/GlobalVariable.h"<br>@@ -76,6 +78,14 @@ LLVMContextRef LLVMGetGlobalContext() {<br> return wrap(&getGlobalContext());<br>}<br><br>+void LLVMContextSetDiagnosticHandler(LLVMContextRef C,<br>+ LLVMDiagnosticHandler Handler,<br>+ void *DiagnosticContext) {<br>+ unwrap(C)->setDiagnosticHandler(<br>+ LLVM_EXTENSION<br>reinterpret_cast<LLVMContext::DiagnosticHandlerTy>(Handler),<br>+ DiagnosticContext);<br>+}<br>+<br>void LLVMContextDispose(LLVMContextRef C) {<br> delete unwrap(C);<br>}<br>@@ -89,6 +99,16 @@ unsigned LLVMGetMDKindID(const char* Name, unsigned SLen)<br>{<br> return LLVMGetMDKindIDInContext(LLVMGetGlobalContext(), Name, SLen);<br>}<br><br>+char *LLVMPrintDiagInfoToString(LLVMDiagnosticInfoRef DI) {<br>+ std::string MsgStorage;<br>+ raw_string_ostream Stream(MsgStorage);<br>+ DiagnosticPrinterRawOStream DP(Stream);<br>+<br>+ unwrap(DI)->print(DP);<br>+ Stream.flush();<br>+<br>+ return strdup(MsgStorage.c_str());<br>+}<br><br>/*===-- Operations on modules<br>---------------------------------------------===*/<br><br>--<br>1.8.3.2<br><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits<br><br><br><br>_______________________________________________<br>llvm-commits mailing list<br>llvm-commits@cs.uiuc.edu<br>http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits<br><br></blockquote>_______________________________________________<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></body></html>