Suppress version control revision info (Git revision id) is appended about tc-build HOT 25 CLOSED

I do not think that the revision info in the compiler string matters for the sake of ccache, unless you have changed something.

According to the ccache docs (https://ccache.dev/manual/latest.html#_configuration_settings):

By default, ccache includes the modification time (“mtime”) and size of the compiler in the hash to ensure that results retrieved from the cache are accurate.

If you build a new compiler, the mtime will change (and probably the size in some small way) and ccache will be invalidated, regardless of the version string in the compiler.

from tc-build.

@msfjarvis
Thanks for the patch. I will try it and report.

The comment is not precise enough.

-        # Don't append versioning info to the compiler version
+        # Don't append version control revision info (Git revision id) to the compiler version
+        'LLVM_APPEND_VC_REV': 'OFF',

This can be changed later.

from tc-build.

What I do is:

cd linux
make distclean
cp -v dileks.config .config
cd ..
dileks-buildscript.sh

from tc-build.

@dileks have you tried to set LLVM_APPEND_VC_REV to false and seeing if that actually helps with cccache hits? You can try this patch and share your findings so we can make a decision about it, since I agree with Nathan's interpretation of the ccache documentation and would love to see evidence to the contrary.

from tc-build.

With patch applied these files are generated:

$ ll ./stage1/include/llvm/Support/VCSRevision.h ./stage1/tools/clang/lib/Basic/VCSVersion.inc ./stage1/tools/lld/Common/VCSVersion.inc
-rw-r--r-- 1 dileks dileks  44 Apr 15 00:06 ./stage1/include/llvm/Support/VCSRevision.h
-rw-r--r-- 1 dileks dileks 258 Apr 15 00:08 ./stage1/tools/clang/lib/Basic/VCSVersion.inc
-rw-r--r-- 1 dileks dileks 126 Apr 15 00:09 ./stage1/tools/lld/Common/VCSVersion.inc

$ cat ./stage1/include/llvm/Support/VCSRevision.h
#undef LLVM_REVISION
#undef LLVM_REPOSITORY

$ cat ./stage1/tools/clang/lib/Basic/VCSVersion.inc
#define LLVM_REVISION "edbe962459da6e3b7b4168118f93a77847b54e02"
#define LLVM_REPOSITORY "https://github.com/llvm/llvm-project"
#define CLANG_REVISION "edbe962459da6e3b7b4168118f93a77847b54e02"
#define CLANG_REPOSITORY "https://github.com/llvm/llvm-project"

$ cat ./stage1/tools/lld/Common/VCSVersion.inc
#define LLD_REVISION "edbe962459da6e3b7b4168118f93a77847b54e02"
#define LLD_REPOSITORY "https://github.com/llvm/llvm-project"

Hmm, that undefs seem not to work properly.

from tc-build.

Might wanna report that to LLVM in that case.

from tc-build.

With this diff the generation of VCSRevision.h is suppressed:

$ git --no-pager diff
diff --git a/llvm/cmake/modules/GenerateVersionFromVCS.cmake b/llvm/cmake/modules/GenerateVersionFromVCS.cmake
index d8ec54df41eb..ed709ac6c645 100644
--- a/llvm/cmake/modules/GenerateVersionFromVCS.cmake
+++ b/llvm/cmake/modules/GenerateVersionFromVCS.cmake
@@ -23,16 +23,10 @@ function(append_info name path)
     get_source_info("${path}" revision repository)
   endif()
   if(revision)
-    file(APPEND "${HEADER_FILE}.tmp"
-      "#define ${name}_REVISION \"${revision}\"\n")
-  else()
     file(APPEND "${HEADER_FILE}.tmp"
       "#undef ${name}_REVISION\n")
   endif()
   if(repository)
-    file(APPEND "${HEADER_FILE}.tmp"
-      "#define ${name}_REPOSITORY \"${repository}\"\n")
-  else()
     file(APPEND "${HEADER_FILE}.tmp"
       "#undef ${name}_REPOSITORY\n")
   endif()
diff --git a/llvm/lib/LTO/LTO.cpp b/llvm/lib/LTO/LTO.cpp
index 297b11de17a9..689f3e87d77f 100644
--- a/llvm/lib/LTO/LTO.cpp
+++ b/llvm/lib/LTO/LTO.cpp
@@ -39,7 +39,6 @@
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/ThreadPool.h"
 #include "llvm/Support/Threading.h"
-#include "llvm/Support/VCSRevision.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Target/TargetMachine.h"
 #include "llvm/Target/TargetOptions.h"
diff --git a/llvm/lib/LTO/ThinLTOCodeGenerator.cpp b/llvm/lib/LTO/ThinLTOCodeGenerator.cpp
index a9e27832917c..0bb518bf6cce 100644
--- a/llvm/lib/LTO/ThinLTOCodeGenerator.cpp
+++ b/llvm/lib/LTO/ThinLTOCodeGenerator.cpp
@@ -48,7 +48,6 @@
 #include "llvm/Support/ThreadPool.h"
 #include "llvm/Support/Threading.h"
 #include "llvm/Support/ToolOutputFile.h"
-#include "llvm/Support/VCSRevision.h"
 #include "llvm/Target/TargetMachine.h"
 #include "llvm/Transforms/IPO.h"
 #include "llvm/Transforms/IPO/FunctionImport.h"
diff --git a/llvm/lib/Object/IRSymtab.cpp b/llvm/lib/Object/IRSymtab.cpp
index e4282b9d6bd3..020c9eb4c0e1 100644
--- a/llvm/lib/Object/IRSymtab.cpp
+++ b/llvm/lib/Object/IRSymtab.cpp
@@ -31,7 +31,6 @@
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/StringSaver.h"
-#include "llvm/Support/VCSRevision.h"
 #include "llvm/Support/raw_ostream.h"
 #include <cassert>
 #include <string>

Versions:

dileks@iniza:~/src/llvm-toolchain/build/stage1/bin$ ./clang-10 --version
ClangBuiltLinux clang version 10.0.1 
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /home/dileks/src/llvm-toolchain/build/stage1/bin/.

dileks@iniza:~/src/llvm-toolchain/build/stage1/bin$ ./ld.lld --version
LLD 10.0.1 (compatible with GNU linkers)

from tc-build.

Now what needs to be tested is:

1. Building a kernel with ccache
2. Rebuilding the toolchain with no VCS info AND no source code/environment changes
3. ccache -z
4. Building a kernel with ccache
5. ccache -s

My guess is that the cache's hit rate is still going to be low because the modification time of the compiler will have changed but we'll see.

from tc-build.

This seems to work:

diff --git a/llvm/cmake/modules/GenerateVersionFromVCS.cmake b/llvm/cmake/modules/GenerateVersionFromVCS.cmake
index d8ec54df41eb..6ea1f6ccc3a3 100644
--- a/llvm/cmake/modules/GenerateVersionFromVCS.cmake
+++ b/llvm/cmake/modules/GenerateVersionFromVCS.cmake
@@ -22,14 +22,14 @@ function(append_info name path)
   if(path)
     get_source_info("${path}" revision repository)
   endif()
-  if(revision)
+  if(revision AND LLVM_APPEND_VC_REV)
     file(APPEND "${HEADER_FILE}.tmp"
       "#define ${name}_REVISION \"${revision}\"\n")
   else()
     file(APPEND "${HEADER_FILE}.tmp"
       "#undef ${name}_REVISION\n")
   endif()
-  if(repository)
+  if(repository AND LLVM_APPEND_VC_REV)
     file(APPEND "${HEADER_FILE}.tmp"
       "#define ${name}_REPOSITORY \"${repository}\"\n")
   else()

Undefs:

$ cd build

$ cat ./stage1/include/llvm/Support/VCSRevision.h 
#undef LLVM_REVISION
#undef LLVM_REPOSITORY

$ cat ./stage1/tools/clang/lib/Basic/VCSVersion.inc
#undef LLVM_REVISION
#undef LLVM_REPOSITORY
#undef CLANG_REVISION
#undef CLANG_REPOSITORY

$ cat ./stage1/tools/lld/Common/VCSVersion.inc
#undef LLD_REVISION
#undef LLD_REPOSITORY

Versions:

$ cd build/stage1/bin

$ ./clang-10 --version
ClangBuiltLinux clang version 10.0.1 
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /home/dileks/src/llvm-toolchain/build/stage1/bin/.

$ ./ld.lld --version
LLD 10.0.1 (compatible with GNU linkers)

from tc-build.

I do not need any changes to the LLVM source for LLVM_APPEND_VC_REV=OFF to work...

$ git show -s --format=%H
2f8b4545f4960778e37114c024073d208751ca89

$ cmake -GNinja \
        -DCMAKE_C_COMPILER=clang \
        -DCMAKE_CXX_COMPILER=clang++ \
        -DLLVM_APPEND_VC_REV=OFF \
        -DLLVM_ENABLE_PROJECTS="clang;lld" \
        -DLLVM_TARGETS_TO_BUILD=X86 \
        -DLLVM_USE_LINKER=lld \
        ../llvm
...

$ ninja clang lld
[2428/2428] Creating executable symlink bin/clang

$ ./bin/clang --version | head -n1
clang version 11.0.0

$ ./bin/ld.lld --version | head -n1
LLD 11.0.0 (compatible with GNU linkers)

from tc-build.

This is with master Git? I am here on release/10.x.

Were there any changes to llvm/cmake/modules/GenerateVersionFromVCS.cmake file?

from tc-build.

@nathanchance
Can you re-test with release/10.x Git branch, please?
I am happy if this works w/o changes.

from tc-build.

This is fixed in master but not release/10.x: llvm/llvm-project@fb5fafb

Not really sure if that is worthy of being added to release/10.x but you can file an issue with Debian or LLVM to see if you really care.

from tc-build.

@nathanchance
OK, thanks for the Link.

My above snippet looks like a more elegant way to fix it.

from tc-build.

@msfjarvis

Can you provide a switchable version of your patch to turn LLVM_APPEND_VC_REVON and OFF?
With my corrected comment?

Thanks.

from tc-build.

Did you run my test in #82 (comment) yet?

There is no point in adding an option to the script to turn this off if it does not actually improve ccache hit rate (which I do not think it is going to from the way the ccache documentation is worded).

In fact, it is potentially developer hostile since a user might flip this option then want to report a bug and not be able to tell us what version of LLVM they are using.

from tc-build.

I am at step #4 - this will take some time.

from tc-build.

No, this is not a ccache-speedup build :-(.

from tc-build.

Patch from llvm-project Git applies cleanly against release/10.x Git branch.

[1] https://github.com/llvm/llvm-project/commit/fb5fafb23cc2d8613f8be2487afe94d8594a88ce.patch

UPDATE: Undefs and Versions OK.

from tc-build.

Yeah I did a test with the below script, which builds two identical versions of clang (as evidenced by the output of sha256sum) then builds kernels with them.

============================
==  Compare clang sha256  ==
============================

4ca7c8bf8315634670828a6ce3de2ea7686f1ddc7ebddfe6a99d79492c5d0f21  install-1/bin/clang
4ca7c8bf8315634670828a6ce3de2ea7686f1ddc7ebddfe6a99d79492c5d0f21  install-2/bin/clang

============================================
==  Building x86_64 kernel with clang #1  ==
============================================

Statistics zeroed
cache hit rate                      1.26 %

============================================
==  Building x86_64 kernel with clang #1  ==
============================================

Statistics zeroed
cache hit rate                     99.89 %

============================================
==  Building x86_64 kernel with clang #2  ==
============================================

Statistics zeroed
cache hit rate                      1.26 %

If this config alone matters, I would have expected the last build to have a similar cache hit to the second build, since the toolchains are the exact same and have no version string:

$ ./install-1/bin/clang --version
ClangBuiltLinux clang version 11.0.0
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /tmp/tmp.bfY8JE5Nxb/./install-1/bin

$ ./install-2/bin/clang --version
ClangBuiltLinux clang version 11.0.0
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /tmp/tmp.bfY8JE5Nxb/./install-2/bin

However, it obviously does not.

Again by default, ccache hashes the mtime and the size of the compiler binary to make sure that it is pulling actual cache hits out. If you rebuild your compiler, even if it is the exact same binary, it looks like you have completely changed the compiler to ccache.

My solution to this whole conundrum is simple: Expect that rebuilding your compiler is going to invalidate ccache and only do it when you know you have the time to let it rebuild.

Otherwise, you might consider fiddling with the compiler_check setting but you might shoot yourself in the foot and pull a file from the cache when it should be rebuilt since the compiler changed.

#!/usr/bin/env bash

function die() {
    printf "\n\033[01;31m%s\033[0m\n" "${1}"
    exit "${2:-33}"
}

function header() {
    BORDER="====$(for _ in $(seq ${#1}); do printf '='; done)===="
    printf '\033[1m\n%s\n%s\n%s\n\n\033[0m' "${BORDER}" "==  ${1}  ==" "${BORDER}"
}

BASE=$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")
cd "${BASE}" || exit ${?}

[[ -d linux ]] || git clone --depth=1 git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
[[ -d tc-build ]] || git clone git://github.com/ClangBuiltLinux/tc-build

grep -q LLVM_APPEND_VC_REV tc-build/build-llvm.py || \
    sed -i "/LLVM_INCLUDE_EXAMPLES/a\ \ \ \ \ \ \ \ 'LLVM_APPEND_VC_REV': 'OFF'," tc-build/build-llvm.py

# Build two identical toolchains
for NUM in $(seq 2); do
    ./tc-build/build-llvm.py --build-stage1-only \
                             --install-folder "${BASE}/install-${NUM}" \
                             --install-stage1-only \
                             --no-ccache \
                             --no-update \
                             --projects "clang;lld" \
                             --shallow-clone \
                             --targets X86 || die "Building LLVM failed"
done

# Compilers should be identical
header "Compare clang sha256"
sha256sum install-{1,2}/bin/clang

# Don't mess with the system's ccache
export CCACHE_DIR=${BASE}/ccache

# Build three kernels
#   1. With compiler #1 to fill ccache (should be a low/zero hit rate)
#   2. With compiler #1 to make sure that ccache works (should be a high hit rate)
#   3. With compiler #2 to see if ccache hits (should not by default)
for NUM in 1 1 2; do
    header "Building x86_64 kernel with clang #${NUM}"
    rm -rf linux/out/x86_64
    ccache -z
    PATH=${BASE}/install-${NUM}/bin:${PATH} \
        make -C linux \
             -j"$(nproc)" \
             LLVM=1 \
             CC="ccache clang" \
             HOSTCC="ccache clang" \
             HOSTCXX="ccache clang++" \
             O=out/x86_64 \
             defconfig all &>/dev/null || die "Building kernel failed!"
    ccache -s | grep "cache hit rate"
done

TL;DR: I do not think that adding a flag for this define should be done.

from tc-build.

Wow! I am very impressed of your analysis, speed of testing and providing use-/test-cases here.

Thanks for your precious time @nathanchance.

The conclusion is simple - on each rebuild of my llvm-toolchain/compiler (even with same code base - nothing new was changed in Git) - ccache is smart enough to detect this change.
Which is IMHO the correct behaviour and the documentation of ccache is verified - Q.E.D. (Quad Erat Demonstrandum).

That's why I did not often change my llvm-toolchain and try to stay on stable releases like release/10.x.

Quote from [1]:

Time is your most precious gift because you only have a set amount of
it. You can make more money, but you can't make more time. When you
give someone your time, you are giving them a portion of your life
that you'll never get back. Your time is your life. That is why the
greatest gift you can give someone is your time.
It is not enough to just say relationships are important; we must
prove it by investing time in them. Words alone are worthless. "My
children, our love should not be just words and talk; it must be true
love, which shows itself in action." Relationships take time and
effort, and the best way to spell love is "T-I-M-E.

― Rick Warren, The Purpose Driven Life: What on Earth Am I Here for?

[1] http://www.goodreads.com/quotes/234195-time-is-your-most-precious-gift-because-you-only-have

[2] https://www.facebook.com/akianeart?fref=ts

UPDATE: Add link to JPEG file

from tc-build.

This should be closed then I suppose?

from tc-build.

@msfjarvis
YES, you can :-).

from tc-build.

UPDATE 28-May-2020:

Bug 46090 - Add "Make LLVM_APPEND_VC_REV=OFF affect > clang, lld, and lldb as well." for 10.0.1 release

[1] https://bugs.llvm.org/show_bug.cgi?id=46090

from tc-build.

UPDATE 24-Jun-2020:

Patch is now in release/10.x Git branch.

[1] llvm/llvm-project@0777c90

from tc-build.