diff mbox

[testsuite] MIPS: Downgrade R6 to R5 if tests need branch-likely instructions.

Message ID A614194ED15B4844BC4C9FB7F21FCD9222530764@HHMAIL01.hh.imgtec.org
State New
Headers show

Commit Message

Toma Tabacu Nov. 4, 2016, 3:25 p.m. UTC 
> From: Matthew Fortune
> Sent: 03 November 2016 13:07
> To: Toma Tabacu; gcc-patches@gcc.gnu.org
> Cc: catherine_moore@mentor.com
> Subject: RE: [PATCH,testsuite] MIPS: Downgrade R6 to R5 if tests need
> branch-likely instructions.
> 
> Toma Tabacu <Toma.Tabacu@imgtec.com> writes:
> > The gcc.target/mips/wrap-delay.c test was failing on mips-img-*
> > toolchains because it was using -mbranch-likely with an R6 target, and
> > branch- likely instructions were removed in R6.
> >
> > This patch makes the testsuite downgrade to R5 if the -mbranch-likely
> > option is present and we're targeting R6.
> >
> > Tested with mips-img-elf and mips-img-linux-gnu.
> 
> Hi Toma,
> 
> Welcome to GCC development, thanks for your first patch. As you can see
> from Catherine's reply the change looks good. I'll just cover some
> housekeeping issues...
> 
> > gcc/testsuite/
> >         * gcc.target/mips/mips.exp: Add check for -mbranch-likely in
> >             condition for R5 downgrade.
> 
> Changelogs are an art form which will take some getting used to. This is
> almost there but needs to reference the function affected.
> 
> 	* gcc.target/mips/mips.exp (mips-dg-options): Downgrade to R5
> 	for -mbranch-likely and infer -mno-branch-likely for R6.
> 
> I have extended the comment as well as there is an additional change
> needed for this patch ideally.
> 
> > diff --git a/gcc/testsuite/gcc.target/mips/mips.exp
> > b/gcc/testsuite/gcc.target/mips/mips.exp
> > index 7c24140..382d69c 100644
> > --- a/gcc/testsuite/gcc.target/mips/mips.exp
> > +++ b/gcc/testsuite/gcc.target/mips/mips.exp
> > @@ -1176,7 +1176,8 @@ proc mips-dg-options { args } {
> >                        || [mips_have_test_option_p options "-mpaired-
> > single"]
> >                        || [mips_have_test_option_p options "-
> > mnan=legacy"]
> >                        || [mips_have_test_option_p options "-
> > mabs=legacy"]
> > -                      || [mips_have_test_option_p options "!HAS_LSA"])
> > } {
> > +                      || [mips_have_test_option_p options "!HAS_LSA"]
> > +                      || [mips_have_test_option_p options "-mbranch-
> > likely"]) } {
> >             if { $gp_size == 32 } {
> >                 mips_make_test_option options "-mips32r5"
> >             } else {
> 
> Please can you make sure to retain the original patch formatting when
> posting. I suspect you have copied this out of a putty session or similar and
> have therefore lost the tabs.
> 
> The extra change is that in the post-arch option processing we will need to
> infer -mno-branch-likely for the $isa_rev > 5 case much like we infer -
> mnan=2008 and -mabs=2008. This is so that when running the testsuite using
> -mips32r5 or earlier, with -mbranch-likely as part of the user-supplied test
> flags, then a test which is upgraded to
> mips32r6 does not attempt to use -mbranch-likely.
> 
> Hope that wasn't too cryptic!
> 
> Thanks,
> Matthew

The updated patch below includes the improved ChangeLog comment, correct
formatting, and the post-arch enforcing of -mno-branch-likely for R6.

Regards,
Toma

gcc/testsuite/ChangeLog:

	* gcc.target/mips/mips.exp (mips-dg-options): Downgrade to R5
	for -mbranch-likely and infer -mno-branch-likely for R6.
diff mbox

Patch

diff --git a/gcc/testsuite/gcc.target/mips/mips.exp b/gcc/testsuite/gcc.target/mips/mips.exp
index 7c24140..6b7c46f 100644
--- a/gcc/testsuite/gcc.target/mips/mips.exp
+++ b/gcc/testsuite/gcc.target/mips/mips.exp
@@ -1176,7 +1176,8 @@  proc mips-dg-options { args } {
 		       || [mips_have_test_option_p options "-mpaired-single"]
 		       || [mips_have_test_option_p options "-mnan=legacy"]
 		       || [mips_have_test_option_p options "-mabs=legacy"]
-		       || [mips_have_test_option_p options "!HAS_LSA"]) } {
+		       || [mips_have_test_option_p options "!HAS_LSA"])
+		       || [mips_have_test_option_p options "-mbranch-likely"]) } {
 	    if { $gp_size == 32 } {
 		mips_make_test_option options "-mips32r5"
 	    } else {
@@ -1345,6 +1346,7 @@  proc mips-dg-options { args } {
 	    mips_make_test_option options "-mno-paired-single"
 	    mips_make_test_option options "-mnan=2008"
 	    mips_make_test_option options "-mabs=2008"
+	    mips_make_test_option options "-mno-branch-likely"
 	}
 	if { [regexp {^-march=(octeon|loongson)} $arch] } {
 	    mips_make_test_option options "-mno-micromips"