This is the mail archive of the
mailing list for the binutils project.
Re: [PATCH/AArch64 2/4] Remove the artificial limit on code alignment through the use of the fixed part of a fragment for output generation only, which required MAX_MEM_FOR_RS_ALIGN_CODE to be large enough to hold the maximum pad.
- From: "Dr. Philipp Tomsich" <philipp dot tomsich at theobroma-systems dot com>
- To: Andrew Pinski <pinskia at gmail dot com>
- Cc: Marcus Shawcroft <marcus dot shawcroft at gmail dot com>, "binutils at sourceware dot org" <binutils at sourceware dot org>
- Date: Tue, 28 Oct 2014 11:58:40 +0100
- Subject: Re: [PATCH/AArch64 2/4] Remove the artificial limit on code alignment through the use of the fixed part of a fragment for output generation only, which required MAX_MEM_FOR_RS_ALIGN_CODE to be large enough to hold the maximum pad.
- Authentication-results: sourceware.org; auth=none
- References: <CA+=Sn1nxGT+x9JgJo=B6iOBKcOz0xWiF5nf2t=6vgVz+eMSpDQ at mail dot gmail dot com> <1388666881-20727-1-git-send-email-philipp dot tomsich at theobroma-systems dot com> <CAFqB+PyuGmvsZ=MUVzm9Vj5YJXuVXfwqZYK7CT6KoLgumt6zoA at mail dot gmail dot com> <CA+=Sn1mCuixK85y+vvuNmmhuJ-SQVB=gy7GES-C7awVpgBxPTA at mail dot gmail dot com>
I had sent a revised patch to the list on Feb 18th:
We’re maintaining this change and use it on production.
So if need be, I can generate a new patch against the current master.
On 28 Oct 2014, at 09:55 , Andrew Pinski <email@example.com> wrote:
> On Tue, Jan 7, 2014 at 4:49 AM, Marcus Shawcroft
> <firstname.lastname@example.org> wrote:
>> A few minor issues....
> Any news on this patch since it never went in? Could you also remove
> md_do_align too?
> Right now even a simple:
> .align 6
> is broken.
> Andrew Pinski
>> On 2 January 2014 12:48, Philipp Tomsich
>> <email@example.com> wrote:
>>> + Here we fill the frag with the appropriate info for padding the output
>>> + stream. The resulting frag will consist of a fixed (fr_fix) and of a
>> GNU style places two spaces after a period. Same comment applies
>> through the patch.
>>> + repeating (fr_var) part.
>>> + The fixed content is always emitted before the repeating content and
>>> + these two parts are used as follows in constructing the output:
>>> + - the fixed part will be used to align to a valid instruction word
>>> + boundary, in case that we start at a misaligned address; as no
>>> + executable instruction can live at the misaligned location, we
>>> + simply fill with zeros;
>>> + - the variable part will be used to cover the remaining padding and
>>> + we fill using the AArch64 NOP instruction.
>>> + Note that the size of a RS_ALIGN_CODE fragment is always 7 to provide
>>> + enough storage space for up to 3 bytes for padding the back to a valid
>>> + instruction alignment and exactly 4 bytes to store the NOP pattern.
>>> + */
>> Remove the trailing white space throughout the patch please.
>>> -aarch64_handle_align (fragS * fragP)
>>> +aarch64_handle_align (fragS * frag)
>> Please don't mix rename / reformat with functional changes, it makes
>> it harder to review the patch. In this case fragP -> frag is an
>> unrelated change, either split the patch into two one with a rename
>> the other with the functional change, or drop the rename completely.
>>> /* NOP = d503201f */
>>> /* AArch64 instructions are always little-endian. */
>> Double space after period.
>>> @@ -5765,69 +5783,32 @@ aarch64_handle_align (fragS * fragP)
>>> int bytes, fix, noop_size;
>>> char *p;
>>> - const char *noop;
>>> - if (fragP->fr_type != rs_align_code)
>>> + if (frag->fr_type != rs_align_code)
>>> - bytes = fragP->fr_next->fr_address - fragP->fr_address - fragP->fr_fix;
>>> - p = fragP->fr_literal + fragP->fr_fix;
>>> - fix = 0;
>>> - if (bytes > MAX_MEM_FOR_RS_ALIGN_CODE)
>>> - bytes &= MAX_MEM_FOR_RS_ALIGN_CODE;
>>> + bytes = frag->fr_next->fr_address - frag->fr_address - frag->fr_fix;
>>> + p = frag->fr_literal + frag->fr_fix;
>>> #ifdef OBJ_ELF
>>> - gas_assert (fragP->tc_frag_data.recorded);
>>> + gas_assert (frag->tc_frag_data.recorded);
>>> - noop = aarch64_noop;
>>> - noop_size = sizeof (aarch64_noop);
>>> - fragP->fr_var = noop_size;
>>> + noop_size = sizeof(aarch64_noop);
>> Inappropriate removal of the white space before the opening parenthesis.
Dr. Philipp Tomsich
Theobroma Systems Design und Consulting GmbH
Seestadtstrasse 27 (Aspern IQ), A-1220 Wien, Austria
Phone: +43 1 2369893-401, Fax: +43 1 2369893-9-401
Cell phone: +43 664 8346109