On Thu, Nov 02, 2023 at 04:33:57PM +0100, Morten Brørup wrote: > > 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.
true. okay let me clean this up. thanks for the feedback. > > > 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?
