Skip to content
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

[CGData] Clang Options #90304

Merged
merged 7 commits into from
Sep 15, 2024
Merged

[CGData] Clang Options #90304

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
966922b
[CGData] Clang Optinos
kyulee-com Apr 26, 2024
3a3da69
Address comments from ellishg
kyulee-com Sep 11, 2024
0710ef4
Address comments from jansvoboda11
kyulee-com Sep 12, 2024
e7be9e2
Remove the directory condition in path per discussion in #90166
kyulee-com Sep 12, 2024
dbab3cc
Document
kyulee-com Sep 12, 2024
d0a95a6
Address 2nd comments from jansvoboda11
kyulee-com Sep 12, 2024
5df8e44
Address 3rd comments from jansvoboda11
kyulee-com Sep 13, 2024
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
12 changes: 12 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 @@ -1894,6 +1894,18 @@ def fprofile_selected_function_group :
Visibility<[ClangOption, CC1Option]>, MetaVarName<"<i>">,
HelpText<"Partition functions into N groups using -fprofile-function-groups and select only functions in group i to be instrumented. The valid range is 0 to N-1 inclusive">,
MarshallingInfoInt<CodeGenOpts<"ProfileSelectedFunctionGroup">>;
def fcodegen_data_generate_EQ : Joined<["-"], "fcodegen-data-generate=">,
Group<f_Group>, Visibility<[ClangOption, CC1Option]>, MetaVarName<"<path>">,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the CC1Option? I don't see this being used in the frontend.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah. It's not needed. Instead, add CLOption similar to the IRPGO flags.

HelpText<"Emit codegen data into the object file. LLD for MachO (currently) merges them into a <path> file. If the path is a directory, it writes to <path>/default.cgdata.">;
def fcodegen_data_generate : Flag<["-"], "fcodegen-data-generate">,
Group<f_Group>, Visibility<[ClangOption, CLOption]>, Alias<fcodegen_data_generate_EQ>,
HelpText<"Emit codegen data into the object file. LLD for MachO (currently) merges them into default.cgdata.">;
def fcodegen_data_use_EQ : Joined<["-"], "fcodegen-data-use=">,
Group<f_Group>, Visibility<[ClangOption, CC1Option]>, MetaVarName<"<path>">,
HelpText<"Use codegen data read from a <path> file. If the path is a directory, it reads from <path>/default.cgdata.">;
def fcodegen_data_use : Flag<["-"], "fcodegen-data-use">,
Group<f_Group>, Visibility<[ClangOption, CC1Option]>, Alias<fcodegen_data_use_EQ>,
HelpText<"Use codegen data read from default.cgdata to optimize the binary">;
def fswift_async_fp_EQ : Joined<["-"], "fswift-async-fp=">,
Group<f_Group>,
Visibility<[ClangOption, CC1Option, CC1AsOption, CLOption]>,
Expand Down
27 changes: 27 additions & 0 deletions clang/lib/Driver/ToolChains/CommonArgs.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2753,6 +2753,33 @@ void tools::addMachineOutlinerArgs(const Driver &D,
addArg(Twine("-enable-machine-outliner=never"));
}
}

auto *CodeGenDataGenArg =
Args.getLastArg(options::OPT_fcodegen_data_generate,
Copy link
Contributor

Choose a reason for hiding this comment

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

Since fcodegen_data_generate is now an alias of fcodegen_data_generate_EQ, do we need to check for both here? I'd expect checking for the _EQ variant should be enough even if the command-line contains the non-_EQ one. Same for the other option.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion! I now use _EQ flag only for various checks.

options::OPT_fcodegen_data_generate_EQ);
auto *CodeGenDataUseArg = Args.getLastArg(options::OPT_fcodegen_data_use,
options::OPT_fcodegen_data_use_EQ);

// We only allow one of them to be specified.
if (CodeGenDataGenArg && CodeGenDataUseArg)
D.Diag(diag::err_drv_argument_not_allowed_with)
<< CodeGenDataGenArg->getAsString(Args)
<< CodeGenDataUseArg->getAsString(Args);

