From: "Andrew Fish" <afish@apple.com>
To: Laszlo Ersek <lersek@redhat.com>
Cc: Leif Lindholm <leif@nuviainc.com>,
"Schaefer, Daniel (DualStudy)" <daniel.schaefer@hpe.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>,
"Chang, Abner (HPS SW/FW Technologist)" <abner.chang@hpe.com>,
"Chen, Gilbert" <gilbert.chen@hpe.com>,
Mike Kinney <michael.d.kinney@intel.com>,
"pete@akeo.ie" <pete@akeo.ie>,
Ard Biesheuvel <ard.biesheuvel@linaro.org>
Subject: Re: [edk2-devel] [PATCH v2 3/3] MdeModulePkg: Use CopyMem instead of GUID assignment
Date: Thu, 12 Mar 2020 12:51:40 -0700 [thread overview]
Message-ID: <AC7B6999-3309-480C-ACBB-A4AC115D3DEB@apple.com> (raw)
In-Reply-To: <ae3fccfa-6204-9f99-6831-3abdbe39969a@redhat.com>
> On Mar 12, 2020, at 12:36 PM, Laszlo Ersek <lersek@redhat.com> wrote:
>
> On 03/12/20 15:03, Leif Lindholm wrote:
>> +Ard, Laszlo.
>>
>> I think it would make sense to move it to MdeModulePkg (or MdePkg) and
>> rename it BaseCompilerIntrinsicsLib (it *is* a BASE library).
>>
>> As I alluded in my reply to Ray - x86 also have this problem, but to a
>> lesser extent, and ended up creating library functions to call instead
>> of using plain C for certain operations.
>> Which was probably the right decision when it was restricted to a
>> very few corner cases.
>
> I think people that are interested in IA32/X64 are happier with explicit
> CopyMem() calls that are optimized (via one of the several BaseMemoryLib
> instances, such as SSE2, REP + string instructions, MMX, "smart"
> (chunked) C code, etc), than with a naive loop, such as the one in
> "ArmPkg/Library/CompilerIntrinsicsLib/memcpy.c", that gets silently
> plugged into an intrinsic (such as a structure assignment).
>
> I mean, compiler intrinsics exist in the first place because they
> implement language features in a fast / performant manner, behind the
> scenes. If we replace the internals of an intrinsic with a slow / naive
> implementation, then the intrinsic has no more right to exist, the
> compiler could / should just generate the normal naive code.
>
> I don't mind the code movement, but I'd like to avoid using
> BaseCompilerIntrinsicsLib on IA32/X64. On those arches, I think it would
> only be an improvement if it had a configurable backend, similarly how
> CopyMem() is currently configurable.
>
> ... I guess I've gotten very used to calling CopyMem(), in place of
> structure assignment.
>
Laszlo,
My brain has flipped too.
For x86 I find smaller structure assignments can cause the optimizer to optimize away the memcpy/memset and you only see issue issues on a NOOPT build since DEBUG builds tend to be size optimized. I tend to hit this issue when I try to turn off optimizations to try and walk vendor code in the debugger. So code review is the 1st line of defense.
Thanks,
Andrew Fish
next prev parent reply other threads:[~2020-03-12 19:51 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-02 10:32 [PATCH v2 0/3] Allow building MdeModulePkg on non-x86 Daniel Schaefer
2020-03-02 10:32 ` [PATCH v2 1/3] MdeModulePkg: Restrict libraries using SMM to x86 Daniel Schaefer
2020-03-02 13:18 ` [edk2-devel] " Liming Gao
2020-03-02 17:38 ` Daniel Schaefer
2020-03-02 10:32 ` [PATCH v2 2/3] MdeModulePkg: Set PcdDxeIplSwitchToLongMode false on non-x86 Daniel Schaefer
2020-03-02 13:19 ` Liming Gao
2020-03-02 17:36 ` Daniel Schaefer
2020-03-02 10:32 ` [PATCH v2 3/3] MdeModulePkg: Use CopyMem instead of GUID assignment Daniel Schaefer
2020-03-02 13:38 ` [edk2-devel] " Liming Gao
2020-03-05 0:39 ` Dandan Bi
2020-03-12 5:58 ` [edk2-devel] " Wang, Jian J
2020-03-12 6:00 ` Abner Chang
2020-03-12 10:55 ` Leif Lindholm
2020-03-12 12:21 ` Ni, Ray
2020-03-12 13:53 ` [EXTERNAL] " Leif Lindholm
2020-03-20 7:24 ` Ni, Ray
2020-03-12 13:24 ` Daniel Schaefer
2020-03-12 14:03 ` Leif Lindholm
2020-03-12 14:33 ` Abner Chang
2020-03-12 14:44 ` Leif Lindholm
2020-03-12 14:57 ` Leif Lindholm
2020-03-13 3:57 ` Abner Chang
2020-03-12 19:42 ` Laszlo Ersek
2020-03-12 21:19 ` Leif Lindholm
2020-03-13 4:08 ` Abner Chang
2020-03-13 10:10 ` Leif Lindholm
2020-03-15 14:59 ` Abner Chang
2020-03-13 16:36 ` Laszlo Ersek
2020-03-12 19:36 ` Laszlo Ersek
2020-03-12 19:51 ` Andrew Fish [this message]
2020-03-12 21:04 ` Leif Lindholm
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=AC7B6999-3309-480C-ACBB-A4AC115D3DEB@apple.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox