diff mbox

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

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

Commit Message

Toma Tabacu Nov. 3, 2016, 10:57 a.m. UTC
Hi,

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.

Regards,
Toma Tabacu

gcc/testsuite/
        * gcc.target/mips/mips.exp: Add check for -mbranch-likely in
            condition for R5 downgrade.

Comments

Moore, Catherine Nov. 3, 2016, 12:54 p.m. UTC | #1
> -----Original Message-----
> From: Toma Tabacu [mailto:Toma.Tabacu@imgtec.com]
> Sent: Thursday, November 3, 2016 6:58 AM
> Subject: [PATCH,testsuite] MIPS: Downgrade R6 to R5 if tests need
> branch-likely instructions.
> 
> 
> gcc/testsuite/
>         * gcc.target/mips/mips.exp: Add check for -mbranch-likely in
>             condition for R5 downgrade.
> 

This patch is OK.
Catherine
Matthew Fortune Nov. 3, 2016, 1:06 p.m. UTC | #2
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
diff mbox

Patch

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 {