[SYCL][Driver] Diagnose incorrect inputs to CLI options (#4515)

Instead of assertions and silent fallthrough scenarios, incorrect
values to SYCL options should lead to user-friendly diagnostics.

The long-term fix for this should be a SYCL-agnostic solution in the
LLVM comunity code - allowed values to driver options are known at the
TableGen level via the `Values<>` class, so the validation should be
happening there too. This would minimize code duplication for
individual compilation pipelines and bring the TableGen CLI library
closer to the `llvm::cl` functionality.

Signed-off-by: Artem Gindinson <[email protected]>

Author: Artem Gindinson

clang/lib/Driver/Driver.cpp

@@ -898,6 +898,29 @@ void Driver::CreateOffloadingDeviceToolChains(Compilation &C,
                                                << "-ffreestanding";
   }
 
+  // Diagnose incorrect inputs to SYCL options.
+  // FIXME: Since the option definition includes the list of possible values,
+  // the validation must be automatic, not requiring separate disjointed code
+  // blocks accross the driver code. Long-term, the detection of incorrect
+  // values must happen at the level of TableGen and Arg class design, with
+  // Compilation/Driver class constructors handling the driver-specific
+  // diagnostic output.
+  auto checkSingleArgValidity = [&](Arg *A,
+                                    SmallVector<StringRef, 4> AllowedValues) {
+    if (!A)
+      return;
+    const char *ArgValue = A->getValue();
+    for (const StringRef AllowedValue : AllowedValues)
+      if (AllowedValue.equals(ArgValue))
+        return;
+    Diag(clang::diag::err_drv_invalid_argument_to_option)
+        << ArgValue << A->getOption().getName();
+  };
+  checkSingleArgValidity(SYCLLink, {"early", "image"});
+  checkSingleArgValidity(
+      C.getInputArgs().getLastArg(options::OPT_fsycl_device_code_split_EQ),
+      {"per_kernel", "per_source", "auto", "off"});
+
   bool HasSYCLTargetsOption = SYCLTargets || SYCLLinkTargets || SYCLAddTargets;
   llvm::StringMap<StringRef> FoundNormalizedTriples;
   llvm::SmallVector<llvm::Triple, 4> UniqueSYCLTriplesVec;

clang/lib/Driver/ToolChains/Clang.cpp

@@ -8909,15 +8909,15 @@ void SYCLPostLink::ConstructJob(Compilation &C, const JobAction &JA,
 
   // See if device code splitting is requested
   if (Arg *A = TCArgs.getLastArg(options::OPT_fsycl_device_code_split_EQ)) {
-    if (StringRef(A->getValue()) == "per_kernel")
+    auto CodeSplitValue = StringRef(A->getValue());
+    if (CodeSplitValue == "per_kernel")
       addArgs(CmdArgs, TCArgs, {"-split=kernel"});
-    else if (StringRef(A->getValue()) == "per_source")
+    else if (CodeSplitValue == "per_source")
       addArgs(CmdArgs, TCArgs, {"-split=source"});
-    else if (StringRef(A->getValue()) == "auto")
+    else if (CodeSplitValue == "auto")
       addArgs(CmdArgs, TCArgs, {"-split=auto"});
-    else
-      // split must be off
-      assert(StringRef(A->getValue()) == "off");
+    else { // Device code split is off
+    }
   } else {
     // auto is the default split mode
     addArgs(CmdArgs, TCArgs, {"-split=auto"});

clang/test/Driver/sycl-offload.c

@@ -87,6 +87,13 @@
 
 /// ###########################################################################
 
+/// Validate SYCL option values
+// RUN:   %clang -### -fsycl-device-code-split=bad_value -fsycl  %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHK-SYCL-BAD-OPT-VALUE -Doption=-fsycl-device-code-split %s
+// RUN:   %clang -### -fsycl-link=bad_value -fsycl  %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHK-SYCL-BAD-OPT-VALUE -Doption=-fsycl-link %s
+// CHK-SYCL-BAD-OPT-VALUE: error: invalid argument 'bad_value' to [[option]]=
+
 /// Check error for -fsycl-targets with bad triple
 // RUN:   %clang -### -fsycl-targets=spir64_bad-unknown-unknown -fsycl  %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHK-SYCL-FPGA-BAD-TRIPLE %s