[PATCH][gold, aarch64] Skip ERRATUM 843419 fix if the sequences have been relaxed by TLS optimization

[Jiong Wang]

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?

>
> 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.
>>
>>
>
>