> From: Tyler Retzlaff [mailto:roret...@linux.microsoft.com] > Sent: Thursday, 2 November 2023 16.28 > > 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,
This will give the correct result, yes. However the 64-bit operation might have a higher performance cost than the 32-bit operation, especially on 32-bit architectures. > in the case of leading i checked the type to make > sure it was already operating on a 64-bit type. If already operating on a 64-bit type, using the 64-bit function is obviously correct. > > too naive?
