Address review comments

Signed-off-by: Arvind Sudarsanam <[email protected]>

Author: asudarsa

clang/lib/Driver/ToolChains/Clang.cpp

@@ -10649,23 +10649,24 @@ static void getNonTripleBasedSYCLPostLinkOpts(const ToolChain &TC,
 }
 
 // Add any sycl-post-link options that rely on a specific Triple in addition
-// to user supplied options. This function is invoked only for old offloading
-// model. For new offloading model, a slightly modified version of this
-// function is called inside clang-linker-wrapper.
+// to user supplied options. This function is invoked only for the old
+// offloading model. For the new offloading model, a slightly modified version
+// of this function is called inside clang-linker-wrapper.
 // NOTE: Any changes made here should be reflected in the similarly named
 // function in clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp.
 static void getTripleBasedSYCLPostLinkOpts(const ToolChain &TC,
                                            const llvm::opt::ArgList &TCArgs,
                                            ArgStringList &PostLinkArgs,
-                                           llvm::Triple Triple, bool SpecConsts,
+                                           llvm::Triple Triple,
+                                           bool SpecConstsSupported,
                                            types::ID OutputType) {
   if (OutputType == types::TY_LLVM_BC) {
     // single file output requested - this means only perform necessary IR
     // transformations (like specialization constant intrinsic lowering) and
     // output LLVMIR
     addArgs(PostLinkArgs, TCArgs, {"-ir-output-only"});
   }
-  if (SpecConsts)
+  if (SpecConstsSupported)
     addArgs(PostLinkArgs, TCArgs, {"-spec-const=native"});
   else
     addArgs(PostLinkArgs, TCArgs, {"-spec-const=emulation"});
@@ -10674,7 +10675,7 @@ static void getTripleBasedSYCLPostLinkOpts(const ToolChain &TC,
   // the behavior in getNonTripleBasedSYCLPostLinkOpts, where the option is
   // added based on the user setting of -fsycl-device-code-split.
   if (!TCArgs.hasArg(options::OPT_fsycl_device_code_split_EQ) &&
-      (!(Triple.getArchName() == "spir64_fpga")))
+      (Triple.getArchName() != "spir64_fpga"))
     addArgs(PostLinkArgs, TCArgs, {"-split=auto"});
 
   // On Intel targets we don't need non-kernel functions as entry points,

clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp

@@ -532,16 +532,15 @@ getTripleBasedSYCLPostLinkOpts(const ArgList &Args,
                                const llvm::Triple Triple) {
   const llvm::Triple HostTriple(Args.getLastArgValue(OPT_host_triple_EQ));
   bool SYCLNativeCPU = (HostTriple == Triple);
-  bool SpecConsts = !(Triple.isNVPTX() || Triple.isAMDGCN() ||
-                      Triple.isSPIRAOT() || SYCLNativeCPU);
-  if (SpecConsts)
+  bool SpecConstsSupported = (!Triple.isNVPTX() && !Triple.isAMDGCN() ||
+                              !Triple.isSPIRAOT() && !SYCLNativeCPU);
+  if (SpecConstsSupported)
     PostLinkArgs.push_back("-spec-const=native");
   else
     PostLinkArgs.push_back("-spec-const=emulation");
 
-  // See if device code splitting is requested.  The logic here works along side
-  // the behavior in getNonTripleBasedSYCLPostLinkOpts, where the option is
-  // added based on the user setting of -fsycl-device-code-split.
+  // See if device code splitting is already requested. If not requested, then
+  // set -split=auto for non-FPGA targets.
   bool NoSplit = true;
   for (auto Arg : PostLinkArgs)
     if (Arg.contains("-split=")) {
@@ -580,10 +579,7 @@ getTripleBasedSYCLPostLinkOpts(const ArgList &Args,
     PostLinkArgs.push_back("-split-esimd");
   PostLinkArgs.push_back("-lower-esimd");
 
-  bool IsAOT = Triple.isNVPTX() || Triple.isAMDGCN() ||
-               Triple.getSubArch() == llvm::Triple::SPIRSubArch_fpga ||
-               Triple.getSubArch() == llvm::Triple::SPIRSubArch_gen ||
-               Triple.getSubArch() == llvm::Triple::SPIRSubArch_x86_64;
+  bool IsAOT = Triple.isNVPTX() || Triple.isAMDGCN() || Triple.isSPIRAOT();
   if (Args.hasFlag(OPT_sycl_add_default_spec_consts_image,
                    OPT_no_sycl_add_default_spec_consts_image, false) &&
       IsAOT)

clang/tools/clang-linker-wrapper/LinkerWrapperOpts.td

@@ -172,7 +172,7 @@ def llvm_spirv_options_EQ : Joined<["--", "-"], "llvm-spirv-options=">,
   HelpText<"Options that will control llvm-spirv step">;
 
 // Extra SYCL options to help generate sycl-post-link options that also depend
-// on target triple.
+// on the target triple.
 def sycl_remove_unused_external_funcs : Flag<["--", "-"], "sycl-remove-unused-external-funcs">,
   Flags<[WrapperOnlyOption, HelpHidden]>;
 def no_sycl_remove_unused_external_funcs : Flag<["--", "-"], "no-sycl-remove-unused-external-funcs">,