> -----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 > > if (enabled) > extension_flags |= aarch64_extensions[i].flag;
