On Thu, 22 Jan 2026, Alex Coplan wrote:
> This patch makes the constructor of bbitmap explicit and deals with the
> fallout throughout the codebase.
>
> The goal of this is to avoid the re-occurrence of bugs such as PR123206.
> In particular, it prevents implicit conversions from other types
> (integers, booleans, pointers (!), etc.) in bbitmap context; instead one
> must directly construct a bbitmap to use it in such a context.
>
> Doing this revealed a bug in bbitmap: the destructive bitwise operators
> (|=, &=, ^=) all returned this (a pointer) which was being implicitly
> converted to a bbitmap. To verify this, I added some selftests for
> these operators which indeed failed. Of course this latent bug becomes
> a compile error when the ctor is marked explicit, which is much nicer.
> This patch fixes that bug.
>
> To see another benefit of this change, consider PR123206 on AArch64. The
> previous patch marks the aarch64_pragma_builtins table as constexpr,
> which gives errors such as this in the presence of the bug:
>
> aarch64-builtins.cc:1736:1: error: the value of ‘global_options’ is not
> usable in a constant expression
>
> but with this change, we get clearer error messages which give more of
> the context:
>
> $GCC/gcc/config/aarch64/aarch64.h:242:39: error: cannot convert ‘bool’ to
> ‘aarch64_feature_flags’ {aka ‘bbitmap<2>’}
> 242 | #define TARGET_SIMD (TARGET_BASE_SIMD && TARGET_NON_STREAMING)
> | ~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~
> | |
> | bool
> $GCC/gcc/config/aarch64/aarch64-simd-pragma-builtins.def:200:48: note: in
> expansion of macro ‘TARGET_SIMD’
> 200 | #define REQUIRED_EXTENSIONS nonstreaming_only (TARGET_SIMD)
> | ^~~~~~~~~~~
> $GCC/gcc/config/aarch64/aarch64-builtins.cc:1720:33: note: in expansion of
> macro ‘REQUIRED_EXTENSIONS’
> 1720 | aarch64_required_extensions::REQUIRED_EXTENSIONS, FLAG_##F},
> | ^~~~~~~~~~~~~~~~~~~
> $GCC/gcc/config/aarch64/aarch64-simd-pragma-builtins.def:51:3: note: in
> expansion of macro ‘ENTRY’
> 51 | ENTRY (N, ternary, T0, T1, T2, T3, U, F)
> | ^~~~~
> $GCC/gcc/config/aarch64/aarch64-simd-pragma-builtins.def:201:1: note: in
> expansion of macro ‘ENTRY_TERNARY’
> 201 | ENTRY_TERNARY (vbsl_mf8, mf8, u8, mf8, mf8, UNSPEC_BSL, QUIET)
> | ^~~~~~~~~~~~~
>
> The main downside of this patch is that it makes usage of
> aarch64_feature_flags slightly less ergonomic; contexts that previously
> passed an integer constant 0 now need an object of type
> aarch64_feature_flags. I've defined a macro AARCH64_FL_NONE to slightly
> reduce the amount of typing needed (compared to aarch64_feature_flags (0)),
> but it does make some of the code a little more verbose.
>
> IMO the trade-off is worth it to avoid further mishaps such as the PR in
> question (and the buggy bbitmap operators), but I would like to hear
> others' thoughts on it, too.
>
> Bootstrapped/regtested on aarch64-linux-gnu, OK for trunk?
The middle-end parts look OK. I supose it would be nice to split
them out in case backporting is required separately from the aarch64
changes (but I didn't know about bbitmap, so maybe it's only used
by aarch64).
Richard.
> I'm happy to split this up or defer completely until next stage 1, if
> folks prefer.
>
> Thanks,
> Alex
>
> gcc/ChangeLog:
>
> PR target/123206
> * Makefile.in (OBJS): Add new bbitmap-selftest.o.
> * bbitmap.h (bbitmap): Make ctor explict, fix destructive operators to
> dereference 'this' on return.
> * common/config/aarch64/aarch64-common.cc (all_extensions): Replace
> uses of
> literal 0 with AARCH64_FL_NONE.
> (all_architectures): Likewise.
> (all_cores): Likewise.
> (aarch64_get_extension_string_for_isa_flags): Likewise, and also switch
> from
> using assignment to braced initialization for aarch64_feature_flags.
> * config/aarch64/aarch64-builtins.cc
> (aarch64_check_required_extensions):
> Likewise.
> (aarch64_general_required_extensions): Likewise, and avoid comparing
> against literal 0.
> * config/aarch64/aarch64-feature-deps.h (get_flags): Use AARCH64_FL_NONE
> instead of literal 0.
> (get_enable): Likewise.
> (alias_prefer_over_flags_##IDENT): Likewise.
> * config/aarch64/aarch64-protos.h (struct aarch64_target_switcher):
> Likewise.
> (struct aarch64_required_extensions): Likewise.
> * config/aarch64/aarch64-simd-pragma-builtins.def: Likewise.
> * config/aarch64/aarch64-sve-builtins-base.def: Likewise.
> * config/aarch64/aarch64-sve-builtins-sme.def: Likewise.
> * config/aarch64/aarch64-sve-builtins-sve2.def: Likewise.
> * config/aarch64/aarch64-sve-builtins.cc (DEF_NEON_SVE_FUNCTION):
> Likewise.
> (function_builder::get_attributes): Don't compare aarch64_feature_flags
> values
> against literal 0.
> * config/aarch64/aarch64-sve-builtins.h
> (function_expander::get_contiguous_base): Replace 0 with
> AARCH64_FL_NONE.
> * config/aarch64/aarch64.cc (all_cores): Likewise.
> (aarch64_override_options): Likewise, plus use of braced initialization
> instead of assignment.
> (aarch64_process_target_attr): Use braced initialization for
> isa_temp.
> (aarch64_valid_sysreg_name_p): Avoid comparison of
> aarch64_feature_flags against literal 0.
> (aarch64_retrieve_sysreg): Likewise.
> * config/aarch64/aarch64.h (AARCH64_FL_NONE): New.
> (TARGET_STREAMING_COMPATIBLE): Avoid comparison of
> aarch64_feature_flags value against literal 0.
> * config/aarch64/driver-aarch64.cc (aarch64_cpu_data): Replace
> use of 0 with AARCH64_FL_NONE.
> (aarch64_arches): Likewise.
> (host_detect_local_cpu): Use braced initialization for
> aarch64_feature_flags.
> * selftest-run-tests.cc (selftest::run_tests): Add bbitmap_cc_tests.
> * selftest.h (bbitmap_cc_tests): New.
> * bbitmap-selftests.cc: New file.
> ---
> gcc/Makefile.in | 1 +
> gcc/bbitmap-selftests.cc | 69 +++++++++++++++++++
> gcc/bbitmap.h | 8 +--
> gcc/common/config/aarch64/aarch64-common.cc | 16 +++--
> gcc/config/aarch64/aarch64-builtins.cc | 12 ++--
> gcc/config/aarch64/aarch64-feature-deps.h | 7 +-
> gcc/config/aarch64/aarch64-protos.h | 10 +--
> .../aarch64/aarch64-simd-pragma-builtins.def | 2 +-
> .../aarch64/aarch64-sve-builtins-base.def | 4 +-
> .../aarch64/aarch64-sve-builtins-sme.def | 4 +-
> .../aarch64/aarch64-sve-builtins-sve2.def | 4 +-
> gcc/config/aarch64/aarch64-sve-builtins.cc | 8 +--
> gcc/config/aarch64/aarch64-sve-builtins.h | 2 +-
> gcc/config/aarch64/aarch64.cc | 18 ++---
> gcc/config/aarch64/aarch64.h | 4 +-
> gcc/config/aarch64/driver-aarch64.cc | 10 +--
> gcc/selftest-run-tests.cc | 1 +
> gcc/selftest.h | 1 +
> 18 files changed, 129 insertions(+), 52 deletions(-)
> create mode 100644 gcc/bbitmap-selftests.cc
>
>
--
Richard Biener <[email protected]>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
