[hip] Properly populate macros based on host processor.

[hliao]

Summary

• The device compilation needs to have a consistent source code compared to the corresponding host compilation. If macros based on the host-specific target processor is not properly populated, the device compilation may fail due to the inconsistent source after the preprocessor. So far, only the host triple is used to build the macros. If a detailed host CPU target or certain features are specified, macros derived from them won't be populated properly, e.g. __SSE3__ won't be added unless +sse3 feature is present. On Windows compilation compatible with MSVC, that missing macros result in that intrinsics are not included and cause device compilation failure on the host-side source.

• This patch addresses this issue by introducing two cc1 options, i.e., -aux-target-cpu and -aux-target-feature. If a specific host CPU target or certain features are specified, the compiler driver will append them during the construction of the offline compilation actions. Then, the toolchain in cc1 phase will populate macros accordingly.

[tra]

On one hand the change makes sense to me and fits well with what we've done so far.
On the other hand, I worry that this is likely to break things.
We sort of have been implicitly relying on not having the macros related to advanced CPU features enabled on device side which typically results in device-side compilation seeing a more portable/simpler version of the code.
Defining the macros that enable more host-side CPU-specific code may trigger interesting compatibility features. Postponed diagnostics combined with ignore (some) errors in the wrong-side-only code should probably deal with most of them, but I suspect we'll see new interesting failure cases. We would not know until we try.

I would be more comfortable if we could have an option to disable this functionality with a command line option, at least temporarily. If things break, the option will avoid turning it into an emergency. If my concern turns out to be false, we can remove the option later.

[hliao]

In D73942#1857384, @tra wrote:

On one hand the change makes sense to me and fits well with what we've done so far.
On the other hand, I worry that this is likely to break things.
We sort of have been implicitly relying on not having the macros related to advanced CPU features enabled on device side which typically results in device-side compilation seeing a more portable/simpler version of the code.
Defining the macros that enable more host-side CPU-specific code may trigger interesting compatibility features. Postponed diagnostics combined with ignore (some) errors in the wrong-side-only code should probably deal with most of them, but I suspect we'll see new interesting failure cases. We would not know until we try.

I would be more comfortable if we could have an option to disable this functionality with a command line option, at least temporarily. If things break, the option will avoid turning it into an emergency. If my concern turns out to be false, we can remove the option later.

sure, I would try to add option to control that. This patch is developed to fix a real issue found on HIP with MSVC without additional CPU settings, like -msse3. Look into the header lib/Headers/x86intrin.h in clang. The including of intrinsic headers is controlled by the pre-defined macros for MSVC compilation. If that macros are not defined correctly, the host code in device-compilation may trigger issues on missing prototypes or something else.

[hliao]

Follow the reviewer's comment and an internal option
--hip-use-aux-triple-only to fall back the original behavior.

[tra]

Thank you for adding the escape hatch option.

clang/include/clang/Driver/Options.td

552 ↗

