Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Trevor Saunders <tbsaunde@tbsaunde.org>
To: Pedro Alves <palves@redhat.com>
Cc: "Maciej W. Rozycki" <macro@imgtec.com>,
	Trevor Saunders <tbsaunde+binutils@tbsaunde.org>,
	Yao Qi <qiyaoltc@gmail.com>,
	gdb-patches@sourceware.org
Subject: Re: [PATCH, doc RFA] Remove support for "target m32rsdi" and "target mips/pmon/ddb/rockhopper/lsi"
Date: Wed, 04 May 2016 00:33:00 -0000	[thread overview]
Message-ID: <20160503235813.GA4366@tsaunders-iceball.corp.tor1.mozilla.com> (raw)
In-Reply-To: <70b62cd3-436b-2495-dd87-4bebf3de9e87@redhat.com>

On Tue, May 03, 2016 at 07:59:10PM +0100, Pedro Alves wrote:
> On 05/03/2016 03:57 PM, Maciej W. Rozycki wrote:
> > On Mon, 2 May 2016, Pedro Alves wrote:
> > 
> >>>  NB it looks to me `mips_r3041_reg_names' is now dead.  We just *might* 
> >>> consider rewiring it like `mips_tx39_reg_names', but that would require 
> >>> defining another BFD machine type and I doubt anybody cares about the 
> >>> R3041 anymore (cf. the relevant comment you've just removed).  So if you 
> >>> care to remove it too, then I'll appreciate it and a change to do so is 
> >>> preapproved.
> >>>
> >>>  Given that the variable is static I wonder why it hasn't triggered a 
> >>> compilation error in the build actually.
> >>
> >> That's because gdb doesn't use -Wunused presently.
> > 
> >  Hmm, I thought it was implied by -Wall.
> 
> Yeah, GDB uses -Wno-usused explicitly.
> 
> > Perhaps we should add it then?  
> 
> Yes, agreed.  That's what Trevor was aiming for IIUC.
> 
> > Releases are built without -Werror so the end users will be safe either 
> > way, and it'll make us easier to avoid code pollution.
> 
> *nod*
> 
> >> I never managed to come back to this, and looks like we won't need to.
> >> Trevor sent a patch that removes mips_r3041_reg_names among a ton
> >> of other unused variables, here:
> >>
> >>  https://sourceware.org/ml/gdb-patches/2016-04/msg00664.html
> > 
> >  Great!  As it happens, I made a patch to remove `mips_r3041_reg_names' on 
> > Friday, but didn't get to actually pushing it -- and we had a bank holiday 
> > yesterday.

Go ahead and push yours if you like, git will deal with it just fine I
expect.

> >  I'll give Trevor's change precedence then as a more general clean-up, 
> > although I'd like to review the MIPS part, as not all variables removed 
> > from mips-tdep.c are actually "trivially unused" (those would be lone 
> > declarations, possibly with initialisers).  

Sure, I'd say they are somewhat trivial compared to other function calls
and variables I have yet to send patches for, but I'm not really
interested in argueing semantics ;-)

> > Especially the heuristic 
> > unwinder bits look highly suspicious to me, where the variable is updated 
> > as the analysis proceeds.  It could be that these variables can indeed go, 
> > but perhaps something is missing that should be there.

perhaps, my guess is that it was necessary at one time, but isn't
anymore.

> > 
> >  I'll try to get this done by the end of tomorrow.
> 
> Thanks.
> 
> FWIW, I don't think that this all needs to go in as one big single
> patch.  We can push in things incrementally, say, push in the obviously
> correct bits, and then push in the bits that remove function calls,
> which may have desirable side effects as separate patches.
> Trevor, if you'd like to proceed like that, feel free to push in
> the patch without the check_typedef, mips, and tracepoint trace status
> bits as first step.

I agree, I'll try to get that done soon, Thanks!

Trev

> 
> Thanks,
> Pedro Alves
> 


  reply	other threads:[~2016-05-04  0:33 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-17 16:00 Pedro Alves
2016-03-22 10:49 ` Yao Qi
2016-03-22 11:54   ` Pedro Alves
2016-03-30 22:19     ` Maciej W. Rozycki
2016-03-31 12:07       ` Pedro Alves
2016-04-02  0:00         ` Maciej W. Rozycki
2016-05-02 11:59           ` Pedro Alves
2016-05-03 14:57             ` Maciej W. Rozycki
2016-05-03 18:59               ` Pedro Alves
2016-05-04  0:33                 ` Trevor Saunders [this message]
2016-05-04  1:03                   ` Maciej W. Rozycki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160503235813.GA4366@tsaunders-iceball.corp.tor1.mozilla.com \
    --to=tbsaunde@tbsaunde.org \
    --cc=gdb-patches@sourceware.org \
    --cc=macro@imgtec.com \
    --cc=palves@redhat.com \
    --cc=qiyaoltc@gmail.com \
    --cc=tbsaunde+binutils@tbsaunde.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox