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.

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

merged 7 commits into from
Sep 15, 2024

Conversation

kyulee-com
Copy link
Contributor

@kyulee-com kyulee-com commented Apr 27, 2024

This adds new Clang flags to support codegen (CG) data:

  • -fcodegen-data-generate{=path}: This flag passes -codegen-data-generate as a boolean to the LLVM backend, causing the raw CG data to be emitted into a custom section. Currently, for LLD MachO only, it also passes --codegen-data-generate-path=<path> so that the indexed CG data file can be automatically produced at link time. For linkers that do not yet support this feature, llvm-cgdata can be used manually to merge this CG data in object files.
  • -fcodegen-data-use{=path}: This flag passes -codegen-data-use-path=<path> to the LLVM backend, enabling the use of specified CG data to optimistically outline functions.
  • The default <path> is set to default.cgdata when not specified.

This depends on #108733.
This is a patch for https://discourse.llvm.org/t/rfc-enhanced-machine-outliner-part-2-thinlto-nolto/78753.

@kyulee-com kyulee-com force-pushed the clang branch 2 times, most recently from 501af43 to 84682d4 Compare May 3, 2024 02:31
@kyulee-com kyulee-com changed the title [Clang][CGData] Flags [CGData] Clang Options May 3, 2024
kyulee-com added a commit that referenced this pull request Sep 10, 2024
This commit introduces support for outlining functions across modules
using codegen data generated from previous codegen. The codegen data
currently manages the outlined hash tree, which records outlining
instances that occurred locally in the past.
    
The machine outliner now operates in one of three modes:

1. CGDataMode::None: This is the default outliner mode that uses the
suffix tree to identify (local) outlining candidates within a module.
This mode is also used by (full)LTO to maintain optimal behavior with
the combined module.
2. CGDataMode::Write (`-codegen-data-generate`): This mode is identical
to the default mode, but it also publishes the stable hash sequences of
instructions in the outlined functions into a local outlined hash tree.
It then encodes this into the `__llvm_outline` section, which will be
dead-stripped at link time.
3. CGDataMode::Read (`-codegen-data-use-path={.cgdata}`): This mode
reads a codegen data file (.cgdata) and initializes a global outlined
hash tree. This tree is used to generate global outlining candidates.
Note that the codegen data file has been post-processed with the raw
`__llvm_outline` sections from all native objects using the
`llvm-cgdata` tool (or a linker, `LLD`, or a new ThinLTO pipeline
later).

This depends on #105398. After
this PR, LLD (#90166) and Clang
(#90304) will follow for each
client side support.
This is a patch for
https://discourse.llvm.org/t/rfc-enhanced-machine-outliner-part-2-thinlto-nolto/78753.
@kyulee-com kyulee-com marked this pull request as ready for review September 11, 2024 16:16
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Sep 11, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Sep 11, 2024

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-driver

Author: Kyungwoo Lee (kyulee-com)

Changes

This adds new Clang flags to support codegen (CG) data:

  • -fcodegen-data-generate{=path}: This flag passes -codegen-data-generate as a boolean to the LLVM backend, causing the raw CG data to be emitted into a custom section. Currently, for LLD MachO only, it also passes --codegen-data-generate-path=&lt;path&gt; so that the indexed CG data file can be automatically produced at link time. For linkers that do not yet support this feature, llvm-cgdata can be used manually to merge this CG data in object files.
  • -fcodegen-data-use{=path}: This flag passes -codegen-data-use-path=&lt;path&gt; to the LLVM backend, enabling the use of specified CG data to optimistically outline functions.
  • The default &lt;path&gt; is set to default.cgdata when not specified.

This depends on #90166.
This is a patch for https://discourse.llvm.org/t/rfc-enhanced-machine-outliner-part-2-thinlto-nolto/78753.


Full diff: https://github.com/llvm/llvm-project/pull/90304.diff

4 Files Affected:

  • (modified) clang/include/clang/Driver/Options.td (+12)
  • (modified) clang/lib/Driver/ToolChains/CommonArgs.cpp (+27)
  • (modified) clang/lib/Driver/ToolChains/Darwin.cpp (+46)
  • (added) clang/test/Driver/codegen-data.c (+42)
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index f78032255f036f..b400af5d99c654 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -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 : Joined<["-"], "fcodegen-data-generate">,
+    Group<f_Group>, Visibility<[ClangOption, CC1Option]>,
+    HelpText<"Emit codegen data into object file. LLD for MachO (for now) merges them into default.cgdata">;
+def fcodegen_data_generate_EQ : Joined<["-"], "fcodegen-data-generate=">,
+    Group<f_Group>, Visibility<[ClangOption, CC1Option]>, MetaVarName<"<directory>">,
+    HelpText<"Emit codegen data into object file. LLD for MachO (for now) merges them into <directory>/default.cgdata">;
+def fcodegen_data_use : Joined<["-"], "fcodegen-data-use">,
+    Group<f_Group>, Visibility<[ClangOption, CC1Option]>,
+    HelpText<"Use codegen data read from default.cgdata to optimize the binary">;
+def fcodegen_data_use_EQ : Joined<["-"], "fcodegen-data-use=">,
+    Group<f_Group>, Visibility<[ClangOption, CC1Option]>, MetaVarName<"<directory>">,
+    HelpText<"Use codegen data read from <directory>/default.cgdata to optimize the binary">;
 def fswift_async_fp_EQ : Joined<["-"], "fswift-async-fp=">,
     Group<f_Group>,
     Visibility<[ClangOption, CC1Option, CC1AsOption, CLOption]>,
diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 2ce6779f4b43e3..5fa502d64c0300 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -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,
+                      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());
+    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,
diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp
index 5e7f9290e2009d..9e72e280109640 100644
--- a/clang/lib/Driver/ToolChains/Darwin.cpp
+++ b/clang/lib/Driver/ToolChains/Darwin.cpp
@@ -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));
+    }
   }
 }
 
