[PATCH] D18119: [ELF][MIPS] Factor out SumVA adjustments into a couple of separate functions. NFC.

George Rimar via llvm-commits llvm-commits at lists.llvm.org
Sat Mar 12 08:39:44 PST 2016


grimar added a comment.

Generally this looks good to me, have few notes though.


================
Comment at: ELF/InputSection.cpp:219
@@ +218,3 @@
+  return Body.getGotVA<ELFT>();
+}
+
----------------
Is that correct ? Originally addend was added:

```
 SymVA = Out<ELFT>::Got->getMipsLocalFullAddr(Body) + A;
 }
}
else {
 SymVA = Body.getGotVA<ELFT>() + A;
```

================
Comment at: ELF/InputSection.cpp:290
@@ -265,3 +289,3 @@
+      else
         SymVA = Body.getGotVA<ELFT>() + A;
       if (Body.IsTls)
----------------
I don't like this place because it has "if (Config->EMachine == EM_MIPS)"
I would do next:
1) rename static uintX_t getMipsGotVA -> static uintX_t getGotVA
2) remove uintX_t SymVA argument, but add uintX_t A

and do:

```
template <class ELFT, class uintX_t>
static uintX_t getGotVA(const SymbolBody &Body, uintX_t A,
                            uint8_t *BufLoc, uint8_t *PairedLoc) {

if (Config->EMachine != EM_MIPS)
 return Body.getGotVA<ELFT>() + A;
...
```

```
else if (Target->needsGot(Type, Body)) {
  SymVA = getGotVA<ELFT>(Body, A, BufLoc, PairedLoc);
...
```


Repository:
  rL LLVM

http://reviews.llvm.org/D18119





More information about the llvm-commits mailing list