Re: [Patch, microblaze, bfd/gas/ld] Add support for MicroBlaze little-endian (microblazeel)

[David Holsgrove <david dot holsgrove at xilinx dot com>]

Hi Michael,

On 9 November 2012 07:41, Michael Eager <eager@eagerm.com> wrote:
> On 11/08/2012 04:25 AM, David Holsgrove wrote:
>>
>> Add microblazeel target support to bfd, gas and ld.
>>
>> binutils/bfd/Changelog
>>
>>   2012-11-08  Edgar E. Iglesias <edgar.iglesias@gmail.com>
>>
>>            * config.bfd: Add microblazeel-*-*
>>            * configure.in: Likewise.
>>            * configure: Regenerate.
>>            * elf32-microblaze.c (microblaze_elf_relocate_section):
>>              Add endian awareness.
>>              (microblaze_elf_merge_private_bfd_data): New.
>>              (microblaze_bfd_write_imm_value_32): New.
>>              (microblaze_bfd_write_imm_value_64): New.
>>              (microblaze_elf_relax_section): Add endian awareness.
>>              (microblaze_elf_add_symbol_hook): Define TARGET_LITTLE_NAME,
>>              TARGET_LITTLE_SYM and bfd_elf32_bfd_merge_private_bfd_data.
>>            * targets.c: Add bfd target bfd_elf32_microblazeel_vec.
>
>
>
> +++ b/bfd/elf32-microblaze.c
> @@ -702,6 +702,7 @@ microblaze_elf_relocate_section (bfd *output_bfd,
>    Elf_Internal_Shdr *symtab_hdr = &elf_tdata (input_bfd)->symtab_hdr;
>    struct elf_link_hash_entry **sym_hashes = elf_sym_hashes (input_bfd);
>    Elf_Internal_Rela *rel, *relend;
> +  int endian = !bfd_little_endian (output_bfd) * 2;
>
> This would be better as
>    int endian =  (bfd_little_endian (output_bfd)) ? 0 : 2;
>
>
> @@ -1275,6 +1291,36 @@ calc_fixup (bfd_vma addr, asection *sec)
>    return fixup;
>  }
>
> +/* Read-modify-write into the bfd, an immediate value into appropriate
> fields of a 32-bit
> + * instruction. */
>
> GNU comment style is to end each comment with a period and two spaces.
> Don't continue comments with an asterisk.   Limit lines to 80 chars.
> There are a number of comments which should be edited.
>
> @ -1627,13 +1676,13 @@ microblaze_elf_relax_section (bfd *abfd,
>                             }
>                         }
>
> -                     immediate = (unsigned short) bfd_get_16 (abfd,
> ocontents +
> -
> irelscan->r_offset + 2);
> +                      unsigned long instr = bfd_get_32 (abfd, ocontents +
> irelscan->r_offset);
> +                      immediate = instr & 0x0000ffff;
>                       target_address = immediate;
>                       offset = calc_fixup (target_address, sec);
>                       immediate -= offset;
>                       irelscan->r_addend -= offset;
> -                     bfd_put_16 (abfd, immediate, ocontents +
> irelscan->r_offset + 2);
> +                      microblaze_bfd_write_imm_value_32 (abfd, ocontents +
> irelscan->r_offset, irelscan->r_addend);
>                     }
>
> Split lines so that they are not longer than 80 chars.  There are several
> long lines.
>

Thanks for the review. I've made the adjustments you suggested and
checked the line lengths for each of the additions made in this patch.
Please find attached revised patch.

regards,
David

> --
> Michael Eager    eager@eagercon.com
> 1960 Park Blvd., Palo Alto, CA 94306  650-325-8077
>
>

Attachment: 0001-Add-support-for-microblaze-little-endian.patch
Description: Binary data