On Thu, Nov 02, 2023 at 08:39:04AM +0100, Morten Brørup wrote: > > From: Tyler Retzlaff [mailto:roret...@linux.microsoft.com] > > Sent: Thursday, 2 November 2023 02.05 > > > > The first set of conversions missed the long 'l' versions of the > > builtins that were being used. This series completes the conversion > > of remaining libraries from __builtin_ctzl and __builtin_clzl. > > NAK to blind search/replace of __builtin_clzl()/clzl(). > > Although the size of long is 64 bit on 64 bit architectures, it only 32 bit > on 32 bit architectures. > > You need to look at the types these builtins operate on: > - E.g. in the hash library (patch 3/5) prim_hitmask[i]/sec_hitmask[i] are > uint32_t, so rte_ctz32() would be the correct replacement. (I am now asking > myself why they were using __builtin_ctzl() instead of __builtin_ctz() > here... Probably by mistake.) > - And if the type is "long", you need conditional compiling (or a wrapper > macro) to choose between the 32 bit or 64 bit variants. > > NB: You can blindly replace __builtin_ctzll()/clzll(), if any, by 64 bit > functions.
they haven't been blindly replaced. but i would like you to validate my thinking. in the case of counting trailing 0s it seems fine if the type is promoted to 64-bits, in the case of leading i checked the type to make sure it was already operating on a 64-bit type. too naive?
