Skip to content

Correct the default Bazel vector instructions options#782

Merged
mhucka merged 8 commits into
masterfrom
mh-more-bazel-changes
Jun 19, 2025
Merged

Correct the default Bazel vector instructions options#782
mhucka merged 8 commits into
masterfrom
mh-more-bazel-changes

Conversation

@mhucka
Copy link
Copy Markdown
Collaborator

@mhucka mhucka commented Jun 18, 2025

The documented way that bazel build, bazel test, and similar commands behave without options like --config=avx is that the AVX, SSE, and other optimization options are not applied. However, this was not what was happening: the default ended up applying them even if --config=sse etc. were not used. Evidently, the previous changes to .bazelrc and dev_tools/test_libs.sh that I did in PR #758 and other recent PRs not correct this.

The changes here invert the way --build_tag_filters and --test_tag_filters are used: they are set to filter out things tagged with avx and sse by default in .bazelrc, and options like --config=sse add the things back.

To make this work, dev_tools/test_libs.sh has to be updated as well.

The documented way that `bazel build`, `bazel test`, and similar
commands behave without options like `--config=avx` is that the AVX,
SSE, and other optimization options are not applied. However, this was
not what was happening: the default ended up applying them. The
previous changes to `.bazelrc` and `dev_tools/test_libs.sh` that I did
in PR #758 and other recent PRs not fully address this.

The changes here invert the way `--build_tag_filters` and
`--test_tag_filters` are used: they are set to filter out things
tagged with `avx` and `sse` by default in `.bazelrc`, and options like
`--config=sse` add the things back.

To make this work, `dev_tools/test_libs.sh` has to be updated as well.
@mhucka mhucka force-pushed the mh-more-bazel-changes branch from 6daef69 to 3f35e62 Compare June 18, 2025 18:11
mhucka added 2 commits June 18, 2025 18:15
The documented way that `bazel build`, `bazel test`, and similar
commands behave without options like `--config=avx` is that the AVX,
SSE, and other optimization options are not applied. However, this was
not what was happening: the default ended up applying them. The
previous changes to `.bazelrc` and `dev_tools/test_libs.sh` that I did
in PR #758 and other recent PRs not fully address this.

The changes here invert the way `--build_tag_filters` and
`--test_tag_filters` are used: they are set to filter out things
tagged with `avx` and `sse` by default in `.bazelrc`, and options like
`--config=sse` add the things back.

To make this work, `dev_tools/test_libs.sh` has to be updated as well.
@mhucka mhucka force-pushed the mh-more-bazel-changes branch from 3f35e62 to 0ce15cf Compare June 18, 2025 18:15
@mhucka mhucka marked this pull request as ready for review June 18, 2025 18:48
@mhucka mhucka enabled auto-merge (squash) June 18, 2025 19:00
Comment thread .bazelrc Outdated
Comment thread dev_tools/test_libs.sh
Comment on lines +35 to +36
[[ "$features" == *avx2* ]] && filters+=",avx"
[[ "$features" == *sse* ]] && filters+=",sse"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This runs into the same problem as in #710 - the shell has the -e option so the script will exit here if any of avx2 or sse features were missing. To have a positive condition, you need to rewrite this as

if [[ "$features" == *avx2* ]]; then
    filters+=",avx"
fi
...

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, I tested the following, and it does not exit early. Do you have an environment where it doesn't work this way?

 #!/usr/bin/env bash

 set -eo pipefail -o errtrace

 declare foo=""
 declare bar="original"

 foo="something"
 [[ "$foo" == *notthere* ]] && bar="new string"

 echo $bar

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I stand corrected - per bash set -e doc for the && and || lists it is only the last command that counts for errexit. I got confused, because on my Debian box it indeed exits early in an interactive session. This happens because interactive bash runs some fancy prompt functions which try to restore the exit status of the last command and that triggers errexit.

With your snippet (and the same system bashrc) this can be reproduced with

bash -i <snippet.bash

TLDR - it is OK as is.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, that's unobvious. Thanks for the info!

Copy link
Copy Markdown
Collaborator

@pavoljuhas pavoljuhas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test_libs.sh may exit too early, please correct.

Comment thread dev_tools/test_libs.sh Outdated
mhucka added 3 commits June 18, 2025 21:23
It's not clear if it's essential to use =, but it seems more likely to
avoid some argument-splitting issues, so let's use that consistently.
On lines 48-49: if the first bazel build invocation succeeds, then the
second is pointless because it's _more_ likely to succeed than the
first. If the first invocation fails, we won't get to the second one
anyway.
@mhucka mhucka disabled auto-merge June 19, 2025 04:40
@mhucka mhucka merged commit bdf48f1 into master Jun 19, 2025
48 checks passed
@mhucka mhucka deleted the mh-more-bazel-changes branch June 19, 2025 04:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants