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> Thanks Liming > > 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 (#60457): https://edk2.groups.io/g/devel/message/60457 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] -=-=-=-=-=-=-=-=-=-=-=-
