[PATCH] D101077: [clang][nfc] Split getOrCheckAMDGPUCodeObjectVersion

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.


================
Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:1592-1598
+  if (auto *CodeObjArg =
+          Args.getLastArg(options::OPT_mcode_object_v3_legacy,
+                          options::OPT_mno_code_object_v3_legacy,
+                          options::OPT_mcode_object_version_EQ)) {
+
+    if (CodeObjArg->getOption().getID() ==
+        options::OPT_mcode_object_version_EQ) {
----------------
can we just call getAMDGPUCodeObjectVersion here instead of duplicating the logic?


================
Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:1626
     } else {
-      auto Remnant =
-          StringRef(CodeObjArg->getValue()).getAsInteger(0, CodeObjVer);
-      if (Diagnose &&
-          (Remnant || CodeObjVer < MinCodeObjVer || CodeObjVer > MaxCodeObjVer))
-        D.Diag(diag::err_drv_invalid_int_value)
-            << CodeObjArg->getAsString(Args) << CodeObjArg->getValue();
+      StringRef(CodeObjArg->getValue()).getAsInteger(0, CodeObjVer);
     }
----------------
JonChesterfield wrote:
> getAsInteger writes to CodeObjVer.
can we return Optional<unsigned> and return None if there is a remnant? Then we can call this function in checkAMDGPUCodeObjectVersion


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101077/new/

https://reviews.llvm.org/D101077