[PATCH] D16468: [ELF] - Attempt to simplify the relocations relaxation code
Rui Ueyama via llvm-commits
llvm-commits at lists.llvm.org
Fri Jan 22 10:32:08 PST 2016
ruiu added inline comments.
================
Comment at: ELF/InputSection.h:85-87
@@ -81,1 +84,5 @@
+ struct RelocData {
+ bool Optimized = false;
+ };
+ struct RelocSec {
----------------
Why do you need a struct that has only one member?
================
Comment at: ELF/InputSection.h:91
@@ +90,3 @@
+ const Elf_Shdr *Sec = nullptr;
+ std::vector<RelocData> Relocs;
+ };
----------------
Is this a bitmap where each bit at position n represents whether or not n'th relocation in `Sec` needs a TLS optimization? If so, I doesn't feel like a clean design. Having side data is somewhat confusing. I'm sorry that I don't have a counter suggestion here, but it feels like this change is somewhat neutral from the readability point of view.
================
Comment at: ELF/Writer.cpp:243-245
@@ -242,5 +254,1 @@
- // Ignore "hint" relocation because it is for optional code optimization.
- if (Target->isHintReloc(Type))
- continue;
-
----------------
Please keep this code here. If you can eliminate data early in a process, do early there rather than later.
http://reviews.llvm.org/D16468
More information about the llvm-commits
mailing list