[llvm-commits] [LLVMdev] [PATCH] Allow per-thread re-direction of outs()/errs()

[Eli Friedman]

On Mon, Jun 4, 2012 at 1:48 PM, Justin Holewinski
<justin.holewinski at gmail.com> wrote:
> On Mon, Jun 4, 2012 at 11:46 AM, Eli Friedman <eli.friedman at gmail.com>
> wrote:
>>
>> On Sun, Jun 3, 2012 at 7:12 PM, Justin Holewinski
>> <justin.holewinski at gmail.com> wrote:
>> > On Sun, Jun 3, 2012 at 4:15 PM, Hal Finkel <hfinkel at anl.gov> wrote:
>> >>
>> >> On Sun, 03 Jun 2012 12:12:06 -0700
>> >> Chris Lattner <clattner at apple.com> wrote:
>> >>
>> >> >
>> >> > On Jun 2, 2012, at 11:01 AM, Mikael Lyngvig wrote:
>> >> >
>> >> > > If I may add my two cents:
>> >> > >
>> >> > > I am planning to use LLVM as the backend for a compiler I am
>> >> > > working on.  And I wholeheartedly agree with Justin that it is a
>> >> > > problem, if LLVM is allowed to freely write to stdout and stderr as
>> >> > > it is a component which can be used in all sorts of code, be it a
>> >> > > GUI IDE, a CLI driver, or whatever.
>> >> >
>> >> > LLVM should not be doing this.
>> >> >
>> >> > > Also, I have a number of times wondered about the somewhat unusual
>> >> > > use of error strings in LLVM (that you pass in a string, which can
>> >> > > be assigned a descriptive error message).  Better would be, IMHO,
>> >> > > to provide an interface along the lines of this:
>> >> > >
>> >> > > class ErrorHandler
>> >> > > {
>> >> > > public:
>> >> > >         virtual void Report(ErrorKind kind, uint code, const
>> >> > > Position &position, const unichar *text, ...) = 0; };
>> >> > >
>> >> > > And then let the client, i.e. the frontend, derive from it and
>> >> > > handle all the output in whichever way it wants to.  The above
>> >> > > example is probably too complex for LLVM, but that's what I use in
>> >> > > my compiler.  ErrorKind is ErrorKind_Fatal, ErrorKind_Error, etc.
>> >> > > Position is simply a filename, a line number, and a character
>> >> > > position.  unichar is either char or wchar_t, depending on the
>> >> > > build mode and target environment.
>> >> >
>> >> > You're right, this would be better.  We have even already have it :)
>> >> >
>> >> >   llvm/Support/ErrorHandling.h
>> >>
>> >> This seems to only handle fatal errors. If that's correct, it will
>> >> probably need to be extended to handle non-fatal errors, warnings,
>> >> suggestions, notes, etc.
>> >
>> >
>> > Okay, it looks like the combination of llvm/Support/ErrorHandling.h and
>> > LLVMContext::emitError solves a part of my use case.  I found the
>> > CrashRecoveryContext class, which seems to allow me to intercept the
>> > report_fatal_error() calls by installing a custom handler that writes to
>> > a
>> > custom stream and then calls abort() instead of the default exit().
>> >  Alternatively, it looks like I can just spawn a thread for the LLVM
>> > work
>> > and call pthread_exit() instead of abort() in the error handler, which
>> > does
>> > not require messing with the process signal handlers.  Is there any
>> > reason
>> > to prefer one over the other (thread vs. signal)?
>> >
>> > This leaves the following issues:
>> >
>> > Why is the error handling stuff in LLVMContext tied to inline asm?  The
>> > error handler setter is even called setInlineAsmDiagnosticHandler.  Can
>> > we
>> > make this more generic?  I glossed right over the emitError methods the
>> > last
>> > time I read through this stuff because I saw "InlineAsm" everywhere and
>> > figured it wasn't the right tool for the job! :)
>>
>> Inline asm is the only place where we have a way of mapping form LLVM
>> IR to frontend source locations at the moment.
>
>
> But is that strictly required?  Lacking the right annotations, we could
> print IR information like function name, basic block name, global variable
> name, etc.

Basic block names are useless to the user.  Function/global names are
not really ideal, but could be reasonable depending on what you're
doing.

>> > As Hal points out, this only covers errors at the moment.  Is there any
>> > resistance to changing emitError to emitDiagnostic and having a
>> > SourceMgr::DiagKind argument?  This should cover the case where a pass
>> > wants
>> > to emit a warning or info message that is not part to DEBUG.
>> >
>> > With LLVMContext::emitDiagnostic, this should cover my use case.
>>
>> I'm not exactly opposed, but what is your use case?
>
>
> Generic warnings and notes, basically any case where compilation can
> continue but the results may be sub-optimal.  For example, this could be
> used by the vectorizer to provide a report on which loops were vectorized,
> and why the others were not.  Backends could use this to report when a
> certain feature is unavailable for the target and software emulation is
> used.  Think of Intel/PGI C/C++/Fortran and their performance notes.
>  Statistics output can give you some of this information, but not all.  Nor
> can it put it into context if you happen to have debug information for
> source level mapping.

Okay.  It seems really important to be able to point at the loop in
question for loop optimization; for other cases, pointing at the
containing function seems reasonable.

-Eli