Re: [PATCH] don't generate long nops on i686 (only when -march=[intel])

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

On Thu, Aug 5, 2010 at 2:04 PM, Quentin Neill
<quentin.neill.gnu@gmail.com> wrote:
> On Mon, Aug 2, 2010 at 7:12 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Mon, Aug 2, 2010 at 4:19 PM, Quentin Neill
>> <quentin.neill.gnu@gmail.com> wrote:
>>> On Tue, Jul 13, 2010 at 8:33 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>> On Tue, Jul 13, 2010 at 5:59 AM, Kyle McMartin <kyle@mcmartin.ca> wrote:
>>>>> From: Kyle McMartin <kyle@redhat.com>
>>>>>
>>>>> Hi,
>>>>>
>>>>> In Fedora, glibc started building itself with -march=i686 flags to
>>>>> binutils as an "optimization." This exposes issues on the Geode, which
>>>>> is i686, but for long nops, since they are apparently not an architected
>>>>> part of the i686 ISA.
>>>>>
>>>>> Clarify this by only enabling long nops if we're generating 64-bit code
>>>>> (as far as I can tell, all x86_64 cpus support this) or explicitly
>>>>> generating code for the pentiumpro and above.
>>>>>
>>>>> Using this patch prevents people from overambitiously optimizing and
>>>>> breaking the Geode.
>>>>>
>>>>> Signed-off-by: Kyle McMartin <kyle@redhat.com>
>>>>
>>>> There is no ChangeLog. Please add CpuNop to i386-opc.h and use it
>>>> to determine when to generate nops.
>>>>
>>>> Thanks.
>>>
>>> Kyle, I was working on a similar patch. ?Here it is for previewing.
>>>
>>> It has a ChangeLog entry but without test cases. ?I will post with the
>>> test cases as soon as I get a clean "make check" .
>>
>> + ?{ STRING_COMMA_LEN (".Nop"), PROCESSOR_UNKNOWN,
>> + ? ?CPU_NOP_FLAGS, 0 },
>> + ?{ STRING_COMMA_LEN (".noNop"), PROCESSOR_UNKNOWN,
>> + ? ?CPU_NOP_FLAGS, 0 },
>>
>> All the others use lower case. Please use ".nop". ?I don't think ".noNop"
>> is correct since it is treated the same as ".nop". ?Please leave it out. ?We
>> can figure something else out later if we really need to turn off NOP.
>>
>> --
>> H.J.
>
> The following patch adds a ".nop" cpu subarch with testcases, to
> control generation of alignment code with nops.
>
> It changes the alignment code generated by -mtune=i686 to not include
> "nopl", by leaving the CpuNop capability out of the CPU_I686_FLAGS.
>
> It passes make -k check on x86.
>
> Okay to commit?
> --
> Quentin
>
>
> opcodes/
> ? ? ? ?* i386-gen.c (cpu_flag_init): Define CpuNop extension
> ? ? ? ? ?flag, add to processor flags for PENTIUMPRO processors
> ? ? ? ? ?and later. ?Note: not to I686 flags.
> ? ? ? ?* i386-opc.h (enum): Add CpuNop.
> ? ? ? ?* i386-opc.tbl (i386_cpu_flags): Add cpunop bit.
>
> gas/
> ? ? ? ?* config/tc-i386.c (arch_entry): Add negated bit to
> ? ? ? ? ?disambiguate flag names starting with "no".
> ? ? ? ? ?(cpu_arch): Add negated bit definitions. ?Add
> ? ? ? ? ?".nop" CPU extension.
> ? ? ? ? ?(i386_align_code): Use new .cpunop bit to decide
> ? ? ? ? ?when to generate alignment using nops.
> ? ? ? ? ?(set_cpu_arch): Use negated bit instead to decide
> ? ? ? ? ?when to use cpu_flags or vs. cpu_flags_and_not.
> ? ? ? ? ?(md_parse_option): Likewise.
>
> gas/testsuite/
> ? ? ? ?* gas/i386/arch-10-1.l: Add nopl instruction.
> ? ? ? ?* gas/i386/arch-10-2.l: Likewise.
> ? ? ? ?* gas/i386/arch-10-3.l: Likewise.
> ? ? ? ?* gas/i386/arch-10-4.l: Likewise.
> ? ? ? ?* gas/i386/arch-10.s: Likewise.
> ? ? ? ?* gas/i386/arch-10.d: Add nopl instruction, and
> ? ? ? ? ?+nopl extension flag to as flags.
> ? ? ? ?* gas/i386/nops-5-i686.d: Change alignment code
> ? ? ? ? ?generated for -mtune=i686.
> ? ? ? ?* gas/i386/nops-5.d: Change alignment code
> ? ? ? ? ?generated for .arch i686.
> ? ? ? ?* gas/i386/x86-64-nops-5-k8.d: Likewise.
> ? ? ? ?* gas/i386/x86-64-nops-5.d: Likewise.
>

Your patch seems corrupted. Please resent it as an attachment.


-- 
H.J.