[X86] Add -mgeneral-regs-only support.

[RKSimon]

I don't know much about target feature inheritance - does this guarantee that the entire sse/avx/avx512 level chain is correctly disabled?

[tianqing]

Respect order of options.

[tianqing]

In D103943#2812638, @RKSimon wrote:

I don't know much about target feature inheritance - does this guarantee that the entire sse/avx/avx512 level chain is correctly disabled?

setFeatureEnabled queries ImpliedFeatures to disable all dependent features.

[pengfei]

clang/include/clang/Driver/Options.td

3200–3201

Will this change affect AArch64 or other targets expect AArch64 and x86?

clang/lib/Basic/Targets/X86.cpp

120

Why do we need to expand it again after we handling in driver?

121

Please follow Lint's comment.

123–156

Shouldn't this be simply skipped under "general-regs-only"?

clang/lib/Driver/ToolChains/Arch/X86.cpp

222–223

Why we need copy the code here? Can it be simply use:

if (Args.getLastArg(options::OPT_mgeneral_regs_only))
  Features.insert(Features.end(), {"-x87", "-mmx", "-sse"});
handleTargetFeaturesGroup(Args, Features, options::OPT_m_x86_Features_Group);

clang/test/CodeGen/attr-target-general-regs-only-x86.c

15

Why we have a -avx512f when enabling avx2?

clang/test/Driver/x86-mgeneral-regs-only.c

27

ditto

[tianqing]

Fix lint comment.

[tianqing]

clang/include/clang/Driver/Options.td

3200–3201

No, using this option on other targets gives "argument unused during compilation" warning.

clang/lib/Basic/Targets/X86.cpp

120

The driver handles command line options, and this handles "target" attributes.
Just adding "+general-regs-only" in the driver also works. But it's not in OPT_m_x86_Features_Group, and current handleTargetFeaturesGroup will ignore it so we still have to copy its code, not much of a difference.

(Note AArch64 only supports options)

123–156

It's still about order of options. Seeing a "general-regs-only" doesn't mean the function is really GPR only, we have to apply all options in order and check the result.

clang/lib/Driver/ToolChains/Arch/X86.cpp

222–223

To make sure later options override earlier ones. This is how GCC behaves.

This is demonstrated in the "GPR" and "AVX2" check lines of the new tests.

clang/test/CodeGen/attr-target-general-regs-only-x86.c

15

See llvm::X86::updateImpliedFeatures(), enabling a feature will enable all features it depends on, disabling a feature also disables all features depending on it. This is "target feature inheritance" mentioned in Simon's comment.

[pengfei]

LGTM.

[RKSimon]

Thanks - that makes sense - LGTM

clang/include/clang/Driver/Options.td

def ForceAAPCSBitfieldLoad : Flag<["-"], "faapcs-bitfield-load">, Group<m_arm_Features_Group>,
   MarshallingInfoFlag<CodeGenOpts<"ForceAAPCSBitfieldLoad">>;
 defm aapcs_bitfield_width : BoolOption<"f", "aapcs-bitfield-width",
   CodeGenOpts<"AAPCSBitfieldWidth">, DefaultTrue,
   NegFlag<SetFalse, [], "Do not follow">, PosFlag<SetTrue, [], "Follow">,
   BothFlags<[NoXarchOption, CC1Option], " the AAPCS standard requirement stating that"
             " volatile bit-field width is dictated by the field container type. (ARM only).">>,
   Group<m_arm_Features_Group>;
 
-def mgeneral_regs_only : Flag<["-"], "mgeneral-regs-only">, Group<m_aarch64_Features_Group>,
-  HelpText<"Generate code which only uses the general purpose registers (AArch64 only)">;
+def mgeneral_regs_only : Flag<["-"], "mgeneral-regs-only">, Group<m_Group>,
+  HelpText<"Generate code which only uses the general purpose registers (AArch64/x86 only)">;
 def mfix_cortex_a53_835769 : Flag<["-"], "mfix-cortex-a53-835769">,
   Group<m_aarch64_Features_Group>,
   HelpText<"Workaround Cortex-A53 erratum 835769 (AArch64 only)">;
 def mno_fix_cortex_a53_835769 : Flag<["-"], "mno-fix-cortex-a53-835769">,
   Group<m_aarch64_Features_Group>,
   HelpText<"Don't workaround Cortex-A53 erratum 835769 (AArch64 only)">;
 def mmark_bti_property : Flag<["-"], "mmark-bti-property">,
   Group<m_aarch64_Features_Group>,

clang/lib/Basic/Targets/X86.cpp

