From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 1B65242941; Fri, 14 Apr 2023 15:25:32 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id E20FD40144; Fri, 14 Apr 2023 15:25:31 +0200 (CEST) Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by mails.dpdk.org (Postfix) with ESMTP id 64BB7400D5 for ; Fri, 14 Apr 2023 15:25:29 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1681478729; x=1713014729; h=date:from:to:cc:subject:message-id:references: content-transfer-encoding:in-reply-to:mime-version; bh=xGfxev1IGhrZ9GM9CaDad00loEhWGHRipsEv1Vg+zb0=; b=OtNdgumO737rtsEqbJGQNfuVJp00iBokIr6Lx0XMhtyUuXYIBbXJ38eg WpgAQ16Ja/SCpa4HqUtO2ge7Q1iMycTXdG0RunL5PjMegxJ0efAGi0hez ut/JzxoBAmmxHxxwW4X1Jb93Edho+kBZRZqOiQ7Y+/iahkRpAxwnO89Ar 16OhXVi/TNTZTRA61/5tevuEVK6q86FR8Co+F5R+cx9u73adPtsyE8g/Y XOB5IPv13oMAPq8QIqiYp85vqvNfX4/TzcrvLDW+kCux+BK2QHoauoai8 6sh/geN0nUIESyOAFrSAULud9DVRcbKob04iMKMCfE9B3GpM8e3P0wGX8 A==; X-IronPort-AV: E=McAfee;i="6600,9927,10679"; a="409663226" X-IronPort-AV: E=Sophos;i="5.99,195,1677571200"; d="scan'208";a="409663226" Received: from orsmga007.jf.intel.com ([10.7.209.58]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Apr 2023 06:25:28 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10679"; a="683320286" X-IronPort-AV: E=Sophos;i="5.99,195,1677571200"; d="scan'208";a="683320286" Received: from fmsmsx601.amr.corp.intel.com ([10.18.126.81]) by orsmga007.jf.intel.com with ESMTP; 14 Apr 2023 06:25:28 -0700 Received: from fmsmsx611.amr.corp.intel.com (10.18.126.91) by fmsmsx601.amr.corp.intel.com (10.18.126.81) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.23; Fri, 14 Apr 2023 06:25:27 -0700 Received: from fmsedg602.ED.cps.intel.com (10.1.192.136) by fmsmsx611.amr.corp.intel.com (10.18.126.91) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.23 via Frontend Transport; Fri, 14 Apr 2023 06:25:27 -0700 Received: from NAM11-BN8-obe.outbound.protection.outlook.com (104.47.58.168) by edgegateway.intel.com (192.55.55.71) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.23; Fri, 14 Apr 2023 06:25:27 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=KyTdKCjmd25HC5pNy7XUxBuSzcoaclfutJZWRrJGXinCH6GWd23W/8sGmvUvY9vBEln+U+8rzveSKKu79l6IEpkKzgyQ1SrGfGXoIIcl7kykfZlciM6aXAQI1jEAqVa3/vTD1BfuE23/3G7URr+96S8meZ6RHasVWV//UsNWGwv8rnLhAdOilGVpEiHjV2ZFgrzrrr8x+Jb5k0IFv0osggfzudOJcfZJW9UdB7iMpO4xN/vg+EaZAa857P5XCTt0ujj4eysi3NQP+ckBeagJ467gV5z0M7wYMgLEhIHOUvNeRYr5BXjuOK0UqeafcShQ6klGLDld5pKRf3sT2PRnKg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=bZ9rhs8UKNeRT4FqK9VCE8Z0yk6ShJo+godZptDojT0=; b=M/FISD1VARFbtPBQBg4H5CNoFEDNEkv9bUfCm/Gb/iA6FqBwdM3owaOs1UT6yjgJFe9+py7Psda2/UP/a1VX55ph5CU27umEapqA4JdiC/6Z+C+l6kdedbyjszl+nwyq0n31o3cfnST0XXbCzUEhylTeX1K2T4NKZ/x5Bc1NcPP8rnNs7wzoIsM1TV9BbhsdhKQfTEs+26n9w/D/XyhZHUe9XUO2PG3tP208vZzekylipWpawLkosc97O3e+Ze38OpUrIBO+iDdIgkupGuznvSCEduPGc8Xf7y1gmBcre7mFSxY6nGo5n+LCFq3Wqt0ukOobSEG1KYbB7JCiJAg+6A== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=intel.com; Received: from DS0PR11MB7309.namprd11.prod.outlook.com (2603:10b6:8:13e::17) by BL1PR11MB5334.namprd11.prod.outlook.com (2603:10b6:208:312::5) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6298.30; Fri, 14 Apr 2023 13:25:20 +0000 Received: from DS0PR11MB7309.namprd11.prod.outlook.com ([fe80::695b:260c:f397:2b69]) by DS0PR11MB7309.namprd11.prod.outlook.com ([fe80::695b:260c:f397:2b69%4]) with mapi id 15.20.6298.030; Fri, 14 Apr 2023 13:25:20 +0000 Date: Fri, 14 Apr 2023 14:25:12 +0100 From: Bruce Richardson To: Morten =?iso-8859-1?Q?Br=F8rup?= CC: Tyler Retzlaff , , , , Subject: Re: [PATCH v5 11/14] eal: expand most macros to empty when using MSVC Message-ID: References: <1680558751-17931-1-git-send-email-roretzla@linux.microsoft.com> <1681421163-18578-1-git-send-email-roretzla@linux.microsoft.com> <1681421163-18578-12-git-send-email-roretzla@linux.microsoft.com> <98CBD80474FA8B44BF855DF32C47DC35D87878@smartserver.smartshare.dk> <98CBD80474FA8B44BF855DF32C47DC35D87879@smartserver.smartshare.dk> Content-Type: text/plain; charset="iso-8859-1" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35D87879@smartserver.smartshare.dk> X-ClientProxiedBy: DUZPR01CA0029.eurprd01.prod.exchangelabs.com (2603:10a6:10:46b::18) To DS0PR11MB7309.namprd11.prod.outlook.com (2603:10b6:8:13e::17) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: DS0PR11MB7309:EE_|BL1PR11MB5334:EE_ X-MS-Office365-Filtering-Correlation-Id: 084807c0-493d-4aeb-9eb8-08db3cebb1a7 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: PgDLLCvpn0q7JKmR9QWABd7ywWaNmigTQJqLheGtj9X8cCiKKyGlGd3UCIK/1utZGZhs4YHkNxlBlAv9ZW0okQt8XLaKqpEUDDNAb2c/la/pgXkyWSmpPUAhop51y1mjbSuQBMN6fnOT164XF5eBO+4Js/HnWLnX3paKVJInwJR/7Jrpbt4boEcWRjzCWza3xlYguwLPOhE/30Z3gCgrn7x85z3mt4xAyDYXuz44qzCCbe+blQC0LJWXpJUxqE3COXb6yUGqhtltCmBaTAjXLIb3D4mK5Cqd7ZXTU7pJPuAYvZY9CyVucT0JqDCZUgXVoyXHasj4qryhebb/cWK61Tt5YAwH8ea9oSo+KOgPURy4IzcALZBRhzRvQ1mAS1Za0Thqgj1S5xpH8RhptQpwB5VNYvAV8mXKf92Km5AaIjMafAWeL4BGwxHPbpES/tNyTRYUULPiJ3l7m4FS3YV0ga+bGIlzWRH1Tf4SJQ8MshOtSrdDsH9p1JnrfFDtOT2oCQRry3YrX6zcG2ugCZjjy94JHK956/jIyXIop56x9QI= X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:DS0PR11MB7309.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230028)(39860400002)(376002)(346002)(396003)(136003)(366004)(451199021)(8676002)(66574015)(83380400001)(966005)(478600001)(6486002)(6666004)(6506007)(26005)(6512007)(186003)(2906002)(44832011)(38100700002)(5660300002)(316002)(6916009)(4326008)(66476007)(41300700001)(66946007)(82960400001)(66556008)(8936002)(86362001); DIR:OUT; SFP:1102; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?iso-8859-1?Q?Ihpe8/F44g4mi5uNyPPD8m6OCr4wPFqVVT/cC3HUtJwB4o4089QNO19oYv?= =?iso-8859-1?Q?xednD3ZAZxEiB2aC+s/8KqfhtJ+NGopZ+XPLaK9h0HwwPMG5QSjurnBppW?= =?iso-8859-1?Q?KVjBqzEipEXYXxbxEw4HbPtdYURxn1rPUKDIT7EvpPTK3X600Qb7UbQI+P?= =?iso-8859-1?Q?X012VlKdeaJ00ibrAOll4B1ARASlAQLihmG1ZMdX/Aj47S+kpCGWGP6Xze?= =?iso-8859-1?Q?VVDCJc2St2ajoXcYF4Z7XnFyn6u131Y9WxFhRssX3u9e367LbJQiYWudZN?= =?iso-8859-1?Q?Yw/7wdYnxi9wKA3iTCk38kCKNWEqPm67/shJY1X7UF5dzjO0CuCtW6gnRE?= =?iso-8859-1?Q?Z+RMUP/QrFUYlXsJFxYHMHbyl6vKLIrr462QgmhL8+eg+2pzFV28s1JO8e?= =?iso-8859-1?Q?Xjte2254yUkGxALkg4wzHauPK6wbmtcRm6JcvoKQ90FF0MBhWOzS6au0J7?= =?iso-8859-1?Q?0PkyKn+MYDlYZ7aNMFjPPg7csczoclxlo2nFglzsC7KzFUFC6lGOIIxG6j?= =?iso-8859-1?Q?3HkXNOJm2fTt7ztsbjCiMM0IWmI4e805FHKyaNjenY7MsptZfG5TMlTvUq?= =?iso-8859-1?Q?/ioWNgxGvL3MoENwSztwZMR5Lvc3l5WVY6Ry0l06GtaWYsxucI+m/tcqFo?= =?iso-8859-1?Q?/h4uhZbW314ZgggKAY860eWg41HJnIkpChSPW5ww4sp0hG+n/iOOx30X6U?= =?iso-8859-1?Q?tqvwAwh8OeHerVWcUirLwg4GXxyTH5kSyoOTihGSHikPUAk6Y7YVU+U21P?= =?iso-8859-1?Q?DjwxOWd/mHaUHkWaXE3SiOlxXlG6gEsxRAkV8wOyk0B3aRUYPMJlVtCigc?= =?iso-8859-1?Q?xbRf5iSAbqmHbk1xI1Uwh8SVnz2eG2vN6b7jYyk1MdUpy3V0FHfZtJJt1u?= =?iso-8859-1?Q?4iowJbQCRKvbGKm0ssHrWB+eCxWs2YhWJBMEAm5YrQp+gIi8VevlNjVJze?= =?iso-8859-1?Q?gPRoZsTlp1EI4j52ruGvj9SP+HtSOtPePfyujrjZPK2I2L1WPwb41tBKVp?= =?iso-8859-1?Q?KvTBvFJ5m0J+oJoaJhoDx+x6AREr5kvZdLwvMl4lAVeyY9hbiZ64bmxOGw?= =?iso-8859-1?Q?5ng6GBHyK9cq9l35x/hjC3CHVCfcCcQ9h6o4GfzkCHmLiLneptXmC7H7+L?= =?iso-8859-1?Q?3vM56OodXHHmD17lYahHs8cuObRvNFGyb5JMEMH7Bz91XSBHpge6HHIZkw?= =?iso-8859-1?Q?UDyosgkpvtYxWEDiSE+qSfOSxmfeaWg9rmvEdkNumF7XLhznIfNd30oh1z?= =?iso-8859-1?Q?dAdDC3x61e64HIHrJoHi/hKjJM6bBB/cRTe9nFS7T7KvloBWFk3gTAinuq?= =?iso-8859-1?Q?WoO7WFsMXyCESlem9cTdgzQ2exQny89WsyAexoU4af1+qk9lwlfpa0CiO2?= =?iso-8859-1?Q?17pFgc3mmB+29Au/63sfQHfRYR9FdI6CrO6CfCjhBfQZ9PVS5c3f2lX0xP?= =?iso-8859-1?Q?B8TE1+TByNe7X/a6vnbyKsEK3UyZgcF2/SOHDDmCMeVY1eRBu3wclGZOsR?= =?iso-8859-1?Q?UTEu/1qzj6xeIAJuKXcRIFoVYELmahXZaSEg4Yg7mC2lgX28ZPCXdYbz+M?= =?iso-8859-1?Q?iRYYme3VN4hW7JIpIic3U+gNSJTTrA3qE4hKZcWQjzHkGIODbSHSBLw9mt?= =?iso-8859-1?Q?o0FeQkAX5VI2bACPb4fY+icFmA6agA/iguZwbx5KIfPfevkuMIVuyscQ?= =?iso-8859-1?Q?=3D=3D?= X-MS-Exchange-CrossTenant-Network-Message-Id: 084807c0-493d-4aeb-9eb8-08db3cebb1a7 X-MS-Exchange-CrossTenant-AuthSource: DS0PR11MB7309.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Apr 2023 13:25:20.2046 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: Y9wcLFzUpmRAk+Kac/kzXn8Qr9+9ATYC7b/j4SwLJioryi+U/4yYhNaG7L3PO3WgHvOy3lc+n2tl5XYYbAa6wdZDbFzOLC9M1j6ThxKHAXw= X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL1PR11MB5334 X-OriginatorOrg: intel.com X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Fri, Apr 14, 2023 at 02:39:03PM +0200, Morten Brørup wrote: > > From: Bruce Richardson [mailto:bruce.richardson@intel.com] > > Sent: Friday, 14 April 2023 11.22 > > > > On Fri, Apr 14, 2023 at 08:45:17AM +0200, Morten Brørup wrote: > > > > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com] > > > > Sent: Thursday, 13 April 2023 23.26 > > > > > > > > For now expand a lot of common rte macros empty. The catch here is we > > > > need to test that most of the macros do what they should but at the same > > > > time they are blocking work needed to bootstrap of the unit tests. > > > > > > > > Later we will return and provide (where possible) expansions that work > > > > correctly for msvc and where not possible provide some alternate macros > > > > to achieve the same outcome. > > > > > > > > Signed-off-by: Tyler Retzlaff > > > > --- > > > > lib/eal/include/rte_branch_prediction.h | 8 ++++++ > > > > lib/eal/include/rte_common.h | 45 > > > > +++++++++++++++++++++++++++++++++ > > > > lib/eal/include/rte_compat.h | 20 +++++++++++++++ > > > > 3 files changed, 73 insertions(+) > > > > > > > > diff --git a/lib/eal/include/rte_branch_prediction.h > > > > b/lib/eal/include/rte_branch_prediction.h > > > > index 0256a9d..d9a0224 100644 > > > > --- a/lib/eal/include/rte_branch_prediction.h > > > > +++ b/lib/eal/include/rte_branch_prediction.h > > > > @@ -25,7 +25,11 @@ > > > > * > > > > */ > > > > #ifndef likely > > > > +#ifndef RTE_TOOLCHAIN_MSVC > > > > #define likely(x) __builtin_expect(!!(x), 1) > > > > +#else > > > > +#define likely(x) (x) > > > > > > This must be (!!(x)), because x may be non-Boolean, e.g. likely(n & 0x10), > > and likely() must return Boolean (0 or 1). > > > > > > > Will this really make a difference? Is there somewhere likely/unlikely > > would be used where we would not get the same conversion to boolean than we > > get using "!!" operator. [NOTE: Not saying we shouldn't put in the !!, just > > wondering if there are actual cases where it affects the output?] > > I agree that it makes no difference the way it is typically used. > > But there are creative developers out there, so these macros definitely need the "!!" conversion to Boolean. > Sure. > > > > > > +#endif > > > > #endif /* likely */ > > > > > > > > /** > > > > @@ -39,7 +43,11 @@ > > > > * > > > > */ > > > > #ifndef unlikely > > > > +#ifndef RTE_TOOLCHAIN_MSVC > > > > #define unlikely(x) __builtin_expect(!!(x), 0) > > > > +#else > > > > +#define unlikely(x) (x) > > > > > > This must also be (!!(x)), for the same reason as above. > > > > > > > +#endif > > > > #endif /* unlikely */ > > > > > > > > #ifdef __cplusplus > > > > diff --git a/lib/eal/include/rte_common.h b/lib/eal/include/rte_common.h > > > > index 2f464e3..1bdaa2d 100644 > > > > --- a/lib/eal/include/rte_common.h > > > > +++ b/lib/eal/include/rte_common.h > > > > @@ -65,7 +65,11 @@ > > > > /** > > > > * Force alignment > > > > */ > > > > +#ifndef RTE_TOOLCHAIN_MSVC > > > > #define __rte_aligned(a) __attribute__((__aligned__(a))) > > > > +#else > > > > +#define __rte_aligned(a) > > > > +#endif > > > > > > It should be reviewed that __rte_aligned() is only used for optimization > > purposes, and is not required for DPDK to function properly. > > > > > > > Good point. > > > > If we look across all of DPDK, things will likely break, as we are relying > > on alignment in various places to use the aligned versions of instructions. > > For example _mm256_load_si256() vs _mm256_loadu_si256() in our x86 > > vectorized driver code. A "git grep _load_si" shows quite a few aligned > > vector load instructions used in our codebase. These will fault and cause a > > crash if the data is not properly aligned. [I suspect that there are similar > > restrictions on other architectures too, just not familiar with their > > intrinsics to check.] > > Another thing that has been annoying me with the use of vector instructions: > > Vector instructions are often used in a way where they cast away the type they are working on, so if that type is modified (e.g. a field is moved), the code will happily build, but fail at runtime. > > When casting away the type for vector instructions, _Static_assert or BUILD_BUG_ON should be used to verify the assumptions about the cast away type. Such a practice might catch some of the places where the missing alignment (and missing structure packing) would fail. > Agreed. And, in fairness, this is sometimes done in our code, e.g. [1], but should probably be more widely done. It's something we should try and catch in reviews of vector code, as it also helps document what exactly we are doing and why. /Bruce [1] http://git.dpdk.org/dpdk/tree/drivers/net/i40e/i40e_rxtx_vec_avx2.c#n183