[PATCH v1] MdePkg: Use __builtin_offset with CLANGPDB toolchain

[PATCH v1] MdePkg: Use __builtin_offset with CLANGPDB toolchain

[Alex James]

CLANGPDB does not define __GNUC__, but it does define __clang__. Check
for the __clang__ preprocessor definition to use __builtin_offsetof to
implement the OFFSET_OF macro.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Alex James <theracermaster@gmail.com>
---
 MdePkg/Include/Base.h | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/MdePkg/Include/Base.h b/MdePkg/Include/Base.h
index 4680e64136..e0bcd0ae67 100644
--- a/MdePkg/Include/Base.h
+++ b/MdePkg/Include/Base.h
@@ -781,11 +781,9 @@ typedef UINTN  *BASE_LIST;
   @return  Offset, in bytes, of field.
 
 **/
-#ifdef __GNUC__
-#if __GNUC__ >= 4
+#if (defined(__GNUC__) && __GNUC__ >= 4) || defined(__clang__)
 #define OFFSET_OF(TYPE, Field) ((UINTN) __builtin_offsetof(TYPE, Field))
 #endif
-#endif
 
 #ifndef OFFSET_OF
 #define OFFSET_OF(TYPE, Field) ((UINTN) &(((TYPE *)0)->Field))
-- 
2.24.0

Re: [edk2-devel] [PATCH v1] MdePkg: Use __builtin_offset with CLANGPDB toolchain

[Philippe Mathieu-Daudé]

On 11/28/19 6:56 AM, Alex James via Groups.Io wrote:
> CLANGPDB does not define __GNUC__, but it does define __clang__. Check
> for the __clang__ preprocessor definition to use __builtin_offsetof to
> implement the OFFSET_OF macro.
> 
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Signed-off-by: Alex James <theracermaster@gmail.com>
> ---
>   MdePkg/Include/Base.h | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/MdePkg/Include/Base.h b/MdePkg/Include/Base.h
> index 4680e64136..e0bcd0ae67 100644
> --- a/MdePkg/Include/Base.h
> +++ b/MdePkg/Include/Base.h
> @@ -781,11 +781,9 @@ typedef UINTN  *BASE_LIST;
>     @return  Offset, in bytes, of field.
>   
>   **/
> -#ifdef __GNUC__
> -#if __GNUC__ >= 4
> +#if (defined(__GNUC__) && __GNUC__ >= 4) || defined(__clang__)
>   #define OFFSET_OF(TYPE, Field) ((UINTN) __builtin_offsetof(TYPE, Field))
>   #endif
> -#endif
>   
>   #ifndef OFFSET_OF
>   #define OFFSET_OF(TYPE, Field) ((UINTN) &(((TYPE *)0)->Field))
> 

Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>

Re: [PATCH v1] MdePkg: Use __builtin_offset with CLANGPDB toolchain

[Liming Gao]

Alex:
  Do you find the real issue without this fix? Or is this change just an enhancement? 

  As you know, now we are in hard code freeze phase. Only functional bug is allowed. 

Thanks
Liming
>-----Original Message-----
>From: Alex James [mailto:theracermaster@gmail.com]
>Sent: Thursday, November 28, 2019 1:57 PM
>To: devel@edk2.groups.io
>Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
><liming.gao@intel.com>
>Subject: [PATCH v1] MdePkg: Use __builtin_offset with CLANGPDB toolchain
>
>CLANGPDB does not define __GNUC__, but it does define __clang__. Check
>for the __clang__ preprocessor definition to use __builtin_offsetof to
>implement the OFFSET_OF macro.
>
>Cc: Michael D Kinney <michael.d.kinney@intel.com>
>Cc: Liming Gao <liming.gao@intel.com>
>Signed-off-by: Alex James <theracermaster@gmail.com>
>---
> MdePkg/Include/Base.h | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
>diff --git a/MdePkg/Include/Base.h b/MdePkg/Include/Base.h
>index 4680e64136..e0bcd0ae67 100644
>--- a/MdePkg/Include/Base.h
>+++ b/MdePkg/Include/Base.h
>@@ -781,11 +781,9 @@ typedef UINTN  *BASE_LIST;
>   @return  Offset, in bytes, of field.
>
>
>
> **/
>
>-#ifdef __GNUC__
>
>-#if __GNUC__ >= 4
>
>+#if (defined(__GNUC__) && __GNUC__ >= 4) || defined(__clang__)
>
> #define OFFSET_OF(TYPE, Field) ((UINTN) __builtin_offsetof(TYPE, Field))
>
> #endif
>
>-#endif
>
>
>
> #ifndef OFFSET_OF
>
> #define OFFSET_OF(TYPE, Field) ((UINTN) &(((TYPE *)0)->Field))
>
>--
>2.24.0

