This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] x86: Add .nop directive to assembler


On Tue, Feb 27, 2018 at 2:25 PM, Alan Modra <amodra@gmail.com> wrote:
> On Mon, Feb 19, 2018 at 05:03:43PM -0800, H.J. Lu wrote:
>> On Mon, Feb 19, 2018 at 4:05 PM, Alan Modra <amodra@gmail.com> wrote:
>> > On Mon, Feb 19, 2018 at 03:24:17PM -0800, H.J. Lu wrote:
>> >> On Mon, Feb 19, 2018 at 3:19 PM, Alan Modra <amodra@gmail.com> wrote:
>> >> > On Mon, Feb 19, 2018 at 10:08:38PM +0000, Maciej W. Rozycki wrote:
>> >> >> On Mon, 19 Feb 2018, Alan Modra wrote:
>> >> >>
>> >> >> > > .nop is similar to .skip, except that it fills with NOPs.  Yes, it can be used
>> >> >> > > wherever .skip can be used.
>> >> >> >
>> >> >> > Given https://sourceware.org/ml/binutils/2018-02/msg00322.html I'm
>> >> >> > inclined to think the name of the directive should change.
>> >> >> > .skipnops perhaps?
>> >> >>
>> >> >>  Just `.nops' maybe?  There doesn't appear to be any matching mnemonic in
>> >> >> opcodes/.
>> >> >
>> >> > I'd be happy with that too.
>> >>
>> >> There is no guarantee that one of those NO_PSEUDO_DOT targets or the new
>> >> NO_PSEUDO_DOT target won't have an instruction called nops or skipnops in
>> >> the future.
>> >
>> > If a target adds an instruction like that, then the target will need
>> > to deal with it.  For example, as the spu target deals with "set" and
>> > "equ" which existed as directives before the spu defined them as
>> > instructions.
>> >
>> > In this case the use of "nop" as an instruction existed before you
>> > decided to define ".nop" as a directive, and lack of testing resulted
>> > in not discovering the NO_PSEUDO_DOT clash.  I suspect we wouldn't be
>> > having this conversation if you had run a full test suite regression,
>> > rather than just testing x86.  You yourself would have chosen
>> > something other than ".nop" as a directive!
>>
>> I would have chosen .nop and handled it for NO_PSEUDO_DOT targets.
>>
>> Here is a patch to rename .nop to .nops.  OK for master?
>>
>> --
>> H.J.
>
>> From eb81600ebb30f513732e395c524bcc9dca4de00a Mon Sep 17 00:00:00 2001
>> From: "H.J. Lu" <hjl.tools@gmail.com>
>> Date: Mon, 19 Feb 2018 16:53:28 -0800
>> Subject: [PATCH] Rename .nop directive to .nops
>>
>> Since directives of NO_PSEUDO_DOT targets don't have the leading '.'
>> and "nop" can be a valid instruction, rename .nop directive to .nops
>> to avoid conflict.
>>
>>       * NEWS: Rename .nop to .nops.
>>       * doc/as.texinfo: Likewise.
>>       * read.c (potable): Add "nops".  Remove "nop".
>>       (s_nop): Renamed to ...
>>       (s_nops): This.
>>       * read.h (s_nop): Renamed to ...
>>       (s_nops): This.
>>       * write.c (cvt_frag_to_fill): Rename .nop to .nops.
>>       (md_generate_nops): Likewise.
>>       (relax_segment): Likewise.
>>       * testsuite/gas/i386/nop-1.d: Updated.
>>       * testsuite/gas/i386/nop-1.s: Likewise.
>>       * testsuite/gas/i386/nop-2.d: Likewise.
>>       * testsuite/gas/i386/nop-2.s: Likewise.
>>       * testsuite/gas/i386/nop-3.d: Likewise.
>>       * testsuite/gas/i386/nop-3.s: Likewise.
>>       * testsuite/gas/i386/nop-4.d: Likewise.
>>       * testsuite/gas/i386/nop-4.s: Likewise.
>>       * testsuite/gas/i386/nop-5.d: Likewise.
>>       * testsuite/gas/i386/nop-5.s: Likewise.
>>       * testsuite/gas/i386/nop-6.d: Likewise.
>>       * testsuite/gas/i386/nop-6.s: Likewise.
>>       * testsuite/gas/i386/nop-bad-1.l: Likewise.
>>       * testsuite/gas/i386/nop-bad-1.s: Likewise.
>>       * testsuite/gas/i386/x86-64-nop-1.d: Likewise.
>>       * testsuite/gas/i386/x86-64-nop-2.d: Likewise.
>>       * testsuite/gas/i386/x86-64-nop-3.d: Likewise.
>>       * testsuite/gas/i386/x86-64-nop-4.d: Likewise.
>>       * testsuite/gas/i386/x86-64-nop-5.d: Likewise.
>>       * testsuite/gas/i386/x86-64-nop-6.d: Likewise.
>
> OK.  I think this is the best solution from a maintenance point of
> view.
>

Andrew, I am going to check in this patch.

-- 
H.J.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]