Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/19514 )

Change subject: [thirdparty] skip build/generation of LLVM benchmarks
......................................................................


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/19514/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19514/1//COMMIT_MSG@9
PS1, Line 9: turns off generation and build of LLVM
> Just curious - how do you decide whether this is a candidate for which gene
The criterion is simple: everything in 3rdparty that's not needed for Kudu 
functionality in runtime can be turned off.  Usually, the imported components 
are given the simplest/shortest set of customized configure/build lparameters, 
but in most cases there are more knobs to turn on/off to make it more optimal 
to fit the set of 3rd party Kudu components.  Some of them are found later on 
when a non-essential piece shows itself one way or another.

In this particular case, there is no need to benchmark LLVM/CLANG as a part of 
building 3rd-party components that Kudu uses.  Even if the generated benchmarks 
were run, I don't know any automation in Kudu 3rd-party build process that 
would take that into consideration.


http://gerrit.cloudera.org:8080/#/c/19514/1//COMMIT_MSG@13
PS1, Line 13: building LLVM on Ubuntu 22.04
> I started a fresh build (without setting CPATH). I was able to build thirdp
I haven't tried to build Kudu, I just made sure all 3rd-party components were 
built successfully with this patch.

I tried to build Kudu later on, and found that it fails with the error like you 
posted.  The fix is to add libc++ header path, and with that it worked for me.  
The only thing is to extract the path from the output of the compiler when run 
with proper flags.  I can take a look how to do so, but if you have done that 
already, please feel free to post your patch for review.


http://gerrit.cloudera.org:8080/#/c/19514/1//COMMIT_MSG@13
PS1, Line 13: without patching
            : the LLVM code itself
> It seems they have fixed this in later releases of llvm but not in this ver
Upgrading LLVM/CLANG is quite a task on itself: you could see from the git 
history of this project that LLVM/CLANG upgrade is usually quite invasive 
change at least for the codegen part.  So, in most cases it's much simpler to 
have a trivial fix like this, especially if it also saves some build time.

If you have already updated LLVM/CLANG to the latest version and everything 
works as expected, feel free to post the patch for review.



--
To view, visit http://gerrit.cloudera.org:8080/19514
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I67592101c5e105b8a4b36fc231b0e67e4585fb0b
Gerrit-Change-Number: 19514
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Ashwani Raina <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <[email protected]>
Gerrit-Reviewer: Yifan Zhang <[email protected]>
Gerrit-Reviewer: Yingchun Lai <[email protected]>
Gerrit-Comment-Date: Mon, 27 Feb 2023 18:19:54 +0000
Gerrit-HasComments: Yes

Reply via email to