Re: [PATCH v1] MdePkg: Use __builtin_offset with CLANGPDB toolchain

[Liming Gao]

BZ https://bugzilla.tianocore.org/show_bug.cgi?id=2393 is submitted to record this issue. 

This is the corner issue when STATIC_ASSERT and OFFSET_OF are used together. So, I think we can fix it after stable tag 201911.

Thanks
Liming
>-----Original Message-----
>From: Alex James [mailto:theracermaster@gmail.com]
>Sent: Thursday, November 28, 2019 1:57 PM
>To: devel@edk2.groups.io
>Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
><liming.gao@intel.com>
>Subject: [PATCH v1] MdePkg: Use __builtin_offset with CLANGPDB toolchain
>
>CLANGPDB does not define __GNUC__, but it does define __clang__. Check
>for the __clang__ preprocessor definition to use __builtin_offsetof to
>implement the OFFSET_OF macro.
>
>Cc: Michael D Kinney <michael.d.kinney@intel.com>
>Cc: Liming Gao <liming.gao@intel.com>
>Signed-off-by: Alex James <theracermaster@gmail.com>
>---
> MdePkg/Include/Base.h | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
>diff --git a/MdePkg/Include/Base.h b/MdePkg/Include/Base.h
>index 4680e64136..e0bcd0ae67 100644
>--- a/MdePkg/Include/Base.h
>+++ b/MdePkg/Include/Base.h
>@@ -781,11 +781,9 @@ typedef UINTN  *BASE_LIST;
>   @return  Offset, in bytes, of field.
>
>
>
> **/
>
>-#ifdef __GNUC__
>
>-#if __GNUC__ >= 4
>
>+#if (defined(__GNUC__) && __GNUC__ >= 4) || defined(__clang__)
>
> #define OFFSET_OF(TYPE, Field) ((UINTN) __builtin_offsetof(TYPE, Field))
>
> #endif
>
>-#endif
>
>
>
> #ifndef OFFSET_OF
>
> #define OFFSET_OF(TYPE, Field) ((UINTN) &(((TYPE *)0)->Field))
>
>--
>2.24.0

Re: [edk2-devel] [PATCH v1] MdePkg: Use __builtin_offset with CLANGPDB toolchain

[Liming Gao]

Alex:
  The change is good. Can you show what test has been done?

Thanks
Liming
>-----Original Message-----
>From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>Liming Gao
>Sent: Friday, November 29, 2019 2:12 PM
>To: Alex James <theracermaster@gmail.com>; devel@edk2.groups.io
>Cc: Kinney, Michael D <michael.d.kinney@intel.com>
>Subject: Re: [edk2-devel] [PATCH v1] MdePkg: Use __builtin_offset with
>CLANGPDB toolchain
>
>BZ https://bugzilla.tianocore.org/show_bug.cgi?id=2393 is submitted to
>record this issue.
>
>This is the corner issue when STATIC_ASSERT and OFFSET_OF are used
>together. So, I think we can fix it after stable tag 201911.
>
>Thanks
>Liming
>>-----Original Message-----
>>From: Alex James [mailto:theracermaster@gmail.com]
>>Sent: Thursday, November 28, 2019 1:57 PM
>>To: devel@edk2.groups.io
>>Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
>><liming.gao@intel.com>
>>Subject: [PATCH v1] MdePkg: Use __builtin_offset with CLANGPDB toolchain
>>
>>CLANGPDB does not define __GNUC__, but it does define __clang__. Check
>>for the __clang__ preprocessor definition to use __builtin_offsetof to
>>implement the OFFSET_OF macro.
>>
>>Cc: Michael D Kinney <michael.d.kinney@intel.com>
>>Cc: Liming Gao <liming.gao@intel.com>
>>Signed-off-by: Alex James <theracermaster@gmail.com>
>>---
>> MdePkg/Include/Base.h | 4 +---
>> 1 file changed, 1 insertion(+), 3 deletions(-)
>>
>>diff --git a/MdePkg/Include/Base.h b/MdePkg/Include/Base.h
>>index 4680e64136..e0bcd0ae67 100644
>>--- a/MdePkg/Include/Base.h
>>+++ b/MdePkg/Include/Base.h
>>@@ -781,11 +781,9 @@ typedef UINTN  *BASE_LIST;
>>   @return  Offset, in bytes, of field.
>>
>>
>>
>> **/
>>
>>-#ifdef __GNUC__
>>
>>-#if __GNUC__ >= 4
>>
>>+#if (defined(__GNUC__) && __GNUC__ >= 4) || defined(__clang__)
>>
>> #define OFFSET_OF(TYPE, Field) ((UINTN) __builtin_offsetof(TYPE, Field))
>>
>> #endif
>>
>>-#endif
>>
>>
>>
>> #ifndef OFFSET_OF
>>
>> #define OFFSET_OF(TYPE, Field) ((UINTN) &(((TYPE *)0)->Field))
>>
>>--
>>2.24.0
>
>
>

