From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 0DD8637B1 for ; Thu, 1 Jun 2017 10:47:24 +0200 (CEST) Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by fmsmga105.fm.intel.com with ESMTP; 01 Jun 2017 01:47:23 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.39,278,1493708400"; d="scan'208";a="109559509" Received: from bricha3-mobl3.ger.corp.intel.com ([10.237.221.42]) by fmsmga006.fm.intel.com with SMTP; 01 Jun 2017 01:47:21 -0700 Received: by (sSMTP sendmail emulation); Thu, 01 Jun 2017 09:47:21 +0100 Date: Thu, 1 Jun 2017 09:47:21 +0100 From: Bruce Richardson To: Sangjin Han Cc: dev@dpdk.org Message-ID: <20170601084720.GA45652@bricha3-MOBL3.ger.corp.intel.com> References: <1496296895-9600-1-git-send-email-sangjin@eecs.berkeley.edu> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1496296895-9600-1-git-send-email-sangjin@eecs.berkeley.edu> Organization: Intel Research and =?iso-8859-1?Q?De=ACvel?= =?iso-8859-1?Q?opment?= Ireland Ltd. User-Agent: Mutt/1.8.1 (2017-04-11) Subject: Re: [dpdk-dev] [PATCH] lpm: fix build error on g++ with -O0 option X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 01 Jun 2017 08:47:25 -0000 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 > --- > 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