Re: [PATCH 3/3] MdePkg/BaseSynchronizationLib: Remove inline X86 assembly code

[Andrew Fish <afish@apple.com>]

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.zhang@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.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
> ---
> .../Library/BaseSynchronizationLib/BaseSynchronizationLib.inf   | 2 --
> 1 file changed, 2 deletions(-)
> 
> diff --git a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.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