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

Rui Ueyama ruiu at google.com
Mon Mar 16 11:25:01 PDT 2015


================
Comment at: lib/ReaderWriter/ELF/ARM/ARMRelocationHandler.cpp:325
@@ +324,3 @@
+      llvm::dbgs() << " P: 0x" << Twine::utohexstr(P);
+      llvm::dbgs() << " result: 0x" << Twine::utohexstr(result) << "\n");
+  applyArmReloc(location, result);
----------------
Off topic, but I always wonder if we want to sprinkle debug outputs like this in the ELF writers. They are longer than actual code and might be too verbose.

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

================
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(
----------------
Can ref.target() return a null pointer? If not, you want to use dyn_cast instead of dyn_cast_or_null.

================
Comment at: lib/ReaderWriter/ELF/ARM/ARMTargetHandler.h:41
@@ +40,3 @@
+    }
+    return *_tpOff;
+  }
----------------
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.

http://reviews.llvm.org/D8353

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






More information about the llvm-commits mailing list