Hu Jiangping <hujiangp...@cn.fujitsu.com> writes:
> Hi!
>
> This patch makes the -falign-foo=0 work as described in the
> documentation. Thanks for all the suggestions, Richard and Segher!
>
> v3: make change more readable and self-consistent
> v2: at a high level handles -falign-foo=0 like -falign-foo
Thanks, this is OK with some minor nits for coding conventions.
Could you add a GNU changelog too? See:
http://www.gnu.org/prep/standards/html_node/Change-Logs.html
for the conventions.
> Regards!
> Hujp
>
> ---
> gcc/opts.c | 24 +++++++++++++++++++-----
> 1 file changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/gcc/opts.c b/gcc/opts.c
> index 499eb900643..dec5ba6d2be 100644
> --- a/gcc/opts.c
> +++ b/gcc/opts.c
> @@ -2007,10 +2007,20 @@ parse_and_check_align_values (const char *flag,
> location LOC. */
>
> static void
> -check_alignment_argument (location_t loc, const char *flag, const char *name)
> +check_alignment_argument (location_t loc,
> + const char *flag,
> + const char *name,
> + int *opt_flag,
> + const char **opt_str)
The function comment needs to mention the new parameters. Maybe just add:
OPT_STR points to the stored -falign-NAME= argument and
OPT_FLAG points to the associated -falign-NAME on/off flag. */
It's more usual in GCC to put as many arguments on one line as possible,
so maybe:
check_alignment_argument (location_t loc, const char *flag, const char *name,
int *opt_flag, const char **opt_str)
> {
> auto_vec<unsigned> align_result;
> parse_and_check_align_values (flag, name, align_result, true, loc);
> +
> + if (align_result.length() >= 1 && align_result[0] == 0)
> + {
> + *opt_flag = 1;
> + *opt_str = NULL;
> + }
The GCC style is to indent the braced “if” body by 2 more spaces:
if (align_result.length() >= 1 && align_result[0] == 0)
{
*opt_flag = 1;
*opt_str = NULL;
}
> }
>
> /* Print help when OPT__help_ is set. */
> @@ -2785,19 +2795,23 @@ common_handle_option (struct gcc_options *opts,
> break;
>
> case OPT_falign_loops_:
> - check_alignment_argument (loc, arg, "loops");
> + check_alignment_argument (loc, arg, "loops",
> + &opts->x_flag_align_loops, &opts->x_str_align_loops);
For multi-line argument lists, the second and subsequent lines should
be indented under the first argument. So maybe something like:
check_alignment_argument (loc, arg, "loops",
&opts->x_flag_align_loops,
&opts->x_str_align_loops);
Thanks,
Richard
