[PATCH] D29445: LTO: add a code-model flag

[Martell Malone via Phabricator via llvm-commits]

martell added a comment.

Updated to raise relevant questions and try to address comments



================
Comment at: tools/gold/gold-plugin.cpp:672
 
+static CodeModel::Model getCodeModel(std::string CodeModel) {
+  unsigned CM =
----------------
jhenderson wrote:
> Is there anywhere we could put this function so that we could share it with ELF/Driver.cpp to prevent essentially duplicate code?
Based on the changes I tracked to update this patch from 3.9.0 to master a lot of the shared code between gold and lld has move into `lib/LTO/LTOBackend.cpp`
I'm not sure if this is the right place for argument parsing though.
I am open to suggestions?


================
Comment at: tools/gold/gold-plugin.cpp:679
+     .Case("large", llvm::CodeModel::Large)
+     .Default(llvm::CodeModel::Default);
+  return static_cast<llvm::CodeModel::Model>(CM);
----------------
jhenderson wrote:
> Should this function emit an error if CodeModel is not recognised? As far as I can see, this will silently accept incorrect switches, which could be harmful (e.g. a typo leads to passing in "meidum" or similar).
The only way I could see emitting an error code initially here was adding the following
`.Case("default", llvm::CodeModel::Default)`
`.Default(llvm::CodeModel::JITDefault);`
Then error in the case where it is JITDefault.
This seems very hacky and not the correct way to do this.

Adding an `llvm_unreachable` seems the correct way to go but `error` is used in lld.
The error message should generally be the same across gold and lld ELF.
I'll wait on suggestions to where we can not have duplication and fix this together in one go.


Repository:
  rL LLVM

https://reviews.llvm.org/D29445