(On Diff #242395)

I think gpu- prefix would work better as it's common for HIP and CUDA.

[hliao]

s/--hip-use-aux-triple-only/--gpu-use-aux-triple-only/g

clang/include/clang/Driver/CC1Options.td

 // This isn't normally used, it is just here so we can parse a
 // CompilerInvocation out of a driver-derived argument vector.
 def cc1 : Flag<["-"], "cc1">;
 def cc1as : Flag<["-"], "cc1as">;
 
 def ast_merge : Separate<["-"], "ast-merge">,
   MetaVarName<"<ast file>">,
   HelpText<"Merge the given AST file into the translation unit being compiled.">;
+def aux_target_cpu : Separate<["-"], "aux-target-cpu">,
+  HelpText<"Target a specific auxiliary cpu type">;
+def aux_target_feature : Separate<["-"], "aux-target-feature">,
+  HelpText<"Target specific auxiliary attributes">;
 def aux_triple : Separate<["-"], "aux-triple">,
   HelpText<"Auxiliary target triple.">;
 def code_completion_at : Separate<["-"], "code-completion-at">,
   MetaVarName<"<file>:<line>:<column>">,
   HelpText<"Dump code-completion information at a location">;
 def remap_file : Separate<["-"], "remap-file">,
   MetaVarName<"<from>;<to>">,
   HelpText<"Replace the contents of the <from> file with the contents of the <to> file">;

clang/include/clang/Frontend/FrontendOptions.h

public:
   /// A list of arguments to forward to LLVM's option processing; this
   /// should only be used for debugging and experimental features.
   std::vector<std::string> LLVMArgs;
 
   /// File name of the file that will provide record layouts
   /// (in the format produced by -fdump-record-layouts).
   std::string OverrideRecordLayoutsFile;
 
-  /// Auxiliary triple for CUDA compilation.
+  /// Auxiliary triple for CUDA/HIP compilation.
   std::string AuxTriple;
 
+  /// Auxiliary target CPU for CUDA/HIP compilation.
+  Optional<std::string> AuxTargetCPU;
+
+  /// Auxiliary target features for CUDA/HIP compilation.
+  Optional<std::vector<std::string>> AuxTargetFeatures;
+
   /// Filename to write statistics to.
   std::string StatsFile;
 
   /// Minimum time granularity (in microseconds) traced by time profiler.
   unsigned TimeTraceGranularity;
 
 public:
   FrontendOptions()

clang/lib/Driver/ToolChains/Clang.cpp

 
 static void getWebAssemblyTargetFeatures(const ArgList &Args,
                                          std::vector<StringRef> &Features) {
   handleTargetFeaturesGroup(Args, Features, options::OPT_m_wasm_Features_Group);
 }
 
 static void getTargetFeatures(const ToolChain &TC, const llvm::Triple &Triple,
                               const ArgList &Args, ArgStringList &CmdArgs,
-                              bool ForAS) {
+                              bool ForAS, bool IsAux = false) {
   const Driver &D = TC.getDriver();
   std::vector<StringRef> Features;
   switch (Triple.getArch()) {
   default:
     break;
   case llvm::Triple::mips:
   case llvm::Triple::mipsel:
   case llvm::Triple::mips64:
for (unsigned I = 0, N = Features.size(); I < N; ++I) {
     // If this feature was overridden, ignore it.
     StringRef Name = Features[I];
     llvm::StringMap<unsigned>::iterator LastI = LastOpt.find(Name.drop_front(1));
     assert(LastI != LastOpt.end());
     unsigned Last = LastI->second;
     if (Last != I)
       continue;
 
-    CmdArgs.push_back("-target-feature");
+    CmdArgs.push_back(IsAux ? "-aux-target-feature" : "-target-feature");
     CmdArgs.push_back(Name.data());
   }
 }
 
 static bool
 shouldUseExceptionTablesForObjCExceptions(const ObjCRuntime &runtime,
                                           const llvm::Triple &Triple) {
   // We use the zero-cost exception tables for Objective-C if the non-fragile
bool AsynchronousUnwindTables =
                    options::OPT_fno_asynchronous_unwind_tables,
                    (TC.IsUnwindTablesDefault(Args) ||
                     TC.getSanitizerArgs().needsUnwindTables()) &&
                        !Freestanding);
   if (Args.hasFlag(options::OPT_funwind_tables, options::OPT_fno_unwind_tables,
                    AsynchronousUnwindTables))
     CmdArgs.push_back("-munwind-tables");
 
+  // Prepare `-aux-target-cpu` and `-aux-target-feature`.
+  if ((IsCuda && JA.isDeviceOffloading(Action::OFK_Cuda)) ||
+      (IsHIP && JA.isDeviceOffloading(Action::OFK_HIP))) {
+    const ArgList &HostArgs = C.getArgsForToolChain(nullptr, StringRef(), Action::OFK_None);
+    std::string HostCPU = getCPUName(HostArgs, *TC.getAuxTriple(), /*FromAs*/false);
+    if (!HostCPU.empty()) {
+      CmdArgs.push_back("-aux-target-cpu");
+      CmdArgs.push_back(Args.MakeArgString(HostCPU));
+    }
+    getTargetFeatures(TC, *TC.getAuxTriple(), HostArgs, CmdArgs, /*ForAS*/false, /*IsAux*/true);
+  }
+
   TC.addClangTargetOptions(Args, CmdArgs, JA.getOffloadingDeviceKind());
 
   // FIXME: Handle -mtune=.
   (void)Args.hasArg(options::OPT_mtune_EQ);
 
   if (Arg *A = Args.getLastArg(options::OPT_mcmodel_EQ)) {
     CmdArgs.push_back("-mcode-model");
     CmdArgs.push_back(A->getValue());

clang/lib/Frontend/CompilerInstance.cpp

if (!hasTarget())
     return false;
 
   // Create TargetInfo for the other side of CUDA/OpenMP/SYCL compilation.
   if ((getLangOpts().CUDA || getLangOpts().OpenMPIsDevice ||
        getLangOpts().SYCLIsDevice) &&
       !getFrontendOpts().AuxTriple.empty()) {
     auto TO = std::make_shared<TargetOptions>();
     TO->Triple = llvm::Triple::normalize(getFrontendOpts().AuxTriple);
+    if (getFrontendOpts().AuxTargetCPU)
+      TO->CPU = getFrontendOpts().AuxTargetCPU.getValue();
+    if (getFrontendOpts().AuxTargetFeatures)
+      TO->FeaturesAsWritten = getFrontendOpts().AuxTargetFeatures.getValue();
     TO->HostTriple = getTarget().getTriple().str();
     setAuxTarget(TargetInfo::CreateTargetInfo(getDiagnostics(), TO));
   }
 
   // Inform the target of the language options.
   //
   // FIXME: We shouldn't need to do this, the target should be immutable once
   // created. This complexity should be lifted elsewhere.

clang/lib/Frontend/CompilerInvocation.cpp

static InputKind ParseFrontendArgs(FrontendOptions &Opts, ArgList &Args,
   Opts.CodeCompleteOpts.IncludeBriefComments
     = Args.hasArg(OPT_code_completion_brief_comments);
   Opts.CodeCompleteOpts.IncludeFixIts
     = Args.hasArg(OPT_code_completion_with_fixits);
 
   Opts.OverrideRecordLayoutsFile =
       std::string(Args.getLastArgValue(OPT_foverride_record_layout_EQ));
   Opts.AuxTriple = std::string(Args.getLastArgValue(OPT_aux_triple));
+  if (Args.hasArg(OPT_aux_target_cpu))
+    Opts.AuxTargetCPU = std::string(Args.getLastArgValue(OPT_aux_target_cpu));
+  if (Args.hasArg(OPT_aux_target_feature))
+    Opts.AuxTargetFeatures = Args.getAllArgValues(OPT_aux_target_feature);
   Opts.StatsFile = std::string(Args.getLastArgValue(OPT_stats_file));
 
   if (const Arg *A = Args.getLastArg(OPT_arcmt_check,
                                      OPT_arcmt_modify,
                                      OPT_arcmt_migrate)) {
     switch (A->getOption().getID()) {
     default:
       llvm_unreachable("missed a case");

clang/test/Driver/hip-host-cpu-features.hip

+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang -### -c -target x86_64-linux-gnu -march=znver2 -x hip --cuda-gpu-arch=gfx803 -nogpulib %s 2>&1 | FileCheck %s -check-prefix=HOSTCPU
+// RUN: %clang -### -c -target x86_64-linux-gnu -msse3 -x hip --cuda-gpu-arch=gfx803 -nogpulib %s 2>&1 | FileCheck %s -check-prefix=HOSTSSE3
+
+// HOSTCPU: "-cc1" "-triple" "amdgcn-amd-amdhsa"
+// HOSTCPU-SAME: "-aux-triple" "x86_64-unknown-linux-gnu"
+// HOSTCPU-SAME: "-aux-target-cpu" "znver2"
+
+// HOSTSSE3: "-cc1" "-triple" "amdgcn-amd-amdhsa"
+// HOSTSSE3-SAME: "-aux-triple" "x86_64-unknown-linux-gnu"
+// HOSTSSE3-SAME: "-aux-target-feature" "+sse3"

clang/test/Preprocessor/hip-host-cpu-macros.cu

+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+#ifdef __HIP_DEVICE_COMPILE__
+DEVICE __SSE3__
+#else
+HOST __SSE3__
+#endif
+
+// RUN: %clang -x hip -E -target x86_64-linux-gnu -msse3 --cuda-gpu-arch=gfx803 -nogpulib -o - %s 2>&1 | FileCheck %s
+
+// CHECK-NOT: SSE3