Re: [PATCH] x86: reject architecture settings that are invalid to be set from the command line

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Thu, Jun 10, 2010 at 12:05 AM, Jan Beulich <JBeulich@novell.com> wrote:
>>>> On 09.06.10 at 18:02, "H.J. Lu" <hjl.tools@gmail.com> wrote:
>> On Wed, Jun 9, 2010 at 8:36 AM, Jan Beulich <JBeulich@novell.com> wrote:
>>> So far, options like -march=i8086 were accepted despite the assembler
>>> subsequently choking on other consistency checks, leading to reasonably
>>> cryptic error messages. This patch makes it so that impossible
>>> architecure settings are neither accepted nor displayed (i.e. it is now
>>> made sure that those settings can only be used via directives).
>>>
>>> gas/
>>> 2010-06-09 ?Jan Beulich ?<jbeulich@novell.com>
>>>
>>> ? ? ? ?* config/tc-i386.c (md_parse_option): Ignore impossible processor
>>> ? ? ? ?types.
>>> ? ? ? ?(show_arch): New parameter 'check'.
>>> ? ? ? ?(md_show_usage): Adjust calls to show_arch().
>>>
>>> --- 2010-06-09/gas/config/tc-i386.c ? ? 2010-06-09 17:04:12.000000000 +0200
>>> +++ 2010-06-09/gas/config/tc-i386.c ? ? 2010-06-09 17:24:59.000000000 +0200
>>> @@ -8166,6 +8166,11 @@ md_parse_option (int c, char *arg)
>>> ? ? ? ? ? ? ?if (strcmp (arch, cpu_arch [j].name) == 0)
>>> ? ? ? ? ? ? ? ?{
>>> ? ? ? ? ? ? ? ? ?/* Processor. ?*/
>>> + ? ? ? ? ? ? ? ? if (! (strcmp (default_arch, "i386")
>>> + ? ? ? ? ? ? ? ? ? ? ? ?? cpu_arch[j].flags.bitfield.cpulm
>>> + ? ? ? ? ? ? ? ? ? ? ? ?: cpu_arch[j].flags.bitfield.cpui386))
>>> + ? ? ? ? ? ? ? ? ? continue;
>>> +
>>> ? ? ? ? ? ? ? ? ?cpu_arch_name = cpu_arch[j].name;
>>> ? ? ? ? ? ? ? ? ?cpu_sub_arch_name = NULL;
>>> ? ? ? ? ? ? ? ? ?cpu_arch_flags = cpu_arch[j].flags;
>>> @@ -8297,7 +8302,7 @@ md_parse_option (int c, char *arg)
>>> ?"
>> ? ? ?"
>>>
>>> ?static void
>>> -show_arch (FILE *stream, int ext)
>>> +show_arch (FILE *stream, int ext, int check)
>>> ?{
>>> ? static char message[] = MESSAGE_TEMPLATE;
>>> ? char *start = message + 27;
>>> @@ -8334,6 +8339,13 @@ show_arch (FILE *stream, int ext)
>>> ? ? ? ? ?/* It is an processor. ?Skip if we show only extension. ?*/
>>> ? ? ? ? ?continue;
>>> ? ? ? ?}
>>> + ? ? ?else if (check && ! (strcmp (default_arch, "i386")
>>> + ? ? ? ? ? ? ? ? ? ? ? ? ?? cpu_arch[j].flags.bitfield.cpulm
>>> + ? ? ? ? ? ? ? ? ? ? ? ? ?: cpu_arch[j].flags.bitfield.cpui386))
>>> + ? ? ? {
>>> + ? ? ? ? /* It is an impossible processor - skip. ?*/
>>> + ? ? ? ? continue;
>>> + ? ? ? }
>>>
>>>
>>
>> Do we need to check cpu_arch[j].flags.bitfield.cpulm? Can we
>> just check cpu_arch[j].flags.bitfield.cpui386 like
>>
>> if (check && !cpu_arch[j].flags.bitfield.cpui386)
>> ? continue?
>>
>
> I'm of the opinion that when the assembler is in 64-bit mode it
> should reject those architectures that aren't 64-bit capable,
> otherwise specifying e.g. -march=i386 has the same ugly effect
> as has passing -march=i8086 in 32-bit mode. And if we reject
> them, we should also not display them as available.
>

On Linux/x86-64, your patch gave me

../as-new --help

  --32/--64               generate 32bit/64bit code
  --divide                ignored
  -march=CPU[,+EXTENSION...]
                          generate code for CPU and EXTENSION, CPU is one of:
                           generic64, nocona, core2, corei7, l1om, opteron, k8,
                           amdfam10, bdver1

I don't see how it can be correct since "--32 -march=i386" works fine.


-- 
H.J.