[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