[PATCH] [lld][ELF][ARM] Implement static (initial exec) TLS model

Denis Protivensky dprotivensky at accesssoftek.com
Tue Mar 17 00:38:52 PDT 2015


================
Comment at: lib/ReaderWriter/ELF/ARM/ARMRelocationPass.cpp:116
@@ +115,3 @@
+  ArrayRef<uint8_t> rawContent() const override {
+    return llvm::makeArrayRef(ARMGotAtomContent);
+  }
----------------
ruiu wrote:
> Can you move the definition of ARMGotAtomContent here as a static local variable?
Good.

================
Comment at: lib/ReaderWriter/ELF/ARM/ARMRelocationPass.cpp:205
@@ +204,3 @@
+  std::error_code handleTLSIE32(const Reference &ref) {
+    if (const auto *target = dyn_cast_or_null<DefinedAtom>(ref.target())) {
+      const_cast<Reference &>(ref).setTarget(
----------------
ruiu wrote:
> Can ref.target() return a null pointer? If not, you want to use dyn_cast instead of dyn_cast_or_null.
I think it shouldn't return null pointer. Some implementations use dyn_cast_or_null, this might have confused me. Will change to dyn_cast.

================
Comment at: lib/ReaderWriter/ELF/ARM/ARMTargetHandler.h:41
@@ +40,3 @@
+    }
+    return *_tpOff;
+  }
----------------
ruiu wrote:
> If TP_TLS section is not found, it seems that this line will fail because tpOff has no value. Maybe you want to return a new _tpOff from the innermost block in the for loop and replace this line with llvm_unreachable?
> 
> Also you might want to use early return.
You're right that it's better to have llvm_unreachable than fail with unknown error. Will change.

http://reviews.llvm.org/D8353

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list