From 0bc70d331a335dccd2fcbd3fe0c13636c64513b9 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Wed, 19 Dec 2018 13:35:53 +0000 Subject: [PATCH] Let TableGen write output only if it changed, instead of doing so in cmake, attempt 2 This relands r330742: """ Let TableGen write output only if it changed, instead of doing so in cmake. Removes one subprocess and one temp file from the build for each tablegen invocation. No intended behavior change. """ In particular, if you see rebuilds after this change that you didn't see before this change, that's unintended and it's fine to revert this change again (but let me know). r330742 got reverted because some people reported that llvm-tblgen ran on every build after it. This could happen if the depfile output got deleted without deleting the main .inc output. To fix, make TableGen always write the depfile, but keep writing the main .inc output only if it has changed. This matches what we did in cmake before. Differential Revision: https://reviews.llvm.org/D55842 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@349624 91177308-0d34-0410-b5e6-96231b3b80d8 --- cmake/modules/TableGen.cmake | 20 +++--------- lib/TableGen/Main.cpp | 32 ++++++++++++++----- .../llvm/utils/TableGen/tablegen.gni | 5 --- 3 files changed, 28 insertions(+), 29 deletions(-) diff --git a/cmake/modules/TableGen.cmake b/cmake/modules/TableGen.cmake index 2d942435222..3c84ae78a34 100644 --- a/cmake/modules/TableGen.cmake +++ b/cmake/modules/TableGen.cmake @@ -25,7 +25,7 @@ function(tablegen project ofn) file(RELATIVE_PATH ofn_rel ${CMAKE_BINARY_DIR} ${CMAKE_CURRENT_BINARY_DIR}/${ofn}) set(additional_cmdline - -o ${ofn_rel}.tmp + -o ${ofn_rel} -d ${ofn_rel}.d WORKING_DIRECTORY ${CMAKE_BINARY_DIR} DEPFILE ${CMAKE_CURRENT_BINARY_DIR}/${ofn}.d @@ -36,7 +36,7 @@ function(tablegen project ofn) file(GLOB local_tds "*.td") file(GLOB_RECURSE global_tds "${LLVM_MAIN_INCLUDE_DIR}/llvm/*.td") set(additional_cmdline - -o ${CMAKE_CURRENT_BINARY_DIR}/${ofn}.tmp + -o ${CMAKE_CURRENT_BINARY_DIR}/${ofn} ) endif() @@ -69,8 +69,7 @@ function(tablegen project ofn) # dependency twice in the result file when # ("${${project}_TABLEGEN_TARGET}" STREQUAL "${${project}_TABLEGEN_EXE}") # but lets us having smaller and cleaner code here. - add_custom_command(OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/${ofn}.tmp - # Generate tablegen output in a temporary file. + add_custom_command(OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/${ofn} COMMAND ${${project}_TABLEGEN_EXE} ${ARGN} -I ${CMAKE_CURRENT_SOURCE_DIR} ${LLVM_TABLEGEN_FLAGS} ${LLVM_TARGET_DEFINITIONS_ABSOLUTE} @@ -83,20 +82,9 @@ function(tablegen project ofn) ${LLVM_TARGET_DEFINITIONS_ABSOLUTE} COMMENT "Building ${ofn}..." ) - add_custom_command(OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/${ofn} - # Only update the real output file if there are any differences. - # This prevents recompilation of all the files depending on it if there - # aren't any. - COMMAND ${CMAKE_COMMAND} -E copy_if_different - ${CMAKE_CURRENT_BINARY_DIR}/${ofn}.tmp - ${CMAKE_CURRENT_BINARY_DIR}/${ofn} - DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/${ofn}.tmp - COMMENT "Updating ${ofn}..." - ) # `make clean' must remove all those generated files: - set_property(DIRECTORY APPEND - PROPERTY ADDITIONAL_MAKE_CLEAN_FILES ${ofn}.tmp ${ofn}) + set_property(DIRECTORY APPEND PROPERTY ADDITIONAL_MAKE_CLEAN_FILES ${ofn}) set(TABLEGEN_OUTPUT ${TABLEGEN_OUTPUT} ${CMAKE_CURRENT_BINARY_DIR}/${ofn} PARENT_SCOPE) set_source_files_properties(${CMAKE_CURRENT_BINARY_DIR}/${ofn} PROPERTIES diff --git a/lib/TableGen/Main.cpp b/lib/TableGen/Main.cpp index df4088ad2f8..02698416609 100644 --- a/lib/TableGen/Main.cpp +++ b/lib/TableGen/Main.cpp @@ -100,23 +100,39 @@ int llvm::TableGenMain(char *argv0, TableGenMainFn *MainFn) { if (Parser.ParseFile()) return 1; - std::error_code EC; - ToolOutputFile Out(OutputFilename, EC, sys::fs::F_Text); - if (EC) - return reportError(argv0, "error opening " + OutputFilename + ":" + - EC.message() + "\n"); + // Write output to memory. + std::string OutString; + raw_string_ostream Out(OutString); + if (MainFn(Out, Records)) + return 1; + + // Always write the depfile, even if the main output hasn't changed. + // If it's missing, Ninja considers the output dirty. If this was below + // the early exit below and someone deleted the .inc.d file but not the .inc + // file, tablegen would never write the depfile. if (!DependFilename.empty()) { if (int Ret = createDependencyFile(Parser, argv0)) return Ret; } - if (MainFn(Out.os(), Records)) - return 1; + // Only updates the real output file if there are any differences. + // This prevents recompilation of all the files depending on it if there + // aren't any. + if (auto ExistingOrErr = MemoryBuffer::getFile(OutputFilename)) + if (std::move(ExistingOrErr.get())->getBuffer() == Out.str()) + return 0; + + std::error_code EC; + ToolOutputFile OutFile(OutputFilename, EC, sys::fs::F_Text); + if (EC) + return reportError(argv0, "error opening " + OutputFilename + ":" + + EC.message() + "\n"); + OutFile.os() << Out.str(); if (ErrorsPrinted > 0) return reportError(argv0, Twine(ErrorsPrinted) + " errors.\n"); // Declare success. - Out.keep(); + OutFile.keep(); return 0; } diff --git a/utils/gn/secondary/llvm/utils/TableGen/tablegen.gni b/utils/gn/secondary/llvm/utils/TableGen/tablegen.gni index 2a5520e9f1b..ce475f58b21 100644 --- a/utils/gn/secondary/llvm/utils/TableGen/tablegen.gni +++ b/utils/gn/secondary/llvm/utils/TableGen/tablegen.gni @@ -60,11 +60,6 @@ template("tablegen") { depfile = "$gen_output.d" td_file = rebase_path(td_file, root_build_dir) - # FIXME: The cmake build lets tablegen write to a temp file and then copies - # it over the final output only if it has changed, for ninja's restat - # optimization. Instead of doing that in cmake, llvm-tblgen should do this - # itself. r330742 tried this, but it caused problems. Fix those and reland, - # so that the gn build has the optimization too. args = [ rebase_path(tblgen_executable, root_build_dir), "-I", -- 2.50.1