public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: "Moore, Catherine" <Catherine_Moore@mentor.com>
To: Andrew Bennett <Andrew.Bennett@imgtec.com>,
	Richard Sandiford	<rdsandiford@googlemail.com>,
	Matthew Fortune <Matthew.Fortune@imgtec.com>
Cc: "gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>
Subject: RE: [PATCH] MIPS: Correctly update the isa and arch_test_option_p variables after the arch dependency handling code in mips.exp
Date: Tue, 14 Jul 2015 21:42:00 -0000	[thread overview]
Message-ID: <FD3DCEAC5B03E9408544A1E416F112420192C89C90@NA-MBX-04.mgc.mentorg.com> (raw)
In-Reply-To: <0DA23CC379F5F945ACB41CF394B98277210F525A@LEMAIL01.le.imgtec.org>



> -----Original Message-----
> From: Andrew Bennett [mailto:Andrew.Bennett@imgtec.com]
> Sent: Tuesday, July 14, 2015 9:24 AM
> To: Richard Sandiford; Matthew Fortune
> Cc: gcc-patches@gcc.gnu.org; Moore, Catherine
> Subject: RE: [PATCH] MIPS: Correctly update the isa and arch_test_option_p
> variables after the arch dependency handling code in mips.exp
> 
> > Yeah, I agree that this doesn't really fit the model that well, but
> > like you say, we're stretching the logic a bit :-).  When I wrote it,
> > the architectures formed a nice tree in which moving to leaf nodes
> > only added features.  So in the pre-r6 days:
> >
> >     # Handle dependencies between the pre-arch options and the arch
> option.
> >     # This should mirror the arch and post-arch code below.
> >     if { !$arch_test_option_p } {
> >
> > increased the architecture from the --target_board default to match
> > the features required by the test, whereas:
> >
> >     # Handle dependencies between the arch option and the post-arch
> options.
> >     # This should mirror the arch and pre-arch code above.
> >     if { $arch_test_option_p } {
> >
> > turned off features from the --target_board default to match a lower
> > architecture required by the test.  So in the pre-r6 days, all the
> > code in the second block was turning something off when going to a
> > lower architecture.  The blocks were mutually-exclusive and writing it
> > this way reduced the number of redundant options.  (Admittedly you
> > could argue that it's daft to worry about that given the kind of
> > command lines you tend to get from the rest of mips.exp. :-))
> >
> > r6 is the first time we've had to turn something off when moving up.
> > -mnan and -mabs are also the first options where old architectures
> > support only A, higher revisions support A and B, and the newest
> > revision supports only B.  I think I'd prefer to acknowledge that and
> > have:
> >
> >     # Handle dependencies between the arch option and the post-arch
> options.
> >     # This should mirror the arch and pre-arch code above.  For pre-r6
> >     # architectures this only needs to be done when we've moved down
> >     # to a lower architecture and might need to turn features off,
> >     # but moving up from pre-r6 to r6 can remove features too.
> >     if { $arch_test_option_p || ($orig_isa_rev < 6 && $isa_rev >= 6) }
> > {
> >
> > I think the existing r6->r5 case really is different: there we're
> > forcing a -mnan option not because the architecture needs it but
> > because the environment might.
> 
> Hi,
> 
> An updated patch and ChangeLog which reflects the comments is below.
> I have tested it on the mips-img-elf and mips-mti-elf toolchains with all the
> valid multilib configurations and there are no new regressions.
> 
> Ok to commit?

Yes, this is OK.
> 
> 
> testsuite/
> 	* gcc.target/mips/mips.exp (mips-dg-options): Allow the post-arch
> 	code to be run when the pre-arch code increases the isa_rev to
> 	mips32r6 or greater.
> 
> 


  reply	other threads:[~2015-07-14 21:26 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-10 15:37 Andrew Bennett
2015-07-10 22:03 ` Matthew Fortune
2015-07-12 14:26   ` Richard Sandiford
2015-07-14 13:23     ` Andrew Bennett
2015-07-14 21:42       ` Moore, Catherine [this message]
2015-07-15  9:45         ` Andrew Bennett

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=FD3DCEAC5B03E9408544A1E416F112420192C89C90@NA-MBX-04.mgc.mentorg.com \
    --to=catherine_moore@mentor.com \
    --cc=Andrew.Bennett@imgtec.com \
    --cc=Matthew.Fortune@imgtec.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=rdsandiford@googlemail.com \
    /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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).