Fix several build issues in CI when using Clang #10800
Conversation
facebook-github-bot
added
the
CLA Signed
✅ Deploy Preview for meta-velox canceled.
|
|
@majetideepak FYI. |
| @@ -59,46 +59,48 @@ target_link_libraries( | |||
|
|
|||
| add_test(velox_wave_exec_test velox_wave_exec_test) | |||
|
|
|||
| add_executable(velox_wave_benchmark WaveBenchmark.cpp) | |||
| if(${VELOX_ENABLE_BENCHMARKS} OR ${VELOX_ENABLE_BENCHMARKS_BASIC}) | |||
There was a problem hiding this comment.
nit: I think if(VELOX_ENABLE_BENCHMARKS) is sufficient here.
There was a problem hiding this comment.
I suppose it isn't a basic benchmark and should only be enabled along the top level benchmarks - the dependency is available then as well. But perhaps we can get a comment about it.
czentgr
force-pushed
the
cz_fix_wave_compile_w_clang
branch
2 times, most recently
from
75219c8
to
bbf8ad8
Compare
majetideepak
added
the
ready-to-merge
|
I know that CUDA/libcudf etc. a much better supported with GCC so potentially switching those parts of when building with clang might be required if hard to solve problems show up. |
|
Yes. We can revisit if problems show up. In this case I believe this was a correct error shown. I read the nvcc also uses a modified LLVM and is similar to Clang actually. So I hope this works out without major problems going forward. |
A number of issues came up when building the CI pipeline with Clang. 1. When CUDA nvcc uses Clang it complains that totalBytes is set but unused. The intent seems to have been that the parameter is an in- and output parameter. 2. If benchmarks are not turned on then the CUDA build fails as dependencies are not available. 3. HDFS virtual function definition not explicitly overridden causing an error (error: 'rename' overrides a member function but is not marked 'override’).
czentgr
force-pushed
the
cz_fix_wave_compile_w_clang
branch
from
bbf8ad8
to
8d63446
Compare
|
@kgpai can you make another pass and help land this? The on-call team likely missed this. Thanks! |
|
@kgpai has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
Conbench analyzed the 1 benchmark run on commit There were no benchmark performance regressions. 🎉 The full Conbench report has more details. |
) Summary: A number of issues came up when building the CI pipeline with Clang. 1. When CUDA nvcc uses Clang it complains that totalBytes is set but unused. �The intent seems to have been that the parameter is an in- and output parameter. 2. If benchmarks are not turned on then the CUDA build fails as dependencies are not available. 3. HDFS virtual function definition not explicitly overridden causing an error �(error: 'rename' overrides a member function but is not marked 'override’). Pull Request resolved: facebookincubator#10800 Reviewed By: DanielHunte Differential Revision: D61862435 Pulled By: kgpai fbshipit-source-id: 3aef27e8ed1c4dd8fe78221c5dd859656fc65e69
) Summary: A number of issues came up when building the CI pipeline with Clang. 1. When CUDA nvcc uses Clang it complains that totalBytes is set but unused. �The intent seems to have been that the parameter is an in- and output parameter. 2. If benchmarks are not turned on then the CUDA build fails as dependencies are not available. 3. HDFS virtual function definition not explicitly overridden causing an error �(error: 'rename' overrides a member function but is not marked 'override’). Pull Request resolved: facebookincubator#10800 Reviewed By: DanielHunte Differential Revision: D61862435 Pulled By: kgpai fbshipit-source-id: 3aef27e8ed1c4dd8fe78221c5dd859656fc65e69
A number of issues came up when building the CI pipeline with Clang.