From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
To: Thomas Monjalon <thomas.monjalon@6wind.com>,
"Chen, Jing D" <jing.d.chen@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH] i40e: fix shared code compile warning
Date: Tue, 24 Jun 2014 14:43:49 +0000 [thread overview]
Message-ID: <2601191342CEEE43887BDE71AB97725821333FA3@IRSMSX105.ger.corp.intel.com> (raw)
In-Reply-To: <4332738.s42vW08oSG@xps13>
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
next prev parent reply other threads:[~2014-06-24 14:43 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-06-24 5:22 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 [this message]
2014-06-24 15:25 ` Zhang, Helin
2014-06-26 12:22 ` Thomas Monjalon
2014-06-25 5:36 ` Chen, Jing D
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2601191342CEEE43887BDE71AB97725821333FA3@IRSMSX105.ger.corp.intel.com \
--to=konstantin.ananyev@intel.com \
--cc=dev@dpdk.org \
--cc=jing.d.chen@intel.com \
--cc=thomas.monjalon@6wind.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).