llvm dev - Nov 2015 - Prefixing DEBUG messages with DEBUG_TYPE (was re: [PATCH] D13259: LLE 6/6: Add LoopLoadElimination pass)

Moving this discussion here from the review thread.

RIght now, we have a few passes that prefix DEBUG message with various
things, so folks using -debug can tell where the debug message came from.

But this is not consistent, and in fact, most passes don't do it.

Worse, the prefixes used do not often match the DEBUG_TYPE of the pass.

It would be nice to fix this, and just have the DEBUG macros output the
debug type in front of messages if that's what folks want.

Of course, it turns out you can't just make the DEBUG macro do something
here, because people use it for more than just outputting messages.

My thought was to just bite the bullet and make a DEBUG_MSG macro that
outputs the DEBUG_TYPE, and replace the appropriate existing uses of
DEBUG(dbgs() <<) with it.

Something like
#define DEBUG_MSG_WITH_TYPE(t, x) DEBUG_WITH_TYPE(t, dbgs() << t
<<":" <<
x;)
#define DEBUG_MSG(x) DEBUG_MSG_WITH_TYPE(DEBUG_TYPE, x)

Suggestions, however, welcome!

---------- Forwarded message ----------
From: Renato Golin <renato.golin at linaro.org>
Date: Fri, Oct 30, 2015 at 10:15 AM
Subject: Re: [PATCH] D13259: LLE 6/6: Add LoopLoadElimination pass
To: anemet at apple.com, hfinkel at anl.gov, dberlin at dberlin.org
Cc: mssimpso at codeaurora.org, sanjoy at playingwithpointers.com,
llvm-commits at lists.llvm.org


rengolin added a comment.

In http://reviews.llvm.org/D13259#278362, @dberlin wrote:
> I would suggest rather than prefix *anything*, anywhere, that if we want
>  prefixes, we just have the DEBUG macro output the debug type in front of
>  the message ;-)

Sounds good to me.


http://reviews.llvm.org/D13259
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20151102/4435155b/attachment-0001.html>

To ask a basic question first: Why do we need prefixes at all? Are the messages
that likely to be confused? As far as I understand this case it was only
mentioned because of inconsistent prefix/no-prefix use, there was no indication
that we should actually have them. So another possible solution would be to
remove prefixes and avoiding the additional visual noise. Why add noise to every
message just to improve upon the rare case of a messages origin being ambiguous?

- Matthias
> On Nov 2, 2015, at 11:04 AM, Daniel Berlin via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> 
> Moving this discussion here from the review thread.
> 
> RIght now, we have a few passes that prefix DEBUG message with various
things, so folks using -debug can tell where the debug message came from.
> 
> But this is not consistent, and in fact, most passes don't do it.
> 
> Worse, the prefixes used do not often match the DEBUG_TYPE of the pass.
> 
> It would be nice to fix this, and just have the DEBUG macros output the
debug type in front of messages if that's what folks want.
> 
> Of course, it turns out you can't just make the DEBUG macro do
something here, because people use it for more than just outputting messages.
> 
> My thought was to just bite the bullet and make a DEBUG_MSG macro that
outputs the DEBUG_TYPE, and replace the appropriate existing uses of
DEBUG(dbgs() <<) with it.
> 
> Something like
> #define DEBUG_MSG_WITH_TYPE(t, x) DEBUG_WITH_TYPE(t, dbgs() << t
<<":" << x;)
> #define DEBUG_MSG(x) DEBUG_MSG_WITH_TYPE(DEBUG_TYPE, x)
> 
> Suggestions, however, welcome!
> 
> ---------- Forwarded message ----------
> From: Renato Golin <renato.golin at linaro.org>
> Date: Fri, Oct 30, 2015 at 10:15 AM
> Subject: Re: [PATCH] D13259: LLE 6/6: Add LoopLoadElimination pass
> To: anemet at apple.com, hfinkel at anl.gov, dberlin at dberlin.org
> Cc: mssimpso at codeaurora.org, sanjoy at playingwithpointers.com,
llvm-commits at lists.llvm.org
> 
> 
> rengolin added a comment.
> 
> In http://reviews.llvm.org/D13259#278362, @dberlin wrote:
> 
> > I would suggest rather than prefix *anything*, anywhere, that if we
want
> >  prefixes, we just have the DEBUG macro output the debug type in front
of
> >  the message ;-)
> 
> 
> Sounds good to me.
> 
> 
> http://reviews.llvm.org/D13259
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