// For codegen data gen, the output file is passed to the linker
// while a boolean flag is passed to the LLVM backend.
if (CodeGenDataGenArg)
addArg(Twine("-codegen-data-generate"));

// For codegen data use, the input file is passed to the LLVM backend.
if (CodeGenDataUseArg) {
SmallString<128> Path(CodeGenDataUseArg->getNumValues() == 0
? ""
: CodeGenDataUseArg->getValue());
Copy link
Contributor

Choose a reason for hiding this comment

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

Would replacing this logic with AliasArgs<["default.cgdata"]> in Options.td on the alias arg work? That way it would be more declarative and consistent with other flags.

if (Path.empty() || llvm::sys::fs::is_directory(Path))
llvm::sys::path::append(Path, "default.cgdata");
addArg(Twine("-codegen-data-use-path=" + Path.str()));
}
}

void tools::addOpenMPDeviceRTL(const Driver &D,
Expand Down
46 changes: 46 additions & 0 deletions clang/lib/Driver/ToolChains/Darwin.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,19 @@ void darwin::Linker::AddLinkArgs(Compilation &C, const ArgList &Args,
llvm::sys::path::append(Path, "default.profdata");
CmdArgs.push_back(Args.MakeArgString(Twine("--cs-profile-path=") + Path));
}

auto *CodeGenDataGenArg =
Args.getLastArg(options::OPT_fcodegen_data_generate,
options::OPT_fcodegen_data_generate_EQ);
if (CodeGenDataGenArg) {
SmallString<128> Path(CodeGenDataGenArg->getNumValues() == 0
? ""
: CodeGenDataGenArg->getValue());
if (Path.empty() || llvm::sys::fs::is_directory(Path))
llvm::sys::path::append(Path, "default.cgdata");
CmdArgs.push_back(
Args.MakeArgString(Twine("--codegen-data-generate-path=") + Path));
}
}
}

