Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9898 )

Change subject: Upgrade to LLVM/Clang 6.0.0
......................................................................


Patch Set 2:

(6 comments)

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++
After reverting some other bits of the patch I was able to remove the need for 
this


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? :)
nope but it is now.


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 ima
if that's the case then I think the above would end up being an empty string 
and this wouldn't trigger, right? I'll ask a mac user to make sure thirdparty 
build still works on osx before committing.


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
I was able to revert this, I guess it wasn't necessary after fixing the above 
-nostdinc++ thing


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
was able to revert this change


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 unhoo
sort of other way around -- if you try to unlock the mutex from a thread which 
isn't registered in TSAN, it will crash because the thread has no TSAN context. 
I'll clarify.



--
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-Reviewer: Todd Lipcon <[email protected]>
Gerrit-Comment-Date: Tue, 17 Apr 2018 23:04:44 +0000
Gerrit-HasComments: Yes

Reply via email to