> -----Original Message----- > From: Alice Carlotti <[email protected]> > Sent: 11 November 2025 19:08 > To: Tamar Christina <[email protected]> > Cc: [email protected]; Richard Earnshaw > <[email protected]>; Kyrylo Tkachov <[email protected]>; Alex > Coplan <[email protected]>; Andrew Pinski > <[email protected]>; Wilco Dijkstra > <[email protected]>; Alfie Richards <[email protected]> > Subject: Re: [PATCH] aarch64: Extend syntax for cpuinfo feature string checks > > 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. >
Sure, you have the shortcut on | but now lose it on " " which is the majority of systems running today. I don't think it's that hard to have keep the old " " shortcircuiting so don't quite see a reason to drop it. Thanks, Tamar > Alice > > > > > > > > > if (enabled) > > > extension_flags |= aarch64_extensions[i].flag;
