tests/semantic: Add architecture specific testcase support #1641
Conversation
lostjeffle
force-pushed
the
jingbo/dev
branch
2 times, most recently
from
01d9912
to
de7c021
Compare
CHANGELOG.md
Outdated
| @@ -106,6 +106,8 @@ and this project adheres to | |||
| - [#1580](https://github.com/iovisor/bpftrace/pull/1580) | |||
| - Printing of small integers with `printf` | |||
| - [#1532](https://github.com/iovisor/bpftrace/pull/1532) | |||
| - Add architecture specific testcase support for semantic_analyser | |||
There was a problem hiding this comment.
We only put the enduser facing changes in the changelog, internal test changes are not relevant for them so you can remove this :)
tests/CMakeLists.txt
Outdated
| @@ -2,6 +2,19 @@ add_compile_options("-Wno-undef") | |||
| add_compile_options("-Wno-switch-default") | |||
| add_compile_options("-Wno-switch-enum") | |||
|
|
|||
| # Architecture-specific preprocessor macro for *.cpp/*.c | |||
| if(CMAKE_SYSTEM_PROCESSOR STREQUAL "aarch64") | |||
| add_definitions(-DARCH_AARCH64) | |||
There was a problem hiding this comment.
I think it's a bit nicer to use the target_compile_definitions
| add_definitions(-DARCH_AARCH64) | |
| target_compile_definitions(bpftrace_test PRIVATE ARCH_AARCH64) |
There was a problem hiding this comment.
Actually 'target_compile_definitions' is used in the original patch, but failed with several automatic build test triggered by GitHub, warning that "target_compile_definitions is not defined". Maybe it's related to the version of cmake, since target_compile_definitions is added since cmake3?
I'm not familiar with cmake and really look forward you guys' professional opinion. Thanks.
There was a problem hiding this comment.
Do you have a link to the failed builds? It should work. We use it in a quite a few places and we require cmake 3.something anyway:
tests/CMakeLists.txt
69: target_compile_definitions(bpftrace_test PRIVATE HAVE_LIBBPF_MAP_BATCH)
73: target_compile_definitions(bpftrace_test PRIVATE HAVE_NAME_TO_HANDLE_AT=1)
76: target_compile_definitions(bpftrace_test PRIVATE HAVE_BCC_PROG_LOAD)
79: target_compile_definitions(bpftrace_test PRIVATE HAVE_BCC_CREATE_MAP)
Maybe you placed it before the bpftrace_test target definition? Needs to go on line 69 or later
I'm not familiar with cmake and really look forward you guys' professional opinion. Thanks.
We all struggle with cmake ;)
lostjeffle
force-pushed
the
jingbo/dev
branch
from
de7c021
to
81c0ed8
Compare
commit 48d4051 ("bpftrace: Add architecture specific testcase support") added architecture specific support for runtime tests. Still there are architecture specific testcases in semantic_analyser, where 'ip' register is hard coded in semantic_analyser.call_reg and semantic_analyser.builtin_functions. Besides registers such as 'ip' and 'bp' are also hard codes in some codegen tests. These testcases are all c/c++ files and thus we need conditional compiling. Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
lostjeffle
force-pushed
the
jingbo/dev
branch
from
81c0ed8
to
1945328
Compare
|
thanks! If we add a lot of cases it might be nice to change the expected value instead of just removing the test but this is fine for now :) |
commit 48d4051 ("bpftrace: Add architecture specific testcase
support") added architecture specific support for runtime tests.
Still there are architecture specific testcases in semantic_analyser,
where 'ip' register is hard coded in semantic_analyser.call_reg and
semantic_analyser.builtin_functions.
These testcases are all c/c++ files and thus we need conditional compiling.
Signed-off-by: Jeffle Xu jefflexu@linux.alibaba.com
#1137 had ever noted that some architecture specific test cases are hard coded, and thus will fail in some architectures not supported.
This patch is supposed to fix the failure in semantic_analyser.builtin_functions, semantic_analyser.call_reg.
Checklist
docs/reference_guide.mdCHANGELOG.md