Martin Liška <mli...@suse.cz> writes:
> On 7/27/20 9:46 AM, Hu Jiangping wrote:
>> Hi!
>> 
>> This patch makes the -falign-foo=0 work as described in the
>> documentation. Thanks for all the suggestions, Richard and Segher!
>
> Hello.
>
> I'm the author of the original code.
>
>> 
>> v3: make change more readable and self-consistent
>> v2: at a high level handles -falign-foo=0 like -falign-foo
>> 
>> 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)
>>   {
>>     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;
>> +  }
>
> Hm, shouldn't the code be placed in parse_and_check_align_values? Note that 
> there's
> one another call gcc/toplev.c (parse_N_M).

I think that's why check_alignment_argument is the right place.
The idea is that we're making -falign-foo=0 do two things:

- imply -falign-foo
- invalidate any previous -falign-foo=… option

It's therefore something that we should only do while iterating
through the command-line in order.

> I bet you likely want to modify result_values in case -falign-foo=0.
> About the -falign-foo=0:m:n2:m3, you can report an error in 
> parse_and_check_align_values?

The problem is that we don't know in general what the target's default is.
It depends on whether this is aligning for functions, loops, labels, etc.
It also depends on the target core, which we might not know yet, and which
might change during compilation based on attributes and pragmas.

So I don't think there's a different value that parse_and_check_align_values
could sensibly insert instead of zero.

Thanks,
Richard

Reply via email to