----- Original Message -----
> From: "Matthias Braun via llvm-dev" <llvm-dev at
lists.llvm.org>
> To: "Daniel Berlin" <dberlin at dberlin.org>
> Cc: "llvm-dev" <llvm-dev at lists.llvm.org>
> Sent: Monday, November 2, 2015 1:16:18 PM
> Subject: Re: [llvm-dev] Prefixing DEBUG messages with DEBUG_TYPE (was re:
[PATCH] D13259: LLE 6/6: Add
> LoopLoadElimination pass)
> 
> To ask a basic question first: Why do we need prefixes at all? Are
> the messages that likely to be confused? As far as I understand this
> case it was only mentioned because of inconsistent prefix/no-prefix
> use, there was no indication that we should actually have them. So
> another possible solution would be to remove prefixes and avoiding
> the additional visual noise. Why add noise to every message just to
> improve upon the rare case of a messages origin being ambiguous?
I generally find the prefixes convenient when searching the debug output for
debug messages from a particular pass.

 -Hal
> 
> - Matthias
> 
> > On Nov 2, 2015, at 11:04 AM, Daniel Berlin via llvm-dev
> > <llvm-dev at lists.llvm.org> wrote:
> > 
> > Moving this discussion here from the review thread.
> > 
> > RIght now, we have a few passes that prefix DEBUG message with
> > various things, so folks using -debug can tell where the debug
> > message came from.
> > 
> > But this is not consistent, and in fact, most passes don't do it.
> > 
> > Worse, the prefixes used do not often match the DEBUG_TYPE of the
> > pass.
> > 
> > It would be nice to fix this, and just have the DEBUG macros output
> > the debug type in front of messages if that's what folks want.
> > 
> > Of course, it turns out you can't just make the DEBUG macro do
> > something here, because people use it for more than just
> > outputting messages.
> > 
> > My thought was to just bite the bullet and make a DEBUG_MSG macro
> > that outputs the DEBUG_TYPE, and replace the appropriate existing
> > uses of DEBUG(dbgs() <<) with it.
> > 
> > Something like
> > #define DEBUG_MSG_WITH_TYPE(t, x) DEBUG_WITH_TYPE(t, dbgs() << t
> > <<":" << x;)
> > #define DEBUG_MSG(x) DEBUG_MSG_WITH_TYPE(DEBUG_TYPE, x)
> > 
> > Suggestions, however, welcome!
> > 
> > ---------- Forwarded message ----------
> > From: Renato Golin <renato.golin at linaro.org>
> > Date: Fri, Oct 30, 2015 at 10:15 AM
> > Subject: Re: [PATCH] D13259: LLE 6/6: Add LoopLoadElimination pass
> > To: anemet at apple.com, hfinkel at anl.gov, dberlin at dberlin.org
> > Cc: mssimpso at codeaurora.org, sanjoy at playingwithpointers.com,
> > llvm-commits at lists.llvm.org
> > 
> > 
> > rengolin added a comment.
> > 
> > In http://reviews.llvm.org/D13259#278362, @dberlin wrote:
> > 
> > > I would suggest rather than prefix *anything*, anywhere, that if
> > > we want
> > >  prefixes, we just have the DEBUG macro output the debug type in
> > >  front of
> > >  the message ;-)
> > 
> > 
> > Sounds good to me.
> > 
> > 
> > http://reviews.llvm.org/D13259
> > _______________________________________________
> > LLVM Developers mailing list
> > llvm-dev at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
> 
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
> 
-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory