* [dpdk-dev] [PATCH] i40e: fix shared code compile warning @ 2014-06-24 5:22 Chen Jing D(Mark) 2014-06-24 8:46 ` Thomas Monjalon 0 siblings, 1 reply; 8+ messages in thread From: Chen Jing D(Mark) @ 2014-06-24 5:22 UTC (permalink / raw) To: dev From: "Chen Jing D(Mark)" <jing.d.chen@intel.com> 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> --- lib/librte_pmd_i40e/Makefile | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/lib/librte_pmd_i40e/Makefile b/lib/librte_pmd_i40e/Makefile index 09f2087..1d9dcb3 100644 --- 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 endif # -- 1.7.7.6 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH] i40e: fix shared code compile warning 2014-06-24 5:22 [dpdk-dev] [PATCH] i40e: fix shared code compile warning Chen Jing D(Mark) @ 2014-06-24 8:46 ` Thomas Monjalon 2014-06-24 9:47 ` Chen, Jing D 0 siblings, 1 reply; 8+ messages in thread From: Thomas Monjalon @ 2014-06-24 8:46 UTC (permalink / raw) To: Chen Jing D(Mark); +Cc: dev 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 -- Thomas ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH] i40e: fix shared code compile warning 2014-06-24 8:46 ` Thomas Monjalon @ 2014-06-24 9:47 ` Chen, Jing D 2014-06-24 10:06 ` Thomas Monjalon 0 siblings, 1 reply; 8+ messages in thread From: Chen, Jing D @ 2014-06-24 9:47 UTC (permalink / raw) To: Thomas Monjalon; +Cc: dev 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH] i40e: fix shared code compile warning 2014-06-24 9:47 ` Chen, Jing D @ 2014-06-24 10:06 ` Thomas Monjalon 2014-06-24 14:43 ` Ananyev, Konstantin 2014-06-25 5:36 ` Chen, Jing D 0 siblings, 2 replies; 8+ messages in thread From: Thomas Monjalon @ 2014-06-24 10:06 UTC (permalink / raw) To: Chen, Jing D; +Cc: dev 2014-06-24 09:47, Chen, Jing D: > From: Thomas Monjalon [mailto:thomas.monjalon@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 > > 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. PS: please try to configure your mailer to add citation marks. -- Thomas ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH] i40e: fix shared code compile warning 2014-06-24 10:06 ` Thomas Monjalon @ 2014-06-24 14:43 ` Ananyev, Konstantin 2014-06-24 15:25 ` Zhang, Helin 2014-06-25 5:36 ` Chen, Jing D 1 sibling, 1 reply; 8+ messages in thread From: Ananyev, Konstantin @ 2014-06-24 14:43 UTC (permalink / raw) To: Thomas Monjalon, Chen, Jing D; +Cc: dev Hi Thomas, > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon > Sent: Tuesday, June 24, 2014 11:06 AM > To: Chen, Jing D > Cc: dev@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@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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH] i40e: fix shared code compile warning 2014-06-24 14:43 ` Ananyev, Konstantin @ 2014-06-24 15:25 ` Zhang, Helin 2014-06-26 12:22 ` Thomas Monjalon 0 siblings, 1 reply; 8+ messages in thread From: Zhang, Helin @ 2014-06-24 15:25 UTC (permalink / raw) To: Ananyev, Konstantin, Thomas Monjalon, Chen, Jing D; +Cc: dev -----Original Message----- From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ananyev, Konstantin Sent: Tuesday, June 24, 2014 10:44 PM To: Thomas Monjalon; Chen, Jing D Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH] i40e: fix shared code compile warning Hi Thomas, > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon > Sent: Tuesday, June 24, 2014 11:06 AM > To: Chen, Jing D > Cc: dev@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@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 --------------------------------------------------------------------------------------------------------------------- Hi guys We should not modify code in shared code, if we do not want to maintain those huge code base. I think Mark's patch good for now, we can report that issue to shared code maintainers, and try to get them to fix it later. Regards, Helin ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH] i40e: fix shared code compile warning 2014-06-24 15:25 ` Zhang, Helin @ 2014-06-26 12:22 ` Thomas Monjalon 0 siblings, 0 replies; 8+ messages in thread From: Thomas Monjalon @ 2014-06-26 12:22 UTC (permalink / raw) To: Zhang, Helin, Ananyev, Konstantin, Chen, Jing D; +Cc: dev 2014-06-24 14:43, Ananyev, Konstantin: > 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. 2014-06-24 15:25, Zhang, Helin: > We should not modify code in shared code, if we do not want to maintain > those huge code base. I think Mark's patch good for now, we can report that > issue to shared code maintainers, and try to get them to fix it later. 2014-06-25 05:36, Chen, Jing D: > my committed patch to ignore the warning on that file and continue the > compile . By doing so, the logic is right and didn't change code's > behavior. We also needn't maintain it. Even in bad case we find bugs > hidden behind the '--CFLAGS_i40e_lan_hmc.0' warning in future, we can fix > it after bug occurred. That's simple. Applied for version 1.7.0. Thanks -- Thomas ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH] i40e: fix shared code compile warning 2014-06-24 10:06 ` Thomas Monjalon 2014-06-24 14:43 ` Ananyev, Konstantin @ 2014-06-25 5:36 ` Chen, Jing D 1 sibling, 0 replies; 8+ messages in thread From: Chen, Jing D @ 2014-06-25 5:36 UTC (permalink / raw) To: Thomas Monjalon; +Cc: dev Hi Thomas, > -----Original Message----- > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > Sent: Tuesday, June 24, 2014 6:06 PM > To: Chen, Jing D > Cc: dev@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@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 > > > > 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/libr > te_pm > > d_i40e > > /i40e/i40e_lan_hmc.c: In function ‘i40e_write_qword’: > > > /jenkins/workspace/DPDK_AUTO_IDT_VM_RHEL65_32_BUILD/DPDK/lib/libr > te_pm > > d_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/libr > te_pm > > d_i40 > > e/i40e/i40e_lan_hmc.c: In function ‘i40e_read_qword’: > > > /jenkins/workspace/DPDK_AUTO_IDT_VM_RHEL65_32_BUILD/DPDK/lib/libr > te_pm > > d_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. > > PS: please try to configure your mailer to add citation marks. > > -- > Thomas [Chen, Jing D] Generally speaking, I can fix the compile error by 2 ways. 1. change shared code with "ULL" suffix, which means that we had to maintain it. In any time shared code updates, we needs to find out what change we've made in history and compare with new shared code. I don’t know who should be responsible for that and how we record the changes. If you have good idea, please share with me. 2. my committed patch to ignore the warning on that file and continue the compile . By doing so, the logic is right and didn't change code's behavior. We also needn't maintain it. Even in bad case we find bugs hidden behind the '--CFLAGS_i40e_lan_hmc.0' warning in future, we can fix it after bug occurred. That's simple. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-06-26 12:22 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-06-24 5:22 [dpdk-dev] [PATCH] i40e: fix shared code compile warning Chen Jing D(Mark) 2014-06-24 8:46 ` Thomas Monjalon 2014-06-24 9:47 ` Chen, Jing D 2014-06-24 10:06 ` Thomas Monjalon 2014-06-24 14:43 ` Ananyev, Konstantin 2014-06-24 15:25 ` Zhang, Helin 2014-06-26 12:22 ` Thomas Monjalon 2014-06-25 5:36 ` Chen, Jing D
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).