@@ -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());
diff --git a/clang/test/Driver/codegen-data.c b/clang/test/Driver/codegen-data.c
new file mode 100644
index 00000000000000..a72850afc59736
--- /dev/null
+++ b/clang/test/Driver/codegen-data.c
@@ -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"

VitaNuo pushed a commit to VitaNuo/llvm-project that referenced this pull request Sep 12, 2024
This commit introduces support for outlining functions across modules
using codegen data generated from previous codegen. The codegen data
currently manages the outlined hash tree, which records outlining
instances that occurred locally in the past.
    
The machine outliner now operates in one of three modes:

1. CGDataMode::None: This is the default outliner mode that uses the
suffix tree to identify (local) outlining candidates within a module.
This mode is also used by (full)LTO to maintain optimal behavior with
the combined module.
2. CGDataMode::Write (`-codegen-data-generate`): This mode is identical
to the default mode, but it also publishes the stable hash sequences of
instructions in the outlined functions into a local outlined hash tree.
It then encodes this into the `__llvm_outline` section, which will be
dead-stripped at link time.
3. CGDataMode::Read (`-codegen-data-use-path={.cgdata}`): This mode
reads a codegen data file (.cgdata) and initializes a global outlined
hash tree. This tree is used to generate global outlining candidates.
Note that the codegen data file has been post-processed with the raw
`__llvm_outline` sections from all native objects using the
`llvm-cgdata` tool (or a linker, `LLD`, or a new ThinLTO pipeline
later).

This depends on llvm#105398. After
this PR, LLD (llvm#90166) and Clang
(llvm#90304) will follow for each
client side support.
This is a patch for
https://discourse.llvm.org/t/rfc-enhanced-machine-outliner-part-2-thinlto-nolto/78753.
Copy link
Contributor

@ellishg ellishg left a comment

Choose a reason for hiding this comment

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

Can we add some documentation to https://github.com/llvm/llvm-project/blob/main/clang/docs/UsersManual.rst? This could also be a separate PR.

LGTM, but I want to give others a chance to review.

@ellishg ellishg mentioned this pull request Sep 12, 2024
@@ -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.

@@ -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.

@kyulee-com
Copy link
Contributor Author

Can we add some documentation to https://github.com/llvm/llvm-project/blob/main/clang/docs/UsersManual.rst? This could also be a separate PR.

LGTM, but I want to give others a chance to review.

Added some documentation, and simplify the flag use by removing the directory case as discussed in #90166.

Comment on lines 2774 to 2776
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.

@jansvoboda11
Copy link
Contributor

The options handling part LGTM, but I'll let others review the actual semantics of this PR.

@kyulee-com
Copy link
Contributor Author

@NuriAmari Do you see or expect any issues with using distributed ThinLTO?

@NuriAmari
Copy link
Contributor

Do you see or expect any issues with using distributed ThinLTO?

No seems good to me!

@kyulee-com
Copy link
Contributor Author

I think I've addressed all the comments and the usage is fairly straightforward.
Can someone take another look?

@kyulee-com kyulee-com merged commit 713a202 into llvm:main Sep 15, 2024
9 checks passed
kyulee-com added a commit that referenced this pull request Oct 3, 2024
This is NFC for #90933.

- Create a lambda function, `RunBackends`, to group the backend
operations into a single function.
- Explicitly pass the `CodeGenOnly` argument to thinBackend, instead of
depending on a configuration value.

Depends on #90304.
This is a patch for
https://discourse.llvm.org/t/rfc-enhanced-machine-outliner-part-2-thinlto-nolto/78753.
xgupta pushed a commit to xgupta/llvm-project that referenced this pull request Oct 4, 2024
This is NFC for llvm#90933.

- Create a lambda function, `RunBackends`, to group the backend
operations into a single function.
- Explicitly pass the `CodeGenOnly` argument to thinBackend, instead of
depending on a configuration value.

Depends on llvm#90304.
This is a patch for
https://discourse.llvm.org/t/rfc-enhanced-machine-outliner-part-2-thinlto-nolto/78753.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants