> From: Matthew Fortune
> Sent: 03 November 2016 13:07
> To: Toma Tabacu; gcc-patches@gcc.gnu.org
> Cc: catherine_mo...@mentor.com
> Subject: RE: [PATCH,testsuite] MIPS: Downgrade R6 to R5 if tests need
> branch-likely instructions.
>
> Toma Tabacu <toma.tab...@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 --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"
