> Sorry for the slow response on this. Like you say, it seems to be a pretty > pervasive problem. In fact I couldn't see anywhere that actually treated - > falign-foo=0 as anything other than -falign-foo=1. > > Technically using an alignment of one for zero is within what the > documentation allows, but not in a useful way. The documentation also isn't > clear about whether: > > -falign-loops=0:8 > > (“align to whatever you think is best, but don't skip more than 8 bytes”) is > supposed to be valid. The implication is that it's OK, but in practice it > doesn't > work. > Thanks, Richard!
> If there isn't anywhere that handles zero in the way that the documentation > implies (i.e. with -falign-loops=0 being equivalent to -falign-loops) then > maybe > we should instead change the documentation to match the actual behaviour. > Yes, I confirmed in source level that there is no target that handles zero in the way that the documentation implies. But, I think just adjusting the documentation to reflect the implementation behavior will cause -falign-functions=0 and -falign-functions=1 to have the same meaning which is a bit confusing from the user's perspective. In fact, I have issued a pr to Bugzilla, and got a comment from Richard Biener. We are considering whether to reject 0, which means both the source and the documentation should be modified a bit. I found that function parse_and_check_align_values in gcc/opts.c maybe the point we want to modify, values less than 0 are currently rejected here. Maybe we can change the '<’ to '<=' in the if statement. But I need test, to make sure there are no regression issues. What do you think? Regards! Hujp > If instead this is a regression from previous compilers, then I guess we > should > fix it. But I think it would be good to have a helper function that tests > whether > the default should be used for a given x_flag_align_foo and x_str_align_foo > pair. That we we could reuse it in other targets and would have only one > place > to update. (For example, we might decide to use > parse_and_check_align_values rather than strcmp.) > > Don't know whether anyone else has any thoughts about the best fix. > > Thanks, and sorry again for the slow reply. > > Richard > > > > > Regards! > > Hujp > > > > --- > > gcc/config/aarch64/aarch64.c | 9 ++++++--- > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/gcc/config/aarch64/aarch64.c > > b/gcc/config/aarch64/aarch64.c index 17dbe673978..697ac676f4d > 100644 > > --- a/gcc/config/aarch64/aarch64.c > > +++ b/gcc/config/aarch64/aarch64.c > > @@ -14221,11 +14221,14 @@ aarch64_override_options_after_change_1 > (struct gcc_options *opts) > > alignment to what the target wants. */ > > if (!opts->x_optimize_size) > > { > > - if (opts->x_flag_align_loops && !opts->x_str_align_loops) > > + if ((opts->x_flag_align_loops && !opts->x_str_align_loops) > > + || (opts->x_str_align_loops && > > + strcmp(opts->x_str_align_loops, "0") == 0)) > > opts->x_str_align_loops = aarch64_tune_params.loop_align; > > - if (opts->x_flag_align_jumps && !opts->x_str_align_jumps) > > + if ((opts->x_flag_align_jumps && !opts->x_str_align_jumps) > > + || (opts->x_str_align_jumps && > > + strcmp(opts->x_str_align_jumps, "0") == 0)) > > opts->x_str_align_jumps = aarch64_tune_params.jump_align; > > - if (opts->x_flag_align_functions && !opts->x_str_align_functions) > > + if ((opts->x_flag_align_functions && !opts->x_str_align_functions) > > + || (opts->x_str_align_functions && > > + strcmp(opts->x_str_align_functions, "0") == 0)) > > opts->x_str_align_functions = aarch64_tune_params.function_align; > > } >