[PATCH] D14129: ELF2: Make .gnu.hash default instead of .hash.

[George Rimar via llvm-commits]

grimar added inline comments.

================
Comment at: ELF/Config.h:63
@@ -63,2 +62,3 @@
+  bool SysvHash = false;
   bool Verbose;
   bool ZNodelete = false;
----------------
May be it worth to rename these 2 to be placed together like HashGnu and HashSysv ? Or just make a single enum ?

================
Comment at: ELF/Driver.cpp:176
@@ -175,3 +175,3 @@
       error("Unknown hash style: " + S);
   }
 
----------------
May be it worth to explicit set flags for "both" ? 


if (S == "both") {
  Config->SysvHash = true;
  Config->GnuHash = true;
}


it makes it more clear (I saw this first time and was need to look into Configuration to check that both works as expected + without that it depends on default initialization which can change).


================
Comment at: test/elf2/discard-none.s:49
@@ -49,2 +48,3 @@
+// CHECK-NEXT:     Value: 0x154
 // CHECK-NEXT:     Size: 0
 // CHECK-NEXT:     Binding: Local
----------------
Do we really need to mantain adresses in such cases ? They changes often, may be we should just skip checking here like:
// CHECK-NEXT:     Value: 0x
?

================
Comment at: test/elf2/shared.s:284
@@ -283,10 +283,3 @@
 
-// CHECK:      HashTable {
-// CHECK-NEXT:   Num Buckets: 4
-// CHECK-NEXT:   Num Chains: 4
-// CHECK-NEXT:   Buckets: [3, 0, 2, 0]
-// CHECK-NEXT:   Chains: [0, 0, 0, 1]
-// CHECK-NEXT: }
-
 .global _start
 _start:
----------------
Was the next part of test removed intentionaly ?

// CHECK:      HashTable {
// CHECK-NEXT:   Num Buckets: 4
// CHECK-NEXT:   Num Chains: 4
// CHECK-NEXT:   Buckets: [3, 0, 2, 0]
// CHECK-NEXT:   Chains: [0, 0, 0, 1]
// CHECK-NEXT: }


http://reviews.llvm.org/D14129