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
