-
Notifications
You must be signed in to change notification settings - Fork 15.2k
benchmarks: Fix sample_symbol_list.txt generation again #167078
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
benchmarks: Fix sample_symbol_list.txt generation again #167078
Conversation
llvm/benchmarks/CMakeLists.txt
Outdated
| # Extract the list of symbols in a random utility as sample data. | ||
| set(SYMBOL_TEST_DATA_FILE "sample_symbol_list.txt") | ||
| set(SYMBOL_TEST_DATA_SOURCE_BINARY ${llc_exe}) | ||
| if(TARGET ${llc_target}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since last review I learned that if argument shouldn't use ${} syntax, otherwice it may be expanded twice (if the first expansion yields a name of existing cmake variable). This requires the variable to always have some value though, otherwise it willbe treated as target name.
Perhaps if (TARGET "${llc_target}" AND TARGET "${llvm_nm_target}") would also work.
This wasn't triggering on a basic build. The various target checks seem to not work as I expect with AND, so split this into separate if TARGET checks.
841582e to
d66b4a9
Compare
s-barannikov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM as long as it works
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/169/builds/16844 Here is the relevant piece of the build log for the reference |
This wasn't triggering on a basic build
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/127/builds/5239 Here is the relevant piece of the build log for the reference |

This wasn't triggering on a basic build. The various target checks
seem to not work as I expect with AND, so split this into separate
if TARGET checks.