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: af...@apple.com; Zhang, Shenglei > <shenglei.zh...@intel.com> > Cc: edk2-devel-01 <edk2-devel@lists.01.org>; Kinney, > Michael D <michael.d.kin...@intel.com> > Subject: RE: [edk2] [PATCH 3/3] > MdePkg/BaseSynchronizationLib: Remove inline X86 > assembly code > > 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. > 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. > > Thanks > Liming > > -----Original Message----- > > From: af...@apple.com [mailto:af...@apple.com] > > Sent: Tuesday, March 5, 2019 12:58 PM > > To: Zhang, Shenglei <shenglei.zh...@intel.com> > > Cc: edk2-devel-01 <edk2-devel@lists.01.org>; Kinney, > Michael D <michael.d.kin...@intel.com>; Gao, Liming > <liming....@intel.com> > > 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 > <shenglei.zh...@intel.com> 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=1163 > > > > > > Cc: Michael D Kinney <michael.d.kin...@intel.com> > > > Cc: Liming Gao <liming....@intel.com> > > > Contributed-under: TianoCore Contribution Agreement > 1.1 > > > Signed-off-by: Shenglei Zhang > <shenglei.zh...@intel.com> > > > --- > > > > .../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 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