Re: [edk2-devel] [PATCH v1] MdePkg: Use __builtin_offset with CLANGPDB toolchain

[Liming Gao]

Alex has verified this change on OVMF boot to shell. I have no other comment for it. Reviewed-by: Liming Gao <liming.gao@intel.com>

Thanks
Liming
>-----Original Message-----
>From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>Liming Gao
>Sent: Tuesday, December 10, 2019 8:50 AM
>To: devel@edk2.groups.io; Gao, Liming <liming.gao@intel.com>; Alex James
><theracermaster@gmail.com>
>Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Liu, Zhiguang
><zhiguang.liu@intel.com>
>Subject: Re: [edk2-devel] [PATCH v1] MdePkg: Use __builtin_offset with
>CLANGPDB toolchain
>
>Alex:
>  The change is good. Can you show what test has been done?
>
>Thanks
>Liming
>>-----Original Message-----
>>From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>>Liming Gao
>>Sent: Friday, November 29, 2019 2:12 PM
>>To: Alex James <theracermaster@gmail.com>; devel@edk2.groups.io
>>Cc: Kinney, Michael D <michael.d.kinney@intel.com>
>>Subject: Re: [edk2-devel] [PATCH v1] MdePkg: Use __builtin_offset with
>>CLANGPDB toolchain
>>
>>BZ https://bugzilla.tianocore.org/show_bug.cgi?id=2393 is submitted to
>>record this issue.
>>
>>This is the corner issue when STATIC_ASSERT and OFFSET_OF are used
>>together. So, I think we can fix it after stable tag 201911.
>>
>>Thanks
>>Liming
>>>-----Original Message-----
>>>From: Alex James [mailto:theracermaster@gmail.com]
>>>Sent: Thursday, November 28, 2019 1:57 PM
>>>To: devel@edk2.groups.io
>>>Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
>>><liming.gao@intel.com>
>>>Subject: [PATCH v1] MdePkg: Use __builtin_offset with CLANGPDB
>toolchain
>>>
>>>CLANGPDB does not define __GNUC__, but it does define __clang__.
>Check
>>>for the __clang__ preprocessor definition to use __builtin_offsetof to
>>>implement the OFFSET_OF macro.
>>>
>>>Cc: Michael D Kinney <michael.d.kinney@intel.com>
>>>Cc: Liming Gao <liming.gao@intel.com>
>>>Signed-off-by: Alex James <theracermaster@gmail.com>
>>>---
>>> MdePkg/Include/Base.h | 4 +---
>>> 1 file changed, 1 insertion(+), 3 deletions(-)
>>>
>>>diff --git a/MdePkg/Include/Base.h b/MdePkg/Include/Base.h
>>>index 4680e64136..e0bcd0ae67 100644
>>>--- a/MdePkg/Include/Base.h
>>>+++ b/MdePkg/Include/Base.h
>>>@@ -781,11 +781,9 @@ typedef UINTN  *BASE_LIST;
>>>   @return  Offset, in bytes, of field.
>>>
>>>
>>>
>>> **/
>>>
>>>-#ifdef __GNUC__
>>>
>>>-#if __GNUC__ >= 4
>>>
>>>+#if (defined(__GNUC__) && __GNUC__ >= 4) || defined(__clang__)
>>>
>>> #define OFFSET_OF(TYPE, Field) ((UINTN) __builtin_offsetof(TYPE, Field))
>>>
>>> #endif
>>>
>>>-#endif
>>>
>>>
>>>
>>> #ifndef OFFSET_OF
>>>
>>> #define OFFSET_OF(TYPE, Field) ((UINTN) &(((TYPE *)0)->Field))
>>>
>>>--
>>>2.24.0
>>
>>
>>
>
>
>

