From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.20; helo=mga02.intel.com; envelope-from=michael.d.kinney@intel.com; receiver=edk2-devel@lists.01.org Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 0EAD921A07A82 for ; Tue, 5 Mar 2019 13:45:25 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 05 Mar 2019 13:45:25 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.58,445,1544515200"; d="scan'208";a="138334655" Received: from orsmsx104.amr.corp.intel.com ([10.22.225.131]) by FMSMGA003.fm.intel.com with ESMTP; 05 Mar 2019 13:45:24 -0800 Received: from orsmsx123.amr.corp.intel.com (10.22.240.116) by ORSMSX104.amr.corp.intel.com (10.22.225.131) with Microsoft SMTP Server (TLS) id 14.3.408.0; Tue, 5 Mar 2019 13:45:24 -0800 Received: from orsmsx113.amr.corp.intel.com ([169.254.9.97]) by ORSMSX123.amr.corp.intel.com ([169.254.1.230]) with mapi id 14.03.0415.000; Tue, 5 Mar 2019 13:45:24 -0800 From: "Kinney, Michael D" To: "Gao, Liming" , "afish@apple.com" , "Zhang, Shenglei" , "Kinney, Michael D" CC: edk2-devel-01 Thread-Topic: [edk2] [PATCH 3/3] MdePkg/BaseSynchronizationLib: Remove inline X86 assembly code Thread-Index: AQHU0vSEcLio4+YtTUm1OMXnEiLc5KX+DF2AgAAJ0gD//3sCkA== Date: Tue, 5 Mar 2019 21:45:23 +0000 Message-ID: References: <20190305014059.17988-1-shenglei.zhang@intel.com> <20190305014059.17988-4-shenglei.zhang@intel.com> <4A89E2EF3DFEDB4C8BFDE51014F606A14E3FBBC2@SHSMSX104.ccr.corp.intel.com> In-Reply-To: <4A89E2EF3DFEDB4C8BFDE51014F606A14E3FBBC2@SHSMSX104.ccr.corp.intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.0.400.15 dlp-reaction: no-action x-originating-ip: [10.22.254.140] MIME-Version: 1.0 Subject: Re: [PATCH 3/3] MdePkg/BaseSynchronizationLib: Remove inline X86 assembly code X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 05 Mar 2019 21:45:26 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Liming, I agree .nasm files replacing .S/.asm files. But the use of inline assembly in C files for some primitives does produce much smaller/faster code. I would prefer that this BZ identify the specific functions that you think would provide better maintainability with no impact to size or performance. Thanks, Mike > -----Original Message----- > From: Gao, Liming > Sent: Tuesday, March 5, 2019 1:33 PM > To: afish@apple.com; Zhang, Shenglei > > Cc: edk2-devel-01 ; Kinney, > Michael D > Subject: RE: [edk2] [PATCH 3/3] > MdePkg/BaseSynchronizationLib: Remove inline X86 > assembly code >=20 > Andrew: > BZ 1163 is to remove inline X86 assembly code in C > source file. But, this patch is wrong. I have gave my > comments to update this patch. >=20 > The change is to reduce the duplicated > implementation. It will be good on the code maintain. > Recently, one patch has to update .c and .nasm both. > Please see https://lists.01.org/pipermail/edk2- > devel/2019-February/037165.html. Beside this change, I > propose to remove all GAS assembly code for IA32 and > X64 arch. After those change, the patch owner will > collect the impact of the image size. >=20 > Thanks > Liming > > -----Original Message----- > > From: afish@apple.com [mailto:afish@apple.com] > > Sent: Tuesday, March 5, 2019 12:58 PM > > To: Zhang, Shenglei > > Cc: edk2-devel-01 ; Kinney, > Michael D ; Gao, Liming > > > Subject: Re: [edk2] [PATCH 3/3] > MdePkg/BaseSynchronizationLib: Remove inline X86 > assembly code > > > > Shenglei, > > > > I was confused by the names of these patches. From a > C point of view this is inline assembly: > > > > VOID > > EFIAPI > > CpuBreakpoint ( > > VOID > > ) > > { > > __asm__ __volatile__ ("int $3"); > > } > > > > These patches seem to be removing GAS and MASM > assembly files, but not the inline assembly *.c files? > > > > We don't want to remove the inline assembly from the > BaseLib as that could have size, performance, and > compiler optimization impact. > > > > For example CpuBreakpoint() for clang with LTO would > end up inlining a single byte. For i385 a call to > assembler would be 5 bytes at each > > call location plus the overhead of the function. So > that is a size increase and a performance overhead. For > a C complier calling an assembly > > function is a serializing event an that can restrict > optimizations. Thus having some limited inline assembly > support is very useful. > > > > Thanks, > > > > Andrew Fish > > > > > On Mar 4, 2019, at 5:40 PM, Shenglei Zhang > wrote: > > > > > > MdePkg BaseSynchronizationLib still uses the inline > X86 assembly code > > > in C code files.It should be updated to consume > nasm only. > > > https://bugzilla.tianocore.org/show_bug.cgi?id=3D1163 > > > > > > Cc: Michael D Kinney > > > Cc: Liming Gao > > > Contributed-under: TianoCore Contribution Agreement > 1.1 > > > Signed-off-by: Shenglei Zhang > > > > --- > > > > .../Library/BaseSynchronizationLib/BaseSynchronizationL > ib.inf | 2 -- > > > 1 file changed, 2 deletions(-) > > > > > > diff --git > a/MdePkg/Library/BaseSynchronizationLib/BaseSynchroniza > tionLib.inf > > > b/MdePkg/Library/BaseSynchronizationLib/BaseSynchroniza > tionLib.inf > > > index 32414b29fa..719dc1938d 100755 > > > --- > a/MdePkg/Library/BaseSynchronizationLib/BaseSynchroniza > tionLib.inf > > > +++ > b/MdePkg/Library/BaseSynchronizationLib/BaseSynchroniza > tionLib.inf > > > @@ -75,13 +75,11 @@ > > > > > > [Sources.ARM] > > > Synchronization.c > > > - Arm/Synchronization.asm | RVCT > > > Arm/Synchronization.S | GCC > > > > > > [Sources.AARCH64] > > > Synchronization.c > > > AArch64/Synchronization.S | GCC > > > - AArch64/Synchronization.asm | MSFT > > > > > > [Packages] > > > MdePkg/MdePkg.dec > > > -- > > > 2.18.0.windows.1 > > > > > > _______________________________________________ > > > edk2-devel mailing list > > > edk2-devel@lists.01.org > > > https://lists.01.org/mailman/listinfo/edk2-devel