[PATCH] D33373: [lld] Infer relocation model from module flags in relocatable LTO link

[Davide Italiano via Phabricator via llvm-commits]

davide added a subscriber: ruiu.
davide added inline comments.


================
Comment at: ELF/LTO.cpp:76-79
+  if (Config->Relocatable)
+    Conf.RelocModel = None;
+  else
+    Conf.RelocModel = Config->Pic ? Reloc::PIC_ : Reloc::Static;
----------------
I'll leave the final decision to @ruiu, but I feel 
```
 if ()
   [...]
 else if()
   [...]
  else()
```
would be more readable in this case.
Not strong about this  one either :)


================
Comment at: test/ELF/lto/relocation-model-pic.ll:3-6
+; RUN: cat %s >%t.pic.ll
+; RUN: echo '!llvm.module.flags = !{!0}' >>%t.pic.ll
+; RUN: echo '!0 = !{i32 1, !"PIC Level", i32 2}' >>%t.pic.ll
+
----------------
eugenis wrote:
> davide wrote:
> > can't this be a separate input in living in `Inputs/` ?
> It can, but then the reader has to go and find this other file to understand what's going on in the test...
> I can do it if you feel strongly about it.
> 
Not necessarily, but this is generally what's done elsewhere in `lld`, so I'd keep it like that for consistency.


Repository:
  rL LLVM

https://reviews.llvm.org/D33373