This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH v2][gold] AArch64: Implement MOVW_UABS_* and MOVW_SABS_* relocations.
- From: Igor Kudrin <ikudrin at accesssoftek dot com>
- To: Han Shen <shenhan at google dot com>
- Cc: Cary Coutant <ccoutant at gmail dot com>, "binutils at sourceware dot org" <binutils at sourceware dot org>
- Date: Mon, 25 Jul 2016 10:29:30 +0000
- Subject: Re: [PATCH v2][gold] AArch64: Implement MOVW_UABS_* and MOVW_SABS_* relocations.
- Authentication-results: sourceware.org; auth=none
- References: <1466787419528.41306@accesssoftek.com> <CAJimCsFmHe2aQSXu5ycME+YwkTusX2d-Xu2ypm6Xr1QOKansBw@mail.gmail.com> <CAJimCsHOchcBY=EaME=cewSTKPuTxxMNvQJ0VpcsTWE5Q5adug@mail.gmail.com> <CACkGtriCVA7ZkrhAptB405ynOOdgzKcDHJY-erPKD6gtROL6+A@mail.gmail.com> <1467306861636.23559@accesssoftek.com>,<CACkGtrjpi=uX-65aqViBk-+ttbP40p74s13gQ90cFSMdpPV8ug@mail.gmail.com>
Hello Han,
Thanks for the review.
Could someone apply the patch, please? I don't have write permissions yet.
Best regards,
Igor Kudrin
________________________________________
From: Han Shen <shenhan@google.com>
Sent: Wednesday, July 6, 2016 10:47 PM
To: Igor Kudrin
Cc: Cary Coutant; binutils@sourceware.org
Subject: Re: [PATCH v2][gold] AArch64: Implement MOVW_UABS_* and MOVW_SABS_* relocations.
Thanks, Igor, LGTM.
Han
On Thu, Jun 30, 2016 at 10:14 AM, Igor Kudrin <ikudrin@accesssoftek.com> wrote:
> Hi Cary, Han,
>
> Thank you very much for the review comments!
>
> Here is an updated patch with the following changes:
> * Rebased to the top.
> * Changed "(uint64_t)1" to "1ULL".
> * Added a comment for "Rvalue_bit_select_impl<0, 0>".
> * Changed "rela_general<64>" to "rela_general<32>".
>
> Best regards,
> Igor Kudrin
>
> ---
> gold/ChangeLog
>
> * aarch64-reloc-property.cc (Rvalue_bit_select_impl): New class.
> (rvalue_bit_select): Use Rvalue_bit_select_impl.
> * aarch64-reloc.def (MOVW_UABS_G0, MOVW_UABS_G0_NC, MOVW_UABS_G1,
> MOVW_UABS_G1_NC, MOVW_UABS_G2, MOVW_UABS_G2_NC, MOVW_UABS_G3,
> MOVW_SABS_G0, MOVW_SABS_G1, MOVW_SABS_G2): New relocations.
> * aarch64.cc (Target_aarch64::Scan::local): Add cases for new
> MOVW_UABS_* and MOVW_SABS_* relocations.
> (Target_aarch64::Scan::global): Likewise.
> (Target_aarch64::Relocate::relocate): Add cases and handlings
> for new MOVW_UABS_* and MOVW_SABS_* relocations.
> * testsuite/Makefile.am (aarch64_relocs): New test.
> * testsuite/Makefile.in: Regenerate.
> * testsuite/aarch64_globals.s: New test source file.
> * testsuite/aarch64_relocs.s: Likewise.
> * testsuite/aarch64_relocs.sh: New test script.
>
> ________________________________________
> From: Han Shen <shenhan@google.com>
> Sent: Wednesday, June 29, 2016 8:48 AM
> To: Cary Coutant
> Cc: Igor Kudrin; binutils@sourceware.org
> Subject: Re: [PATCH][gold] AArch64: Implement MOVW_UABS_* and MOVW_SABS_* relocations.
>
> Hi Igor, could you add comments to the specialization that L=U=0 means
> no check, otherwise readers might mistake it for selecting LSB. (The
> origin code should have added it.)
>
> +class Rvalue_bit_select_impl<0, 0>
> +{
> +public:
> + static uint64_t
> + calc(uint64_t x)
> + {
> + return x;
> + }
> +};
>
> Also in the below segment - "rela_general<64>" should be
> "rela_general<32>", cause we are patching a 32 bit mov insn.
>
> + case elfcpp::R_AARCH64_MOVW_UABS_G3:
> + reloc_status = Reloc::template rela_general<64>(
> + view, object, psymval, addend, reloc_property);
> + break;
>
> Thanks,
> Han
>
> On Tue, Jun 28, 2016 at 3:18 PM, Cary Coutant <ccoutant@gmail.com> wrote:
>> [+shenhan this time]
>>
>> On Tue, Jun 28, 2016 at 3:17 PM, Cary Coutant <ccoutant@gmail.com> wrote:
>>>> This patch implements R_AARCH64_MOVW_UABS_G* and R_AARCH64_MOVW_SABS_G*
>>>> relocations for AArch64 target.
>>>>
>>>> Best regards,
>>>> Igor Kudrin
>>>>
>>>> ---
>>>> gold/ChangeLog
>>>>
>>>> * aarch64-reloc-property.cc (Rvalue_bit_select_impl): New class.
>>>> (rvalue_bit_select): Use Rvalue_bit_select_impl.
>>>> * aarch64-reloc.def (MOVW_UABS_G0, MOVW_UABS_G0_NC, MOVW_UABS_G1,
>>>> MOVW_UABS_G1_NC, MOVW_UABS_G2, MOVW_UABS_G2_NC, MOVW_UABS_G3,
>>>> MOVW_SABS_G0, MOVW_SABS_G1, MOVW_SABS_G2): New relocations.
>>>> * aarch64.cc (Target_aarch64::Scan::local): Add cases for new
>>>> MOVW_UABS_* and MOVW_SABS_* relocations.
>>>> (Target_aarch64::Scan::global): Likewise.
>>>> (Target_aarch64::Relocate::relocate): Add cases and handlings
>>>> for new MOVW_UABS_* and MOVW_SABS_* relocations.
>>>> * testsuite/Makefile.am (aarch64_relocs): New test.
>>>> * testsuite/Makefile.in: Regenerate.
>>>> * testsuite/aarch64_globals.s: New test source file.
>>>> * testsuite/aarch64_relocs.s: New test source file.
>>>> * testsuite/aarch64_relocs.sh: New test script.
>>>
>>> + static uint64_t
>>> + calc(uint64_t x)
>>> + {
>>> + return (x & (((uint64_t)1 << (U+1)) - 1)) >> L;
>>> + }
>>>
>>> I know this was the same before your patch, but I'd prefer using
>>> "1ULL" instead of the C-style cast.
>>>
>>> Han, can you take a look at this patch?
>>>
>>> -cary
>
>
>
> --
> Han Shen | Software Engineer | shenhan@google.com | +1-650-440-3330
--
Han Shen | Software Engineer | shenhan@google.com | +1-650-440-3330