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 D9AB74291B; Tue, 11 Apr 2023 11:11:04 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 6F93440DFD; Tue, 11 Apr 2023 11:11:04 +0200 (CEST) Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by mails.dpdk.org (Postfix) with ESMTP id 882BE40A8B for ; Tue, 11 Apr 2023 11:11:02 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1681204262; x=1712740262; h=date:from:to:cc:subject:message-id:references: content-transfer-encoding:in-reply-to:mime-version; bh=CevU0V1CxVcjAJwduhvkfxis/5BgHzp1uqasl7IIwdc=; b=iq22cibAfAzLAYiSu88rs0QcWOL7wBR1yw4bXbn9TLjSk63/sqnYaQK1 y8A3W00chDfJHLBGYqJMtabV2h5uJo2lL62hrk1mhcxJ44iI8/om8okwz 8FgmRwHlOuGDITg+d500IhGjYyDyZ4FFf/YyRoldqDwM/gRz/5Dz366js 0bw4Mp1+B5FvwP9tWp2rDfnh/fJjKcrDMwLqzpxpA2aIMonrukGmKzMI2 OWC1I/kvWJz0lHrK31pURvStLNvxSTR6asfJzzrzLM3AQqUWzdcoi8iCm tQ+Dubhi+0QQPL6XQHekbClQicvMKdX9lsUCClbYMgOkSnkq1VPkSy3nJ A==; X-IronPort-AV: E=McAfee;i="6600,9927,10676"; a="327665048" X-IronPort-AV: E=Sophos;i="5.98,336,1673942400"; d="scan'208";a="327665048" Received: from orsmga006.jf.intel.com ([10.7.209.51]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 11 Apr 2023 02:10:58 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10676"; a="665909367" X-IronPort-AV: E=Sophos;i="5.98,336,1673942400"; d="scan'208";a="665909367" Received: from fmsmsx603.amr.corp.intel.com ([10.18.126.83]) by orsmga006.jf.intel.com with ESMTP; 11 Apr 2023 02:10:58 -0700 Received: from fmsmsx610.amr.corp.intel.com (10.18.126.90) by fmsmsx603.amr.corp.intel.com (10.18.126.83) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.23; Tue, 11 Apr 2023 02:10:58 -0700 Received: from fmsmsx603.amr.corp.intel.com (10.18.126.83) by fmsmsx610.amr.corp.intel.com (10.18.126.90) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.23; Tue, 11 Apr 2023 02:10:57 -0700 Received: from fmsedg601.ED.cps.intel.com (10.1.192.135) by fmsmsx603.amr.corp.intel.com (10.18.126.83) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.23 via Frontend Transport; Tue, 11 Apr 2023 02:10:57 -0700 Received: from NAM04-DM6-obe.outbound.protection.outlook.com (104.47.73.49) by edgegateway.intel.com (192.55.55.70) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.23; Tue, 11 Apr 2023 02:10:57 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=G3zx5D9GHGlPJ9fK410vBxnmjwXpDn0MuPsKcJgTGqKEhVSv+/c3N/l8DTv4Ahvs6opcRBkRVcW/RpyEwIn4n549KavLBvnVuQCN6NhaWiE0fVoJlrejTi4KcRaToaaEi/1ctZcuRTkIduUg8rgRQRNUZbsnWLnScbE30TaRD+3s/wgoixfWr2EHuQY0TbRwxRnvxPhsBmFHnaXUwj2jUf6HRg6X4iYvPdVJTvkALOgaC31nnE3rBwDTy/XnkD8/xtdeNjJuaD/x6SjVXZBqi1bCXcHkFHBpfJHvD6SuUtkOwV9iO/3WRNVkYB9LyHbGT0VNEfyN9cBxU5HRk+ra8w== 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=gpySkXF62CNjYClhTooFCe8cUqJl+romseUeZihh+o8=; b=SGD4y7QGm7agp5O1M5NPXnBwIJZpDSWdVGjRK0U45yJnDBFhWCE4GIQN1/gNlFPGA4mWtO+c2uWsNQ9X8mmcDYGtWx9OOGnrm/sNo6FIzBWv16q9U+IVwoVR4SLrvOFQgbw3GCe0hxGTI86kvjTM/+v/IpP4rHw4VpXeObXe5EZtllzVQ9kpU3azAL/ZgJY/lq05waotc5S7qtrSDib2X6wq2We+6MMxU0r8k9uHLS1u/RGnpVgNUbO7bcRpWBcjRfgMLmIYWIMd6LnUncKC2/n7xKcWFqM0YI1irYQNGEE9ooZzxT4yXHada1ltLoC4BgrNusR3vA5W7HlPxG6xWg== 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 IA1PR11MB6538.namprd11.prod.outlook.com (2603:10b6:208:3a2::5) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6277.30; Tue, 11 Apr 2023 09:10:55 +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.6277.038; Tue, 11 Apr 2023 09:10:55 +0000 Date: Tue, 11 Apr 2023 10:10:48 +0100 From: Bruce Richardson To: Tyler Retzlaff CC: Konstantin Ananyev , Konstantin Ananyev , "dev@dpdk.org" , "david.marchand@redhat.com" , "thomas@monjalon.net" , "mb@smartsharesystems.com" Subject: Re: [PATCH 3/9] eal: use barrier intrinsics when compiling with msvc Message-ID: References: <1680558751-17931-1-git-send-email-roretzla@linux.microsoft.com> <1680558751-17931-4-git-send-email-roretzla@linux.microsoft.com> <754a877d020a4a199a5310b469e876a7@huawei.com> <20230404154906.GC23247@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> <20230405000441.GA24769@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> <20230406000740.GB2287@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> <624ce08e-6f67-e078-3f37-c9dab7b094f4@yandex.ru> <20230410205848.GB4798@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20230410205848.GB4798@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> X-ClientProxiedBy: LO2P123CA0064.GBRP123.PROD.OUTLOOK.COM (2603:10a6:600:1::28) To DS0PR11MB7309.namprd11.prod.outlook.com (2603:10b6:8:13e::17) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: DS0PR11MB7309:EE_|IA1PR11MB6538:EE_ X-MS-Office365-Filtering-Correlation-Id: 248c21bd-c4cc-4a18-5119-08db3a6ca7c2 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: sdBXxFFpVuraksVfStEcrQefYQkevQqMWQahF1bCma4srny779IxXeQejUdPQnyubXmXvyJB0uCznikf5CZ3GhEuN7OYJ9wNQanvgUx+rsBErNikk7+bKr5NEPzoAunjDeOWKF1PlQVa/SI0aifeur2TglSRUAG3/J7k3iHe3V3yQWywackXL1TNkOzCZgMpASEOjfQf5x3LElc6x9bS7fNMc7diyteH3fap2Kaw5WlWt3WDzQdr6Zh2ADakxRY9SJHKycEZFL5yrJ28/Y5/9iuUaf9CVNWlwpUXGQXp8fpCkJ5OBlTiwSOu49y+ggoYrWbzc5mpEvIJAPWGoBgvNvpAI4z+xHitVlBbJrdNhLekwmt4rtZv2p8RukiHvotlm8AIwxL2ZkFoen/wj0NRP4m+j2E1LoY+vMDGT1g0okE/Z+KFMln9zaTdHA5jMiuNHbcCywbqrJB6YoDEwXml/o5fb3VjoBrRSywdqNsJnnN2Sq2nVBjv+9/D2UBr1NgtakrDS9AgOmDnSuJb3dB/NbbB4MiVFvYe72cTCb4MoCwYbbI8bV7KukMzRF1CeKLXNVPAkPNtch15vTSVQXxsXeNWhsm1b/BZVNh2q2wyCs8= 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)(136003)(376002)(39860400002)(366004)(396003)(346002)(451199021)(478600001)(5660300002)(6506007)(966005)(6486002)(6512007)(26005)(316002)(54906003)(6666004)(186003)(2906002)(38100700002)(44832011)(66946007)(8676002)(6916009)(66556008)(41300700001)(8936002)(4326008)(82960400001)(86362001)(83380400001)(66476007)(67856001); DIR:OUT; SFP:1102; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?Z1RwK2JtOCtLbXJOTXBrV0w5VnI0d3V2RjVqUDZVdTF6akNvOEtLZFFEVFBB?= =?utf-8?B?ZVRDNUlNY1NwNXY1VGh0NVF6L2Y3R242THYyR0dpK2ZrV2h3Q0FPOVdvZlRi?= =?utf-8?B?V2dSZ3IvWTI3OEdsZEVqZHpTc0s3SGhrV3lxUFRRa1VHZmZtUFN2cnM0dnEy?= =?utf-8?B?SjdGUG1SOTlJVFZwQkUvZzlhRktHbDNHWGlWV1hFdVR0MnZsamZGN01kZjhO?= =?utf-8?B?Ti80d1dOQ09NVHhYZENGcnVVSmZ0VDJsT2pkUjd2MFR4OVJGQ2Z4VlB3VlhP?= =?utf-8?B?ZmZEbjR4YW4zR3RNa0hnOVVMMkRwQkszd2N2SGJpMVkzWnorTlM4OUJReUtT?= =?utf-8?B?S2VXUnUyMVovdVF2S1BQWTZ6alF3bS9PRW9KdW5TYjUxamtoR2Q2WXBUNU5E?= =?utf-8?B?cVZDYkFhMzhLTnpodUMzWHl5ZXQzRmlRd3FlUnFJekoreXZWcG43TU52N0ZK?= =?utf-8?B?eWFGMFZ0My9PS0pwOTdaWThNZzBkejk3QVBLTERza2VKK255RFBlQ1Ywc0Jv?= =?utf-8?B?bHRkcWdaS2NiekZoWlVTSUtWUUJjeDBqN1FwbytQNGcxL3F6MHBVZnJPNjBy?= =?utf-8?B?dHJxMXArb2tUUnpqQ3NTZ0hEUkl2Z3lRdGtzdXRtOG5rdE1GODY3bFg4aysy?= =?utf-8?B?NDlxUFd5OFhCUTRUQjVhSEVEMXRoMVZtYjl4bnVSeWs3L2djOXlxMlJhRGY1?= =?utf-8?B?NnJFV09JY1VHTUVEUDhKeXBWOWpkcmZjZDM1TjRiYmNIaXJ6c1NWdDloMDJG?= =?utf-8?B?VXVSdTlnYXNWdUxSVHBBS1ZiU0sySkdEMERwM0ZtT0RmUjJQNEp0dWh1MzFn?= =?utf-8?B?WEY2WDNDbVpjWDMvczdKU2lkN2ozL1VKRFBnNmtkUmJ2SUJ6eHB1d3hEMHpX?= =?utf-8?B?YnlmQldpVXVKNHNPM1QrUjZIKzhhMHhuS1J1Y2tPSmwvT1dXdmVRR2dVd3VE?= =?utf-8?B?dVdzS3d3R082UE03ZEl1RzZ2U2tXdnVHemx3NEhKZVpMamYyWGdnODhTY1dL?= =?utf-8?B?R0NqUWYvMUpabUEzenJGNWxVZ1o2aG82TitndmRjbEl0Ui9zMkNwL0dnbVQ0?= =?utf-8?B?RnZRaXpQcFZpaHg1blAzcGZFVnlxZ0h0ZDhOT3o2UERjS0NlMlk0enFXQm5y?= =?utf-8?B?bEFUbTJFSys3NmRQN3k1dlNaMlAwOGl2NVFZV3FTdDdnb1V5bHBKQ3IyckNm?= =?utf-8?B?OElaNzlzT2tZVWxMcXhLQzdIb2kzOTBHenNjNEZ5emVYUHA1bzU1cDgzQmVj?= =?utf-8?B?cFY1RTVUWUUxb0dha0Zna1kwaUZvKzhIa0xGcytWVDhZQmlyVGg5Uk4rcFp3?= =?utf-8?B?ZlFZRGp2OSsyOHZqTVU3SllLYm1XRXZhZnQ5WjZuUVNOQTZmeHBETWkySXVC?= =?utf-8?B?QWJ1VXA2TWpDdDNpWS9aQ1Y4R3BZemg2VXhYb2piWmdUejFUN0tZMVE4TjNv?= =?utf-8?B?MTdkVkVaRFlzT2dacXJRUnlVYjE0c3diaU9iK1QxdlZQU3lMR25rL2NmUXdZ?= =?utf-8?B?MTVvdFVTMnM3dVdHSjJBOGh1Nk5NTjg1UDlKM01hNUlVNFhsZGZaR25qZExL?= =?utf-8?B?YTRtc1kweWdBVTlsOHc5Q1JDUTE3WDJGKzRyMEl5cjU4d0F4enk0Wm1DY3Br?= =?utf-8?B?NE9qazl2eElDOHA2MzUwNlNrSElEWEgyQi9mM1AyVmViV0xlNmJEWkxZTGpC?= =?utf-8?B?ZnlNN3J1WWs1aXBnSTd1S3J4NHFWQ3Roa2ZkT0FudTQ0UlFCQ3lSU3JqQ016?= =?utf-8?B?MkJlRVJQUjVHV1JYNXMwNTdTcDlOMzZXV1N2UnMybUNFMytEUXF3WXBPQThB?= =?utf-8?B?MDFYT1E2eGxMUG96cVBqdUpqM3VMSDhUak5qSUJGT0E3aDdpTGVSQUEwYk9t?= =?utf-8?B?SzdzSWhtYWlMM0MxMWRtNUdCRXlkRmZ6TWdJNkZSUjJIZG9VQXc3RTVNd1RF?= =?utf-8?B?bllET3JnK1RWYkZVTkF2TWQxTkZwNHNvWVR4ZEMvaTd6T1BkYU9JTGk0VkVz?= =?utf-8?B?OTJDOE1nNjc1Y0t2TTJDYmIxU2VWSDl5ZmRnZEhHbGVYM0V4MTV5U3YrM3NO?= =?utf-8?B?UzRPN0kzeXV3OGtzSlBpcUkvSDV5S1RFbllHeXQ1TUo4aURuYnhFdzEzNHhi?= =?utf-8?B?V3BYVUk5UytEaVBNcUlXcFFGZklNUy9URjUwak44VnBrRjZVZ1RTNnJLR2JM?= =?utf-8?B?SkE9PQ==?= X-MS-Exchange-CrossTenant-Network-Message-Id: 248c21bd-c4cc-4a18-5119-08db3a6ca7c2 X-MS-Exchange-CrossTenant-AuthSource: DS0PR11MB7309.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Apr 2023 09:10:55.2021 (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: 4a10EliwlvaVt4F76qYyQQWZBo5HGhaqGnqXlFfc3UUpDmAzbH3ycwWvzsQlDeSbFkkiZGDfUBfikcNau7fhhAz/OVNav0fzjGkTOt0a2ro= X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR11MB6538 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 Mon, Apr 10, 2023 at 01:58:48PM -0700, Tyler Retzlaff wrote: > On Mon, Apr 10, 2023 at 09:02:00PM +0100, Konstantin Ananyev wrote: > > 06/04/2023 01:07, Tyler Retzlaff пишет: > > >On Wed, Apr 05, 2023 at 10:57:02AM +0000, Konstantin Ananyev wrote: > > >> > > >>>>>>>Inline assembly is not supported for msvc x64 instead use > > >>>>>>>_{Read,Write,ReadWrite}Barrier() intrinsics. > > >>>>>>> > > >>>>>>>Signed-off-by: Tyler Retzlaff > > >>>>>>>--- > > >>>>>>> lib/eal/include/generic/rte_atomic.h | 4 ++++ > > >>>>>>> lib/eal/x86/include/rte_atomic.h | 10 +++++++++- > > >>>>>>> 2 files changed, 13 insertions(+), 1 deletion(-) > > >>>>>>> > > >>>>>>>diff --git a/lib/eal/include/generic/rte_atomic.h b/lib/eal/include/generic/rte_atomic.h > > >>>>>>>index 234b268..e973184 100644 > > >>>>>>>--- a/lib/eal/include/generic/rte_atomic.h > > >>>>>>>+++ b/lib/eal/include/generic/rte_atomic.h > > >>>>>>>@@ -116,9 +116,13 @@ > > >>>>>>> * Guarantees that operation reordering does not occur at compile time > > >>>>>>> * for operations directly before and after the barrier. > > >>>>>>> */ > > >>>>>>>+#ifndef RTE_TOOLCHAIN_MSVC > > >>>>>>> #define rte_compiler_barrier() do { \ > > >>>>>>> asm volatile ("" : : : "memory"); \ > > >>>>>>> } while(0) > > >>>>>>>+#else > > >>>>>>>+#define rte_compiler_barrier() _ReadWriteBarrier() > > >>>>>>>+#endif > > >>>>>>> > > >>>>>>> /** > > >>>>>>> * Synchronization fence between threads based on the specified memory order. > > >>>>>>>diff --git a/lib/eal/x86/include/rte_atomic.h b/lib/eal/x86/include/rte_atomic.h > > >>>>>>>index f2ee1a9..5cce9ba 100644 > > >>>>>>>--- a/lib/eal/x86/include/rte_atomic.h > > >>>>>>>+++ b/lib/eal/x86/include/rte_atomic.h > > >>>>>>>@@ -27,9 +27,13 @@ > > >>>>>>> > > >>>>>>> #define rte_rmb() _mm_lfence() > > >>>>>>> > > >>>>>>>+#ifndef RTE_TOOLCHAIN_MSVC > > >>>>>>> #define rte_smp_wmb() rte_compiler_barrier() > > >>>>>>>- > > >>>>>>> #define rte_smp_rmb() rte_compiler_barrier() > > >>>>>>>+#else > > >>>>>>>+#define rte_smp_wmb() _WriteBarrier() > > >>>>>>>+#define rte_smp_rmb() _ReadBarrier() > > >>>>>>>+#endif > > >>>>>>> > > >>>>>>> /* > > >>>>>>> * From Intel Software Development Manual; Vol 3; > > >>>>>>>@@ -66,11 +70,15 @@ > > >>>>>>> static __rte_always_inline void > > >>>>>>> rte_smp_mb(void) > > >>>>>>> { > > >>>>>>>+#ifndef RTE_TOOLCHAIN_MSVC > > >>>>>>> #ifdef RTE_ARCH_I686 > > >>>>>>> asm volatile("lock addl $0, -128(%%esp); " ::: "memory"); > > >>>>>>> #else > > >>>>>>> asm volatile("lock addl $0, -128(%%rsp); " ::: "memory"); > > >>>>>>> #endif > > >>>>>>>+#else > > >>>>>>>+ rte_compiler_barrier(); > > >>>>>>>+#endif > > >>>>>> > > >>>>>>It doesn't look right to me: compiler_barrier is not identical to LOCK-ed operation, > > >>>>>>and is not enough to serve as a proper memory barrier for SMP. > > >>>>> > > >>>>>i think i'm confused by the macro naming here. i'll take another look > > >>>>>thank you for raising it. > > >>>>> > > >>>>>> > > >>>>>>Another ore generic comment - do we really need to pollute all that code with RTE_TOOLCHAIN_MSVC ifdefs? > > >>>>>>Right now we have ability to have subdir per arch (x86/arm/etc.). > > >>>>>>Can we treat x86+windows+msvc as a special arch? > > >>>>> > > >>>>>i asked this question previously and confirmed in the technical board > > >>>>>meeting. the answer i received was that the community did not want new > > >>>>>directory/headers introduced for compiler support matrix and i should > > >>>>>use #ifdef in the existing headers. > > >>>> > > >>>>Ok, can I then ask at least to minimize number of ifdefs to absolute > > >>>>minimum? > > >>> > > >>>in principal no objection at all, one question though is what to do with > > >>>comment based documentation attached to macros? e.g. > > >>> > > >>>#ifdef SOME_FOO > > >>>/* some documentation */ > > >>>#define some_macro > > >>>#else > > >>>#define some_macro > > >>>#endif > > >>> > > >>>#ifdef SOME_FOO > > >>>/* some documentation 2 */ > > >>>#define some_macro2 > > >>>#else > > >>>#define some_macro2 > > >>>#endif > > >>> > > >>>i can either duplicate the documentation for every define so it stays > > >>>"attached" or i can only document the first expansion. let me know what > > >>>you expect. > > >>> > > >>>so something like this? > > >>> > > >>>#ifdef SOME_FOO > > >>>/* some documentation */ > > >>>#define some_macro > > >>>/* some documentation 2 */ > > >>>#define some_macro2 > > >>>#else > > >>>#define some_macro > > >>>#define some_macro2 > > >>>#endif > > >>> > > >>>or should all documentation be duplicated? which can become a teadious > > >>>redundancy for anyone maintaining it. keep in mind we might have to make > > >>>an exception for rte_common.h because it seems doing this would be > > >>>really ugly there. take a look let me know. > > >> > > >>My personal preference would be to keep one documentation block for both cases > > >>(yes, I suppose it needs to be updated if required): > > >> > > >>/* some documentation, probably for both SOME_FOO on/off */ > > >>#ifdef SOME_FOO > > >>#define some_macro > > >>#else > > >>#define some_macro > > >>#endif > > >> > > >> > > >>> > > >>>>It is really hard to read an follow acode that is heavily ifdefed. > > >>>>Let say above we probably don't need to re-define > > >>>>rte_smp_rmb/rte_smp_wmb, as both are boiled down to > > >>>>compiler_barrier(), which is already redefined. > > >>> > > >>>can you take a look at v2 of the patch and re-prescribe your advise > > >>>here? in v2 only the intel macros expand to the compiler barrier. though > > >>>i find this vexing since as you pointed out it seems they aren't > > >>>supposed to be compiler only barriers according to the documentation in > > >>>generic/rte_atomic.h they are intended to be memory barriers. > > >> > > >>Commented, pls check if I explained my thoughts clear enough there. > > >>> > > >>>please help me if i've goofed up in this regard. > > >>> > > >>>>Another question - could it be visa-versa approach: > > >>>>can we replace some inline assembly with common instincts whenever possible? > > >>> > > >>>msvc has only intrinsics and the conditional expansion for msvc is to > > >>>use those intrinsics, gcc doesn't generally define intrinsics for processor > > >>>specific code does it? > > >> > > >>AFAIK latest gcc (and clang) versions do support majority of these instincts: __rdtsc, _xbegin/_xend, etc. > > >>So my thought was - might be we can use same instincts for all compilers... > > >>One implication I can think about - older versions of gcc. > > >>But might be we can re-order things and have inlines only for these oldere gcc versions? > > > > > >i'm going to propose if we do this we do it as a separate change later. > > > > > >i fear it could turn into the following dance which seems not a lot > > >better given i'm sure some people will argue there is no benefit to > > >removing inline assembly for gcc/clang. my preference is not to get > > >side-tracked on refactoring with the short merge window. > > > > > >#if (defined(__clang__) && clang version < x) || > > > (defined(__GNUC__) && gcc version < x) > > >__asm(whatever... > > >#else > > >__rdtsc() > > >#endif > > > > > > Played a bit with https://godbolt.org/. > > It seems that __rdtsc() and RMT builtins (xbegin/xend/xtest) are > > supported all way down to gcc 4.8.1. > > Do you know what version of clang comes with RHEL 7. Because for 23.07 > release at least I need to know it won't break clang compile on RHEL 7 > either (which is what makes this painful). > I don't know if we ever stated that we need to support clang from RHEL 7. I always assumed that it was just gcc 4.8 compiler, but I suppose we never stated either that we didn't need to support it.... > I played around with clang 7 (which i'm not sure is the right version) > it had __rdtsc() but it did not have xbegin/xend/xtest. For the various > instructions they're appearing in different versions of the compilers. > Running Centos 7 in a VM, I see Clang v3.4.2. > I'm starting to wonder if i should drop the intrinsics changes into a > separate series? >