Hi Thomas, > -----Original Message----- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Thomas Monjalon > Sent: Tuesday, June 24, 2014 11:06 AM > To: Chen, Jing D > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH] i40e: fix shared code compile warning > > 2014-06-24 09:47, Chen, Jing D: > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > > > 2014-06-24 13:22, Chen Jing D: > > > > +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 > >
Actually, I suppose there is a way to fix that issue without modifying shared code. As I know Pablo plans to prepare another patch to deal with it in a proper way. > > 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_i40 > > e/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_i40 > > e/i40e/i40e_lan_hmc.c: In function ?i40e_read_qword?: > > /jenkins/workspace/DPDK_AUTO_IDT_VM_RHEL65_32_BUILD/DPDK/lib/librte_pmd_i40 > > e/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) > > Maybe I don't understand. You are saying you can fix the compiler warning by > adding ULL to the constant. This is a simple patch and is a lot nicer than > CFLAGS_i40e_lan_hmc.o += -Wno-error > Even if the asm code seems right, it would be more secure to remove this > warning. > Yes, it is much nicer to fix it in i40e_lan_hmc.c. But I don't really want us to open that door. So my vote would be to initial Mark's patch: add '-Wno-error' in the Makefile. Konstantin
