From f62dffa9f07a45683bce1a130919cbe666dabbe6 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Fri, 22 Jun 2018 16:32:53 -0400 Subject: [PATCH 1/5] DSPTool: Factor out assembly file retrieval Keeps the retrieval behavior isolated and lessens the amount of variables within PerformAssembly's scope. --- Source/DSPTool/DSPTool.cpp | 48 +++++++++++++++++++++++--------------- 1 file changed, 29 insertions(+), 19 deletions(-) diff --git a/Source/DSPTool/DSPTool.cpp b/Source/DSPTool/DSPTool.cpp index bd100e6d83..5cb0547b30 100644 --- a/Source/DSPTool/DSPTool.cpp +++ b/Source/DSPTool/DSPTool.cpp @@ -2,7 +2,11 @@ // Licensed under GPLv2+ // Refer to the license.txt file included. -#include "Common/Common.h" +#include +#include +#include + +#include "Common/CommonTypes.h" #include "Common/FileUtil.h" #include "Common/StringUtil.h" #include "Core/DSP/DSPCodeUtil.h" @@ -235,6 +239,23 @@ static bool PerformDisassembly(const std::string& input_name, const std::string& return true; } +static std::vector GetAssemblerFiles(const std::string& source) +{ + std::vector files; + std::size_t last_pos = 0; + std::size_t pos = 0; + + while ((pos = source.find('\n', last_pos)) != std::string::npos) + { + std::string temp = source.substr(last_pos, pos - last_pos); + if (!temp.empty()) + files.push_back(std::move(temp)); + last_pos = pos + 1; + } + + return files; +} + static bool PerformAssembly(const std::string& input_name, const std::string& output_name, const std::string& output_header_name, bool multiple, bool force, bool output_size) @@ -250,34 +271,23 @@ static bool PerformAssembly(const std::string& input_name, const std::string& ou { if (multiple) { + source.append("\n"); + // When specifying a list of files we must compile a header // (we can't assemble multiple files to one binary) // since we checked it before, we assume output_header_name isn't empty - int lines; - std::vector* codes; - std::vector files; - std::string header, currentSource; - size_t lastPos = 0, pos = 0; - - source.append("\n"); - - while ((pos = source.find('\n', lastPos)) != std::string::npos) - { - std::string temp = source.substr(lastPos, pos - lastPos); - if (!temp.empty()) - files.push_back(temp); - lastPos = pos + 1; - } - - lines = (int)files.size(); + std::string header; + std::string currentSource; + const std::vector files = GetAssemblerFiles(source); + int lines = static_cast(files.size()); if (lines == 0) { printf("ERROR: Must specify at least one file\n"); return false; } - codes = new std::vector[lines]; + std::vector* codes = new std::vector[lines]; for (int i = 0; i < lines; i++) { From 537d09e1d4e01a2a3d36221c1df152b3cefd3953 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Fri, 22 Jun 2018 16:38:12 -0400 Subject: [PATCH 2/5] DSPTool: Remove unnecessary c_str() calls These functions already accept std::string instances, so c_str here just causes an unnecessary copy of the string to be made. --- Source/DSPTool/DSPTool.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Source/DSPTool/DSPTool.cpp b/Source/DSPTool/DSPTool.cpp index 5cb0547b30..340682a3cb 100644 --- a/Source/DSPTool/DSPTool.cpp +++ b/Source/DSPTool/DSPTool.cpp @@ -267,7 +267,7 @@ static bool PerformAssembly(const std::string& input_name, const std::string& ou } std::string source; - if (File::ReadFileToString(input_name.c_str(), source)) + if (File::ReadFileToString(input_name, source)) { if (multiple) { @@ -291,7 +291,7 @@ static bool PerformAssembly(const std::string& input_name, const std::string& ou for (int i = 0; i < lines; i++) { - if (!File::ReadFileToString(files[i].c_str(), currentSource)) + if (!File::ReadFileToString(files[i], currentSource)) { printf("ERROR reading %s, skipping...\n", files[i].c_str()); lines--; From d81e3fddce71172facbbef446e2220b1459d332f Mon Sep 17 00:00:00 2001 From: Lioncash Date: Fri, 22 Jun 2018 16:44:32 -0400 Subject: [PATCH 3/5] DSPTool: Make CodeToHeader() and CodesToHeader() return a std::string directly Instead of using an out-reference, we can modernize these to return the std::string directly. While we're at it, also remove the unused name parameter. --- Source/DSPTool/DSPTool.cpp | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/Source/DSPTool/DSPTool.cpp b/Source/DSPTool/DSPTool.cpp index 340682a3cb..1dcd6e2f63 100644 --- a/Source/DSPTool/DSPTool.cpp +++ b/Source/DSPTool/DSPTool.cpp @@ -43,14 +43,15 @@ void DSP::Host::UpdateDebugger() { } -static void CodeToHeader(const std::vector& code, std::string filename, const char* name, - std::string& header) +static std::string CodeToHeader(const std::vector& code, const std::string& filename) { std::vector code_padded = code; + // Pad with nops to 32byte boundary while (code_padded.size() & 0x7f) code_padded.push_back(0); - header.clear(); + + std::string header; header.reserve(code_padded.size() * 4); header.append("#define NUM_UCODES 1\n\n"); std::string filename_without_extension; @@ -69,13 +70,14 @@ static void CodeToHeader(const std::vector& code, std::string filename, con header.append("\n\t},\n"); header.append("};\n"); + return header; } -static void CodesToHeader(const std::vector* codes, const std::vector* filenames, - u32 num_codes, const char* name, std::string& header) +static std::string CodesToHeader(const std::vector* codes, + const std::vector* filenames, u32 num_codes) { std::vector> codes_padded; - u32 reserveSize = 0; + std::size_t reserveSize = 0; for (u32 i = 0; i < num_codes; i++) { codes_padded.push_back(codes[i]); @@ -83,9 +85,10 @@ static void CodesToHeader(const std::vector* codes, const std::vector* codes, const std::vector files = GetAssemblerFiles(source); @@ -310,7 +313,7 @@ static bool PerformAssembly(const std::string& input_name, const std::string& ou } } - CodesToHeader(codes, &files, lines, output_header_name.c_str(), header); + const std::string header = CodesToHeader(codes, &files, lines); File::WriteStringToFile(header, output_header_name + ".h"); delete[] codes; @@ -337,8 +340,7 @@ static bool PerformAssembly(const std::string& input_name, const std::string& ou } if (!output_header_name.empty()) { - std::string header; - CodeToHeader(code, input_name, output_header_name.c_str(), header); + const std::string header = CodeToHeader(code, input_name); File::WriteStringToFile(header, output_header_name + ".h"); } } From 83dab8dd365e890189c3608dd9aa4a0ad532bba0 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Fri, 22 Jun 2018 16:56:57 -0400 Subject: [PATCH 4/5] DSPTool: Get rid of raw new and delete We can just use a vector of a vector, which also has the benefit of keeping the size accounted for as well, allowing us to get rid of a count parameter for CodesToHeader(). --- Source/DSPTool/DSPTool.cpp | 38 ++++++++++++++++++-------------------- 1 file changed, 18 insertions(+), 20 deletions(-) diff --git a/Source/DSPTool/DSPTool.cpp b/Source/DSPTool/DSPTool.cpp index 1dcd6e2f63..04d18293ea 100644 --- a/Source/DSPTool/DSPTool.cpp +++ b/Source/DSPTool/DSPTool.cpp @@ -73,46 +73,46 @@ static std::string CodeToHeader(const std::vector& code, const std::string& return header; } -static std::string CodesToHeader(const std::vector* codes, - const std::vector* filenames, u32 num_codes) +static std::string CodesToHeader(const std::vector>& codes, + const std::vector& filenames) { std::vector> codes_padded; - std::size_t reserveSize = 0; - for (u32 i = 0; i < num_codes; i++) + std::size_t reserve_size = 0; + for (std::size_t i = 0; i < codes.size(); i++) { codes_padded.push_back(codes[i]); // Pad with nops to 32byte boundary - while (codes_padded.at(i).size() & 0x7f) - codes_padded.at(i).push_back(0); + while (codes_padded[i].size() & 0x7f) + codes_padded[i].push_back(0); - reserveSize += codes_padded.at(i).size(); + reserve_size += codes_padded[i].size(); } std::string header; - header.reserve(reserveSize * 4); - header.append(StringFromFormat("#define NUM_UCODES %u\n\n", num_codes)); + header.reserve(reserve_size * 4); + header.append(StringFromFormat("#define NUM_UCODES %zu\n\n", codes.size())); header.append("const char* UCODE_NAMES[NUM_UCODES] = {\n"); - for (u32 i = 0; i < num_codes; i++) + for (const std::string& in_filename : filenames) { std::string filename; - if (!SplitPath(filenames->at(i), nullptr, &filename, nullptr)) - filename = filenames->at(i); + if (!SplitPath(in_filename, nullptr, &filename, nullptr)) + filename = in_filename; header.append(StringFromFormat("\t\"%s\",\n", filename.c_str())); } header.append("};\n\n"); header.append("const unsigned short dsp_code[NUM_UCODES][0x1000] = {\n"); - for (u32 i = 0; i < num_codes; i++) + for (std::size_t i = 0; i < codes.size(); i++) { - if (codes[i].size() == 0) + if (codes[i].empty()) continue; header.append("\t{\n\t\t"); - for (u32 j = 0; j < codes_padded.at(i).size(); j++) + for (std::size_t j = 0; j < codes_padded[i].size(); j++) { if (j && ((j & 15) == 0)) header.append("\n\t\t"); - header.append(StringFromFormat("0x%04x, ", codes_padded.at(i).at(j))); + header.append(StringFromFormat("0x%04x, ", codes_padded[i][j])); } header.append("\n\t},\n"); } @@ -290,7 +290,7 @@ static bool PerformAssembly(const std::string& input_name, const std::string& ou return false; } - std::vector* codes = new std::vector[lines]; + std::vector> codes(lines); for (int i = 0; i < lines; i++) { @@ -313,10 +313,8 @@ static bool PerformAssembly(const std::string& input_name, const std::string& ou } } - const std::string header = CodesToHeader(codes, &files, lines); + const std::string header = CodesToHeader(codes, files); File::WriteStringToFile(header, output_header_name + ".h"); - - delete[] codes; } else { From 43daebbc667294d1377e90564b6d64ad820ebb09 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Fri, 22 Jun 2018 17:00:47 -0400 Subject: [PATCH 5/5] DSPTool: Get rid of unnecessary casts --- Source/DSPTool/DSPTool.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Source/DSPTool/DSPTool.cpp b/Source/DSPTool/DSPTool.cpp index 04d18293ea..148c0e44a8 100644 --- a/Source/DSPTool/DSPTool.cpp +++ b/Source/DSPTool/DSPTool.cpp @@ -283,7 +283,7 @@ static bool PerformAssembly(const std::string& input_name, const std::string& ou std::string currentSource; const std::vector files = GetAssemblerFiles(source); - int lines = static_cast(files.size()); + std::size_t lines = files.size(); if (lines == 0) { printf("ERROR: Must specify at least one file\n"); @@ -292,7 +292,7 @@ static bool PerformAssembly(const std::string& input_name, const std::string& ou std::vector> codes(lines); - for (int i = 0; i < lines; i++) + for (std::size_t i = 0; i < lines; i++) { if (!File::ReadFileToString(files[i], currentSource)) { @@ -308,7 +308,7 @@ static bool PerformAssembly(const std::string& input_name, const std::string& ou } if (output_size) { - printf("%s: %d\n", files[i].c_str(), (int)codes[i].size()); + printf("%s: %zu\n", files[i].c_str(), codes[i].size()); } } } @@ -328,7 +328,7 @@ static bool PerformAssembly(const std::string& input_name, const std::string& ou if (output_size) { - printf("%s: %d\n", input_name.c_str(), (int)code.size()); + printf("%s: %zu\n", input_name.c_str(), code.size()); } if (!output_name.empty())