bool X86TargetInfo::initFeatureMap(
 
   using namespace llvm::X86;
 
   SmallVector<StringRef, 16> CPUFeatures;
   getFeaturesForCPU(CPU, CPUFeatures);
   for (auto &F : CPUFeatures)
     setFeatureEnabled(Features, F, true);
 
   if (!TargetInfo::initFeatureMap(Features, Diags, CPU, FeaturesVec))
     return false;
 
   // Can't do this earlier because we need to be able to explicitly enable
   // or disable these features and the things that they depend upon.
 
   // Enable popcnt if sse4.2 is enabled and popcnt is not explicitly disabled.
   auto I = Features.find("sse4.2");
   if (I != Features.end() && I->getValue() &&
       llvm::find(FeaturesVec, "-popcnt") == FeaturesVec.end())
     Features["popcnt"] = true;
 
   // Additionally, if SSE is enabled and mmx is not explicitly disabled,
   // then enable MMX.
   I = Features.find("sse");
   if (I != Features.end() && I->getValue() &&
       llvm::find(FeaturesVec, "-mmx") == FeaturesVec.end())
     Features["mmx"] = true;
 
   // Enable xsave if avx is enabled and xsave is not explicitly disabled.
   I = Features.find("avx");
   if (I != Features.end() && I->getValue() &&
       llvm::find(FeaturesVec, "-xsave") == FeaturesVec.end())
     Features["xsave"] = true;
 
+  I = Features.find("general-regs-only");
+  if (I != Features.end()) {
+    bool HasGeneralRegsOnly = I->getValue();
+    Features.erase(I);
+    if (HasGeneralRegsOnly) {
+      setFeatureEnabled(Features, "x87", false);
+      setFeatureEnabled(Features, "mmx", false);
+      setFeatureEnabled(Features, "sse", false);
+    }
+  }
+
   return true;
 }
 
 void X86TargetInfo::setFeatureEnabled(llvm::StringMap<bool> &Features,
                                       StringRef Name, bool Enabled) const {
   if (Name == "sse4") {
     // We can get here via the __target__ attribute since that's not controlled
     // via the -msse4/-mno-sse4 command line alias. Handle this the same way
     // here - turn on the sse4.2 if enabled, turn off the sse4.1 level if
return llvm::StringSwitch<bool>(Name)
       .Case("clzero", true)
       .Case("cx16", true)
       .Case("enqcmd", true)
       .Case("f16c", true)
       .Case("fma", true)
       .Case("fma4", true)
       .Case("fsgsbase", true)
       .Case("fxsr", true)
+      .Case("general-regs-only", true)
       .Case("gfni", true)
       .Case("hreset", true)
       .Case("invpcid", true)
       .Case("kl", true)
       .Case("widekl", true)
       .Case("lwp", true)
       .Case("lzcnt", true)
       .Case("mmx", true)

clang/lib/Driver/ToolChains/Arch/X86.cpp

if (Triple.isAndroid()) {
     if (ArchType == llvm::Triple::x86_64) {
       Features.push_back("+sse4.2");
       Features.push_back("+popcnt");
       Features.push_back("+cx16");
     } else
       Features.push_back("+ssse3");
   }
 
+  if (Args.getLastArg(options::OPT_mgeneral_regs_only)) {
+    Features.push_back("-x87");
+    Features.push_back("-mmx");
+    Features.push_back("-sse");
+  }
+
   // Translate the high level `-mretpoline` flag to the specific target feature
   // flags. We also detect if the user asked for retpoline external thunks but
   // failed to ask for retpolines themselves (through any of the different
   // flags). This is a bit hacky but keeps existing usages working. We should
   // consider deprecating this and instead warn if the user requests external
   // retpoline thunks and *doesn't* request some form of retpolines.
   auto SpectreOpt = clang::driver::options::ID::OPT_INVALID;
   if (Args.hasArgNoClaim(options::OPT_mretpoline, options::OPT_mno_retpoline,
if (SpectreOpt != clang::driver::options::ID::OPT_INVALID &&
       LVIOpt != clang::driver::options::ID::OPT_INVALID) {
     D.Diag(diag::err_drv_argument_not_allowed_with)
         << D.getOpts().getOptionName(SpectreOpt)
         << D.getOpts().getOptionName(LVIOpt);
   }
 
   // Now add any that the user explicitly requested on the command line,
   // which may override the defaults.
   handleTargetFeaturesGroup(Args, Features, options::OPT_m_x86_Features_Group);
 }

clang/test/CodeGen/attr-target-general-regs-only-x86.c

+// Test general-regs-only target attribute on x86
+
+// RUN: %clang_cc1 -triple i386-unknown-linux-gnu -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm %s -o - | FileCheck %s
+
+void __attribute__((target("general-regs-only"))) foo() { }
+
+// CHECK: "target-features"="{{.*}}-mmx{{.*}}-sse{{.*}}-x87{{.*}}"

clang/test/Driver/x86-mgeneral-regs-only.c

+// Test the -mgeneral-regs-only option on x86
+
+// RUN: %clang -target i386-unknown-linux-gnu -mgeneral-regs-only %s -### 2>&1 | FileCheck %s
+// RUN: %clang -target x86_64-unknown-linux-gnu -mgeneral-regs-only %s -### 2>&1 | FileCheck %s
+
+// CHECK: "-target-feature" "-x87"
+// CHECK: "-target-feature" "-mmx"
+// CHECK: "-target-feature" "-sse"