On 6/14/2024 4:27 PM, Stephen Hemminger wrote: > On Fri, 14 Jun 2024 10:01:02 +0100 > Ferruh Yigit <ferruh.yi...@amd.com> wrote: > >> On 6/13/2024 8:13 PM, Stephen Hemminger wrote: >>> On Thu, 13 Jun 2024 17:51:14 +0100 >>> Ferruh Yigit <ferruh.yi...@amd.com> wrote: >>> >>>>> Hi Sivaprasad, >>>>> >>>>> Is this '(lcoreid_t)' cast required? Because of integer promotion I >>>>> think result will be correct without casting. >>>>> >>>>> (And without integer promotion considered, casting needs to be done on >>>>> one of the variables, not to the result, because result may be already >>>>> cast down I think. Anyway this is not required for this case since >>>>> variables are u16.) >>>>> >>>> >>>> Why casing required (for record) is, >>>> 'nb_txq' -> uint16_t, promoted to 'int' >>>> 'nb_fwd_ports' -> uint16_t, promoted to 'int' >>>> (nb_txq * nb_fwd_ports) -> result 'int' >>>> nb_fwd_lcores -> 'uint32_t' >>>> >>>> comparison between 'int' & 'uint32_t' gives warning. After some compiler >>>> version it is smart enough to not give a warning, but casting is >>>> required for old compilers. >>>> >>>> And back to my comment above, casting one of the parameter to >>>> 'lcoreid_t' also works, as it forcing promotion to 'unsigned int' >>>> instead. But logically casting looks odd, so keeping casting result. >>> >>> Where is the integer promotion happening? >>> >> >> Raslan reported following compile error, this version of the patch has >> the cast, but merged version, v3, doesn't. >> >> >> ``` >> ../../root/dpdk/app/test-pmd/config.c: In function 'icmp_echo_config_setup': >> ../../root/dpdk/app/test-pmd/config.c:5159:30: error: comparison between >> signed and unsigned integer expressions [-Werror=sign-compare] >> if ((nb_txq * nb_fwd_ports) < nb_fwd_lcores) > > That does look like a compiler bug. uint16 multiplied by uint16 should be > uint16. >
Probably, this warning goes away with newer version of compiler. > Not sure why DPDK keeps using small types like uint16 so much, doesn't have > any > real benefit since all this is in registers. ^