Re: [dpdk-dev] [PATCH] lpm: fix build error on g++ with -O0 option

[Bruce Richardson <bruce.richardson@intel.com>]

On Thu, Jun 01, 2017 at 06:01:35AM +0000, Sangjin Han wrote:
> When rte_lpm.h is used on x86, -O0 option (no optimization at all)
> given to g++ (not gcc) causes a compile error like this:
> 
> error: the last argument must be an 8-bit immediate
>    i24 = _mm_srli_si128(i24, sizeof(uint64_t));
> 
> -O0 option is useful for debugging and code coverage measurement, but
> this error prevents C++ programs from building. This patch replaces
> "sizeof(uint64_t)" with a constant literal "8" to work around the issue.
> Tested with g++ 5.4.1.
> 
> Signed-off-by: Sangjin Han <sangjin@eecs.berkeley.edu>
> ---
>  lib/librte_lpm/rte_lpm_sse.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/librte_lpm/rte_lpm_sse.h b/lib/librte_lpm/rte_lpm_sse.h
> index ef33c6a..2e17df3 100644
> --- a/lib/librte_lpm/rte_lpm_sse.h
> +++ b/lib/librte_lpm/rte_lpm_sse.h
> @@ -78,7 +78,7 @@ rte_lpm_lookupx4(const struct rte_lpm *lpm, xmm_t ip, uint32_t hop[4],
>  
>  	/* extract values from tbl24[] */
>  	idx = _mm_cvtsi128_si64(i24);
> -	i24 = _mm_srli_si128(i24, sizeof(uint64_t));
> +	i24 = _mm_srli_si128(i24, 8);
>  

I will admit that I don't really like this fix, as it reduces the
comprehensibility of the code. It seems more like a compiler bug than an
issue with the code itself.

That being said, since it does actually cause problems for end users, we
need to fix it, so this patch should be merged. However, I think we
should have a comment explaining where the "8" comes from and how the
hard-coded value is used to work around the compilation error. If
possible, please also note what compiler versions are affected, so that
if we ever drop support for those versions, we can change the 8 back to
a sizeof().

With suitable comment added:
Acked-by: Bruce Richardson <bruce.richardson@intel.com>