We use this in lld, so it's kinda needed for parity with what's used there.
I might try to add an option for that in the gold-plugin as well.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
lib/LTO/LTOBackend.cpp | ||
---|---|---|
145 ↗ | (On Diff #69887) | Please use better variable name. (I know the rest of the file doesn't, but we should fix it and not spread this). |
lib/LTO/LTOBackend.cpp | ||
---|---|---|
145 ↗ | (On Diff #69887) | Instead of taking a config, you can pass directly the string and the bool you need. |
165 ↗ | (On Diff #69887) | We always verify the input. |
Sorry, I was distracted by other things. This should address part of the comments and provide feedback where I have a different opinion. Thanks!
lib/LTO/LTOBackend.cpp | ||
---|---|---|
125 ↗ | (On Diff #70478) | It seems like this should be provided in LLVM somewhere, why would this be different between opt or clang for instance and LTO? |
lib/LTO/LTOBackend.cpp | ||
---|---|---|
145 ↗ | (On Diff #69887) | I changed the arguments to the function. About centralizing somewhere, maybe. |
145 ↗ | (On Diff #69887) | I'd rather be consistent with what's there already, and change the whole naming convention in a second pass, if you don't mind. |
165 ↗ | (On Diff #69887) | Not in lld, but, OK, I think it's a decent idea, so I made the change. |
lib/LTO/LTOBackend.cpp | ||
---|---|---|
180 ↗ | (On Diff #70478) |
Sorry, it does not make sense to me, opt has to need the same thing.
If you don't want to break consistency, that's fine: first change the names in the whole file, and then update this patch. |