[PATCH] D18499: [ELF] - Implemented prototype of location counter support.
Rui Ueyama via llvm-commits
llvm-commits at lists.llvm.org
Tue Apr 12 12:59:28 PDT 2016
ruiu added inline comments.
================
Comment at: ELF/LinkerScript.cpp:51-65
@@ +50,17 @@
+
+ StringRef Tok = Tokens[I];
+ if (Tok == ".") {
+ Result += LocCounter;
+ continue;
+ }
+
+ uint64_t Val;
+ if (Tok.getAsInteger(0, Val)) {
+ error("undeclared variable " + Tok);
+ return 0;
+ }
+
+ Result += Val;
+ }
+ return Result;
+}
----------------
Define this method
uint64_t getInteger(StringRef S) {
uint64_t V;
if (S.getAsInteger(0, V)) {
setError("malformed number: " + V);
return 0;
}
retrurn V;
}
Then you can simplify this to
StringRef Tok = Tokens[I];
if (Tok == ".")
Result += LocCounter;
else
Result += getInteger(Tok);
================
Comment at: ELF/LinkerScript.cpp:94
@@ +93,3 @@
+void LinkerScript::setSectionsAddresses(
+ llvm::StringMap<OutputSectionBase<ELFT> *> &S,
+ std::vector<LocationNode> &Loc) {
----------------
Are you aware of the fact that StringMap copies keys? Maybe DenseMap is more efficient because it doesn't copy StringRefs?
================
Comment at: ELF/LinkerScript.cpp:113
@@ +112,3 @@
+ uintX_t Align = Sec->getAlign();
+ if (Sec->getFlags() & SHF_TLS && Sec->getType() == SHT_NOBITS) {
+ uintX_t TVA = VA + ThreadBssOffset;
----------------
Needs () around &.
================
Comment at: ELF/LinkerScript.cpp:131
@@ +130,3 @@
+template <class ELFT>
+void LinkerScript::assignAddresses(std::vector<OutputSectionBase<ELFT> *> &S) {
+ typedef typename ELFT::uint uintX_t;
----------------
This function seems a bit too complicated. This function construct temporary data to feed to setSectionsAddresses. Do we really need to do that? Isn't there any way to make it simpler?
================
Comment at: ELF/LinkerScript.cpp:154
@@ +153,3 @@
+ for (OutputSectionBase<ELFT> *Sec : S)
+ if (NameToSec.count(Sec->getName()) == 0) {
+ StringRef Name = Sec->getName();
----------------
Flip the condition and use early continue.
http://reviews.llvm.org/D18499
More information about the llvm-commits
mailing list