https://gcc.gnu.org/bugzilla/show_bug.cgi?id=123206
--- Comment #4 from Alex Coplan <acoplan at gcc dot gnu.org> --- (In reply to Alice Carlotti from comment #3) > > 1. Make aarch64-builtins.cc:aarch64_pragma_builtins declared constexpr. > > This would prevent uses of global_options via TARGET_* macros. > > That sounds like reasonable improvement. Yeah, at least with the current setup it seems like an easy change that would immediately prevent this sort of problem. > > However, I'll note that we might some day need to support more flexible > gating here. I don't think this has come up (architecturally) in SIMD yet, > but there are multiple cases in SVE/SME where we ought to support an > intrinsic when either of two different features is enabled. I suppose at the moment aarch64_feature_flags only allows expressing a set of feature flags where all of them are required to match; I guess you're saying we'd also need to be able to express the union of two feature flags/sets in the future? > > > 2. Make the ctor of aarch64_feature_flags (i.e. bbitmap) declared explicit. > > > > I originally wanted to prevent any initialisation from a single int, but > Richard Sandiford suggested we should allow it - see > https://gcc.gnu.org/pipermail/gcc-patches/2024-May/652191.html for the > original discussion. I don't disagree with Richard, I think we should keep the behaviour of the current ctor (including allowing construction from a single int), but additionally I think we should mark the current ctor as explicit. That would prevent any implicit conversions from e.g. bool or int to aarch64_feature_flags in a context that expects an instance of aarch64_feature_flags. That would mean e.g. nonstreaming_only (0) or nonstreaming_only (TARGT_SIMD) would become invalid, but nonstreaming_only (aarch64_feature_flags (0)) would be valid: as the name implies, you just have to be explicit about when you want to construct an instance. > > However, I think we can do better. The specific issue we have in this case > is that our constructor allow conversion from a boolean type. In future we > might be able to narrow the range of accepted types using C++20 type > constraints, but that isn't an option now. Instead, we can address the > current issue by adding a more specific constructor and marking it private - > e.g.: > > private: > constexpr bbitmap(bool x) : val{x} {} So I suppose the usual way of doing that sort of thing would be to declare that bool ctor with = delete, but I think making the ctor explicit is a more comprehensive solution; it forces users to be intentional about when they want to convert anything to a set of feature flags. WDYT?
