Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/9898 )
Change subject: Upgrade to LLVM/Clang 6.0.0 ...................................................................... Patch Set 2: (7 comments) http://gerrit.cloudera.org:8080/#/c/9898/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/9898/1//COMMIT_MSG@58 PS1, Line 58: by CodegenTest.TestDumpMC, which matches the results with LLVM 4. Remind me why this is important? Helps with debugging? http://gerrit.cloudera.org:8080/#/c/9898/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/9898/2//COMMIT_MSG@38 PS2, Line 38: /usr/bin/ld: cannot find -lc++ Probably because there's no -L<...> telling the linker where to find libc++. But I wonder why it's missing. Can you look at the generated makefiles for LLVMHello vs. for other LLVM targets and see if it's explicitly missing? http://gerrit.cloudera.org:8080/#/c/9898/2/thirdparty/build-definitions.sh File thirdparty/build-definitions.sh: http://gerrit.cloudera.org:8080/#/c/9898/2/thirdparty/build-definitions.sh@182 PS2, Line 182: # Disable some builds we don't care about. Wow, that's dedication. I assume it's sorted alphabetically? :) http://gerrit.cloudera.org:8080/#/c/9898/2/thirdparty/build-definitions.sh@270 PS2, Line 270: TOOLS_ARGS="$TOOLS_ARGS -DGCC_INSTALL_PREFIX=$GCC_INSTALL_PREFIX" Is there a use case for building LLVM in the 'normal' way using LLVM? I imagine that's the case on macOS; does this still work then? http://gerrit.cloudera.org:8080/#/c/9898/2/thirdparty/build-definitions.sh@319 PS2, Line 319: -DCMAKE_EXE_LINKER_FLAGS="$EXTRA_LDFLAGS" \ : -DCMAKE_SHARED_LINKER_FLAGS="$EXTRA_LDFLAGS" \ I wonder if this change caused LLVMHello to fail to link. Maybe it doesn't use CMAKE_EXE_LINKER_FLAGS but does use CMAKE_CXX_FLAGS? http://gerrit.cloudera.org:8080/#/c/9898/2/thirdparty/build-thirdparty.sh File thirdparty/build-thirdparty.sh: http://gerrit.cloudera.org:8080/#/c/9898/2/thirdparty/build-thirdparty.sh@416 PS2, Line 416: EXTRA_CXXFLAGS="-fsanitize=thread $EXTRA_CXXFLAGS" Isn't this already done on L483? Why do we need to specify it here if it'll only be passed into C dependency builds? FWIW I think this was omitted intentionally because libcxxabi doesn't support sanitizing and TSAN is enabled for libcxx in a different way. http://gerrit.cloudera.org:8080/#/c/9898/2/thirdparty/patches/libunwind-trace-cache-destructor.patch File thirdparty/patches/libunwind-trace-cache-destructor.patch: http://gerrit.cloudera.org:8080/#/c/9898/2/thirdparty/patches/libunwind-trace-cache-destructor.patch@7 PS2, Line 7: Freeing a trace cache requires acquiring a mutex, and when running in : ThreadSanitizer, TSAN itself unhooks from a thread in the last : destructor iteration. So, if we try to free the trace cache in the last : destructor iteration, we end up crashing TSAN. Not quite understanding this. Is the implication that it's illegal to unhook TSAN while holding a mutex? -- To view, visit http://gerrit.cloudera.org:8080/9898 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I137180f0b167872d903d855282607bac17f92a83 Gerrit-Change-Number: 9898 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Tue, 03 Apr 2018 17:57:11 +0000 Gerrit-HasComments: Yes
