On 5/29/20 4:29 PM, Gao, Liming wrote:
Ard:

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ard Biesheuvel
Sent: Friday, May 29, 2020 1:47 PM
To: devel@edk2.groups.io; Gao, Liming <liming....@intel.com>; ler...@redhat.com; Leif 
Lindholm <l...@nuviainc.com>
Cc: phi...@redhat.com; mli...@suse.cz
Subject: Re: [edk2-devel] [PATCH] MdePkg/Include: AARCH64: disable outline 
atomics on GCC 10.2+

On 5/29/20 5:18 AM, Liming Gao via groups.io wrote:
Leif:
   I get the point that the linux distribution default GCC version may be 10 or 
above. Without this fix, those developers can’t pass
build edk2-stable202005. So, you think this is a critical issue to catch stable 
tag 202005.

Ard:
    For this patch, I have two minor comments.
1) I suggest to remove Link: 
https://bugzilla.tianocore.org/show_bug.cgi?id=2723 from comments, because this 
information has
been in the commit message.

I think it would be helpful to keep it but I won't insist.


I agree this is useful. But, we record it in the commit message. I prefer to 
remove this link from source code.
With this change, Reviewed-by: Liming Gao <liming....@intel.com>



Works for me.

I will send a v2 after the stable tag is released.


2) Can we think __GNUC_MINOR__ is always defined? Do we need to check its value 
after check whether it is defined or not?


Yes __GNUC_MINOR__ is always defined.



-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo Ersek
Sent: 2020年5月29日 4:03
To: Leif Lindholm <l...@nuviainc.com>
Cc: Ard Biesheuvel <ard.biesheu...@arm.com>; devel@edk2.groups.io; Gao, Liming 
<liming....@intel.com>; phi...@redhat.com;
mli...@suse.cz
Subject: Re: [edk2-devel] [PATCH] MdePkg/Include: AARCH64: disable outline 
atomics on GCC 10.2+

On 05/28/20 12:05, Leif Lindholm wrote:
On Wed, May 27, 2020 at 11:12:23 +0200, Laszlo Ersek wrote:
Oh and I think both this patch and the assembly language
implementation for the atomics should be delayed after the stable
tag. gcc-10 is a new toolchain; so even if we don't introduce a
new toolchain tag such as
GCC10 for it, whatever we do in order to make it work, that's
feature enablement in my book.

Works for me. By the time the next stable tag comes around, early
adopters that are now on GCC 10.1 will likely have moved to 10.2 by
that time, and so we may not need the assembly patch at all.

I'm not ecstatic that we'll be releasing the first stable tag known
to break with current toolchains.

If this breakage affects "current toolchains", then why was
<https://bugzilla.tianocore.org/show_bug.cgi?id=2723> only reported
on 2020-May-19, four days into the soft feature freeze?

I agree the timing is crap.

This isn't just affecting random crazies pulling latest toolchains
down, but people using their distro defaults (native or cross).

... "people using their distro defaults" to *not* build upstream edk2
until 2020-May-19, apparently.

Or distro defaults changing in between. I mean, we could say "Arch is
the same as any other distro's unstable", but I wouldn't want to go
down that route - I know people who use it for developing also for
qemu and linux.

Argh, I also just realised the error report I saw two days after Ard's
intrinsics patch hit the list was not a public report. Yes, if this
had affected only in-development/unstable distributions, I agree this
isn't something we should try to deal with upstream.

I don't recall if 10.1 ended up being default in F32, but it was
definitely included. In Arch, it does appear default.

Debian/Ubuntu are unaffected in their stable releases.

I agree it's a transitional issue, but I would really prefer to have
the intrinsics included in the release.

OK, let's delay the release then, by a few days. I agree the present
patch may qualify as a bugfix, but the other patch with the assembly
language intrinsics doesn't. If it's really that important to have in
the upcoming stable tag, then it's worth delaying the tag for. I'm
fine delaying the release for it; it wouldn't be without precedent.

I would argue it *is* a bugfix, since it only has an effect on builds
that would otherwise fail.

OK. That's a good argument. From my POV, feel free to merge (both patches).

Thanks
Laszlo

But I also do think it is important enough to delay the release if we
feel that is necessary.

/
      Leif

Also, I think Ard's assembly language patch needs a Tested-by from
Gary at the least (reporter of TianoCore#2723). Please reach out to
him in that thread.

... More precisely, please *ping* Gary for a Tested-by in that
thread, because Ard CC'd him from the start, and even credited Gary
in the commit message.

Thanks,
Laszlo















-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#60460): https://edk2.groups.io/g/devel/message/60460
Mute This Topic: https://groups.io/mt/74396053/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to