[PATCH] D18499: [ELF] - Implemented prototype of location counter support.

[Rui Ueyama via llvm-commits]

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