On Tue, Nov 11, 2025 at 02:04:07PM +0000, Tamar Christina wrote: > > -----Original Message----- > > From: Alice Carlotti <[email protected]> > > Sent: 11 November 2025 02:05 > > To: [email protected] > > Cc: Richard Earnshaw <[email protected]>; Tamar Christina > > <[email protected]>; Kyrylo Tkachov <[email protected]>; Alex > > Coplan <[email protected]>; Andrew Pinski > > <[email protected]>; Wilco Dijkstra > > <[email protected]>; Alfie Richards <[email protected]> > > Subject: [PATCH] aarch64: Extend syntax for cpuinfo feature string checks > > > > Some SVE features in the toolchain need to be enabled when either of two > > different kernel HWCAPS (and corresponding cpuinfo strings) are enabled > > (one for non-streaming mode and one for streaming mode). > > > > Add support for using "|" to separate alternative lists of required > > features. > > > > > > Bootstrapped and regression tested, and tested further by changing two of > > the > > feature string definitions to "sve2 | blah" and "testing123 | fphp asimdhp" > > and > > rerunning the aarch64-cpunative.exp tests. > > > > This change is required for Alfie's new feature flags patch series. > > Ok for master? > > > > > > diff --git a/gcc/config/aarch64/driver-aarch64.cc > > b/gcc/config/aarch64/driver-aarch64.cc > > index > > 0333746ee00422e47a8349fad1127a1abface877..be98c5b5d1bcb33b4492 > > 7e6e365a3a39e46ee203 100644 > > --- a/gcc/config/aarch64/driver-aarch64.cc > > +++ b/gcc/config/aarch64/driver-aarch64.cc > > @@ -368,18 +368,30 @@ host_detect_local_cpu (int argc, const char **argv) > > continue; > > } > > > > + /* This may be a multi-token feature string. We need to match > > + all parts in one of the "|" separated sublists. */ > > bool enabled = true; > > - > > - /* This may be a multi-token feature string. We need > > - to match all parts, which could be in any order. */ > > - std::set<std::string> tokens; > > - split_words (val, tokens); > > - std::set<std::string>::iterator it; > > - > > - /* Iterate till the first feature isn't found or all of them > > - are found. */ > > - for (it = tokens.begin (); enabled && it != tokens.end (); ++it) > > - enabled = enabled && features.count (*it); > > + size_t cur = 0; > > + while (cur < val.length ()) > > + { > > + size_t end = val.find_first_of (" ", cur); > > + if (end == std::string::npos) > > + end = val.length (); > > + std::string word = val.substr (cur, end - cur); > > + cur = end + 1; > > + > > + if (word == "|") > > + { > > + /* If we've matched everything in the current sublist, we > > + can stop now. */ > > + if (enabled) > > + break; > > + /* Otherwise, start again with the next sublist. */ > > + enabled = true; > > + continue; > > + } > > + enabled = enabled && features.count (word); > > + } > > Overall I think this is fine, but you've now lost the short circuiting that > was being > done before where the for loop before would stop as soon as enabled == false. > > Perhaps make this a nested loop instead? Where the outer loop checks for | and > the inner loop scans for the whitespace? HWCAPS strings are getting long so > would be good to keep the short circuit. > > Thanks, > Tamar
We still have some short-circuiting that bypasses the set lookup when enabled is false. I don't think it's worth the added complexity of scanning ahead for "|" just to avoid constructing another short string. The total impact would be constructing at most eight extra short strings in the entire driver invocation, and all of them are present on most modern systems anyway. Alice > > > > > if (enabled) > > extension_flags |= aarch64_extensions[i].flag;
