Hi Fredrik,

 Thank you for your submission.  Your change looks very good to me, except 
for a bunch of minor nits in your proposed ChangeLog entry.

>       * 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'.

?

>       * 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.  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?

>       * 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.

>       * 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.

?

 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.

 Please also fix indentation throughout; the subsequent lines of entries 
are horizontally aligned with the asterisk above and not indented any 
further.

 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.

  Maciej

Reply via email to