diff --git a/common/string_helpers.cpp b/common/string_helpers.cpp index 4ca000cc46c9..905f681a874a 100644 --- a/common/string_helpers.cpp +++ b/common/string_helpers.cpp @@ -18,6 +18,7 @@ #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringExtras.h" #include "llvm/ADT/StringRef.h" +#include "llvm/Support/Allocator.h" #include "llvm/Support/ConvertUTF.h" namespace Carbon { @@ -211,33 +212,19 @@ auto StringRefContainsPointer(llvm::StringRef ref, const char* ptr) -> bool { auto BuildCStrArgs(llvm::StringRef tool_path, llvm::ArrayRef args, - llvm::OwningArrayRef& cstr_arg_storage) + llvm::BumpPtrAllocator& alloc) -> llvm::SmallVector { - return BuildCStrArgs(tool_path, /*prefix_args=*/{}, args, cstr_arg_storage); + return BuildCStrArgs(tool_path, /*prefix_args=*/{}, args, alloc); } auto BuildCStrArgs(llvm::StringRef tool_path, llvm::ArrayRef prefix_args, llvm::ArrayRef args, - llvm::OwningArrayRef& cstr_arg_storage) + llvm::BumpPtrAllocator& alloc) -> llvm::SmallVector { - // Render the arguments into null-terminated C-strings. Command lines can get - // quite long in build systems so this tries to minimize the memory allocation - // overhead. - - // Precompute the total C-string data size needed. - int total_size = tool_path.size() + 1; - for (llvm::StringRef arg : args) { - // Accumulate both the string size and a null terminator byte. - total_size += arg.size() + 1; - } - - // Allocate one chunk of storage for the actual C-strings, and reserve a - // vector of pointers into the storage. - cstr_arg_storage = llvm::OwningArrayRef(total_size); ssize_t i = 0; auto make_cstr = [&](llvm::StringRef arg) { - char* cstr = &cstr_arg_storage[i]; + char* cstr = alloc.Allocate(arg.size() + 1); memcpy(cstr, arg.data(), arg.size()); cstr[arg.size()] = '\0'; i += arg.size() + 1; diff --git a/common/string_helpers.h b/common/string_helpers.h index 793d836c70cb..69e12db9cc4f 100644 --- a/common/string_helpers.h +++ b/common/string_helpers.h @@ -12,6 +12,7 @@ #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringRef.h" +#include "llvm/Support/Allocator.h" namespace Carbon { @@ -44,7 +45,7 @@ auto StringRefContainsPointer(llvm::StringRef ref, const char* ptr) -> bool; // lines to avoid extra allocations and growth passes. auto BuildCStrArgs(llvm::StringRef tool_path, llvm::ArrayRef args, - llvm::OwningArrayRef& cstr_arg_storage) + llvm::BumpPtrAllocator& alloc) -> llvm::SmallVector; // An overload of `BuildCStrArgs` with the same core behavior as the above, but @@ -61,7 +62,7 @@ auto BuildCStrArgs(llvm::StringRef tool_path, auto BuildCStrArgs(llvm::StringRef tool_path, llvm::ArrayRef prefix_args, llvm::ArrayRef args, - llvm::OwningArrayRef& cstr_arg_storage) + llvm::BumpPtrAllocator& alloc) -> llvm::SmallVector; } // namespace Carbon diff --git a/common/string_helpers_test.cpp b/common/string_helpers_test.cpp index 120d0ba01ca5..98b7fea36211 100644 --- a/common/string_helpers_test.cpp +++ b/common/string_helpers_test.cpp @@ -12,6 +12,7 @@ #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/StringRef.h" +#include "llvm/Support/Allocator.h" using ::testing::Eq; using ::testing::Optional; @@ -233,23 +234,23 @@ TEST(ParseBlockStringLiteral, OkMultipleSlashes) { } TEST(BuildCStrArgs, NoArgs) { - llvm::OwningArrayRef storage; - auto result = BuildCStrArgs("tool", {}, storage); + llvm::BumpPtrAllocator alloc; + auto result = BuildCStrArgs("tool", {}, alloc); ASSERT_THAT(result.size(), Eq(1)); EXPECT_THAT(result[0], StrEq("tool")); } TEST(BuildCStrArgs, OneArg) { - llvm::OwningArrayRef storage; - auto result = BuildCStrArgs("tool", {"arg1"}, storage); + llvm::BumpPtrAllocator alloc; + auto result = BuildCStrArgs("tool", {"arg1"}, alloc); ASSERT_THAT(result.size(), Eq(2)); EXPECT_THAT(result[0], StrEq("tool")); EXPECT_THAT(result[1], StrEq("arg1")); } TEST(BuildCStrArgs, MultipleArgs) { - llvm::OwningArrayRef storage; - auto result = BuildCStrArgs("tool", {"arg1", "arg2"}, storage); + llvm::BumpPtrAllocator alloc; + auto result = BuildCStrArgs("tool", {"arg1", "arg2"}, alloc); ASSERT_THAT(result.size(), Eq(3)); EXPECT_THAT(result[0], StrEq("tool")); EXPECT_THAT(result[1], StrEq("arg1")); @@ -257,16 +258,16 @@ TEST(BuildCStrArgs, MultipleArgs) { } TEST(BuildCStrArgsWithPrefix, NoArgs) { - llvm::OwningArrayRef storage; - auto result = BuildCStrArgs("tool", {}, {}, storage); + llvm::BumpPtrAllocator alloc; + auto result = BuildCStrArgs("tool", {}, {}, alloc); ASSERT_THAT(result.size(), Eq(1)); EXPECT_THAT(result[0], StrEq("tool")); } TEST(BuildCStrArgsWithPrefix, PrefixOnly) { - llvm::OwningArrayRef storage; + llvm::BumpPtrAllocator alloc; std::string prefix_args[] = {"p_arg1", "p_arg2"}; - auto result = BuildCStrArgs("tool", prefix_args, {}, storage); + auto result = BuildCStrArgs("tool", prefix_args, {}, alloc); ASSERT_THAT(result.size(), Eq(3)); EXPECT_THAT(result[0], StrEq("tool")); EXPECT_THAT(result[1], Eq(prefix_args[0].c_str())); @@ -274,8 +275,8 @@ TEST(BuildCStrArgsWithPrefix, PrefixOnly) { } TEST(BuildCStrArgsWithPrefix, ArgsOnly) { - llvm::OwningArrayRef storage; - auto result = BuildCStrArgs("tool", {}, {"arg1", "arg2"}, storage); + llvm::BumpPtrAllocator alloc; + auto result = BuildCStrArgs("tool", {}, {"arg1", "arg2"}, alloc); ASSERT_THAT(result.size(), Eq(3)); EXPECT_THAT(result[0], StrEq("tool")); EXPECT_THAT(result[1], StrEq("arg1")); @@ -283,9 +284,9 @@ TEST(BuildCStrArgsWithPrefix, ArgsOnly) { } TEST(BuildCStrArgsWithPrefix, BothPrefixAndArgs) { - llvm::OwningArrayRef storage; + llvm::BumpPtrAllocator alloc; std::string prefix_args[] = {"p_arg1", "p_arg2"}; - auto result = BuildCStrArgs("tool", prefix_args, {"arg1", "arg2"}, storage); + auto result = BuildCStrArgs("tool", prefix_args, {"arg1", "arg2"}, alloc); ASSERT_THAT(result.size(), Eq(5)); EXPECT_THAT(result[0], StrEq("tool")); EXPECT_THAT(result[1], Eq(prefix_args[0].c_str())); diff --git a/toolchain/base/clang_invocation.cpp b/toolchain/base/clang_invocation.cpp index 53f225253db3..c8b1dc31cd2f 100644 --- a/toolchain/base/clang_invocation.cpp +++ b/toolchain/base/clang_invocation.cpp @@ -95,9 +95,9 @@ auto BuildClangInvocation(Diagnostics::Consumer& consumer, // The clang driver inconveniently wants an array of `const char*`, so convert // the arguments. - llvm::OwningArrayRef cstr_arg_storage; + llvm::BumpPtrAllocator alloc; llvm::SmallVector cstr_args = BuildCStrArgs( - install_paths.clang_path().native(), args, extra_args, cstr_arg_storage); + install_paths.clang_path().native(), args, extra_args, alloc); // Build a diagnostics engine. Note that we don't have any diagnostic options // yet; they're produced by running the driver. diff --git a/toolchain/driver/clang_runner.cpp b/toolchain/driver/clang_runner.cpp index 81bf9617c9ec..34900de007d3 100644 --- a/toolchain/driver/clang_runner.cpp +++ b/toolchain/driver/clang_runner.cpp @@ -237,14 +237,13 @@ auto ClangRunner::RunInternal( std::optional libunwind_path, std::optional libcxx_path, bool enable_leaking) -> bool { - // Rebuild the args as C-string args. - llvm::OwningArrayRef cstr_arg_storage; + llvm::BumpPtrAllocator alloc; // Handle special dispatch for CC1 commands as they don't use the driver and // we don't synthesize any default arguments there. if (!args.empty() && args[0].starts_with("-cc1")) { llvm::SmallVector cstr_args = - BuildCStrArgs(clang_path_.native(), args, cstr_arg_storage); + BuildCStrArgs(clang_path_.native(), args, alloc); if (args[0] == "-cc1") { CARBON_VLOG("Dispatching `-cc1` command line..."); int exit_code = @@ -297,7 +296,7 @@ auto ClangRunner::RunInternal( // Rebuild the args as C-string args. llvm::SmallVector cstr_args = - BuildCStrArgs(clang_path_.native(), prefix_args, args, cstr_arg_storage); + BuildCStrArgs(clang_path_.native(), prefix_args, args, alloc); CARBON_VLOG("Running Clang driver with the following arguments:\n"); for (const char* cstr_arg : llvm::ArrayRef(cstr_args)) { diff --git a/toolchain/driver/lld_runner.cpp b/toolchain/driver/lld_runner.cpp index 5bd30026e73f..da79ca73731c 100644 --- a/toolchain/driver/lld_runner.cpp +++ b/toolchain/driver/lld_runner.cpp @@ -14,6 +14,7 @@ #include "common/vlog.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/StringRef.h" +#include "llvm/Support/Allocator.h" // Declare the supported driver flavor entry points. // @@ -33,9 +34,9 @@ auto LldRunner::LinkHelper(llvm::StringLiteral label, -> bool { // Allocate one chunk of storage for the actual C-strings and a vector of // pointers into the storage. - llvm::OwningArrayRef cstr_arg_storage; + llvm::BumpPtrAllocator alloc; llvm::SmallVector cstr_args = - BuildCStrArgs(path, args, cstr_arg_storage); + BuildCStrArgs(path, args, alloc); CARBON_VLOG("Running LLD {0}-platform link with args:\n", label); for (const char* cstr_arg : cstr_args) { diff --git a/toolchain/driver/llvm_runner.cpp b/toolchain/driver/llvm_runner.cpp index d6da036b9607..80f3ffbd3964 100644 --- a/toolchain/driver/llvm_runner.cpp +++ b/toolchain/driver/llvm_runner.cpp @@ -15,6 +15,7 @@ #include "lld/Common/Driver.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/StringRef.h" +#include "llvm/Support/Allocator.h" namespace Carbon { @@ -24,9 +25,9 @@ auto LLVMRunner::Run(LLVMTool tool, llvm::ArrayRef args) // Allocate one chunk of storage for the actual C-strings and a vector of // pointers into the storage. - llvm::OwningArrayRef cstr_arg_storage; + llvm::BumpPtrAllocator alloc; llvm::SmallVector cstr_args = - BuildCStrArgs(path, args, cstr_arg_storage); + BuildCStrArgs(path, args, alloc); CARBON_VLOG("Running LLVM's {0} tool with args:\n", tool.name()); for (const char* cstr_arg : cstr_args) {