On 29/08/2024 3:07 pm, Jan Beulich wrote: > On 29.08.2024 00:03, Andrew Cooper wrote: >> All of the ffs()/fls() infrastructure is in fact (attr) const, because it >> doesn't even read global state. This allows the compiler even more >> flexibility to optimise. >> >> No functional change. >> >> Reported-by: Jan Beulich <jbeul...@suse.com> >> Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com> >> --- >> CC: Jan Beulich <jbeul...@suse.com> >> CC: Roger Pau Monné <roger....@citrix.com> >> CC: Stefano Stabellini <sstabell...@kernel.org> >> CC: Julien Grall <jul...@xen.org> >> CC: Volodymyr Babchuk <volodymyr_babc...@epam.com> >> CC: Bertrand Marquis <bertrand.marq...@arm.com> >> CC: Michal Orzel <michal.or...@amd.com> >> CC: Oleksii Kurochko <oleksii.kuroc...@gmail.com> >> CC: Shawn Anastasio <sanasta...@raptorengineering.com> >> >> v2: >> * New >> --- >> xen/include/xen/bitops.h | 16 ++++++++-------- >> 1 file changed, 8 insertions(+), 8 deletions(-) > The various arch_* forms didn't have __pure and hence also don't gain > attr_const presumably because we deem these attributes ineffectual for > always-inline functions? On that basis > Reviewed-by: Jan Beulich <jbeul...@suse.com>
It's not quite that. The non static-inline ones definitely do need the attribute. That's the only thing the optimiser has to operate with. The static inlines shouldn't need it for GCC's benefit. GCC will apparently (according to buzilla) silently drop the attribute if it believes it to have been erroneous. However, Eclair does care about the attributes even on static-inlines as part of it's side-effects analysis for various rules. Therefore I've been putting the attributes on the APIs we expect code to be using, but not on the arch_* infrastructure. Whether this is the right balance remains to be seen. ~Andrew
