This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
[PATCH][gold, aarch64] Skip ERRATUM 843419 fix if the sequences have been relaxed by TLS optimization
- From: Jiong Wang <jiong dot wang at foss dot arm dot com>
- To: Han Shen <shenhan at google dot com>, Andrew Pinski <pinskia at gmail dot com>
- Cc: binutils <binutils at sourceware dot org>, Cary Coutant <ccoutant at gmail dot com>, Luis Lozano <llozano at google dot com>
- Date: Thu, 8 Jun 2017 11:35:21 +0100
- Subject: [PATCH][gold, aarch64] Skip ERRATUM 843419 fix if the sequences have been relaxed by TLS optimization
- Authentication-results: sourceware.org; auth=none
- References: <CACkGtriWCQ=g-0WGY0wuLUp6PZ+sgpFcJz2cEbtSE5RCkdXTmA@mail.gmail.com> <CA+=Sn1kwk5CXGSmMNYkKcaxxJq2g4OCfB7agDYerq00nsaSnkA@mail.gmail.com> <CACkGtri0jy=YocqcpTgT7ceE+Ogd9ZsbThvh8eTtE8VYpq_sTw@mail.gmail.com> <6284fdb4-6075-ab71-ab01-22657e505425@foss.arm.com>
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.
diff --git a/gold/aarch64.cc b/gold/aarch64.cc
index 2470986..3bfb96d 100644
--- a/gold/aarch64.cc
+++ b/gold/aarch64.cc
@@ -110,6 +110,10 @@ public:
is_adrp(const Insntype insn)
{ return (insn & 0x9F000000) == 0x90000000; }
+ static bool
+ is_mrs_tpidr_el0(const Insntype insn)
+ { return (insn & 0xFFFFFFE0) == 0xd53bd040; }
+
static unsigned int
aarch64_rm(const Insntype insn)
{ return aarch64_bits(insn, 16, 5); }
@@ -2010,9 +2014,35 @@ AArch64_relobj<size, big_endian>::try_fix_erratum_843419_optimized(
E843419_stub<size, big_endian>* e843419_stub =
reinterpret_cast<E843419_stub<size, big_endian>*>(stub);
AArch64_address pc = pview.address + e843419_stub->adrp_sh_offset();
- Insntype* adrp_view = reinterpret_cast<Insntype*>(
- pview.view + e843419_stub->adrp_sh_offset());
+ unsigned int adrp_offset = e843419_stub->adrp_sh_offset ();
+ Insntype* adrp_view = reinterpret_cast<Insntype*>(pview.view + adrp_offset);
Insntype adrp_insn = adrp_view[0];
+ Insntype prev_insn;
+
+ if (adrp_offset)
+ {
+ Insntype* prev_view
+ = reinterpret_cast<Insntype*>(pview.view + adrp_offset - 4);
+ prev_insn = prev_view[0];
+ }
+
+ // TLS relaxation might change ADRP instruction into other instructions.
+ // The following check identifies the following cases:
+ //
+ // 1. If the instruction pointed to by adrp_sh_offset is "mrs R, tpidr_el0".
+ // This may come from IE -> LE relaxation etc. ADRP has been turned
+ // into MRS as expected, we could safely skip the erratum fix.
+ //
+ // 2. If the instruction before adrp_sh_offset is "mrs R, tpidr_el0" and
+ // the instruction at adrp_sh_offset is not ADRP. This may come from
+ // LD -> LE relaxation etc. Similar as case 1, we can safely skip the
+ // erratum fix.
+ if (Insn_utilities::is_mrs_tpidr_el0(adrp_insn)
+ || (!Insn_utilities::is_adrp(adrp_insn)
+ && adrp_offset && Insn_utilities::is_mrs_tpidr_el0(prev_insn)))
+ return false;
+
+ /* If we reach here, the first instruction must be ADRP. */
gold_assert(Insn_utilities::is_adrp(adrp_insn));
// Get adrp 33-bit signed imm value.
int64_t adrp_imm = Insn_utilities::