From 0c0891836f205bdf35323f1b49bc291bbbf1f12d Mon Sep 17 00:00:00 2001 From: Chris Kitching Date: Mon, 16 Oct 2017 14:46:49 +0100 Subject: [PATCH 1/3] Operate on a temp file in-place This copies to the output after operation, instead of working _on_ the output. This allows includes to work correctly, while supporting output paths anywhere on the filesystem. Fixes #208 Fixes #206 [ROCm/hip commit: 2263cb9f72ee006d88f9479c4127453d9b401daa] --- projects/hip/hipify-clang/src/Cuda2Hip.cpp | 45 ++++++++++++---------- 1 file changed, 25 insertions(+), 20 deletions(-) diff --git a/projects/hip/hipify-clang/src/Cuda2Hip.cpp b/projects/hip/hipify-clang/src/Cuda2Hip.cpp index 553ea5d8af..5318222408 100644 --- a/projects/hip/hipify-clang/src/Cuda2Hip.cpp +++ b/projects/hip/hipify-clang/src/Cuda2Hip.cpp @@ -4174,6 +4174,12 @@ void printAllStats(const std::string &csvFile, int64_t totalFiles, int64_t conve csv.close(); } +void copyFile(const std::string& src, const std::string& dst) { + std::ifstream source(src, std::ios::binary); + std::ofstream dest(dst, std::ios::binary); + dest << source.rdbuf(); +} + int main(int argc, const char **argv) { auto start = std::chrono::steady_clock::now(); auto begin = start; @@ -4235,18 +4241,21 @@ int main(int argc, const char **argv) { } dst += ".hip"; } - // backup source file since tooling may change "inplace" - if (!NoBackup || !Inplace) { - std::ifstream source(src, std::ios::binary); - std::ofstream dest(Inplace ? dst + ".prehip" : dst, std::ios::binary); - dest << source.rdbuf(); - source.close(); - dest.close(); - } - RefactoringTool Tool(OptionsParser.getCompilations(), dst); + + std::string tmpFile = src + ".hipify-tmp"; + + // Create a copy of the file to work on. When we're done, we'll move this onto the + // output (which may mean overwriting the input, if we're in-place). + // Should we fail for some reason, we'll just leak this file and not corrupt the input. + copyFile(src, tmpFile); + + // RefactoringTool operates on the file in-place. Giving it the output path is no good, + // because that'll break relative includes, and we don't want to overwrite the input file. + // So what we do is operate on a copy, which we then move to the output. + RefactoringTool Tool(OptionsParser.getCompilations(), tmpFile); ast_matchers::MatchFinder Finder; - HipifyPPCallbacks PPCallbacks(&Tool.getReplacements(), src); - Cuda2HipCallback Callback(&Tool.getReplacements(), &Finder, &PPCallbacks, src); + HipifyPPCallbacks PPCallbacks(&Tool.getReplacements(), tmpFile); + Cuda2HipCallback Callback(&Tool.getReplacements(), &Finder, &PPCallbacks, tmpFile); addAllMatchers(Finder, &Callback); @@ -4288,17 +4297,13 @@ int main(int argc, const char **argv) { if (!Tool.applyAllReplacements(Rewrite)) { DEBUG(dbgs() << "Skipped some replacements.\n"); } + + // Either move the tmpfile to the output, or remove it. if (!NoOutput) { Result += Rewrite.overwriteChangedFiles(); - } - if (!Inplace && !NoOutput) { - size_t pos = dst.rfind("."); - if (pos != std::string::npos) { - rename(dst.c_str(), dst.substr(0, pos).c_str()); - } - } - if (NoOutput) { - remove(dst.c_str()); + rename(tmpFile.c_str(), dst.c_str()); + } else { + remove(tmpFile.c_str()); } if (PrintStats) { if (fileSources.size() == 1) { From a2629da99c7286d4c02ace1b30287530004c09e9 Mon Sep 17 00:00:00 2001 From: Chris Kitching Date: Mon, 16 Oct 2017 14:48:52 +0100 Subject: [PATCH 2/3] If an output path is given _use it_ Don't append .hip to a user-provided output file... [ROCm/hip commit: e4e17a56bb7e1378928dacdd5b50eedee898ecf6] --- projects/hip/hipify-clang/src/Cuda2Hip.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/projects/hip/hipify-clang/src/Cuda2Hip.cpp b/projects/hip/hipify-clang/src/Cuda2Hip.cpp index 5318222408..d36e2ba6bf 100644 --- a/projects/hip/hipify-clang/src/Cuda2Hip.cpp +++ b/projects/hip/hipify-clang/src/Cuda2Hip.cpp @@ -4239,7 +4239,6 @@ int main(int argc, const char **argv) { llvm::errs() << "[HIPIFY] conflict: both -o and -inplace options are specified.\n"; return 1; } - dst += ".hip"; } std::string tmpFile = src + ".hipify-tmp"; From 49f9c4fefff5df6f081a4f722055041874e8a08b Mon Sep 17 00:00:00 2001 From: Chris Kitching Date: Wed, 18 Oct 2017 19:19:18 +0100 Subject: [PATCH 3/3] Unconditionally append .hip as the default output filename [ROCm/hip commit: 296c5a33ce8c059639d07b06384bac20afb51d59] --- projects/hip/hipify-clang/src/Cuda2Hip.cpp | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/projects/hip/hipify-clang/src/Cuda2Hip.cpp b/projects/hip/hipify-clang/src/Cuda2Hip.cpp index d36e2ba6bf..4a2dc58f4d 100644 --- a/projects/hip/hipify-clang/src/Cuda2Hip.cpp +++ b/projects/hip/hipify-clang/src/Cuda2Hip.cpp @@ -4225,20 +4225,14 @@ int main(int argc, const char **argv) { } for (const auto & src : fileSources) { if (dst.empty()) { - dst = src; - if (!Inplace) { - size_t pos = dst.rfind("."); - if (pos != std::string::npos && pos + 1 < dst.size()) { - dst = dst.substr(0, pos) + ".hip." + dst.substr(pos + 1, dst.size() - pos - 1); - } else { - dst += ".hip.cu"; - } - } - } else { if (Inplace) { - llvm::errs() << "[HIPIFY] conflict: both -o and -inplace options are specified.\n"; - return 1; + dst = src; + } else { + dst = src + ".hip"; } + } else if (Inplace) { + llvm::errs() << "[HIPIFY] conflict: both -o and -inplace options are specified.\n"; + return 1; } std::string tmpFile = src + ".hipify-tmp";