[llvm] r331702 - Revert r330742: Let TableGen write output only if it changed, instead of doing so in cmake.

Chandler Carruth via llvm-commits llvm-commits at lists.llvm.org
Mon May 7 16:41:48 PDT 2018


Author: chandlerc
Date: Mon May  7 16:41:48 2018
New Revision: 331702

URL: http://llvm.org/viewvc/llvm-project?rev=331702&view=rev
Log:
Revert r330742: Let TableGen write output only if it changed, instead of doing so in cmake.

This change causes us to re-run tablegen for every single target on
every single build. This is much, much worse than the problem being
fixed AFAICT.

On my system, it makes a clean rebuild of `llc` with nothing changed go
from .5s to over 8s. On systems with less parallelism, slower file
systems, or high process startup overhead this will be even more
extreme.

The only way I see this could be a win is in clean builds where we churn
the filesystem. But I think incremental rebuild is more important, and
so if we want to re-instate this, it needs to be done in a way that
doesn't trigger constant re-runs of tablegen.

Modified:
    llvm/trunk/cmake/modules/TableGen.cmake
    llvm/trunk/lib/TableGen/Main.cpp

Modified: llvm/trunk/cmake/modules/TableGen.cmake
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/cmake/modules/TableGen.cmake?rev=331702&r1=331701&r2=331702&view=diff
==============================================================================
--- llvm/trunk/cmake/modules/TableGen.cmake (original)
+++ llvm/trunk/cmake/modules/TableGen.cmake Mon May  7 16:41:48 2018
@@ -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}
+      -o ${ofn_rel}.tmp
       -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}
+      -o ${CMAKE_CURRENT_BINARY_DIR}/${ofn}.tmp
       )
   endif()
 
@@ -69,7 +69,8 @@ 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}
+  add_custom_command(OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/${ofn}.tmp
+    # Generate tablegen output in a temporary file.
     COMMAND ${${project}_TABLEGEN_EXE} ${ARGN} -I ${CMAKE_CURRENT_SOURCE_DIR}
     ${LLVM_TABLEGEN_FLAGS}
     ${LLVM_TARGET_DEFINITIONS_ABSOLUTE}
@@ -82,9 +83,20 @@ 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})
+  set_property(DIRECTORY APPEND
+    PROPERTY ADDITIONAL_MAKE_CLEAN_FILES ${ofn}.tmp ${ofn})
 
   set(TABLEGEN_OUTPUT ${TABLEGEN_OUTPUT} ${CMAKE_CURRENT_BINARY_DIR}/${ofn} PARENT_SCOPE)
   set_source_files_properties(${CMAKE_CURRENT_BINARY_DIR}/${ofn} PROPERTIES

Modified: llvm/trunk/lib/TableGen/Main.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/TableGen/Main.cpp?rev=331702&r1=331701&r2=331702&view=diff
==============================================================================
--- llvm/trunk/lib/TableGen/Main.cpp (original)
+++ llvm/trunk/lib/TableGen/Main.cpp Mon May  7 16:41:48 2018
@@ -96,21 +96,8 @@ int llvm::TableGenMain(char *argv0, Tabl
   if (Parser.ParseFile())
     return 1;
 
-  // Write output to memory.
-  std::string OutString;
-  raw_string_ostream Out(OutString);
-  if (MainFn(Out, 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);
+  ToolOutputFile Out(OutputFilename, EC, sys::fs::F_Text);
   if (EC)
     return reportError(argv0, "error opening " + OutputFilename + ":" +
                                   EC.message() + "\n");
@@ -118,12 +105,14 @@ int llvm::TableGenMain(char *argv0, Tabl
     if (int Ret = createDependencyFile(Parser, argv0))
       return Ret;
   }
-  OutFile.os() << Out.str();
+
+  if (MainFn(Out.os(), Records))
+    return 1;
 
   if (ErrorsPrinted > 0)
     return reportError(argv0, Twine(ErrorsPrinted) + " errors.\n");
 
   // Declare success.
-  OutFile.keep();
+  Out.keep();
   return 0;
 }




More information about the llvm-commits mailing list