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.


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.

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]