Expand Down Expand Up @@ -633,6 +646,39 @@ void darwin::Linker::ConstructJob(Compilation &C, const JobAction &JA,
CmdArgs.push_back("-mllvm");
CmdArgs.push_back("-enable-linkonceodr-outlining");

// Propagate codegen data flags to the linker for the LLVM backend.
auto *CodeGenDataGenArg =
Args.getLastArg(options::OPT_fcodegen_data_generate,
options::OPT_fcodegen_data_generate_EQ);
auto *CodeGenDataUseArg = Args.getLastArg(options::OPT_fcodegen_data_use,
options::OPT_fcodegen_data_use_EQ);

// We only allow one of them to be specified.
const Driver &D = getToolChain().getDriver();
if (CodeGenDataGenArg && CodeGenDataUseArg)
D.Diag(diag::err_drv_argument_not_allowed_with)
<< CodeGenDataGenArg->getAsString(Args)
<< CodeGenDataUseArg->getAsString(Args);

// For codegen data gen, the output file is passed to the linker
// while a boolean flag is passed to the LLVM backend.
if (CodeGenDataGenArg) {
CmdArgs.push_back("-mllvm");
CmdArgs.push_back("-codegen-data-generate");
}

// For codegen data use, the input file is passed to the LLVM backend.
if (CodeGenDataUseArg) {
SmallString<128> Path(CodeGenDataUseArg->getNumValues() == 0
? ""
: CodeGenDataUseArg->getValue());
if (Path.empty() || llvm::sys::fs::is_directory(Path))
llvm::sys::path::append(Path, "default.cgdata");
CmdArgs.push_back("-mllvm");
CmdArgs.push_back(
Args.MakeArgString("-codegen-data-use-path=" + Path.str()));
}

// Setup statistics file output.
SmallString<128> StatsFile =
getStatsFileName(Args, Output, Inputs[0], getToolChain().getDriver());
Expand Down
42 changes: 42 additions & 0 deletions clang/test/Driver/codegen-data.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// Verify only one of codegen-data flag is passed.
// RUN: not %clang -### -S --target=aarch64-linux-gnu -fcodegen-data-generate -fcodegen-data-use %s 2>&1 | FileCheck %s --check-prefix=CONFLICT
// RUN: not %clang -### -S --target=arm64-apple-darwin -fcodegen-data-generate -fcodegen-data-use %s 2>&1 | FileCheck %s --check-prefix=CONFLICT
// CONFLICT: error: invalid argument '-fcodegen-data-generate' not allowed with '-fcodegen-data-use'

// Verify the codegen-data-generate (boolean) flag is passed to LLVM
// RUN: %clang -### -S --target=aarch64-linux-gnu -fcodegen-data-generate %s 2>&1| FileCheck %s --check-prefix=GENERATE
// RUN: %clang -### -S --target=arm64-apple-darwin -fcodegen-data-generate %s 2>&1| FileCheck %s --check-prefix=GENERATE
// GENERATE: "-mllvm" "-codegen-data-generate"

// Verify the codegen-data-use-path flag (with a default value) is passed to LLVM.
// RUN: %clang -### -S --target=aarch64-linux-gnu -fcodegen-data-use %s 2>&1| FileCheck %s --check-prefix=USE
// RUN: %clang -### -S --target=arm64-apple-darwin -fcodegen-data-use %s 2>&1| FileCheck %s --check-prefix=USE
// RUN: mkdir -p %t.d/some/dir
// RUN: %clang -### -S --target=aarch64-linux-gnu -fcodegen-data-use=%t.d/some/dir %s 2>&1 | FileCheck %s --check-prefix=USE-DIR
// RUN: %clang -### -S --target=arm64-apple-darwin -fcodegen-data-use=%t.d/some/dir %s 2>&1 | FileCheck %s --check-prefix=USE-DIR
// RUN: %clang -### -S --target=aarch64-linux-gnu -fcodegen-data-use=file %s 2>&1 | FileCheck %s --check-prefix=USE-FILE
// RUN: %clang -### -S --target=arm64-apple-darwin -fcodegen-data-use=file %s 2>&1 | FileCheck %s --check-prefix=USE-FILE
// USE: "-mllvm" "-codegen-data-use-path=default.cgdata"
// USE-DIR: "-mllvm" "-codegen-data-use-path={{.*}}.d/some/dir{{/|\\\\}}default.cgdata"
// USE-FILE: "-mllvm" "-codegen-data-use-path=file"

// Verify the codegen-data-generate (boolean) flag with a LTO.
// RUN: %clang -### -flto --target=aarch64-linux-gnu -fcodegen-data-generate %s 2>&1 | FileCheck %s --check-prefix=GENERATE-LTO
// GENERATE-LTO: {{ld(.exe)?"}}
// GENERATE-LTO-SAME: "-plugin-opt=-codegen-data-generate"
// RUN: %clang -### -flto --target=arm64-apple-darwin -fcodegen-data-generate %s 2>&1 | FileCheck %s --check-prefix=GENERATE-LTO-DARWIN
// GENERATE-LTO-DARWIN: {{ld(.exe)?"}}
// GENERATE-LTO-DARWIN-SAME: "-mllvm" "-codegen-data-generate"

// Verify the codegen-data-use-path flag with a LTO is passed to LLVM.
// RUN: %clang -### -flto=thin --target=aarch64-linux-gnu -fcodegen-data-use %s 2>&1 | FileCheck %s --check-prefix=USE-LTO
// USE-LTO: {{ld(.exe)?"}}
// USE-LTO-SAME: "-plugin-opt=-codegen-data-use-path=default.cgdata"
// RUN: %clang -### -flto=thin --target=arm64-apple-darwin -fcodegen-data-use %s 2>&1 | FileCheck %s --check-prefix=USE-LTO-DARWIN
// USE-LTO-DARWIN: {{ld(.exe)?"}}
// USE-LTO-DARWIN-SAME: "-mllvm" "-codegen-data-use-path=default.cgdata"

// For now, LLD MachO supports for generating the codegen data at link time.
// RUN: %clang -### -fuse-ld=lld -B%S/Inputs/lld --target=arm64-apple-darwin -fcodegen-data-generate %s 2>&1 | FileCheck %s --check-prefix=GENERATE-LLD-DARWIN
// GENERATE-LLD-DARWIN: {{ld(.exe)?"}}
// GENERATE-LLD-DARWIN-SAME: "--codegen-data-generate-path=default.cgdata"
Loading