diff --git a/compiler/plugins/target/LLVMCPU/LLVMTargetOptions.cpp b/compiler/plugins/target/LLVMCPU/LLVMTargetOptions.cpp index a9da1c9b3f26..314cdd6cb898 100644 --- a/compiler/plugins/target/LLVMCPU/LLVMTargetOptions.cpp +++ b/compiler/plugins/target/LLVMCPU/LLVMTargetOptions.cpp @@ -33,46 +33,61 @@ bool resolveCPUAndCPUFeatures(llvm::StringRef inCpu, llvm::StringRef inCpuFeatures, const llvm::Triple &triple, std::string &outCpu, std::string &outCpuFeatures) { - // Resolve "host" - if (inCpu == "host" || inCpuFeatures == "host") { - // If either Cpu or CpuFeatures is "host", the other must be either also - // host or the default value. - bool isCpuHostOrDefault = - inCpu.empty() || inCpu == "host" || inCpu == "generic"; - bool isCpuFeaturesHostOrDefault = - inCpuFeatures.empty() || inCpuFeatures == "host"; - if (!(isCpuHostOrDefault && isCpuFeaturesHostOrDefault)) { - llvm::errs() - << "error: If either cpu or CpuFeatures is `host`, the other must " - "be either also `host` or the default value\n"; + // Note: only set out* variables right before returning if successful. + std::string cpu = inCpu.str(); + std::string cpuFeatures = inCpuFeatures.str(); + + // Resolve "host" cpu and cpu features. + // Note: CPU feature detection in LLVM may be incomplete. Passing explicit + // feature lists instead of "host" should be preferred when possible. + if (cpu == "host" || cpuFeatures == "host") { + bool isCpuHostOrEmpty = cpu.empty() || cpu == "host"; + bool isCpuFeaturesHostOrEmpty = + cpuFeatures.empty() || cpuFeatures == "host"; + if (!(isCpuHostOrEmpty && isCpuFeaturesHostOrEmpty)) { + llvm::errs() << "error: If either Cpu or CpuFeatures is 'host', the " + "other must also be 'host' or empty (Cpu: '" + << cpu << "', CpuFeatures: '" << cpuFeatures << "')\n"; + // Note: not populating outCpu or outCpuFeatures here! Fatal error later. return false; } - outCpu = triple.isX86() ? llvm::sys::getHostCPUName().str() : ""; + cpu = llvm::sys::getHostCPUName().str(); llvm::SubtargetFeatures features; for (auto &feature : llvm::sys::getHostCPUFeatures()) { features.AddFeature(feature.first(), feature.second); } - outCpuFeatures = features.getString(); + cpuFeatures = features.getString(); } else { - outCpu = inCpu; - outCpuFeatures = inCpuFeatures; + if (cpu.empty()) { + // TODO(#18561): Require explicit "generic" and treat unset as an error? + llvm::errs() << "warning: LLVMTarget cpu unspecified, defaulting to " + "`generic`. Performance may suffer. Use " + "`--iree-llvmcpu-target-cpu=` or the target attribute to " + "set the target cpu.\n"; + cpu = "generic"; + } } // Target-specific CPU feature tweaks that we need unconditionally. if (triple.isAArch64()) { - llvm::SubtargetFeatures targetCpuFeatures(outCpuFeatures); + llvm::SubtargetFeatures targetCpuFeatures(cpuFeatures); // x18 is platform-reserved per the Aarch64 procedure call specification. targetCpuFeatures.AddFeature("reserve-x18", true); - outCpuFeatures = targetCpuFeatures.getString(); + cpuFeatures = targetCpuFeatures.getString(); } - if (outCpu.empty() || inCpu == "host" || inCpu == "generic" || - inCpu.starts_with("generic-")) { + // If features were already fully populated for "host", or compiling for a + // "generic" target (with no features), return now. + if (cpu == "host" || cpuFeatures == "host" || // + cpu == "generic" || inCpu.starts_with("generic-")) { + outCpu = cpu; + outCpuFeatures = cpuFeatures; return true; } - // If CPU is non-host and non-generic then we need to populate the - // corresponding features. - llvm::SubtargetFeatures targetCpuFeatures(outCpuFeatures); + + // If CPU is non-host and non-generic then we can join the requested features + // with the target cpu's available features (if LLVM can determine them). + llvm::SubtargetFeatures targetCpuFeatures(cpuFeatures); auto addCpuFeatures = [&](const auto &getFeaturesForCPU, auto &cpuFeatureList) { getFeaturesForCPU(outCpu, cpuFeatureList, false); @@ -87,14 +102,11 @@ bool resolveCPUAndCPUFeatures(llvm::StringRef inCpu, llvm::SmallVector cpuFeatureList; addCpuFeatures(llvm::RISCV::getFeaturesForCPU, cpuFeatureList); } else { - llvm::errs() - << "error: Resolution of target CPU to target CPU features is not " - "implemented on " - "this target architecture. Pass explicit CPU features " - "instead of a CPU " - "on this architecture, or implement that.\n"; - return false; + // No `getFeaturesForCPU` function for this arch. Just use the explicit + // features that we were provided, if any. + // TODO(#18561): We could log a warning here if cpuFeatures is non-empty. } + outCpu = cpu; outCpuFeatures = targetCpuFeatures.getString(); return true; } @@ -122,15 +134,19 @@ LLVMTarget::LLVMTarget() { llvmTargetOptions.UniqueSectionNames = true; } -std::optional LLVMTarget::create(std::string_view triple, - std::string_view cpu, - std::string_view cpuFeatures, +std::optional LLVMTarget::create(std::string triple, + std::string cpu, + std::string cpuFeatures, bool requestLinkEmbedded) { LLVMTarget target; target.linkEmbedded = requestLinkEmbedded; - target.triple = triple; - llvm::Triple targetTriple(target.triple); + // TODO(#18561): Require this be set to "host" explicitly? + if (triple.empty() || triple == "host") { + triple = llvm::sys::getProcessTriple(); + } + llvm::Triple targetTriple(triple); + // Special casing if linkEmbedded. if (targetTriple.isWasm()) { // The embedded ELF loader is not supported on WebAssembly, so force it off. @@ -138,17 +154,23 @@ std::optional LLVMTarget::create(std::string_view triple, } if (target.linkEmbedded) { // Force the triple to something compatible with embedded linking. + // Note: this leaves the ArchType and SubArchType unchanged. + // Embedded linking is portable across OS (e.g. Win32 to Linux) but it + // _is not_ portable across architectures (e.g. x86_64 to arch64). targetTriple.setVendor(llvm::Triple::VendorType::UnknownVendor); - targetTriple.setEnvironment(llvm::Triple::EnvironmentType::EABI); targetTriple.setOS(llvm::Triple::OSType::UnknownOS); + targetTriple.setEnvironment(llvm::Triple::EnvironmentType::EABI); targetTriple.setObjectFormat(llvm::Triple::ObjectFormatType::ELF); - target.triple = targetTriple.str(); } - if (!resolveCPUAndCPUFeatures(cpu, cpuFeatures, llvm::Triple(triple), - target.cpu, target.cpuFeatures)) { + target.triple = targetTriple.str(); + + if (!resolveCPUAndCPUFeatures(cpu, cpuFeatures, targetTriple, target.cpu, + target.cpuFeatures)) { // Something bad happened, and our target might not be what the user expects // but we need to continue to avoid breaking existing users. Hopefully // resolveCPUAndCPUFeatures logged a helpful error already. + // TODO(#18561): propagate the error and fail to compile - stderr output is + // not enough. Could return std::nullopt here. } return target; @@ -333,9 +355,10 @@ LLVMTarget::loadFromConfigAttr(Location loc, DictionaryAttr config, "accompanied by 'target_triple'"; return {}; } + std::optional maybeTarget = - LLVMTarget::create(*triple, cpu ? *cpu : "generic", - cpuFeatures ? *cpuFeatures : "", linkEmbedded); + LLVMTarget::create(triple->str(), cpu.value_or("").str(), + cpuFeatures.value_or("").str(), linkEmbedded); if (!maybeTarget) { return {}; } @@ -666,10 +689,6 @@ LLVMTargetOptions LLVMCPUTargetCLOptions::getTargetOptions() { targetOptions.wasmLinkerPath = wasmLinkerPath; targetOptions.keepLinkerArtifacts = keepLinkerArtifacts; - if (targetTriple.empty()) { - targetTriple = llvm::sys::getProcessTriple(); - } - std::optional maybeTarget = LLVMTarget::create( targetTriple, targetCPU, targetCPUFeatures, linkEmbedded); if (maybeTarget) { diff --git a/compiler/plugins/target/LLVMCPU/LLVMTargetOptions.h b/compiler/plugins/target/LLVMCPU/LLVMTargetOptions.h index 3d232386f084..0018159721cb 100644 --- a/compiler/plugins/target/LLVMCPU/LLVMTargetOptions.h +++ b/compiler/plugins/target/LLVMCPU/LLVMTargetOptions.h @@ -66,9 +66,8 @@ struct LLVMTarget { void storeToConfigAttrs(MLIRContext *context, SmallVector &config) const; - static std::optional create(std::string_view triple, - std::string_view cpu, - std::string_view cpuFeatures, + static std::optional create(std::string triple, std::string cpu, + std::string cpuFeatures, bool requestLinkEmbedded); static std::optional createForHost(); @@ -177,7 +176,7 @@ struct LLVMCPUTargetCLOptions { // Default device options. std::string targetTriple = ""; - std::string targetCPU = "generic"; + std::string targetCPU = ""; std::string targetCPUFeatures = ""; bool linkEmbedded = LLVMTarget::DEFAULT_LINK_EMBEDDED; bool linkStatic = LLVMTarget::DEFAULT_LINK_STATIC;