This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]