[PATCH] D31528: [ELF][MIPS] Multi-GOT implementation

[George Rimar via Phabricator via llvm-commits]

grimar added a comment.

Few quick minor comments/suggestions.



================
Comment at: ELF/DriverUtils.cpp:31
 using namespace llvm;
+using namespace llvm::opt;
 using namespace llvm::sys;
----------------
Seems unused.


================
Comment at: ELF/InputFiles.h:116
+  // Index of MIPS GOT built for this file.
+  size_t MipsGotIndex = -1;
+
----------------
May be type could be Optional<> ?


================
Comment at: ELF/SyntheticSections.cpp:729
+    auto E = G.Tls.find(Sym);
+    assert(E != G.Tls.end());
+    return E->second * Config->Wordsize;
----------------
Does not seem assert is needed here. It anyways will either assert or crash on error,
not a huge difference probably. I would simplify:

```
if (Sym->isTls())
    return G.Tls.find(Sym)->second * Config->Wordsize;
if (Sym->IsPreemptible)
    return G.Global.find(Sym)->second * Config->Wordsize;
...

```

The same in `getGlobalDynOffset` and other places.


================
Comment at: ELF/SyntheticSections.h:337
+  // holds primary ans secondaty GOT's.
+  std::vector<FileGot> Gots;
+
----------------
Looks it can be `MapVector<InputFile *, FileGot>`.
Then probably you will not need to add `InputFile::MipsGotIndex` member.



================
Comment at: ELF/SyntheticSections.h:403
+  DynamicReloc(uint32_t Type, const InputSectionBase *InputSec,
+               uint64_t  OffsetInSec, const OutputSection *OutputSec,
+               int64_t Addend)
----------------
Excessive space: `uint64_t  OffsetInSec`


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D31528