Re: [dpdk-dev] [PATCH] i40e: fix shared code compile warning

["Chen, Jing D" <jing.d.chen@intel.com>]

Hi Thomas, 

-----Original Message-----
From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] 
Sent: Tuesday, June 24, 2014 4:47 PM
To: Chen, Jing D
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH] i40e: fix shared code compile warning

Hi,

2014-06-24 13:22, Chen Jing D:
> Fix a compile warning in shared code on 32-bits RHEL6.3/6.5.
> 
> Signed-off-by: Chen Jing D(Mark) <jing.d.chen@intel.com>

In such case you should show the error message in the log.
	lib/librte_pmd_i40e/i40e/i40e_lan_hmc.c:917:
	error: integer constant is too large for ‘long’ type

> --- a/lib/librte_pmd_i40e/Makefile
> +++ b/lib/librte_pmd_i40e/Makefile
> @@ -52,6 +52,7 @@ CFLAGS_SHARED_DRIVERS += 
> -Wno-missing-field-initializers  CFLAGS_SHARED_DRIVERS += 
> -Wno-pointer-to-int-cast  CFLAGS_SHARED_DRIVERS += 
> -Wno-format-nonliteral  CFLAGS_SHARED_DRIVERS += -Wno-format-security
> +CFLAGS_i40e_lan_hmc.o += -Wno-error

I know we shouldn't modify base drivers. But this one seems to be an important error. In such case, we already modified base driver. Recently:
	http://dpdk.org/ml/archives/dev/2014-June/003498.html

I think it's different. The logic is right after adding the patch. Below is my finding. 

In this case, it met the error when compile on 32-bits OS. The message is :

/jenkins/workspace/DPDK_AUTO_IDT_VM_RHEL65_32_BUILD/DPDK/lib/librte_pmd_i40e/i40e/i40e_lan_hmc.c: In function ‘i40e_write_qword’:
/jenkins/workspace/DPDK_AUTO_IDT_VM_RHEL65_32_BUILD/DPDK/lib/librte_pmd_i40e/i40e/i40e_lan_hmc.c:917: error: integer constant is too large for ‘long’ type
/jenkins/workspace/DPDK_AUTO_IDT_VM_RHEL65_32_BUILD/DPDK/lib/librte_pmd_i40e/i40e/i40e_lan_hmc.c: In function ‘i40e_read_qword’:
/jenkins/workspace/DPDK_AUTO_IDT_VM_RHEL65_32_BUILD/DPDK/lib/librte_pmd_i40e/i40e/i40e_lan_hmc.c:1097: error: integer constant is too large for ‘long’ type

I found the code that cause errors. 'mask' is 'uint64_t' type and is assigned to value 0Xffff_ffff_ffff_ffff. Compiler assumes the constant is 'int' type by default. If changed it to oxffff_ffff_ffff_ffffULL, the warning will be gone.

	if (ce_info->width < 64)
		mask = ((u64)1 << ce_info->width) - 1;
	else
		mask = 0xFFFFFFFFFFFFFFFF;

besides that, I dis-assembler the code with the patch and get below segment. It seems right. 

        if (ce_info->width < 64)
    1946:       8b 45 0c                mov    0xc(%ebp),%eax
    1949:       0f b7 40 04             movzwl 0x4(%eax),%eax
    194d:       66 83 f8 3f             cmp    $0x3f,%ax
    1951:       77 30                   ja     1983 <i40e_write_qword+0x62>
                mask = ((u64)1 << ce_info->width) - 1;
    1953:       8b 45 0c                mov    0xc(%ebp),%eax
    1956:       0f b7 40 04             movzwl 0x4(%eax),%eax
    195a:       0f b7 c8                movzwl %ax,%ecx
    195d:       b8 01 00 00 00          mov    $0x1,%eax
    1962:       ba 00 00 00 00          mov    $0x0,%edx
    1967:       0f a5 c2                shld   %cl,%eax,%edx
    196a:       d3 e0                   shl    %cl,%eax
    196c:       f6 c1 20                test   $0x20,%cl
    196f:       74 04                   je     1975 <i40e_write_qword+0x54>
    1971:       89 c2                   mov    %eax,%edx
    1973:       31 c0                   xor    %eax,%eax
    1975:       83 c0 ff                add    $0xffffffff,%eax
    1978:       83 d2 ff                adc    $0xffffffff,%edx
    197b:       89 45 e0                mov    %eax,-0x20(%ebp)
    197e:       89 55 e4                mov    %edx,-0x1c(%ebp)
    1981:       eb 0e                   jmp    1991 <i40e_write_qword+0x70>
        else
                mask = 0xFFFFFFFFFFFFFFFF;
    1983:       c7 45 e0 ff ff ff ff    movl   $0xffffffff,-0x20(%ebp)
    198a:       c7 45 e4 ff ff ff ff    movl   $0xffffffff,-0x1c(%ebp)
--
Thomas