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.web10.890.1590771093593779742 for ; Fri, 29 May 2020 09:51:34 -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 316FB1045; Fri, 29 May 2020 09:51:32 -0700 (PDT) Received: from [192.168.1.69] (unknown [10.37.8.220]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 98CE53F52E; Fri, 29 May 2020 09:51:30 -0700 (PDT) Subject: Re: [edk2-devel] [PATCH] MdePkg/Include: AARCH64: disable outline atomics on GCC 10.2+ To: "Gao, Liming" , "devel@edk2.groups.io" , "lersek@redhat.com" , Leif Lindholm Cc: "philmd@redhat.com" , "mliska@suse.cz" , "Kinney, Michael D" , "afish@apple.com" 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: <51509986-4950-30b9-73af-874bb991e355@arm.com> Date: Fri, 29 May 2020 18:51:28 +0200 User-Agent: Mozilla/5.0 (X11; Linux aarch64; rv:68.0) Gecko/20100101 Thunderbird/68.8.1 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 4:29 PM, Gao, Liming wrote: > Ard: >=20 >> -----Original Message----- >> From: devel@edk2.groups.io On Behalf Of Ard Bies= heuvel >> Sent: Friday, May 29, 2020 1:47 PM >> To: devel@edk2.groups.io; Gao, Liming ; lersek@re= dhat.com; Leif Lindholm >> Cc: philmd@redhat.com; mliska@suse.cz >> Subject: Re: [edk2-devel] [PATCH] MdePkg/Include: AARCH64: disable outl= ine 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=E2=80=99t pass >> build edk2-stable202005. So, you think this is a critical issue to catc= h stable tag 202005. >>> >>> Ard: >>> For this patch, I have two minor comments. >>> 1) I suggest to remove Link: https://bugzilla.tianocore.org/show_bug.c= gi?id=3D2723 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. >> >=20 > I agree this is useful. But, we record it in the commit message. I prefe= r to remove this link from source code. > With this change, Reviewed-by: Liming Gao >=20 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 On Behalf Of Laszlo = Ersek >>> 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 out= line 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 b= y >>>>>>> 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 reporte= d >>>>> 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 edk= 2 >>>>> 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 hav= e >>>>>> 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 i= n >>>>> 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 patc= hes). >>> >>> 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 >>>>> >>>> >>> >>> >>> >>> >>> >>> >>> >> >> >>=20 >=20