[SYCL] Correct CFE behavior with named lambdas in unnamed-lambda mode (#3990)

The library is responsible to replace a non-named kernel's name with the
type of the lambda.  However, it still allows the user to specify the
name if they'd like, and uses that as the name instead.

The current implementation ALWAYS marked the kernel functor as the name
of the kernel, but this isn't correct, it isn't the name of the kernel
unless the library makes it so.  This ends up causing user-named kernels
that have their names changed by the functor they pass in.

We can't mark the first template parameter, since it might be a user
provided name, and we can't mark the functor since the user might have
provided a name.

However, if they are both the same (after unqualification and
canonicalization) that means that the library has done such a
replacement, and thus needs to be marked.

Author: Erich Keane

clang/include/clang/Driver/Options.td

@@ -470,6 +470,7 @@ defvar hip = LangOpts<"HIP">;
 defvar gnu_mode = LangOpts<"GNUMode">;
 defvar asm_preprocessor = LangOpts<"AsmPreprocessor">;
 defvar cpp_modules = LangOpts<"CPlusPlusModules">;
+defvar sycl_ver = LangOpts<"SYCLVersion">;
 
 defvar std = !strconcat("LangStandard::getLangStandardForKind(", lang_std.KeyPath, ")");
 
@@ -2599,9 +2600,14 @@ def fsycl_link_EQ : Joined<["-"], "fsycl-link=">,
 def fsycl_link : Flag<["-"], "fsycl-link">, Alias<fsycl_link_EQ>,
   AliasArgs<["early"]>, Flags<[CC1Option, CoreOption]>,
   HelpText<"Generate partially linked device object to be used with the host link">;
-def fsycl_unnamed_lambda : Flag<["-"], "fsycl-unnamed-lambda">,
-  Flags<[CC1Option, CoreOption]>, HelpText<"Allow unnamed SYCL lambda kernels">,
-  MarshallingInfoFlag<LangOpts<"SYCLUnnamedLambda">>;
+defm sycl_unnamed_lambda
+    : BoolFOption<
+          "sycl-unnamed-lambda", LangOpts<"SYCLUnnamedLambda">,
+          Default<!strconcat(
+              sycl_ver.KeyPath,
+              " >= clang::LangOptions::SYCLMajorVersion::SYCL_2020")>,
+          PosFlag<SetTrue, [], "Allow">, NegFlag<SetFalse, [], "Disallow">,
+          BothFlags<[CC1Option, CoreOption], " unnamed SYCL lambda kernels">>;
 def fsycl_help_EQ : Joined<["-"], "fsycl-help=">,
   Flags<[NoXarchOption, CoreOption]>, HelpText<"Emit help information from the "
   "related offline compilation tool. Valid values: all, fpga, gen, x86_64.">,
@@ -5896,9 +5902,6 @@ def sycl_std_EQ : Joined<["-"], "sycl-std=">, Group<sycl_Group>,
   Flags<[CC1Option, NoArgumentUnused, CoreOption]>,
   HelpText<"SYCL language standard to compile for.">,
   Values<"2020,2017,121,1.2.1,sycl-1.2.1">,
-  NormalizedValues<["SYCL_2020", "SYCL_2017", "SYCL_2017", "SYCL_2017", "SYCL_2017"]>,
-  NormalizedValuesScope<"LangOptions">,
-  MarshallingInfoEnum<LangOpts<"SYCLVersion">, "SYCL_None">,
   ShouldParseIf<!strconcat(fsycl_is_device.KeyPath, "||", fsycl_is_host.KeyPath)>;
 
 def fsycl_default_sub_group_size

clang/include/clang/Sema/Sema.h

@@ -322,7 +322,7 @@ class SYCLIntegrationHeader {
   };
 
 public:
-  SYCLIntegrationHeader(bool UnnamedLambdaSupport, Sema &S);
+  SYCLIntegrationHeader(Sema &S);
 
   /// Emits contents of the header into given stream.
   void emit(raw_ostream &Out);
@@ -334,8 +334,8 @@ class SYCLIntegrationHeader {
   ///  Signals that subsequent parameter descriptor additions will go to
   ///  the kernel with given name. Starts new kernel invocation descriptor.
   void startKernel(StringRef KernelName, QualType KernelNameType,
-                   StringRef KernelStableName, SourceLocation Loc,
-                   bool IsESIMD);
+                   StringRef KernelStableName, SourceLocation Loc, bool IsESIMD,
+                   bool IsUnnamedKernel);
 
   /// Adds a kernel parameter descriptor to current kernel invocation
   /// descriptor.
@@ -375,10 +375,10 @@ class SYCLIntegrationHeader {
   // there are four free functions the kernel may call (this_id, this_item,
   // this_nd_item, this_group)
   struct KernelCallsSYCLFreeFunction {
-    bool CallsThisId;
-    bool CallsThisItem;
-    bool CallsThisNDItem;
-    bool CallsThisGroup;
+    bool CallsThisId = false;
+    bool CallsThisItem = false;
+    bool CallsThisNDItem = false;
+    bool CallsThisGroup = false;
   };
 
   // Kernel invocation descriptor
@@ -404,7 +404,15 @@ class SYCLIntegrationHeader {
     // this_id(), etc)
     KernelCallsSYCLFreeFunction FreeFunctionCalls;
 
-    KernelDesc() = default;
+    // If we are in unnamed kernel/lambda mode AND this is one that the user
+    // hasn't provided an explicit name for.
+    bool IsUnnamedKernel;
+
+    KernelDesc(StringRef Name, QualType NameType, StringRef StableName,
+               SourceLocation KernelLoc, bool IsESIMD, bool IsUnnamedKernel)
+        : Name(Name), NameType(NameType), StableName(StableName),
+          KernelLocation(KernelLoc), IsESIMDKernel(IsESIMD),
+          IsUnnamedKernel(IsUnnamedKernel) {}
   };
 
   /// Returns the latest invocation descriptor started by
@@ -426,9 +434,6 @@ class SYCLIntegrationHeader {
   /// integration header emission time.
   llvm::SmallVector<SpecConstID, 4> SpecConsts;
 
-  /// Whether header is generated with unnamed lambda support
-  bool UnnamedLambdaSupport;
-
   Sema &S;
 };
 
@@ -13293,8 +13298,7 @@ class Sema final {
   /// Lazily creates and returns SYCL integration header instance.
   SYCLIntegrationHeader &getSyclIntegrationHeader() {
     if (SyclIntHeader == nullptr)
-      SyclIntHeader = std::make_unique<SYCLIntegrationHeader>(
-          getLangOpts().SYCLUnnamedLambda, *this);
+      SyclIntHeader = std::make_unique<SYCLIntegrationHeader>(*this);
     return *SyclIntHeader.get();
   }
 

clang/lib/Driver/ToolChains/Clang.cpp

@@ -4675,8 +4675,10 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
       // Ensure the default version in SYCL mode is 2020.
       CmdArgs.push_back("-sycl-std=2020");
     }
-    if (Args.hasArg(options::OPT_fsycl_unnamed_lambda))
-      CmdArgs.push_back("-fsycl-unnamed-lambda");
+
+    if (!Args.hasFlag(options::OPT_fsycl_unnamed_lambda,
+                      options::OPT_fno_sycl_unnamed_lambda))
+      CmdArgs.push_back("-fno-sycl-unnamed-lambda");
 
     // Add the Unique ID prefix
     StringRef UniqueID = D.getSYCLUniqueID(Input.getBaseInput());

clang/lib/Frontend/CompilerInvocation.cpp

@@ -3546,6 +3546,20 @@ void CompilerInvocation::GenerateLangArgs(const LangOptions &Opts,
   case LangOptions::SubGroupSizeType::None:
     break;
   }
+
+  if (Opts.isSYCL()) {
+    switch (Opts.SYCLVersion) {
+    case LangOptions::SYCL_2017:
+      GenerateArg(Args, OPT_sycl_std_EQ, "2017", SA);
+      break;
+    case LangOptions::SYCL_2020:
+      GenerateArg(Args, OPT_sycl_std_EQ, "2020", SA);
+      break;
+    case LangOptions::SYCL_None:
+      // Do nothing, case where we were given an invalid value.
+      break;
+    }
+  }
 }
 
 bool CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args,
@@ -3637,6 +3651,28 @@ bool CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args,
       LangStd = OpenCLLangStd;
   }
 
