From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.115; helo=mga14.intel.com; envelope-from=liming.gao@intel.com; receiver=edk2-devel@lists.01.org Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) (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 2132E211D5075 for ; Tue, 5 Mar 2019 13:32:58 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 05 Mar 2019 13:32:57 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.58,445,1544515200"; d="scan'208";a="163205582" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by orsmga001.jf.intel.com with ESMTP; 05 Mar 2019 13:32:57 -0800 Received: from fmsmsx112.amr.corp.intel.com (10.18.116.6) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.408.0; Tue, 5 Mar 2019 13:32:56 -0800 Received: from shsmsx102.ccr.corp.intel.com (10.239.4.154) by FMSMSX112.amr.corp.intel.com (10.18.116.6) with Microsoft SMTP Server (TLS) id 14.3.408.0; Tue, 5 Mar 2019 13:32:55 -0800 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.74]) by shsmsx102.ccr.corp.intel.com ([169.254.2.163]) with mapi id 14.03.0415.000; Wed, 6 Mar 2019 05:32:53 +0800 From: "Gao, Liming" To: "afish@apple.com" , "Zhang, Shenglei" CC: edk2-devel-01 , "Kinney, Michael D" Thread-Topic: [edk2] [PATCH 3/3] MdePkg/BaseSynchronizationLib: Remove inline X86 assembly code Thread-Index: AQHU0vSE94gh21G/NUi0Z1yYhGmykqX9ACSAgACMrPA= Date: Tue, 5 Mar 2019 21:32:54 +0000 Message-ID: <4A89E2EF3DFEDB4C8BFDE51014F606A14E3FBBC2@SHSMSX104.ccr.corp.intel.com> References: <20190305014059.17988-1-shenglei.zhang@intel.com> <20190305014059.17988-4-shenglei.zhang@intel.com> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ctpclassification: CTP_NT x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNGRiYTAxNjctYmMwNS00MjRkLTkxYzItYWRjYTI1MTkwNjZmIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiWXZpekRZRTRXTnlGSEIxMnB3eTQ2OHBiNHVRWGIxWkd1dTI5bGpGV0FTVHcrTUVYOUR4VjZiYVpPNFBBcjZiRCJ9 dlp-product: dlpe-windows dlp-version: 11.0.400.15 dlp-reaction: no-action x-originating-ip: [10.239.127.40] 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:32:58 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable 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. 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. Pl= ease see https://lists.01.org/pipermail/edk2-devel/2019-February/037165.htm= l. Beside this change, I propose to remove all GAS assembly code for IA32 a= nd 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 inl= ine X86 assembly code >=20 > Shenglei, >=20 > I was confused by the names of these patches. From a C point of view this= is inline assembly: >=20 > VOID > EFIAPI > CpuBreakpoint ( > VOID > ) > { > __asm__ __volatile__ ("int $3"); > } >=20 > These patches seem to be removing GAS and MASM assembly files, but not th= e inline assembly *.c files? >=20 > We don't want to remove the inline assembly from the BaseLib as that coul= d have size, performance, and compiler optimization impact. >=20 > For example CpuBreakpoint() for clang with LTO would end up inlining a si= ngle 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 increa= se 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. >=20 > Thanks, >=20 > Andrew Fish >=20 > > On Mar 4, 2019, at 5:40 PM, Shenglei Zhang w= rote: > > > > 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/BaseSynchronizationLib.inf | 2 -- > > 1 file changed, 2 deletions(-) > > > > diff --git a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationL= ib.inf > b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf > > index 32414b29fa..719dc1938d 100755 > > --- a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf > > +++ b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.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