This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH][gold, aarch64] Skip ERRATUM 843419 fix if the sequences have been relaxed by TLS optimization
Thanks,
On Wed, Jun 14, 2017 at 1:42 AM, Jiong Wang <jiong.wang@foss.arm.com> wrote:
> On 13/06/17 23:17, Han Shen via binutils wrote:
>>
>> Whereas the correct behavior should leave the code
>> untouched, because adrp is replaced by relaxation and shall never
>> trigger the erratum.
>
>
> Hi Han,
>
> This is exactly what I want to do.
>
>> What do you think?
>
>
> Look more on fix_errata + try_try_fix_erratum_843419_optimized, I think we
> should return true as return true means this sequence becomes safe after
> either
> instruction rewrite or double-check that we don't want to install the
> branch-to-stub,
> is this looks correct to you?
Yup, let's change "return false;" -> "return true;" (and add proper comments).
Also, I assume this relaxation-changing-instructions behavior should
not break erratum 835769? Erratum 843419 must have adrp as its first
insn, whereas 835769 begins with LD/ST.
Otherwise, LGTM
We shall still wait for Cary's approval.
Han
>
>
>>
>> Han
>>
>> On Thu, Jun 8, 2017 at 3:35 AM, Jiong Wang <jiong.wang@foss.arm.com>
>> wrote:
>>>
>>> On 07/06/17 10:26, Jiong Wang wrote:
>>>>
>>>> On 18/07/16 17:46, Han Shen wrote:
>>>>
>>>>> Hi Andrew, thanks for reporting this. Could you send me the objs and
>>>>> the command line? (I tried to build hhvm on aarch64 machine, seemed to
>>>>> me this needs a few third_packages that need to be installed through
>>>>> apt-get (mysql, for example), since I am not a superuser on the
>>>>> machine, it is not easy for me to build the whole thing from scratch
>>>>> ...)
>>>>>
>>>>> On Fri, Jul 15, 2016 at 10:42 PM, Andrew Pinski <pinskia@gmail.com>
>>>>> wrote:
>>>>>>
>>>>>> On Thu, Jul 16, 2015 at 2:44 PM, Han Shen <shenhan@google.com> wrote:
>>>>>>>
>>>>>>> Hi Cary, this is the patch for erratum 843419 fix optimization.
>>>>>>>
>>>>>>> Usually we apply branch-to-stub fix for all erratum. For 843419,
>>>>>>> under
>>>>>>> some
>>>>>>> condition, instead of generating jumps, we re-write 'adrp' with 'adr'
>>>>>>> (only
>>>>>>> applicable if adrp calculation result fits in adr range), thus break
>>>>>>> such
>>>>>>> erratum sequence and eliminate performance penalty (2-jump/fix).
>>>>>>>
>>>>>>> Test - build on x86_64 platform and aarch64 platform using opt and
>>>>>>> debug (-O0).
>>>>>>> Pass unit tests. Pass gold local test suite. Pass tests from
>>>>>>> arm.
>>>>>>>
>>>>>>> Ok for trunk?
>>>>>>
>>>>>> Hi,
>>>>>> I am getting an internal error some of the time when linking HHVM
>>>>>> :
>>>>>> Erratum 843419 found and fixed at
>>>>>> "../runtime/libhphp_runtime.a(bytecode.cpp.o)", section 10882, offset
>>>>>> 0x0000022c.
>>>>>> Erratum 843419 found and fixed at
>>>>>> "../runtime/libhphp_runtime.a(unique-stubs.cpp.o)", section 7040,
>>>>>> offset 0x00000218.
>>>>>> Erratum 843419 found and fixed at
>>>>>> "../runtime/libhphp_runtime.a(bytecode.cpp.o)", section 10882, offset
>>>>>> 0x0000022c.
>>>>>> Erratum 843419 found and fixed at
>>>>>> "../runtime/libhphp_runtime.a(unique-stubs.cpp.o)", section 7040,
>>>>>> offset 0x00000218.
>>>>>> /usr/bin/ld.gold: internal error in try_fix_erratum_843419_optimized,
>>>>>> at ../../gold/aarch64.cc:2007
>>>>>> collect2: error: ld returned 1 exit status
>>>>>>
>>>>>> Is there anything which you need to debug this issue?
>>>>>>
>>>>>> Thanks,
>>>>>> Andrew
>>>>>>
>>>>>>> gold/ChangeLog
>>>>>>>
>>>>>>> 2015-07-15 Han Shen <shenhan@google.com>
>>>>>>>
>>>>>>> Optimize erratum 843419 fix.
>>>>>>>
>>>>>>> gold/ChangeLog: * aarch64.cc
>>>>>>> (AArch64_insn_utilities::is_adr):
>>>>>>> New
>>>>>>> method. (AArch64_insn_utilities::aarch64_adr_encode_imm):
>>>>>>> New
>>>>>>> method.
>>>>>>> (AArch64_insn_utilities::aarch64_adrp_decode_imm): New
>>>>>>> method.
>>>>>>> (E843419_stub): New sub-class of Erratum_stub.
>>>>>>> (AArch64_relobj::try_fix_erratum_843419_optimized): New
>>>>>>> method.
>>>>>>> (AArch64_relobj::section_needs_reloc_stub_scanning): Try
>>>>>>> optimized fix.
>>>>>>> (AArch64_relobj::create_erratum_stub): Add 1 argument.
>>>>>>> (Target_aarch64::scan_erratum_843419_span): Pass in adrp
>>>>>>> insn
>>>>>>> offset.
>>>>>>>
>>>>>>> --
>>>>>>> Han Shen
>>>>
>>>>
>>>> I have encountered the same issue.
>>>>
>>>> On latest GOLD, the following assertion triggered:
>>>>
>>>> gold/aarch64.cc +2016
>>>> gold_assert(Insn_utilities::is_adrp(adrp_insn));
>>>>
>>>> A quick debug shows the offending instruction sequence is like the
>>>> following:
>>>>
>>>>
>>>> 91000401 add x1, x0, #0x1
>>>> d53bd042 mrs x2, tpidr_el0
>>>> d2a00000 movz x0, #0x0, lsl #16 <- adrp_sh_offset
>>>> f285cf00 movk x0, #0x2e78
>>>> 8b000040 add x0, x2, x0
>>>> f9001401 str x1, [x0,#40]
>>>>
>>>> This sequence looks like the sequence for TLS local executable mode, So,
>>>> I
>>>> am
>>>> wondering if this issue is caused by TLS relaxtaion?
>>>>
>>>> Before relaxataion they will be adrp + add instructions and the stub was
>>>> recorded
>>>> at that time. Then the later relaxation change the instructions and
>>>> break
>>>> the
>>>> assumptions.
>>>
>>>
>>>
>>> Here is a proposed fix, please have a look, it fixed the HHVM build.
>>>
>>> This patch simply returns false from the erratum fix function. From my
>>> understanding, this ignores that particular stub. I feel this is
>>> reasonable
>>> as
>>> after TLS relaxataion the original sequence does not contain adrp
>>> instruction
>>> anymore.
>>> This patch return false in to cases:
>>> * if the instruction pointed by adrp_sh_offset is "mrs R, tpidr_el0".
>>> IE -> LE relaxation etc. may generate this.
>>> * if the instruction pointed by adrp_sh_offset is not ADRP and the
>>> instruction
>>> before adrp_sh_offset is "mrs R, tpidr_el0", LD -> LE relaxation etc
>>> may
>>> generate this.
>>>
>>>
>>> gold/
>>> 2017-06-08 Jiong Wang <jiong.wang@arm.com>
>>>
>>> * aarch64.cc (Insn_utilities::is_mrs_tpidr_el0): New method.
>>> (AArch64_relobj<size,
>>> big_endian>::try_fix_erratum_843419_optimized):
>>> Skip if there is TLS relaxation.
>>>
>>>
>>
>>
>
--
Han Shen | Software Engineer | shenhan@google.com | +1-650-440-3330