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

On Jun 1, 2012, at 3:48 PM, Justin Holewinski wrote:
> This isn't the right approach.  Nothing in the library part of the
compiler should be hard coding a stream to write to.  What are you trying to
accomplish?
> 
> There are a lot of places where warning/debug information is passed
directly to errs().  For example, take the Linker class.  You can tell it to
omit errors/warnings, but it would be preferable to let it emit the
errors/warnings to some compiler-controlled stream for message triaging.
Yes, this is broken.  The fix should be in the linker though, not in errs().
> Perhaps I'm just getting the wrong impression from the current LLVM
code base.  Take this snippet from LinkModules.cpp:
> 
>   if (!SrcM->getDataLayout().empty() &&
!DstM->getDataLayout().empty() &&
>       SrcM->getDataLayout() != DstM->getDataLayout())
>     errs() << "WARNING: Linking two modules of different data
layouts!\n";
>   if (!SrcM->getTargetTriple().empty() &&
>       DstM->getTargetTriple() != SrcM->getTargetTriple()) {
>     errs() << "WARNING: Linking two modules of different target
triples: ";
>     if (!SrcM->getModuleIdentifier().empty())
>       errs() << SrcM->getModuleIdentifier() << ":
";
>     errs() << "'" << SrcM->getTargetTriple()
<< "' and '"
>            << DstM->getTargetTriple() <<
"'\n";
>   }
> 
> Would I be correct in assuming that this is actually "bad" code
since it's a library function that writes directly to stderr?
Yes, this is extremely unfriendly to using the linker as a library.  It would be
much better for the linker to return its error in a proper way (i.e. extending
llvm/Support/system_error.h like llvm/Object/Error.h does).
> Is the recommended approach to pass a stream to the constructor of any pass
or function that wants to emit output outside of debug mode?  My concern with
this approach is that the compiler would then need to know which passes expect
an output stream, and which do not.  The supplied patch allows passes to output
information without having to worry about where the output is going, it is up to
the compiler to specify where this output goes.
The right fix for this is to fix the code to not report errors textually.

-Chris

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20120601/4c470137/attachment.html>

> From: llvmdev-bounces at cs.uiuc.edu [mailto:llvmdev-bounces at
cs.uiuc.edu] On Behalf Of Chris Lattner
> Subject: Re: [LLVMdev] [llvm-commits] [PATCH] Allow per-thread re-direction
of outs()/errs()
> It would be much better for the linker to return its error in a proper way
> (i.e. extending llvm/Support/system_error.h like llvm/Object/Error.h does).
> The right fix for this is to fix the code to not report errors textually.
Unfortunately, the use of outs() and (especially) errs() is rampant - a simple
grep of the 3.1 source tree shows about 1,500 instances.  One of the first
things we had to implement in order to make LLVM usable is something very
similar to what Justin has proposed.  Centralizing control of the output in
outs()/errs() would seem to be an ideal way to let users of LLVM as a library
customize it as they see fit without requiring massive source changes.

 - Chuck


THIS COMMUNICATION MAY CONTAIN CONFIDENTIAL AND/OR OTHERWISE PROPRIETARY
MATERIAL and is thus for use only by the intended recipient. If you received
this in error, please contact the sender and delete the e-mail and its
attachments from all computers.

On Jun 1, 2012, at 7:08 PM, Caldarale, Charles R wrote:
>> From: llvmdev-bounces at cs.uiuc.edu [mailto:llvmdev-bounces at
cs.uiuc.edu] On Behalf Of Chris Lattner
>> Subject: Re: [LLVMdev] [llvm-commits] [PATCH] Allow per-thread
re-direction of outs()/errs()
> 
>> It would be much better for the linker to return its error in a proper
way
>> (i.e. extending llvm/Support/system_error.h like llvm/Object/Error.h
does).
> 
>> The right fix for this is to fix the code to not report errors
textually.
> 
> Unfortunately, the use of outs() and (especially) errs() is rampant - a
simple grep of the 3.1 source tree shows about 1,500 instances.  One of the
first things we had to implement in order to make LLVM usable is something very
similar to what Justin has proposed.  Centralizing control of the output in
outs()/errs() would seem to be an ideal way to let users of LLVM as a library
customize it as they see fit without requiring massive source changes.
Virtually all of those are in DEBUG statements or in llvm/tools.  Neither are
relevant to the discussion.

-Chris

On Fri, Jun 1, 2012 at 7:08 PM, Caldarale, Charles R <
Chuck.Caldarale at unisys.com> wrote:
> > From: llvmdev-bounces at cs.uiuc.edu [mailto:llvmdev-bounces at
cs.uiuc.edu]
> On Behalf Of Chris Lattner
> > Subject: Re: [LLVMdev] [llvm-commits] [PATCH] Allow per-thread
> re-direction of outs()/errs()
>
> > It would be much better for the linker to return its error in a proper
> way
> > (i.e. extending llvm/Support/system_error.h like llvm/Object/Error.h
> does).
>
I like the way this works, but it seems like it's still relatively unused
in LLVM except for a few low-level classes.  Is there a plan to thread this
into the rest of LLVM, like the pass framework?  My concern is not so much
the passes that are currently using errs() to output the equivalent of "not
implemented" messages, but passes that may have legitimate reasons to
output warnings/errors, or signify an error condition.  If a compiler gets
broken IR, it's reasonable to expect some pass to fail.  In such a case, it
would be nice to get an error message and error result without having to
abort the entire OS process (llvm_unreachable is an example of something I
would like to be able to intercept and handle gracefully).  Ideally, I
would like a build of LLVM that would never result in a call to abort(),
only a result status I act on in a higher-level library.

>
> > The right fix for this is to fix the code to not report errors
textually.
>
> Unfortunately, the use of outs() and (especially) errs() is rampant - a
> simple grep of the 3.1 source tree shows about 1,500 instances.  One of the
> first things we had to implement in order to make LLVM usable is something
> very similar to what Justin has proposed.  Centralizing control of the
> output in outs()/errs() would seem to be an ideal way to let users of LLVM
> as a library customize it as they see fit without requiring massive source
> changes.
>
>  - Chuck
>
>
> THIS COMMUNICATION MAY CONTAIN CONFIDENTIAL AND/OR OTHERWISE PROPRIETARY
> MATERIAL and is thus for use only by the intended recipient. If you
> received this in error, please contact the sender and delete the e-mail and
> its attachments from all computers.
>
>

-- 

Thanks,

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