[PATCH] D37386: [AMDGPU] Implement infrastructure to set options in AMDGPUToolChain

[Evgeny Mankov via Phabricator via cfe-commits]

emankov added inline comments.


================
Comment at: lib/Driver/ToolChains/AMDGPU.cpp:60-62
+  for (auto *A : Args) {
+    DAL->append(A);
+  }
----------------
redundant braces


================
Comment at: lib/Driver/ToolChains/AMDGPU.cpp:74-75
+    // as they defined that way in Options.td
+    if (!Args.hasArg(options::OPT_O) && !Args.hasArg(options::OPT_O0) &&
+        !Args.hasArg(options::OPT_O4) && !Args.hasArg(options::OPT_Ofast))
+      DAL->AddJoinedArg(nullptr, Opts.getOption(options::OPT_O),
----------------
One hasArg might be used for all.


================
Comment at: lib/Driver/ToolChains/AMDGPU.h:50
   Tool *buildLinker() const override;
+  const std::string getOptionDefault(options::ID OptID) const {
+    auto opt = OptionsDefault.find(OptID);
----------------
const Ref might be returned here.


================
Comment at: lib/Driver/ToolChains/AMDGPU.h:51-52
+  const std::string getOptionDefault(options::ID OptID) const {
+    auto opt = OptionsDefault.find(OptID);
+    return opt->second;
+  }
----------------
Check on not found opt is needed.


================
Comment at: lib/Driver/ToolChains/AMDGPU.h:60
   bool IsIntegratedAssemblerDefault() const override { return true; }
+  llvm::opt::DerivedArgList *
+  TranslateArgs(const llvm::opt::DerivedArgList &Args, StringRef BoundArch,
----------------
Return arg on the same line.


https://reviews.llvm.org/D37386