Sandra Loosemore <san...@codesourcery.com> writes:
> On 01/10/2017 07:24 AM, Yunqiang Su wrote:
> > Hi, folks, any idea about this patch?
>
> I can only comment on the documentation parts.
I am reviewing the patch but need to determine if the changes are sufficient and
safe to meet the goal. This area is complex in the MIPS backend so it will be a
few days to respond.
Sandra: Thanks for your docs review, much appreciated.
Thanks,
Matthew
>
> >> diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi
> >> index b911d76dd66..e548733537d 100644
> >> --- a/gcc/doc/install.texi
> >> +++ b/gcc/doc/install.texi
> >> @@ -1338,6 +1338,23 @@ In the absence of this configuration option the
> >> default
> convention is
> >> the legacy encoding, as when neither of the @option{-mnan=2008} and
> >> @option{-mnan=legacy} command-line options has been used.
> >>
> >> +@item --with-madd4=@var{type}
> >> +On MIPS targets, set the default behaivor of madd.fmt/msub.fmt. The
>
> s/behaivor/behavior/
>
> Are you really setting/describing the behavior of these instructions on
> the target, or are you setting how GCC generates code for them?
>
> >> +possibilities for @var{type} are:
> >> +@table @code
> >> +@item unfused
> >> +The madd.fmt/msub.fmt instructions are unfused, as with the
> >> +@option{-mmadd4=unfused} command-line option.
> >> +@item fused
> >> +The madd.fmt/msub.fmt instructions are fused, as with the
> >> +@option{-mmadd4=fused} command-line option.
> >> +@item no
> >> +The madd.fmt/msub.fmt are disabled, as with the @option{-mmadd4=no}
>
> s/are/instructions are/
> >> +command-line option.
> >> +@end table
> >> +In the absence of this configuration option the default convention is
> >> +the unfused, while for MIPS r6 and Loongson, the default is fused.
>
> s/the unfused/@samp{unfused}
> s/fused/@samp{fused}
>
> The MIPS maintainers may correct me on this, but it looks to me like
> "MIPS r6" is not an official name of anything. I see "MIPS Release 6"
> on the Imagination web site and in the MIPS processor documentation.
>
> Finally, I'm confused about whether the default is based on the
> processor selected at compile time, or depends on some other
> configure-time option.
>
> >> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> >> index 1096254085f..e88040cdceb 100644
> >> --- a/gcc/doc/invoke.texi
> >> +++ b/gcc/doc/invoke.texi
> >> @@ -19289,6 +19289,29 @@ their trailing significand field being 0.
> >> The default is @option{-mnan=legacy} unless GCC has been configured with
> >> @option{--with-nan=2008}.
> >>
> >> +@item -mmadd4=unfused
> >> +@item -mmadd4=fused
> >> +@item -mmadd4=no
>
> Use @itemx instead of @item for entries other than the first.
>
> >> +@opindex mmadd4=unfused
> >> +@opindex mmadd4=fused
> >> +@opindex mmadd4=no
>
> I think it's sufficient to just have a single index entry
>
> @opindex mmadd4=
>
> >> +These options control the type of madd.fmt/msub.fmt, unfused, fused
> >> +or disabled at all.
>
> "type of" is confusing. Maybe just say
>
> These options control generation of madd.fmt/msub.fmt instructions.
>
> >> +
> >> +The @option{-mmadd4=unfused} option will set type of madd.fmt/msub.fmt
>
> Same concerns about "type of" here, plus
> s/will set/sets/
>
> >> +to be unfused if they are generated.
> >> +
> >> +The @option{-mmadd4=fused} option will set type of madd.fmt/msub.fmt
> >> +to be fused if they are generated.
>
> Same here.
>
> >> +
> >> +The @option{-mmadd4=no} option will disable madd.fmt/msub.fmt
> >> +to be generated at all.
>
> s/will disable/prevents/
> s/to be generated/from being generated/
>
>
> >> +
> >> +The default is @option{-mmadd4=unfused} unless GCC has been configured
> >> +for MIPS r6+ or Loongson. If runtime flags for MIPS r6 or Loongson is
> >> +given, the type will always be `fused', no matter what value
> >> +@option{-mmadd4=} is.
> >> +
>
> Same concerns about "MIPS r6".
>
> I suggest rewriting the second sentence as something like
>
> "If compiling for a MIPS Release 6 or Loongson processor, this option is
> ignored and the behavior is as if @samp{fused} were specified."
>
> I think we are talking about controlling code generation determined by
> compilation options, and not detecting runtime processor flag bits, right?
>
> -Sandra
