[PATCH] D13815: [ELF2] Add support for Gnu Hash section

[Rafael Ávila de Espíndola via llvm-commits]

rafael added inline comments.

================
Comment at: ELF/OutputSections.cpp:324
@@ +323,3 @@
+  // GNU Hash table requirement: it should be power of 2.
+  return static_cast<unsigned>(NextPowerOf2((NumHashed - 1) / sizeof(Elf_Off)));
+}
----------------
ikudrin wrote:
> rafael wrote:
> > ikudrin wrote:
> > > On the fresh look, I've forgotten about the requirement for this value to be a power of 2.
> > Why the -1?
> > Why the static_cast?
> > 
> For 64-bit target: calcMaskWords(0..8) = 1, calcMaskWords(9..16) = 2, calcMaskWords(17..32) = 4... I feel like it'd be better to have a unit test here.
> 
> The return type of NextPowerOf2 is uint64_t. I try to avoid a potential warning here about value truncation.
OK, I missed the fact  that NextPowerOf returns the power of 2 that is grater, not greater or equal.

Can you add your example as a comment?



http://reviews.llvm.org/D13815