[Driver][VE] Change to enable VPU flag by default

[kaz7]

Summary

Change to enable VPU flag for VE by default in order to support vector
intrinsics from clang.

[efocht]

Looks good.
The default should be able to use intrinsics, hiding this only makes things complicated and gets users confused.

[kaz7]

Thanks @efocht . I appreciate if someone working on clang can review this patch too.

[kaz7]

I randamly added reviewrs who review code related to feature flags and features group in clang. I appreciate if some of you guys have a time to check this patch too. Thank you so much.

[MaskRay]

clang/include/clang/Driver/Options.td

5166

Other feature group options don't set CC1Option: we do not need them as CC1 options (e.g. -Xclang -msse4)

[MaskRay]

clang/include/clang/Driver/Options.td

5169

In general, we just need documentation for the non-default option (let's say -mvevpu). Documentation for the opposite -mno-mvevpu is just boilerplate and not very useful.

clang/test/Driver/ve-features.c

2

-target has been deprecated since Clang 3.4. Use --target=

[kaz7]

Update to follow suggestions. Thank you.

[kaz7]

clang/include/clang/Driver/Options.td

5166

Thank you. I was wondering which group options are required here.

5169

I see. Changed.

clang/test/Driver/ve-features.c

2

I didn't know that. Thank you!

[kaz7]

ping

[kaz7]

Hi @MaskRay , thank you for reviewing this patch last time. Is it possible to review updated this patch again? Thanks!

[MaskRay]

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

22

Delete the comment. The code speaks itself.

if (Args.hasArg(options::OPT_mvevpu, options::OPT_mno_vevpu, true)
  Features.push_back("+vpu");

clang/test/Driver/ve-features.c

2

I think we typically spend just two RUN lines:

• one for the default
• one for -mvevpu -mno-vevpu

The additional coverage for having 3 RUN lines is probably not useful.

[MaskRay]

[VE][Clang] Change to enable VPU flag by default

For components like llvm/, clang/, we usually use a more specific tag. I'd pick [Driver] over [Clang].

[kaz7]

Change to not control backend default bahavior but support VPU flag in
the backend for VE.

[kaz7]

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

22

Thanks. I remove comments and simplify existing code.

clang/test/Driver/ve-features.c

2

Thank you for suggesting. I consider what this patch should do and change the purpose to support VPU flag in the backend for VE. As a result, Tests are two RUN lines.

• one for enable VPU
• one for disable VPU

[kaz7]

In D157813#4621978, @MaskRay wrote:

[VE][Clang] Change to enable VPU flag by default

For components like llvm/, clang/, we usually use a more specific tag. I'd pick [Driver] over [Clang].

I also change it to following.

[Driver][VE] Support VPU flag as a feature for VE

Thank you so much.

[MaskRay]

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

22

Sorry for my typo. We should use the 3-argument hasFlag here. It's simpler than getLastArg+matches

[kaz7]

By using hasFlag, change to enable VPU flag for VE by default again.

[kaz7]

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

22

I see. I was wondering that part too. Now, I change it to use hasFlag function with default value.

clang/test/Driver/ve-features.c

2

Because we use hasFlag with default value, test is changed again for following two RUN lines.

• one for the default
• one for -mvevpu -mno-vevpu

[kaz7]

Thank you for correction. I clean code again.

clang/include/clang/Driver/Options.td

 // The features added by this group will not be added to target features.
 // These are explicitly handled.
 def m_wasm_Features_Driver_Group : OptionGroup<"<wasm driver features group>">,
                                    Group<m_Group>, DocName<"WebAssembly Driver">;
 def m_x86_Features_Group : OptionGroup<"<x86 features group>">,
                            Group<m_Group>, Flags<[CoreOption]>, DocName<"X86">;
 def m_riscv_Features_Group : OptionGroup<"<riscv features group>">,
                              Group<m_Group>, DocName<"RISC-V">;
+def m_ve_Features_Group : OptionGroup<"<ve features group>">,
+                          Group<m_Group>, DocName<"VE">;
 
 def m_libc_Group : OptionGroup<"<m libc group>">, Group<m_mips_Features_Group>,
                    Flags<[HelpHidden]>;
 
 def O_Group : OptionGroup<"<O group>">, Group<CompileOnly_Group>,
               DocName<"Optimization level">, DocBrief<[{
 Flags controlling how much optimization should be performed.}]>;
 

 def mshstk : Flag<["-"], "mshstk">, Group<m_x86_Features_Group>;
 def mno_shstk : Flag<["-"], "mno-shstk">, Group<m_x86_Features_Group>;
 def mretpoline_external_thunk : Flag<["-"], "mretpoline-external-thunk">, Group<m_x86_Features_Group>;
 def mno_retpoline_external_thunk : Flag<["-"], "mno-retpoline-external-thunk">, Group<m_x86_Features_Group>;
 def mvzeroupper : Flag<["-"], "mvzeroupper">, Group<m_x86_Features_Group>;
 def mno_vzeroupper : Flag<["-"], "mno-vzeroupper">, Group<m_x86_Features_Group>;
 } // let Flags = [TargetSpecific]
 
+// VE feature flags
+let Flags = [TargetSpecific] in {
+def mvevpu : Flag<["-"], "mvevpu">, Group<m_ve_Features_Group>,
+  HelpText<"Emit VPU instructions for VE">;
+def mno_vevpu : Flag<["-"], "mno-vevpu">, Group<m_ve_Features_Group>;
+} // let Flags = [TargetSpecific]
+
 // These are legacy user-facing driver-level option spellings. They are always
 // aliases for options that are spelled using the more common Unix / GNU flag
 // style of double-dash and equals-joined flags.
 def target_legacy_spelling : Separate<["-"], "target">,
                              Alias<target>,
                              Flags<[CoreOption]>;
 
 // Special internal option to handle -Xlinker --no-demangle.

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

 #include "llvm/Option/ArgList.h"
 
 using namespace clang::driver;
 using namespace clang::driver::tools;
 using namespace clang;
 using namespace llvm::opt;
 
 void ve::getVETargetFeatures(const Driver &D, const ArgList &Args,
-                             std::vector<StringRef> &Features) {}
+                             std::vector<StringRef> &Features) {
+  if (auto *A = Args.getLastArg(options::OPT_mvevpu, options::OPT_mno_vevpu)) {
+    if (A->getOption().matches(options::OPT_mvevpu))
+      Features.push_back("+vpu");
+    else
+      Features.push_back("-vpu");
+  }
+}

clang/test/Driver/ve-features.c

+// RUN: %clang --target=ve-unknown-linux-gnu -### %s -mvevpu 2>&1 | FileCheck %s -check-prefix=VEVPU
+// RUN: %clang --target=ve-unknown-linux-gnu -### %s -mno-vevpu 2>&1 | FileCheck %s -check-prefix=NO-VEVPU
+
+// VEVPU: "-target-feature" "+vpu"
+// NO-VEVPU-NOT: "-target-feature" "+vpu"