+  // We have to parse this manually before the marshalling, otherwise we can't
+  // use the marshalling to set other flags based on the SYCL version.
+  if (Args.hasArg(OPT_fsycl_is_device) || Args.hasArg(OPT_fsycl_is_host)) {
+    if (const Arg *A = Args.getLastArg(OPT_sycl_std_EQ)) {
+      Opts.setSYCLVersion(
+          llvm::StringSwitch<LangOptions::SYCLMajorVersion>(A->getValue())
+              .Case("2020", LangOptions::SYCL_2020)
+              .Cases("2017", "121", "1.2.1", "sycl-1.2.1",
+                     LangOptions::SYCL_2017)
+              .Default(LangOptions::SYCL_None));
+
+      if (Opts.SYCLVersion == LangOptions::SYCL_None)
+        Diags.Report(diag::err_drv_invalid_value)
+            << A->getAsString(Args) << A->getValue();
+    } else {
+      // If the user supplied -fsycl-is-device or -fsycl-is-host, but failed to
+      // provide -sycl-std=, we want to default it to whatever the default SYCL
+      // version is.
+      Opts.setSYCLVersion(LangOptions::SYCL_Default);
+    }
+  }
+
   // Parse SYCL Default Sub group size.
   if (const Arg *A = Args.getLastArg(OPT_fsycl_default_sub_group_size)) {
     StringRef Value = A->getValue();
@@ -3687,16 +3723,6 @@ bool CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args,
     }
   }
 
-  if ((Args.hasArg(OPT_fsycl_is_device) || Args.hasArg(OPT_fsycl_is_host)) &&
-      !Args.hasArg(OPT_sycl_std_EQ)) {
-    // If the user supplied -fsycl-is-device or -fsycl-is-host, but failed to
-    // provide -sycl-std=, we want to default it to whatever the default SYCL
-    // version is. I could not find a way to express this with the options
-    // tablegen because we still want this value to be SYCL_None when the user
-    // is not in device or host mode.
-    Opts.setSYCLVersion(LangOptions::SYCL_Default);
-  }
-
   if (Opts.ObjC) {
     if (Arg *arg = Args.getLastArg(OPT_fobjc_runtime_EQ)) {
       StringRef value = arg->getValue();