GCC 10 enabled a feature by default that was introduced in GCC 9,
which results in atomic operations to be emitted as function calls
to intrinsics provided by a runtime library.

Atomics are hardly used in EDK2, which runs on a single CPU anyway,
and any benefit that would result from reusing library code that
implements these operations is defeated by the fact that every EDK2
module will need to have its own copy anyway.

So let's disable this feature on GCC versions that support the
pragma to do so (GCC 10.2 and up)

Link: https://bugzilla.tianocore.org/show_bug.cgi?id=2723
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
---
The GCC support for this pragma has already been pulled into the 10.2
release branch. I think we should consider adding this to the stable
tag, so that the issue can easily be resolved by upgrading the compiler.
Whether we add the intrinsics too is a separate matter, but we can
revisit that later.

 MdePkg/Include/AArch64/ProcessorBind.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/MdePkg/Include/AArch64/ProcessorBind.h b/MdePkg/Include/AArch64/ProcessorBind.h
index 896bf273ac7a..a3ca8f09e51c 100644
--- a/MdePkg/Include/AArch64/ProcessorBind.h
+++ b/MdePkg/Include/AArch64/ProcessorBind.h
@@ -24,6 +24,17 @@
 #pragma pack()
 #endif
 
+#if defined(__GNUC__) && !defined(__clang__)
+
+//
+// Disable GCC outline atomics
+// Link: https://bugzilla.tianocore.org/show_bug.cgi?id=2723
+//
+#if __GNUC__ > 10 || (__GNUC__ == 10 && __GNUC_MINOR__ >= 2)
+#pragma GCC target "no-outline-atomics"
+#endif
+#endif
+
 #if defined(_MSC_EXTENSIONS)
 
 //
-- 
2.17.1


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

