> From: Matthew Fortune > Sent: 04 November 2016 16:49 > 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: > > > From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches- > > > ow...@gcc.gnu.org] On Behalf Of Toma Tabacu > > > Sent: 04 November 2016 15:25 > > > To: Matthew Fortune; 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. > > > > > > > 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. > > Thanks, committed as r241850. > > Matthew
I've noticed that there is a typo in my surname in the ChangeLog entry (in the name and in the email address). Regards, Toma Tabacu
