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]

Re: [gold][aarch64]Patch to support TLS


> elfcpp/ChangeLog:
> 2014-09-11  Han Shen  <shenhan@google.com>
>         * aarch64.h (R_AARCH64_TLS_DTPREL64): Switch enum value with ...
>         (R_AARCH64_TLS_DTPMOD64): ... enum value.
>
> gold/ChangeLog:
> 2014-09-11  Han Shen  <shenhan@google.com>
>             Jing Yu  <jingyu@google.com>

Looks good. I found a few typos and made some minor comments. The
patch is OK with these fixes. Thanks!

-cary


+// Above if from Table 4-19, TLS descriptor relocations, 560-569.

s/if/is/

+ tls::Tls_optimization tlsopt =Target_aarch64<size, big_endian>::
+  optimize_tls_reloc(!parameters->options().shared(), r_type);

Space after '='. (Two locations)

+    // Create preserved PLT and GOT entries for the resolver.

s/preserved/reserved/

@@ -2537,22 +3412,39 @@ Target_aarch64<size, big_endian>::Relocate::relocate(
...
       gold_error_at_location(relinfo, relnum, rela.get_r_offset(),
+     _("unsupported reloc during scan global %u"),

This is not during the scan pass -- it's in relocate().
Anyway, telling the user what pass we're in is unlikely to mean
anything. It would help to print the name of the symbol here:
"unsupported reloc against global symbol xxx".

+ gold_error_at_location(relinfo, relnum, rela.get_r_offset(),
+       _("unsupport gd_to_ie relaxation on %u"),
+       r_type);

s/unsupport/unsupported/

+  // The origin sequence is -

s/origin/original/

+  // Unlike tls_ie_to_le, we change the 3 insns in one funcation call when we

"function"

+  // relocation type to process. So before proceed, we need to make sure

"before proceeding"

+  if(!(insn1 == 0x90000000
+       && insn2 == 0x91000000
+       && insn3 == 0x94000000))

It would help the reader if you used macros for the various instruction
bit patterns here and below.

+      gold_error(_("unexpected reloc insn sequence while relaxing "
+   "tls gd to le for reloc %u."), r_type);

Does this need to be an error? Can it be a warning instead?

+    default:
+      gold_error(_("Don't support tlsdesc gd_to_ie optimization on reloc %u"),
+ r_type);

For consistency with other messages, this should say "unsupported"
instead of "don't support". (And we don't capitalize the first word
of error messages.)


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