From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mx.groups.io with SMTP id smtpd.web11.29344.1590731200405005198 for ; Thu, 28 May 2020 22:46:40 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: ard.biesheuvel@arm.com) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id E50F455D; Thu, 28 May 2020 22:46:38 -0700 (PDT) Received: from [192.168.1.81] (unknown [10.37.8.249]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 6EB263F305; Thu, 28 May 2020 22:46:37 -0700 (PDT) Subject: Re: [edk2-devel] [PATCH] MdePkg/Include: AARCH64: disable outline atomics on GCC 10.2+ To: devel@edk2.groups.io, liming.gao@intel.com, "lersek@redhat.com" , Leif Lindholm Cc: "philmd@redhat.com" , "mliska@suse.cz" References: <20200522101202.15016-1-ard.biesheuvel@arm.com> <36effdcd-91a7-da00-855d-570e64d650b8@redhat.com> <5587f0f0-9f30-62ce-9d07-5eed4ac6daa7@arm.com> <20200526143725.GM1923@vanye> <0f2f90b9-e2fe-5658-e507-299a387e0ce5@redhat.com> <20200528100515.GC1923@vanye> <6d6e3bb8-647c-42c4-5143-f64f5e6ba81b@redhat.com> From: "Ard Biesheuvel" Message-ID: Date: Fri, 29 May 2020 07:46:34 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: quoted-printable 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=E2=80=99t pass build e= dk2-stable202005. So, you think this is a critical issue to catch stable ta= g 202005. >=20 > Ard: > For this patch, I have two minor comments. > 1) I suggest to remove Link: https://bugzilla.tianocore.org/show_bug.cgi= ?id=3D2723 from comments, because this information has been in the commit m= essage. I think it would be helpful to keep it but I won't insist. > 2) Can we think __GNUC_MINOR__ is always defined? Do we need to check it= s value after check whether it is defined or not? > Yes __GNUC_MINOR__ is always defined. > -----Original Message----- > From: devel@edk2.groups.io On Behalf Of Laszlo Er= sek > Sent: 2020=E5=B9=B45=E6=9C=8829=E6=97=A5 4:03 > To: Leif Lindholm > Cc: Ard Biesheuvel ; devel@edk2.groups.io; Gao, = Liming ; philmd@redhat.com; mliska@suse.cz > Subject: Re: [edk2-devel] [PATCH] MdePkg/Include: AARCH64: disable outli= ne atomics on GCC 10.2+ >=20 > 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 >>> 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. >=20 > OK. That's a good argument. From my POV, feel free to merge (both patche= s). >=20 > Thanks > Laszlo >=20 >> 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 >>> >> >=20 >=20 >=20 >=20 >=20 >=20 >=20