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/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.


On Tue, Oct 28, 2014 at 3:58 AM, Dr. Philipp Tomsich
<philipp.tomsich@theobroma-systems.com> wrote:
> Andrew,
>
> I had sent a revised patch to the list on Feb 18th:
>         https://sourceware.org/ml/binutils/2014-02/msg00108.html
>
> Weâre maintaining this change and use it on production.
> So if need be, I can generate a new patch against the current master.

Yes that would be useful as it seems to fix
https://bugs.linaro.org/show_bug.cgi?id=815 .

Thanks,
Andrew Pinski


>
> Best,
> Philipp.
>
> On 28 Oct 2014, at 09:55 , Andrew Pinski <pinskia@gmail.com> wrote:
>
>> On Tue, Jan 7, 2014 at 4:49 AM, Marcus Shawcroft
>> <marcus.shawcroft@gmail.com> wrote:
>>> Hi,
>>>
>>> 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:
>> start64:
>> nop
>> .align 6
>> done:
>>  nop
>>
>> is broken.
>>
>> Thanks,
>> Andrew Pinski
>>
>>
>>>
>>> On 2 January 2014 12:48, Philipp Tomsich
>>> <philipp.tomsich@theobroma-systems.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.
>>>
>>>>
>>>> void
>>>> -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)
>>>>     return;
>>>>
>>>> -  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);
>>>> #endif
>>>>
>>>> -  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.
>>>
>>> Thanks
>>> /Marcus
>
> 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
> http://www.theobroma-systems.com
>


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