This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] x86: Add .nop directive to assembler
- From: "Jan Beulich" <JBeulich at suse dot com>
- To: "H.J. Lu" <hjl dot tools at gmail dot com>
- Cc: "Andrew Cooper" <andrew dot cooper3 at citrix dot com>, "Maciej W. Rozycki" <macro at mips dot com>, "Binutils" <binutils at sourceware dot org>
- Date: Tue, 13 Feb 2018 01:50:04 -0700
- Subject: Re: [PATCH] x86: Add .nop directive to assembler
- Authentication-results: sourceware.org; auth=none
- References: <CAMe9rOr9n4XwM4wMhR3RNP4EXUE37nu0ui3SL4-nUvLAF_T9yA@mail.gmail.com>
>>> On 12.02.18 at 20:38, <hjl.tools@gmail.com> wrote:
> On Mon, Feb 12, 2018 at 7:48 AM, Maciej W. Rozycki <macro@mips.com> wrote:
>> On Mon, 12 Feb 2018, H.J. Lu wrote:
>>
>>> My implementation uses the existing relaxation frame work.
>>> When we are processing .nop, we don't know exactly how big the
>>> the NOP size will be. We allocate a frag with the maximum size
>>> and set the exact size after relaxation. After relaxation is done,
>>> all frags are converted to rs_fill. We can add rs_fill_nop to
>>> support arbitrary .nop directive size. But I don't know if it is
>>> necessary.
>>
>> Right, so this is needed for argument expressions using forward
>> references. Understood and accepted. Thank your for patience.
>>
>
> Implement the '.nop SIZE[, LIMIT]' directive for x86 assembler. This
> directive emits SIZE bytes filled with 'NOP' instructions. SIZE is
> absolute expression, which must be between 0 and 512. LIMIT specifies
> the size limit of a single 'NOP' instruction. If the comma and LIMIT
> are omitted, LIMIT is assumed to the maximum supported size of a single
> 'NOP' instruction. The valid values of LIMIT are between 1 and 8 for
> 16-bit mode, between 1 and 10 for 32-bit mode, between 1 and 11 for
> 64-bit mode. This directive is only allowed in text sections.
Did you consider generalizing .skip instead (e.g. by allowing its
FILL to be "@NOP" alongside an absolute expression)? I have
to admit that adding a new directive looks a little odd to me
when all you want is some more flexibility with an existing one.
Also I'm not sure I really follow what the upper bounds for
LIMIT in the different modes are being derived from. Without
a comment next to the patterns that's going to remain guesswork
forever. For example, why would
static const unsigned char alt64_12[] =
{0x67,0x66,0x2e,0x40,0x0f,0x1f,0x84,0x00,0x00,0x00,0x00,0x00};
not be a possibility? Or, like at least AMD suggests, multiple 0x66
prefixes?
> This is implemented by adding a relax state, rs_space_nop, to enum
> _relax_state, which is similar to rs_space, but it fills with NOPs,
> instead of a single byte. A pseudo relocation, BFD_RELOC_NOP_DIRECTIVE,
> is added to fix up frag data with the proper number of NOPs. The new
> rs_space_nop state is processed only when TARGET_USE_NOP_DIRECTIVE is
> defined.
In your earlier reply to Maciej didn't you indicate you restrict
LIMIT only because you don't want to go through the
complexity of introducing a new relaxation state?
> To enable .nop directive, a target backend should
>
> 1. Define TARGET_USE_NOP_DIRECTIVE.
With this, why is the new directive being added to i386's
md_pseudo_table[], instead of the arch independent one in
read.c?
> 2. Create a rs_space_nop frag for .nop directive.
> 3. Update md_convert_frag to create a fixup with BFD_RELOC_NOP_DIRECTIVE
> for rs_space_nop frag.
> 4. Update md_apply_fix to process fixup with BFD_RELOC_NOP_DIRECTIVE.
>
> OK for master?
>
> Andrew, please test my current users/hjl/nop branch.
On the original thread Andrew had indicated that producing a single
byte NOP at the end of a sequence of NOPs is undesirable. Your
i386_output_nops() appears to do just that, however.
Additionally - what's wrong with emitting NOPs to a non-executable
section?
Finally, would you mind making the diagnostic complaining about too
large a LIMIT also report the upper bound (not the least because - as
per above remark - this may change over time)?
Jan