RE: [Patch, ARM]Enable veneer routine for R_ARM_THM_JUMP19

["Tony Wang" <tony dot wang at arm dot com>]

New patch updated, fix Will's failure.

And it looks strange to me why binutils change the first branch instruction into a 32bit data, which introduced endian difference. Maybe anyone can help on me to understand why the first branch instruction is optimized out in some situation? However, as the jump24 is just doing the same thing, in this patch, I just keep conform with the old test case.

BR,
Tony
> -----Original Message-----
> From: Will Newton [mailto:will.newton@linaro.org]
> Sent: Monday, August 04, 2014 6:33 PM
> To: Tony Wang
> Cc: binutils@sourceware.org; nickc@redhat.com
> Subject: Re: [Patch, ARM]Enable veneer routine for R_ARM_THM_JUMP19
> 
> On 4 August 2014 11:27, Tony Wang <tony.wang@arm.com> wrote:
> >> -----Original Message-----
> >> From: Will Newton [mailto:will.newton@linaro.org]
> >> Sent: Monday, August 04, 2014 5:50 PM
> >> To: Tony Wang
> >> Cc: binutils@sourceware.org; nickc@redhat.com
> >> Subject: Re: [Patch, ARM]Enable veneer routine for
> R_ARM_THM_JUMP19
> >>
> >> On 4 August 2014 10:40, Tony Wang <tony.wang@arm.com> wrote:
> >> >> -----Original Message-----
> >> >> From: Will Newton [mailto:will.newton@linaro.org]
> >> >> Sent: Monday, August 04, 2014 5:14 PM
> >> >> To: Tony Wang
> >> >> Cc: binutils@sourceware.org; nickc@redhat.com
> >> >> Subject: Re: [Patch, ARM]Enable veneer routine for
> >> R_ARM_THM_JUMP19
> >> >>
> >> >> On 4 August 2014 05:40, Tony Wang <tony.wang@arm.com> wrote:
> >> >> > Hi there,
> >> >> >
> >> >> > According to AAPCS-ELF specification R_ARM_THM_JUMP19 should
> >> >> > also be veneered if the target is outside the addressable span
> >> >> > of the branch instruction or where interworking happened.
> >> >> >
> >> >> > So the attached patch implements the veneer routine for
> >> >> > R_ARM_THM_JUMP19, and two new macros
> >> >> THM2_MAX_FWD_COND_BRANCH_OFFSET
> >> >> > and THM2_MAX_BWD_COND_BRANCH_OFFSET are introduced.
> Also
> >> >> updated the
> >> >> > conditional branch for both cases mentioned above.
> >> >> >
> >> >> > Tested with Binutils regression test, no new regressions. No
> >> >> > regression on gcc trunk with target cortex m4
> >> >> >
> >> >> > Ok for the trunk?
> >> >> >
> >> >> > bfd/ChangeLog:
> >> >> > 2014-08-4  Tony Wang <tony.wang@arm.com>
> >> >> >
> >> >> >      * elf32-arm.c (elf32_arm_final_link_relocate): Implement
> >> >> > the veneer routine for R_ARM_THM_JUMP19.
> >> >> >      (arm_type_of_stub): Add conditional clause for
> >> R_ARM_THM_JUMP19
> >> >> >      (elf32_arm_size_stub): Ditto.
> >> >> >
> >> >> > ld/testsuite/ChangeLog:
> >> >> > 2014-08-4  Tony Wang <tony.wang@arm.com>
> >> >> >
> >> >> >      * ld-arm/jump-reloc-veneers-cond.s: New test.
> >> >> >      * ld-arm/farcall-cond-thumb-arm.s: Ditto.
> >> >> >      * ld-arm/jump-reloc-veneers-cond-short.d: Expected output
> >> >> > for target without a veneer generation.
> >> >> >      * ld-arm/jump-reloc-veneers-cond-long.d: Expected output
> >> >> > for target with a veneer generation.
> >> >> >      * ld-arm/farcall-cond-thumb-arm.d: Expected output for
> >> >> > inter working veneer generation.
> >> >> >      * ld-arm/arm-elf.exp: Add tests for conditional branch veneer.
> >> >>
> >> >> Looks ok to me (although I am not a maintainer). Small details but
> >> >> logical operations bind more loosely than almost every other
> >> >> operator so in some cases you can drop the brackets around
> >> >> assignments in conditionals and make them a bit easier to read.
> >> >
> >> > I agreed with your point.
> >> >
> >> >>Also the casts to unsigned int in
> >> >> elf32_arm_size_stubs seem unnecessary.
> >> >
> >> > From my point of view, it might still needed here, as the enum type
> >> > not
> >> always equals to unsigned int.
> >>
> >> Fair enough, its not a big issue.
> >>
> >> I also noticed a testsuite failure for armeb:
> >>
> >> /home/will/linaro/binutils-arm/ld/../binutils/objdump -d
> >> tmpdir/jump-reloc- veneers-cond-long regexp_diff match failure
> >> regexp "^    80..:      8002f040        .word   0x8002f040$"
> >> line   "    8000:       f0408002        .word   0xf0408002"
> >> FAIL: R_ARM_THM_JUMP19 Relocation veneers: Long
> >>
> >
> > Hi Will,
> > Thanks a lot for your report, and are you using the big endian toolchain? It
> looks like an endian issue. I only tested it with little endian toolchain.
> 
> I configured binutils with --target=armeb-none-eabi, although I also
> found that armeb EABI targets seem to be broken in general so I
> submitted a patch to fix that.
> 
> --
> Will Newton
> Toolchain Working Group, Linaro

Attachment: relocation_jump19.diff
Description: Binary data