Skip to content

[X86][clang-cl] Add CL option /vlen #166375

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
phoebewang merged 3 commits into from
Nov 8, 2025
Merged

[X86][clang-cl] Add CL option /vlen #166375

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bdecf07
[X86][clang-cl] Add CL option /arch
phoebewang Nov 4, 2025
266f047
Address review comments
phoebewang Nov 7, 2025
83d24bd
Address review comment
phoebewang Nov 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions clang/include/clang/Basic/DiagnosticDriverKinds.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,9 @@ def err_drv_amdgpu_ieee_without_no_honor_nans : Error<
"invalid argument '-mno-amdgpu-ieee' only allowed with relaxed NaN handling">;
def err_drv_argument_not_allowed_with : Error<
"invalid argument '%0' not allowed with '%1'">;
def warn_drv_argument_not_allowed_with : Warning<
"invalid argument '%0' not allowed with '%1'">,
InGroup<OptionIgnored>;
def err_drv_cannot_open_randomize_layout_seed_file : Error<
"cannot read randomize layout seed file '%0'">;
def err_drv_invalid_version_number : Error<
Expand Down
4 changes: 4 additions & 0 deletions clang/include/clang/Driver/Options.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9218,6 +9218,10 @@ def : CLFlag<"Qscatter-">, Alias<mno_scatter>,

def _SLASH_arch : CLCompileJoined<"arch:">,
HelpText<"Set architecture for code generation">;
def _SLASH_vlen : CLCompileJoined<"vlen">,
HelpText<"Set vector length for autovectorization and other optimizations">;
def _SLASH_vlen_default : CLFlag<"vlen">,
HelpText<"Set default vector length for autovectorization and other optimizations">;
Copy link
Collaborator

@zmodem zmodem Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these should be _SLASH_vlen (for the default) and _SLASH_vlen_EQ for the one ending in = (and that should be included in the flag spelling.

But, since only two values are allowed, it might be simpler to do something like:

def _SLASH_vlen : CLFLag<"vlen"> ...
def _SLASH_vlen_EQ_256 : CLFLag<"vlen=256"> ...
def _SLASH_vlen_EQ_512 : CLFlag<"vlen=512"> ...

Does Clang provide good diagnostics when passing an -mprefer-vector-width that doesn't fit the target architecture?

If so, could we make _SLASH_vlen_EQ_256 an alias for -mprefer-vector-width=256 and so on, and not have to add any new driver logic at all?

Copy link
Contributor Author

@phoebewang phoebewang Nov 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea! No, unfortunately: https://godbolt.org/z/hhvjP8W34


def _SLASH_M_Group : OptionGroup<"</M group>">, Group<cl_compile_Group>;
def _SLASH_volatile_Group : OptionGroup<"</volatile group>">,
Expand Down
31 changes: 31 additions & 0 deletions clang/lib/Driver/ToolChains/Clang.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8266,6 +8266,37 @@ void Clang::AddClangCLArgs(const ArgList &Args, types::ID InputType,
<< "/kernel";
}

if (Args.hasArg(options::OPT__SLASH_vlen_default)) {
// Override /vlen= options.
Copy link
Collaborator

@zmodem zmodem Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the last flag take precedence? For example, in /arch:AVX512 /vlen /vlen=256, shouldn't 256 win? What does MSVC do?

Copy link
Contributor Author

@phoebewang phoebewang Nov 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, MSVC also takes precedence of the last flag: https://godbolt.org/z/43Ga39WhE

for (const Arg *A : Args.filtered(options::OPT__SLASH_vlen))
A->claim();
// Nothing to do for AVX512F/AVX512.
} else if (const Arg *A = Args.getLastArg(options::OPT__SLASH_vlen)) {
StringRef VLen = A->getValue();
llvm::SmallSet<std::string, 4> Arch512 = {"AVX512F", "AVX512"};
llvm::SmallSet<std::string, 4> Arch256 = {"AVX", "AVX2"};

StringRef Arch = Args.getLastArgValue(options::OPT__SLASH_arch);
Copy link
Collaborator

@zmodem zmodem Nov 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if the user didn't pass /arch?
What if they passed -mavx512?

Copy link
Contributor Author

@phoebewang phoebewang Nov 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. MSVC has a default value for /arch. We should match it here. -mavx512xxx is a single feature, which is orthogonal to /arch. We don't need to check them.

if (Arch != "") {
if (VLen == "=512") {
if (Arch512.contains(Arch.str()))
CmdArgs.push_back("-mprefer-vector-width=512");
else
D.Diag(diag::warn_drv_argument_not_allowed_with)
<< "/vlen=512" << std::string("/arch:").append(Arch);
} else if (VLen == "=256") {
if (Arch512.contains(Arch.str()))
CmdArgs.push_back("-mprefer-vector-width=256");
else if (!Arch256.contains(Arch.str()))
D.Diag(diag::warn_drv_argument_not_allowed_with)
<< "/vlen=256" << std::string("/arch:").append(Arch);
} else {
D.Diag(diag::warn_drv_unknown_argument_clang_cl)
<< std::string("/vlen").append(VLen);
}
}
}

Arg *MostGeneralArg = Args.getLastArg(options::OPT__SLASH_vmg);
Arg *BestCaseArg = Args.getLastArg(options::OPT__SLASH_vmb);
if (MostGeneralArg && BestCaseArg)
Expand Down
21 changes: 21 additions & 0 deletions clang/test/Driver/cl-x86-flags.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,27 @@
// tune: "-target-cpu" "sandybridge"
// tune-SAME: "-tune-cpu" "haswell"

// RUN: %clang_cl -m64 -arch:AVX512 -vlen=512 --target=x86_64-pc-windows -### -- 2>&1 %s | FileCheck -check-prefix=vlen512 %s
// vlen512: "-mprefer-vector-width=512"

// RUN: %clang_cl -m64 -arch:AVX512 -vlen=256 --target=x86_64-pc-windows -### -- 2>&1 %s | FileCheck -check-prefix=vlen256 %s
// vlen256: "-mprefer-vector-width=256"

// RUN: %clang_cl -m64 -arch:AVX512 -vlen=512 -vlen --target=x86_64-pc-windows -### -- 2>&1 %s | FileCheck -check-prefix=noprefer %s
Copy link
Collaborator

@zmodem zmodem Nov 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about novlen instead of noprefer?

Copy link
Contributor Author

@phoebewang phoebewang Nov 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

// noprefer-NOT: -mprefer-vector-width

// RUN: %clang_cl -m64 -arch:AVX2 -vlen=512 --target=x86_64-pc-windows -### -- 2>&1 %s | FileCheck -check-prefix=avx2vlen512 %s
// avx2vlen512: invalid argument '/vlen=512' not allowed with '/arch:AVX2'

// RUN: %clang_cl -m64 -arch:AVX2 -vlen=256 --target=x86_64-pc-windows -### -- 2>&1 %s | FileCheck -check-prefix=avx2vlen256 %s
// avx2vlen256-NOT: invalid argument

// RUN: %clang_cl -m32 -arch:SSE2 -vlen=256 --target=i386-pc-windows -### -- 2>&1 %s | FileCheck -check-prefix=sse2vlen256 %s
// sse2vlen256: invalid argument '/vlen=256' not allowed with '/arch:SSE2'

// RUN: %clang_cl -m64 -arch:AVX -vlen256 --target=x86_64-pc-windows -### -- 2>&1 %s | FileCheck -check-prefix=avxvlen256 %s
// avxvlen256: unknown argument ignored in clang-cl: '/vlen256'

void f(void) {
}

Expand Down
Loading