[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