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

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.
> 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?

-Eli

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.

>
> > 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.

>
> -Eli
>


-- 

Thanks,

Justin Holewinski
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20120604/7b0a4f01/attachment.html>

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