View/Reply Online (#60108): https://edk2.groups.io/g/devel/message/60108
Mute This Topic: https://groups.io/mt/74396053/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On Fri, May 22, 2020 at 12:12:02 +0200, Ard Biesheuvel wrote:
> GCC 10 enabled a feature by default that was introduced in GCC 9,
> which results in atomic operations to be emitted as function calls
> to intrinsics provided by a runtime library.
> 
> Atomics are hardly used in EDK2, which runs on a single CPU anyway,
> and any benefit that would result from reusing library code that
> implements these operations is defeated by the fact that every EDK2
> module will need to have its own copy anyway.
> 
> So let's disable this feature on GCC versions that support the
> pragma to do so (GCC 10.2 and up)
> 
> Link: https://bugzilla.tianocore.org/show_bug.cgi?id=2723
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
>
> ---
> The GCC support for this pragma has already been pulled into the 10.2
> release branch. I think we should consider adding this to the stable
> tag, so that the issue can easily be resolved by upgrading the compiler.
> Whether we add the intrinsics too is a separate matter, but we can
> revisit that later.

Hmm. I am just slightly concerned over referencing GCC 10.2 before GCC
10.2 is released.
Presumably, there will be no ill effects if that pragma is enabled also
on 10.2, we just won't need it? If so ...

>  MdePkg/Include/AArch64/ProcessorBind.h | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/MdePkg/Include/AArch64/ProcessorBind.h b/MdePkg/Include/AArch64/ProcessorBind.h
> index 896bf273ac7a..a3ca8f09e51c 100644
> --- a/MdePkg/Include/AArch64/ProcessorBind.h
> +++ b/MdePkg/Include/AArch64/ProcessorBind.h
> @@ -24,6 +24,17 @@
>  #pragma pack()
>  #endif
>  
> +#if defined(__GNUC__) && !defined(__clang__)
> +
> +//
> +// Disable GCC outline atomics
> +// Link: https://bugzilla.tianocore.org/show_bug.cgi?id=2723
> +//
> +#if __GNUC__ > 10 || (__GNUC__ == 10 && __GNUC_MINOR__ >= 2)

Could we do >= 10 here for now, and update to this conditional once
we've verified that the change does really make it into 10.2?

If so, I support including it in the stable tag.

/
    Leif

> +#pragma GCC target "no-outline-atomics"
> +#endif
> +#endif
> +
>  #if defined(_MSC_EXTENSIONS)
>  
>  //
> -- 
> 2.17.1
> 

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

View/Reply Online (#60131): https://edk2.groups.io/g/devel/message/60131
Mute This Topic: https://groups.io/mt/74396053/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On 5/22/20 1:41 PM, Leif Lindholm wrote:
> On Fri, May 22, 2020 at 12:12:02 +0200, Ard Biesheuvel wrote:
>> GCC 10 enabled a feature by default that was introduced in GCC 9,
>> which results in atomic operations to be emitted as function calls
>> to intrinsics provided by a runtime library.
>>
>> Atomics are hardly used in EDK2, which runs on a single CPU anyway,
>> and any benefit that would result from reusing library code that
>> implements these operations is defeated by the fact that every EDK2
>> module will need to have its own copy anyway.
>>
>> So let's disable this feature on GCC versions that support the
>> pragma to do so (GCC 10.2 and up)
>>
>> Link: https://bugzilla.tianocore.org/show_bug.cgi?id=2723
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
>>
>> ---
>> The GCC support for this pragma has already been pulled into the 10.2
>> release branch. I think we should consider adding this to the stable
>> tag, so that the issue can easily be resolved by upgrading the compiler.
>> Whether we add the intrinsics too is a separate matter, but we can
>> revisit that later.
> 
> Hmm. I am just slightly concerned over referencing GCC 10.2 before GCC
> 10.2 is released.
> Presumably, there will be no ill effects if that pragma is enabled also
> on 10.2, we just won't need it? If so ...
> 

No the pragma is only enabled on 10.2 and later. Older GCCs will choke 
on it.

>>   MdePkg/Include/AArch64/ProcessorBind.h | 11 +++++++++++
>>   1 file changed, 11 insertions(+)
>>
>> diff --git a/MdePkg/Include/AArch64/ProcessorBind.h b/MdePkg/Include/AArch64/ProcessorBind.h
>> index 896bf273ac7a..a3ca8f09e51c 100644
>> --- a/MdePkg/Include/AArch64/ProcessorBind.h
>> +++ b/MdePkg/Include/AArch64/ProcessorBind.h
>> @@ -24,6 +24,17 @@
>>   #pragma pack()
>>   #endif
>>   
>> +#if defined(__GNUC__) && !defined(__clang__)
>> +
>> +//
>> +// Disable GCC outline atomics
>> +// Link: https://bugzilla.tianocore.org/show_bug.cgi?id=2723
>> +//
>> +#if __GNUC__ > 10 || (__GNUC__ == 10 && __GNUC_MINOR__ >= 2)
> 
> Could we do >= 10 here for now, and update to this conditional once
> we've verified that the change does really make it into 10.2?
> 

That would break all GCC 10, even if no users of the outline atomics are 
part of the build.

> If so, I support including it in the stable tag.
> 
> /
>      Leif
> 
>> +#pragma GCC target "no-outline-atomics"
>> +#endif
>> +#endif
>> +
>>   #if defined(_MSC_EXTENSIONS)
>>   
>>   //
>> -- 
>> 2.17.1
>>


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

View/Reply Online (#60134): https://edk2.groups.io/g/devel/message/60134
Mute This Topic: https://groups.io/mt/74396053/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On Fri, May 22, 2020 at 14:05:02 +0200, Ard Biesheuvel wrote:
> On 5/22/20 1:41 PM, Leif Lindholm wrote:
> > On Fri, May 22, 2020 at 12:12:02 +0200, Ard Biesheuvel wrote:
> > > GCC 10 enabled a feature by default that was introduced in GCC 9,
> > > which results in atomic operations to be emitted as function calls
> > > to intrinsics provided by a runtime library.
> > > 
> > > Atomics are hardly used in EDK2, which runs on a single CPU anyway,
> > > and any benefit that would result from reusing library code that
> > > implements these operations is defeated by the fact that every EDK2
> > > module will need to have its own copy anyway.
> > > 
> > > So let's disable this feature on GCC versions that support the
> > > pragma to do so (GCC 10.2 and up)
> > > 
> > > Link: https://bugzilla.tianocore.org/show_bug.cgi?id=2723
> > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
> > > 
> > > ---
> > > The GCC support for this pragma has already been pulled into the 10.2
> > > release branch. I think we should consider adding this to the stable
> > > tag, so that the issue can easily be resolved by upgrading the compiler.
> > > Whether we add the intrinsics too is a separate matter, but we can
> > > revisit that later.
> > 
> > Hmm. I am just slightly concerned over referencing GCC 10.2 before GCC
> > 10.2 is released.
> > Presumably, there will be no ill effects if that pragma is enabled also
> > on 10.2, we just won't need it? If so ...
> > 
> 
> No the pragma is only enabled on 10.2 and later. Older GCCs will choke on
> it.

Urgh, right. I guess I'm having a "can't read" day.

Still. That means that if the pragma gets pulled last second from
10.2, we now explicitly break builds for 10.2.
No matter how low the risk of that happening, this doesn't make me
comfortable.

/
    Leif

> > >   MdePkg/Include/AArch64/ProcessorBind.h | 11 +++++++++++
> > >   1 file changed, 11 insertions(+)
> > > 
> > > diff --git a/MdePkg/Include/AArch64/ProcessorBind.h b/MdePkg/Include/AArch64/ProcessorBind.h
> > > index 896bf273ac7a..a3ca8f09e51c 100644
> > > --- a/MdePkg/Include/AArch64/ProcessorBind.h
> > > +++ b/MdePkg/Include/AArch64/ProcessorBind.h
> > > @@ -24,6 +24,17 @@
> > >   #pragma pack()
> > >   #endif
> > > +#if defined(__GNUC__) && !defined(__clang__)
> > > +
> > > +//
> > > +// Disable GCC outline atomics
> > > +// Link: https://bugzilla.tianocore.org/show_bug.cgi?id=2723
> > > +//
> > > +#if __GNUC__ > 10 || (__GNUC__ == 10 && __GNUC_MINOR__ >= 2)
> > 
> > Could we do >= 10 here for now, and update to this conditional once
> > we've verified that the change does really make it into 10.2?
> > 
> 
> That would break all GCC 10, even if no users of the outline atomics are
> part of the build.
> 
> > If so, I support including it in the stable tag.
> > 
> > /
> >      Leif
> > 
> > > +#pragma GCC target "no-outline-atomics"
> > > +#endif
> > > +#endif
> > > +
> > >   #if defined(_MSC_EXTENSIONS)
> > >   //
> > > -- 
> > > 2.17.1
> > > 
> 

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

View/Reply Online (#60136): https://edk2.groups.io/g/devel/message/60136
Mute This Topic: https://groups.io/mt/74396053/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On 5/22/20 2:14 PM, Leif Lindholm wrote:
> On Fri, May 22, 2020 at 14:05:02 +0200, Ard Biesheuvel wrote:
>> On 5/22/20 1:41 PM, Leif Lindholm wrote:
>>> On Fri, May 22, 2020 at 12:12:02 +0200, Ard Biesheuvel wrote:
>>>> GCC 10 enabled a feature by default that was introduced in GCC 9,
>>>> which results in atomic operations to be emitted as function calls
>>>> to intrinsics provided by a runtime library.
>>>>
>>>> Atomics are hardly used in EDK2, which runs on a single CPU anyway,
>>>> and any benefit that would result from reusing library code that
>>>> implements these operations is defeated by the fact that every EDK2
>>>> module will need to have its own copy anyway.
>>>>
>>>> So let's disable this feature on GCC versions that support the
>>>> pragma to do so (GCC 10.2 and up)
>>>>
>>>> Link: https://bugzilla.tianocore.org/show_bug.cgi?id=2723
>>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
>>>>
>>>> ---
>>>> The GCC support for this pragma has already been pulled into the 10.2
>>>> release branch. I think we should consider adding this to the stable
>>>> tag, so that the issue can easily be resolved by upgrading the compiler.
>>>> Whether we add the intrinsics too is a separate matter, but we can
>>>> revisit that later.
>>>
>>> Hmm. I am just slightly concerned over referencing GCC 10.2 before GCC
>>> 10.2 is released.
>>> Presumably, there will be no ill effects if that pragma is enabled also
>>> on 10.2, we just won't need it? If so ...
>>>
>>
>> No the pragma is only enabled on 10.2 and later. Older GCCs will choke on
>> it.
> 
> Urgh, right. I guess I'm having a "can't read" day.
> 
> Still. That means that if the pragma gets pulled last second from
> 10.2, we now explicitly break builds for 10.2.
> No matter how low the risk of that happening, this doesn't make me
> comfortable.
> 

Indeed. So assuming that we won't take the atomics patch for the stable 
tag, we already know it will be broken for all GCC 10 and up.


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

View/Reply Online (#60138): https://edk2.groups.io/g/devel/message/60138
Mute This Topic: https://groups.io/mt/74396053/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On Fri, May 22, 2020 at 14:42:34 +0200, Ard Biesheuvel wrote:
> > > > > ---
> > > > > The GCC support for this pragma has already been pulled into the 10.2
> > > > > release branch. I think we should consider adding this to the stable
> > > > > tag, so that the issue can easily be resolved by upgrading the compiler.
> > > > > Whether we add the intrinsics too is a separate matter, but we can
> > > > > revisit that later.
> > > > 
> > > > Hmm. I am just slightly concerned over referencing GCC 10.2 before GCC
> > > > 10.2 is released.
> > > > Presumably, there will be no ill effects if that pragma is enabled also
> > > > on 10.2, we just won't need it? If so ...
> > > > 
> > > 
> > > No the pragma is only enabled on 10.2 and later. Older GCCs will choke on
> > > it.
> > 
> > Urgh, right. I guess I'm having a "can't read" day.
> > 
> > Still. That means that if the pragma gets pulled last second from
> > 10.2, we now explicitly break builds for 10.2.
> > No matter how low the risk of that happening, this doesn't make me
> > comfortable.
> 
> Indeed. So assuming that we won't take the atomics patch for the stable tag,
> we already know it will be broken for all GCC 10 and up.

Which isn't ideal, as we're seeing people picking that up.

I think in a list of not great options I would probably prefer getting
the intrinsics into the stable tag.

/
    Leif

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

View/Reply Online (#60140): https://edk2.groups.io/g/devel/message/60140
Mute This Topic: https://groups.io/mt/74396053/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On 05/22/20 12:12, Ard Biesheuvel wrote:
> GCC 10 enabled a feature by default that was introduced in GCC 9,
> which results in atomic operations to be emitted as function calls
> to intrinsics provided by a runtime library.
> 
> Atomics are hardly used in EDK2, which runs on a single CPU anyway,
> and any benefit that would result from reusing library code that
> implements these operations is defeated by the fact that every EDK2
> module will need to have its own copy anyway.
> 
> So let's disable this feature on GCC versions that support the
> pragma to do so (GCC 10.2 and up)
> 
> Link: https://bugzilla.tianocore.org/show_bug.cgi?id=2723
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
> ---
> The GCC support for this pragma has already been pulled into the 10.2
> release branch. I think we should consider adding this to the stable
> tag, so that the issue can easily be resolved by upgrading the compiler.
> Whether we add the intrinsics too is a separate matter, but we can
> revisit that later.
> 
>  MdePkg/Include/AArch64/ProcessorBind.h | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/MdePkg/Include/AArch64/ProcessorBind.h b/MdePkg/Include/AArch64/ProcessorBind.h
> index 896bf273ac7a..a3ca8f09e51c 100644
> --- a/MdePkg/Include/AArch64/ProcessorBind.h
> +++ b/MdePkg/Include/AArch64/ProcessorBind.h
> @@ -24,6 +24,17 @@
>  #pragma pack()
>  #endif
>  
> +#if defined(__GNUC__) && !defined(__clang__)
> +
> +//
> +// Disable GCC outline atomics
> +// Link: https://bugzilla.tianocore.org/show_bug.cgi?id=2723
> +//
> +#if __GNUC__ > 10 || (__GNUC__ == 10 && __GNUC_MINOR__ >= 2)
> +#pragma GCC target "no-outline-atomics"
> +#endif
> +#endif
> +
>  #if defined(_MSC_EXTENSIONS)
>  
>  //
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

But I think it should be merged later, after GCC 10.2 is out.

(Obviously I don't "insist" that we follow this approach, I'm just OK
with it.)

Thanks
Laszlo


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

View/Reply Online (#60169): https://edk2.groups.io/g/devel/message/60169
Mute This Topic: https://groups.io/mt/74396053/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On 05/22/20 22:01, Laszlo Ersek wrote:
> On 05/22/20 12:12, Ard Biesheuvel wrote:
>> GCC 10 enabled a feature by default that was introduced in GCC 9,
>> which results in atomic operations to be emitted as function calls
>> to intrinsics provided by a runtime library.
>>
>> Atomics are hardly used in EDK2, which runs on a single CPU anyway,
>> and any benefit that would result from reusing library code that
>> implements these operations is defeated by the fact that every EDK2
>> module will need to have its own copy anyway.
>>
>> So let's disable this feature on GCC versions that support the
>> pragma to do so (GCC 10.2 and up)
>>
>> Link: https://bugzilla.tianocore.org/show_bug.cgi?id=2723
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
>> ---
>> The GCC support for this pragma has already been pulled into the 10.2
>> release branch. I think we should consider adding this to the stable
>> tag, so that the issue can easily be resolved by upgrading the compiler.
>> Whether we add the intrinsics too is a separate matter, but we can
>> revisit that later.
>>
>>  MdePkg/Include/AArch64/ProcessorBind.h | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/MdePkg/Include/AArch64/ProcessorBind.h b/MdePkg/Include/AArch64/ProcessorBind.h
>> index 896bf273ac7a..a3ca8f09e51c 100644
>> --- a/MdePkg/Include/AArch64/ProcessorBind.h
>> +++ b/MdePkg/Include/AArch64/ProcessorBind.h
>> @@ -24,6 +24,17 @@
>>  #pragma pack()
>>  #endif
>>  
>> +#if defined(__GNUC__) && !defined(__clang__)
>> +
>> +//
>> +// Disable GCC outline atomics
>> +// Link: https://bugzilla.tianocore.org/show_bug.cgi?id=2723
>> +//
>> +#if __GNUC__ > 10 || (__GNUC__ == 10 && __GNUC_MINOR__ >= 2)
>> +#pragma GCC target "no-outline-atomics"
>> +#endif
>> +#endif
>> +
>>  #if defined(_MSC_EXTENSIONS)
>>  
>>  //
>>
> 
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> 
> But I think it should be merged later, after GCC 10.2 is out.
> 
> (Obviously I don't "insist" that we follow this approach, I'm just OK
> with it.)

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.

Thanks,
Laszlo


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

View/Reply Online (#60170): https://edk2.groups.io/g/devel/message/60170
Mute This Topic: https://groups.io/mt/74396053/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On 5/22/20 10:05 PM, Laszlo Ersek via groups.io wrote:
> On 05/22/20 22:01, Laszlo Ersek wrote:
>> On 05/22/20 12:12, Ard Biesheuvel wrote:
>>> GCC 10 enabled a feature by default that was introduced in GCC 9,
>>> which results in atomic operations to be emitted as function calls
>>> to intrinsics provided by a runtime library.
>>>
>>> Atomics are hardly used in EDK2, which runs on a single CPU anyway,
>>> and any benefit that would result from reusing library code that
>>> implements these operations is defeated by the fact that every EDK2
>>> module will need to have its own copy anyway.
>>>
>>> So let's disable this feature on GCC versions that support the
>>> pragma to do so (GCC 10.2 and up)
>>>
>>> Link: https://bugzilla.tianocore.org/show_bug.cgi?id=2723
>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
>>> ---
>>> The GCC support for this pragma has already been pulled into the 10.2
>>> release branch. I think we should consider adding this to the stable
>>> tag, so that the issue can easily be resolved by upgrading the compiler.
>>> Whether we add the intrinsics too is a separate matter, but we can
>>> revisit that later.
>>>
>>>   MdePkg/Include/AArch64/ProcessorBind.h | 11 +++++++++++
>>>   1 file changed, 11 insertions(+)
>>>
>>> diff --git a/MdePkg/Include/AArch64/ProcessorBind.h b/MdePkg/Include/AArch64/ProcessorBind.h
>>> index 896bf273ac7a..a3ca8f09e51c 100644
>>> --- a/MdePkg/Include/AArch64/ProcessorBind.h
>>> +++ b/MdePkg/Include/AArch64/ProcessorBind.h
>>> @@ -24,6 +24,17 @@
>>>   #pragma pack()
>>>   #endif
>>>   
>>> +#if defined(__GNUC__) && !defined(__clang__)
>>> +
>>> +//
>>> +// Disable GCC outline atomics
>>> +// Link: https://bugzilla.tianocore.org/show_bug.cgi?id=2723
>>> +//
>>> +#if __GNUC__ > 10 || (__GNUC__ == 10 && __GNUC_MINOR__ >= 2)
>>> +#pragma GCC target "no-outline-atomics"
>>> +#endif
>>> +#endif
>>> +
>>>   #if defined(_MSC_EXTENSIONS)
>>>   
>>>   //
>>>
>>
>> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>>
>> But I think it should be merged later, after GCC 10.2 is out.
>>
>> (Obviously I don't "insist" that we follow this approach, I'm just OK
>> with it.)
> 
> 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.



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

View/Reply Online (#60177): https://edk2.groups.io/g/devel/message/60177
Mute This Topic: https://groups.io/mt/74396053/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On Sat, May 23, 2020 at 00:09:52 +0200, Ard Biesheuvel wrote:
> > > > diff --git a/MdePkg/Include/AArch64/ProcessorBind.h b/MdePkg/Include/AArch64/ProcessorBind.h
> > > > index 896bf273ac7a..a3ca8f09e51c 100644
> > > > --- a/MdePkg/Include/AArch64/ProcessorBind.h
> > > > +++ b/MdePkg/Include/AArch64/ProcessorBind.h
> > > > @@ -24,6 +24,17 @@
> > > >   #pragma pack()
> > > >   #endif
> > > > +#if defined(__GNUC__) && !defined(__clang__)
> > > > +
> > > > +//
> > > > +// Disable GCC outline atomics
> > > > +// Link: https://bugzilla.tianocore.org/show_bug.cgi?id=2723
> > > > +//
> > > > +#if __GNUC__ > 10 || (__GNUC__ == 10 && __GNUC_MINOR__ >= 2)
> > > > +#pragma GCC target "no-outline-atomics"
> > > > +#endif
> > > > +#endif
> > > > +
> > > >   #if defined(_MSC_EXTENSIONS)
> > > >   //
> > > 
> > > Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> > > 
> > > But I think it should be merged later, after GCC 10.2 is out.
> > > 
> > > (Obviously I don't "insist" that we follow this approach, I'm just OK
> > > with it.)
> > 
> > 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.

This isn't just affecting random crazies pulling latest toolchains
down, but people using their distro defaults (native or cross).
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.

/
    Leif

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

View/Reply Online (#60280): https://edk2.groups.io/g/devel/message/60280
Mute This Topic: https://groups.io/mt/74396053/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On 05/26/20 16:37, Leif Lindholm wrote:
> On Sat, May 23, 2020 at 00:09:52 +0200, Ard Biesheuvel wrote:
>>>>> diff --git a/MdePkg/Include/AArch64/ProcessorBind.h b/MdePkg/Include/AArch64/ProcessorBind.h
>>>>> index 896bf273ac7a..a3ca8f09e51c 100644
>>>>> --- a/MdePkg/Include/AArch64/ProcessorBind.h
>>>>> +++ b/MdePkg/Include/AArch64/ProcessorBind.h
>>>>> @@ -24,6 +24,17 @@
>>>>>   #pragma pack()
>>>>>   #endif
>>>>> +#if defined(__GNUC__) && !defined(__clang__)
>>>>> +
>>>>> +//
>>>>> +// Disable GCC outline atomics
>>>>> +// Link: https://bugzilla.tianocore.org/show_bug.cgi?id=2723
>>>>> +//
>>>>> +#if __GNUC__ > 10 || (__GNUC__ == 10 && __GNUC_MINOR__ >= 2)
>>>>> +#pragma GCC target "no-outline-atomics"
>>>>> +#endif
>>>>> +#endif
>>>>> +
>>>>>   #if defined(_MSC_EXTENSIONS)
>>>>>   //
>>>>
>>>> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>>>>
>>>> But I think it should be merged later, after GCC 10.2 is out.
>>>>
>>>> (Obviously I don't "insist" that we follow this approach, I'm just OK
>>>> with it.)
>>>
>>> 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?

> 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.

> 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.

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 (#60330): https://edk2.groups.io/g/devel/message/60330
Mute This Topic: https://groups.io/mt/74396053/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On Wed, May 27, 2020 at 11:12:23AM +0200, Laszlo Ersek wrote:
> On 05/26/20 16:37, Leif Lindholm wrote:
> > On Sat, May 23, 2020 at 00:09:52 +0200, Ard Biesheuvel wrote:
> >>>>> diff --git a/MdePkg/Include/AArch64/ProcessorBind.h b/MdePkg/Include/AArch64/ProcessorBind.h
> >>>>> index 896bf273ac7a..a3ca8f09e51c 100644
> >>>>> --- a/MdePkg/Include/AArch64/ProcessorBind.h
> >>>>> +++ b/MdePkg/Include/AArch64/ProcessorBind.h
> >>>>> @@ -24,6 +24,17 @@
> >>>>>   #pragma pack()
> >>>>>   #endif
> >>>>> +#if defined(__GNUC__) && !defined(__clang__)
> >>>>> +
> >>>>> +//
> >>>>> +// Disable GCC outline atomics
> >>>>> +// Link: https://bugzilla.tianocore.org/show_bug.cgi?id=2723
> >>>>> +//
> >>>>> +#if __GNUC__ > 10 || (__GNUC__ == 10 && __GNUC_MINOR__ >= 2)
> >>>>> +#pragma GCC target "no-outline-atomics"
> >>>>> +#endif
> >>>>> +#endif
> >>>>> +
> >>>>>   #if defined(_MSC_EXTENSIONS)
> >>>>>   //
> >>>>
> >>>> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> >>>>
> >>>> But I think it should be merged later, after GCC 10.2 is out.
> >>>>
> >>>> (Obviously I don't "insist" that we follow this approach, I'm just OK
> >>>> with it.)
> >>>
> >>> 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?
> 
> > 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.
> 
openSUSE Tumbleweed is going to change the default gcc version to 10.
I actually wrote a workaround patch to add "-mno-outline-atomics" and
apply it conditionally in the rpm spec file. Anyway, we can live with
the workaround, so it's not intolerant to me if the intrinsics patch is
not included.

> > 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.
> 
> 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.
> 
I thought the patch would be abondoned so didn't try it. Will test it soon.

Cheers,

Gary Lin

> Thanks,
> Laszlo
> 
> 
> 
> 

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

View/Reply Online (#60332): https://edk2.groups.io/g/devel/message/60332
Mute This Topic: https://groups.io/mt/74396053/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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. 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 (#60394): https://edk2.groups.io/g/devel/message/60394
Mute This Topic: https://groups.io/mt/74396053/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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 (#60412): https://edk2.groups.io/g/devel/message/60412
Mute This Topic: https://groups.io/mt/74396053/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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. 
2) Can we think __GNUC_MINOR__ is always defined? Do we need to check its value after check whether it is defined or not?

Thanks
Liming
-----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 <leif@nuviainc.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>; devel@edk2.groups.io; Gao, Liming <liming.gao@intel.com>; philmd@redhat.com; mliska@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 (#60425): https://edk2.groups.io/g/devel/message/60425
Mute This Topic: https://groups.io/mt/74396053/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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.

> 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 <leif@nuviainc.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>; devel@edk2.groups.io; Gao, Liming <liming.gao@intel.com>; philmd@redhat.com; mliska@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 (#60427): https://edk2.groups.io/g/devel/message/60427
Mute This Topic: https://groups.io/mt/74396053/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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.gao@intel.com>; lersek@redhat.com; Leif Lindholm <leif@nuviainc.com>
> Cc: philmd@redhat.com; mliska@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.gao@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 <leif@nuviainc.com>
> > Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>; devel@edk2.groups.io; Gao, Liming <liming.gao@intel.com>; philmd@redhat.com;
> mliska@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/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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.gao@intel.com>; lersek@redhat.com; Leif Lindholm <leif@nuviainc.com>
>> Cc: philmd@redhat.com; mliska@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.gao@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 <leif@nuviainc.com>
>>> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>; devel@edk2.groups.io; Gao, Liming <liming.gao@intel.com>; philmd@redhat.com;
>> mliska@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/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Ard: 
  Lefi requests to catch this change into 202005 stable tag. I also highlight this request in hard feature freeze notice mail https://edk2.groups.io/g/devel/message/60421.

  If no objection before the middle of next week (2020-06-03), this patch can be merged with the updated comments.

Thanks
Liming
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ard Biesheuvel
> Sent: Saturday, May 30, 2020 12:51 AM
> To: Gao, Liming <liming.gao@intel.com>; devel@edk2.groups.io; lersek@redhat.com; Leif Lindholm <leif@nuviainc.com>
> Cc: philmd@redhat.com; mliska@suse.cz; Kinney, Michael D <michael.d.kinney@intel.com>; afish@apple.com
> Subject: Re: [edk2-devel] [PATCH] MdePkg/Include: AARCH64: disable outline atomics on GCC 10.2+
> 
> 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.gao@intel.com>; lersek@redhat.com; Leif Lindholm <leif@nuviainc.com>
> >> Cc: philmd@redhat.com; mliska@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.gao@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 <leif@nuviainc.com>
> >>> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>; devel@edk2.groups.io; Gao, Liming <liming.gao@intel.com>; philmd@redhat.com;
> >> mliska@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 (#60470): https://edk2.groups.io/g/devel/message/60470
Mute This Topic: https://groups.io/mt/74396053/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Hi Liming,

Apologies if I have caused confusion (since we have been discussing
the same bug on both patches). This patch is *not* required for the
stable tag. It will be useful to include *after* the stable tag.

The intrinsics patch on its own resolves the problem (by providing
resolutions for the generated function calls), whereas *this* patch
tells GCC 10.2 or later (not yet released) not to generate those calls
in the first place.

Best Regards,

Leif

On Sat, May 30, 2020 at 15:10:07 +0000, Gao, Liming wrote:
> Ard: 
>   Lefi requests to catch this change into 202005 stable tag. I also
>   highlight this request in hard feature freeze notice mail
>   https://edk2.groups.io/g/devel/message/60421.
> 
>   If no objection before the middle of next week (2020-06-03), this
>   patch can be merged with the updated comments.
> 
> Thanks
> Liming
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ard Biesheuvel
> > Sent: Saturday, May 30, 2020 12:51 AM
> > To: Gao, Liming <liming.gao@intel.com>; devel@edk2.groups.io; lersek@redhat.com; Leif Lindholm <leif@nuviainc.com>
> > Cc: philmd@redhat.com; mliska@suse.cz; Kinney, Michael D <michael.d.kinney@intel.com>; afish@apple.com
> > Subject: Re: [edk2-devel] [PATCH] MdePkg/Include: AARCH64: disable outline atomics on GCC 10.2+
> > 
> > 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.gao@intel.com>; lersek@redhat.com; Leif Lindholm <leif@nuviainc.com>
> > >> Cc: philmd@redhat.com; mliska@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.gao@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 <leif@nuviainc.com>
> > >>> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>; devel@edk2.groups.io; Gao, Liming <liming.gao@intel.com>; philmd@redhat.com;
> > >> mliska@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 (#60471): https://edk2.groups.io/g/devel/message/60471
Mute This Topic: https://groups.io/mt/74396053/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Leif:
  Thanks for your clarification. So, you request only one in ArmPkg to catch this stable tag 202005. 

  https://edk2.groups.io/g/devel/message/59961 [PATCH v2] ArmPkg/CompilerIntrinsicsLib: provide atomics intrinsics

Thanks
Liming
> -----Original Message-----
> From: Leif Lindholm <leif@nuviainc.com>
> Sent: Saturday, May 30, 2020 11:23 PM
> To: Gao, Liming <liming.gao@intel.com>
> Cc: devel@edk2.groups.io; ard.biesheuvel@arm.com; lersek@redhat.com; philmd@redhat.com; mliska@suse.cz; Kinney, Michael D
> <michael.d.kinney@intel.com>; afish@apple.com
> Subject: Re: [edk2-devel] [PATCH] MdePkg/Include: AARCH64: disable outline atomics on GCC 10.2+
> 
> Hi Liming,
> 
> Apologies if I have caused confusion (since we have been discussing
> the same bug on both patches). This patch is *not* required for the
> stable tag. It will be useful to include *after* the stable tag.
> 
> The intrinsics patch on its own resolves the problem (by providing
> resolutions for the generated function calls), whereas *this* patch
> tells GCC 10.2 or later (not yet released) not to generate those calls
> in the first place.
> 
> Best Regards,
> 
> Leif
> 
> On Sat, May 30, 2020 at 15:10:07 +0000, Gao, Liming wrote:
> > Ard:
> >   Lefi requests to catch this change into 202005 stable tag. I also
> >   highlight this request in hard feature freeze notice mail
> >   https://edk2.groups.io/g/devel/message/60421.
> >
> >   If no objection before the middle of next week (2020-06-03), this
> >   patch can be merged with the updated comments.
> >
> > Thanks
> > Liming
> > > -----Original Message-----
> > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ard Biesheuvel
> > > Sent: Saturday, May 30, 2020 12:51 AM
> > > To: Gao, Liming <liming.gao@intel.com>; devel@edk2.groups.io; lersek@redhat.com; Leif Lindholm <leif@nuviainc.com>
> > > Cc: philmd@redhat.com; mliska@suse.cz; Kinney, Michael D <michael.d.kinney@intel.com>; afish@apple.com
> > > Subject: Re: [edk2-devel] [PATCH] MdePkg/Include: AARCH64: disable outline atomics on GCC 10.2+
> > >
> > > 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.gao@intel.com>; lersek@redhat.com; Leif Lindholm <leif@nuviainc.com>
> > > >> Cc: philmd@redhat.com; mliska@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.gao@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 <leif@nuviainc.com>
> > > >>> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>; devel@edk2.groups.io; Gao, Liming <liming.gao@intel.com>;
> philmd@redhat.com;
> > > >> mliska@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 (#60472): https://edk2.groups.io/g/devel/message/60472
Mute This Topic: https://groups.io/mt/74396053/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Yes please.

Best Regards,

Leif

On Sat, May 30, 2020 at 15:32:16 +0000, Liming Gao wrote:
> Leif:
>   Thanks for your clarification. So, you request only one in ArmPkg to catch this stable tag 202005. 
> 
>   https://edk2.groups.io/g/devel/message/59961 [PATCH v2] ArmPkg/CompilerIntrinsicsLib: provide atomics intrinsics
> 
> Thanks
> Liming
> > -----Original Message-----
> > From: Leif Lindholm <leif@nuviainc.com>
> > Sent: Saturday, May 30, 2020 11:23 PM
> > To: Gao, Liming <liming.gao@intel.com>
> > Cc: devel@edk2.groups.io; ard.biesheuvel@arm.com; lersek@redhat.com; philmd@redhat.com; mliska@suse.cz; Kinney, Michael D
> > <michael.d.kinney@intel.com>; afish@apple.com
> > Subject: Re: [edk2-devel] [PATCH] MdePkg/Include: AARCH64: disable outline atomics on GCC 10.2+
> > 
> > Hi Liming,
> > 
> > Apologies if I have caused confusion (since we have been discussing
> > the same bug on both patches). This patch is *not* required for the
> > stable tag. It will be useful to include *after* the stable tag.
> > 
> > The intrinsics patch on its own resolves the problem (by providing
> > resolutions for the generated function calls), whereas *this* patch
> > tells GCC 10.2 or later (not yet released) not to generate those calls
> > in the first place.
> > 
> > Best Regards,
> > 
> > Leif
> > 
> > On Sat, May 30, 2020 at 15:10:07 +0000, Gao, Liming wrote:
> > > Ard:
> > >   Lefi requests to catch this change into 202005 stable tag. I also
> > >   highlight this request in hard feature freeze notice mail
> > >   https://edk2.groups.io/g/devel/message/60421.
> > >
> > >   If no objection before the middle of next week (2020-06-03), this
> > >   patch can be merged with the updated comments.
> > >
> > > Thanks
> > > Liming
> > > > -----Original Message-----
> > > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ard Biesheuvel
> > > > Sent: Saturday, May 30, 2020 12:51 AM
> > > > To: Gao, Liming <liming.gao@intel.com>; devel@edk2.groups.io; lersek@redhat.com; Leif Lindholm <leif@nuviainc.com>
> > > > Cc: philmd@redhat.com; mliska@suse.cz; Kinney, Michael D <michael.d.kinney@intel.com>; afish@apple.com
> > > > Subject: Re: [edk2-devel] [PATCH] MdePkg/Include: AARCH64: disable outline atomics on GCC 10.2+
> > > >
> > > > 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.gao@intel.com>; lersek@redhat.com; Leif Lindholm <leif@nuviainc.com>
> > > > >> Cc: philmd@redhat.com; mliska@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.gao@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 <leif@nuviainc.com>
> > > > >>> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>; devel@edk2.groups.io; Gao, Liming <liming.gao@intel.com>;
> > philmd@redhat.com;
> > > > >> mliska@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 (#60473): https://edk2.groups.io/g/devel/message/60473
Mute This Topic: https://groups.io/mt/74396053/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On 5/30/20 5:10 PM, Gao, Liming wrote:
> Ard:
>    Lefi requests to catch this change into 202005 stable tag. I also highlight this request in hard feature freeze notice mail https://edk2.groups.io/g/devel/message/60421.
> 
>    If no objection before the middle of next week (2020-06-03), this patch can be merged with the updated comments.
> 

If the intent is to allow things to stabilize a bit with this patch 
applied, before creating the tag, then the patch should be pushed 
earlier, no?

Then, we give it a few days so that we can revert it again if anyone 
finds any issues with it.

Pushing it right before creating the tag does not sound to me like the 
correct way to approach this.



>> -----Original Message-----
>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ard Biesheuvel
>> Sent: Saturday, May 30, 2020 12:51 AM
>> To: Gao, Liming <liming.gao@intel.com>; devel@edk2.groups.io; lersek@redhat.com; Leif Lindholm <leif@nuviainc.com>
>> Cc: philmd@redhat.com; mliska@suse.cz; Kinney, Michael D <michael.d.kinney@intel.com>; afish@apple.com
>> Subject: Re: [edk2-devel] [PATCH] MdePkg/Include: AARCH64: disable outline atomics on GCC 10.2+
>>
>> 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.gao@intel.com>; lersek@redhat.com; Leif Lindholm <leif@nuviainc.com>
>>>> Cc: philmd@redhat.com; mliska@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.gao@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 <leif@nuviainc.com>
>>>>> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>; devel@edk2.groups.io; Gao, Liming <liming.gao@intel.com>; philmd@redhat.com;
>>>> mliska@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 (#60501): https://edk2.groups.io/g/devel/message/60501
Mute This Topic: https://groups.io/mt/74396053/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Ard:
  First, Lefi has clarified the request. This patch doesn't need to catch the stable tag. 

  Second, I plan to wait 1~2 days to collect the feedback, the merge the changes. I want to avoid revert change. 
  I don't mean to merge them, then immediately create the stable tag. Sorry if I bring confuse to you. 

  Last, on current plan https://edk2.groups.io/g/devel/message/60474, I will merge two patches tomorrow (06-02), then create the stable tag (06-03).

Thanks
Liming
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ard Biesheuvel
> Sent: Monday, June 1, 2020 6:32 PM
> To: Gao, Liming <liming.gao@intel.com>; devel@edk2.groups.io; lersek@redhat.com; Leif Lindholm <leif@nuviainc.com>
> Cc: philmd@redhat.com; mliska@suse.cz; Kinney, Michael D <michael.d.kinney@intel.com>; afish@apple.com
> Subject: Re: [edk2-devel] [PATCH] MdePkg/Include: AARCH64: disable outline atomics on GCC 10.2+
> 
> On 5/30/20 5:10 PM, Gao, Liming wrote:
> > Ard:
> >    Lefi requests to catch this change into 202005 stable tag. I also highlight this request in hard feature freeze notice mail
> https://edk2.groups.io/g/devel/message/60421.
> >
> >    If no objection before the middle of next week (2020-06-03), this patch can be merged with the updated comments.
> >
> 
> If the intent is to allow things to stabilize a bit with this patch
> applied, before creating the tag, then the patch should be pushed
> earlier, no?
> 
> Then, we give it a few days so that we can revert it again if anyone
> finds any issues with it.
> 
> Pushing it right before creating the tag does not sound to me like the
> correct way to approach this.
> 
> 
> 
> >> -----Original Message-----
> >> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ard Biesheuvel
> >> Sent: Saturday, May 30, 2020 12:51 AM
> >> To: Gao, Liming <liming.gao@intel.com>; devel@edk2.groups.io; lersek@redhat.com; Leif Lindholm <leif@nuviainc.com>
> >> Cc: philmd@redhat.com; mliska@suse.cz; Kinney, Michael D <michael.d.kinney@intel.com>; afish@apple.com
> >> Subject: Re: [edk2-devel] [PATCH] MdePkg/Include: AARCH64: disable outline atomics on GCC 10.2+
> >>
> >> 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.gao@intel.com>; lersek@redhat.com; Leif Lindholm <leif@nuviainc.com>
> >>>> Cc: philmd@redhat.com; mliska@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.gao@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 <leif@nuviainc.com>
> >>>>> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>; devel@edk2.groups.io; Gao, Liming <liming.gao@intel.com>;
> philmd@redhat.com;
> >>>> mliska@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 (#60522): https://edk2.groups.io/g/devel/message/60522
Mute This Topic: https://groups.io/mt/74396053/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-