Re: [edk2-devel] [PATCH v1] MdePkg: Use __builtin_offset with CLANGPDB toolchain

[Liming Gao]

merge@796b380ca7d263ca504b82fe5317a78d3546d537

>-----Original Message-----
>From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>Liming Gao
>Sent: Wednesday, December 18, 2019 10:07 AM
>To: devel@edk2.groups.io; Gao, Liming <liming.gao@intel.com>; Alex James
><theracermaster@gmail.com>
>Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Liu, Zhiguang
><zhiguang.liu@intel.com>
>Subject: Re: [edk2-devel] [PATCH v1] MdePkg: Use __builtin_offset with
>CLANGPDB toolchain
>
>Alex has verified this change on OVMF boot to shell. I have no other comment
>for it. Reviewed-by: Liming Gao <liming.gao@intel.com>
>
>Thanks
>Liming
>>-----Original Message-----
>>From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>>Liming Gao
>>Sent: Tuesday, December 10, 2019 8:50 AM
>>To: devel@edk2.groups.io; Gao, Liming <liming.gao@intel.com>; Alex James
>><theracermaster@gmail.com>
>>Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Liu, Zhiguang
>><zhiguang.liu@intel.com>
>>Subject: Re: [edk2-devel] [PATCH v1] MdePkg: Use __builtin_offset with
>>CLANGPDB toolchain
>>
>>Alex:
>>  The change is good. Can you show what test has been done?
>>
>>Thanks
>>Liming
>>>-----Original Message-----
>>>From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>>>Liming Gao
>>>Sent: Friday, November 29, 2019 2:12 PM
>>>To: Alex James <theracermaster@gmail.com>; devel@edk2.groups.io
>>>Cc: Kinney, Michael D <michael.d.kinney@intel.com>
>>>Subject: Re: [edk2-devel] [PATCH v1] MdePkg: Use __builtin_offset with
>>>CLANGPDB toolchain
>>>
>>>BZ https://bugzilla.tianocore.org/show_bug.cgi?id=2393 is submitted to
>>>record this issue.
>>>
>>>This is the corner issue when STATIC_ASSERT and OFFSET_OF are used
>>>together. So, I think we can fix it after stable tag 201911.
>>>
>>>Thanks
>>>Liming
>>>>-----Original Message-----
>>>>From: Alex James [mailto:theracermaster@gmail.com]
>>>>Sent: Thursday, November 28, 2019 1:57 PM
>>>>To: devel@edk2.groups.io
>>>>Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
>>>><liming.gao@intel.com>
>>>>Subject: [PATCH v1] MdePkg: Use __builtin_offset with CLANGPDB
>>toolchain
>>>>
>>>>CLANGPDB does not define __GNUC__, but it does define __clang__.
>>Check
>>>>for the __clang__ preprocessor definition to use __builtin_offsetof to
>>>>implement the OFFSET_OF macro.
>>>>
>>>>Cc: Michael D Kinney <michael.d.kinney@intel.com>
>>>>Cc: Liming Gao <liming.gao@intel.com>
>>>>Signed-off-by: Alex James <theracermaster@gmail.com>
>>>>---
>>>> MdePkg/Include/Base.h | 4 +---
>>>> 1 file changed, 1 insertion(+), 3 deletions(-)
>>>>
>>>>diff --git a/MdePkg/Include/Base.h b/MdePkg/Include/Base.h
>>>>index 4680e64136..e0bcd0ae67 100644
>>>>--- a/MdePkg/Include/Base.h
>>>>+++ b/MdePkg/Include/Base.h
>>>>@@ -781,11 +781,9 @@ typedef UINTN  *BASE_LIST;
>>>>   @return  Offset, in bytes, of field.
>>>>
>>>>
>>>>
>>>> **/
>>>>
>>>>-#ifdef __GNUC__
>>>>
>>>>-#if __GNUC__ >= 4
>>>>
>>>>+#if (defined(__GNUC__) && __GNUC__ >= 4) || defined(__clang__)
>>>>
>>>> #define OFFSET_OF(TYPE, Field) ((UINTN) __builtin_offsetof(TYPE, Field))
>>>>
>>>> #endif
>>>>
>>>>-#endif
>>>>
>>>>
>>>>
>>>> #ifndef OFFSET_OF
>>>>
>>>> #define OFFSET_OF(TYPE, Field) ((UINTN) &(((TYPE *)0)->Field))
>>>>
>>>>--
>>>>2.24.0
>>>
>>>
>>>
>>
>>
>>
>
>
>