This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] MIPS: Add `-mfix-r5900' option for the R5900 short loop erratum
Many thanks for your prompt review, Maciej!
> > * gcc/config/mips/mips.c (mips_reorg_process_insns)
> > (mips_option_override): Default to working around R5900
> > errata only if the processor was selected explicitly.
>
> I think this only describes the `mips_option_override' part. ChangeLog
> entries are best concise, so how about just:
>
> * config/mips/mips.c (mips_reorg_process_insns)
> (mips_option_override): Handle `-mfix-r5900'.
>
> ?
Done!
> > * gcc/config/mips/mips.h: Declare `mfix-r5900' and
> > `mno-fix-r5900'.
>
> This has to be:
>
> * config/mips/mips.h (ASM_SPEC): Add `mfix-r5900' and
> `mno-fix-r5900'.
>
> as you're adding to the ASM_SPEC definition.
Yes, done!
> Also your patch lines are
> way off and I had to seach for the place this change applies to -- have
> you been using current upstream trunk with this change?
Hmm... I thought so, but obviously 2 November was a week old. I'm sorry
about that, I have rebased v2 now. [ As mentioned in the commit message,
the test was done with GCC 8.2.0 since there was an unrelated problem
compiling some sanitizer component at the time. I will try to test a
current GCC version during the weekend. ]
> > * gcc/config/mips/mips.opt: Define MASK_FIX_R5900.
>
> Likewise:
>
> * config/mips/mips.opt (mfix-r5900): New option.
>
> as it's an entry for `mfix-r5900' that you're adding.
Done!
> > * gcc/doc/invoke.texi: Document the R5900, `mfix-r5900' and
> > `mno-fix-r5900'.
>
> It looks like missing bits, did you mean:
>
> * doc/invoke.texi: Document the `r5900' processor name, and
> `-mfix-r5900' and `-mno-fix-r5900' options.
>
> ?
Yes, done!
> All these entries apply to the gcc/ChangeLog rather than the top-level
> one, so please drop the leading gcc/ part of the paths, and indicate it at
> the beginning instead.
Done!
> Please also fix indentation throughout; the subsequent lines of entries
> are horizontally aligned with the asterisk above and not indented any
> further.
Ah, yes, of course.
> Please resubmit your change with the ChangeLog entry updated.
> Regrettably I have no authority to approve your submission, but hopefully
> a maintainer will so that it can make it to GCC 9.
I will post v2 shortly. Let's hope for the best. Thanks again!
Fredrik