From 091533bd66ca1546ccc5cad546512ea428f55431 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Thu, 3 Oct 2019 21:22:28 +0000 Subject: [PATCH] Reland r349624: Let TableGen write output only if it changed, instead of doing so in cmake Move the write-if-changed logic behind a flag and don't pass it with the MSVC generator. msbuild doesn't have a restat optimization, so not doing write-if-change there doesn't have a cost, and it should fix whatever causes PR43385. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@373664 91177308-0d34-0410-b5e6-96231b3b80d8 --- cmake/modules/TableGen.cmake | 30 +++++++-------- lib/TableGen/Main.cpp | 37 +++++++++++++++---- .../llvm/utils/TableGen/tablegen.gni | 5 --- 3 files changed, 43 insertions(+), 29 deletions(-) diff --git a/cmake/modules/TableGen.cmake b/cmake/modules/TableGen.cmake index 38e755d6d15..a1c28870c14 100644 --- a/cmake/modules/TableGen.cmake +++ b/cmake/modules/TableGen.cmake @@ -23,7 +23,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 @@ -34,7 +34,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() @@ -58,6 +58,15 @@ function(tablegen project ofn) endif() endif() + if (CMAKE_GENERATOR MATCHES "Visual Studio") + # Visual Studio has problems with llvm-tblgen's native --write-if-changed + # behavior. Since it doesn't do restat optimizations anyway, just don't + # pass --write-if-changed there. + set(tblgen_change_flag) + else() + set(tblgen_change_flag "--write-if-changed") + endif() + # We need both _TABLEGEN_TARGET and _TABLEGEN_EXE in the DEPENDS list # (both the target and the file) to have .inc files rebuilt on # a tablegen change, as cmake does not propagate file-level dependencies @@ -67,11 +76,11 @@ 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} + ${tblgen_change_flag} ${additional_cmdline} # The file in LLVM_TARGET_DEFINITIONS may be not in the current # directory and local_tds may not contain it, so we must @@ -81,20 +90,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 bf97e37f324..48ded6c45a4 100644 --- a/lib/TableGen/Main.cpp +++ b/lib/TableGen/Main.cpp @@ -49,6 +49,9 @@ static cl::list MacroNames("D", cl::desc("Name of the macro to be defined"), cl::value_desc("macro name"), cl::Prefix); +static cl::opt +WriteIfChanged("write-if-changed", cl::desc("Only write output if it changed")); + static int reportError(const char *ProgName, Twine Msg) { errs() << ProgName << ": " << Msg; errs().flush(); @@ -99,23 +102,41 @@ int llvm::TableGenMain(char *argv0, TableGenMainFn *MainFn) { if (Parser.ParseFile()) return 1; - std::error_code EC; - ToolOutputFile Out(OutputFilename, EC, sys::fs::OF_None); - 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; + if (WriteIfChanged) { + // 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::OF_None); + 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 a0e2e7967d6..cb588abbaa6 100644 --- a/utils/gn/secondary/llvm/utils/TableGen/tablegen.gni +++ b/utils/gn/secondary/llvm/utils/TableGen/tablegen.gni @@ -64,11 +64,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.40.0