[PATCH] D38407: [ELF] - Do --hash-style=both by default.

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 4 02:29:50 PDT 2017


grimar added inline comments.


================
Comment at: ELF/Driver.cpp:259-261
   // The MIPS ABI as of 2016 does not support the GNU-style symbol lookup
-  // table which is a relatively new feature.
-  if (Config->EMachine == EM_MIPS && Config->GnuHash)
-    error("the .gnu.hash section is not compatible with the MIPS target.");
+  // table which is a relatively new feature. Other targets defaults to
+  // both GNU and classic ELF hash styled sections.
----------------
ruiu wrote:
> This function is not a place to put this code, as described in the function comment. This function checks for validity of option combinations. You shouldn't set a new value to a Config member.
I updated this function name and description. Problem is that I need to know EMachine type,
which is updated after we scan input files, what happens after reading normal config reading flow.
The place where this function is located and called and the fact that it is the place where
"the .gnu.hash section is not compatible with the MIPS target." error is reported at the same time
makes me think it is a right place to update as it allows to localize such a bit hacky situation.

Alternative probably could be to intoduce one more function to set up options that have different
defaults basing on target, but it does not seem it worth to do for a single option we have atm.


https://reviews.llvm.org/D38407





More information about the llvm-commits mailing list