[PATCH] D16575: [ELF] - redesign of program headers creation code.

Rui Ueyama via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 26 17:23:17 PST 2016


ruiu added inline comments.

================
Comment at: ELF/Writer.cpp:1122-1130
@@ +1121,11 @@
+
+  uintX_t VA = Target->getVAStart() + sizeof(Elf_Ehdr);
+  uintX_t FileOff = sizeof(Elf_Ehdr);
+  uintX_t TlsVA = 0;
+  uintX_t TlsFileOff = 0;
+  Elf_Phdr *InterpHdr = nullptr;
+  Elf_Phdr GnuRelroPhdr = {};
+  bool RelroAligned = false;
+  bool FirstLoad = true;
+  uintX_t I = 0;
+  for (auto &PHdr : Map) {
----------------
Do you think you can reduce number of variables and boolean flags used in this function by separating into small functions?

The fundamental reason why the previous code was not easy to read is because too many flags are entangled. If the code makes a decision based on n boolean flags, the possible combination is 2^n, which is exponential. Moreover, some variables are updated in the middle of a process, that made it hard to follow the logic. (If a variable gets a new value in one path but don't in the other, you'll get 2^n combinations, again.) We'd like to use less information. I think it is acceptable to iterate over the same data structure more than once because I don't think this is a performance-critical path.


http://reviews.llvm.org/D16575





More information about the llvm-commits mailing list