diff mbox series

[Xen-devel,2/2] xen/mm: Introduce {G, M}FN_INVALID_INITIALIZER

Message ID 20170627093320.9811-2-julien.grall@arm.com
State Accepted
Commit bbbe89351275ed6ac09207fe9aeb80b606af1a4d
Headers show
Series [Xen-devel,1/2] Revert "mm: don't use _{g, m}fn for defining INVALID_{G, M}FN" | expand

Commit Message

Julien Grall June 27, 2017, 9:33 a.m. UTC
The current implementation of {G,M}FN_INVALID cannot be used to
initialize global variable because the initializer element is not a
constant.

Due to a bug in GCC 4.9 and older ([1]), it is not easy to find a common
value to initialize a variable and directly passed as an argument.

Introduce 2 news define {G,M}FN_INVALID_INITIALIZER to be used for
initializing a variable.

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64856

Signed-off-by: Julien Grall <julien.grall@arm.com>

---
    Build tested it with:
        * ARM: GCC 4.9.4, 5.1, 4.3, 6.1.1, 7.1.0
        * x86: Clang 3.5.0, 3.6.0, 3.6.2, 3.8.0, 3.9.0, 4.0.0

    With introducing a dummy global variable common/mm.c:

    mfn_t foo = INVALID_MFN_INITIALIZER
---
 xen/include/xen/mm.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Tim Deegan June 27, 2017, 9:46 a.m. UTC | #1
At 10:33 +0100 on 27 Jun (1498559600), Julien Grall wrote:
> The current implementation of {G,M}FN_INVALID cannot be used to
> initialize global variable because the initializer element is not a
> constant.
> 
> Due to a bug in GCC 4.9 and older ([1]), it is not easy to find a common
> value to initialize a variable and directly passed as an argument.
> 
> Introduce 2 news define {G,M}FN_INVALID_INITIALIZER to be used for
> initializing a variable.
> 
> [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64856
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Acked-by: Tim Deegan <tim@xen.org> (and Ack for the revert too)
but please choose either { ~0UL } or {~0UL} and use it for both.

> ---
>     Build tested it with:
>         * ARM: GCC 4.9.4, 5.1, 4.3, 6.1.1, 7.1.0
>         * x86: Clang 3.5.0, 3.6.0, 3.6.2, 3.8.0, 3.9.0, 4.0.0
> 
>     With introducing a dummy global variable common/mm.c:
> 
>     mfn_t foo = INVALID_MFN_INITIALIZER
> ---
>  xen/include/xen/mm.h | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
> index 0050fba498..251db4ffa1 100644
> --- a/xen/include/xen/mm.h
> +++ b/xen/include/xen/mm.h
> @@ -57,6 +57,11 @@
>  TYPE_SAFE(unsigned long, mfn);
>  #define PRI_mfn          "05lx"
>  #define INVALID_MFN      _mfn(~0UL)
> +/*
> + * To be used for global variable initialization. This workaround a bug
> + * in GCC < 5.0.
> + */
> +#define INVALID_MFN_INITIALIZER {~0UL}
>  
>  #ifndef mfn_t
>  #define mfn_t /* Grep fodder: mfn_t, _mfn() and mfn_x() are defined above */
> @@ -90,6 +95,11 @@ static inline bool_t mfn_eq(mfn_t x, mfn_t y)
>  TYPE_SAFE(unsigned long, gfn);
>  #define PRI_gfn          "05lx"
>  #define INVALID_GFN      _gfn(~0UL)
> +/*
> + * To be used for global variable initialization. This workaround a bug
> + * in GCC < 5.0 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64856
> + */
> +#define INVALID_GFN_INITIALIZER { ~0UL }
>  
>  #ifndef gfn_t
>  #define gfn_t /* Grep fodder: gfn_t, _gfn() and gfn_x() are defined above */
> -- 
> 2.11.0
>
Julien Grall June 27, 2017, 9:47 a.m. UTC | #2
On 27/06/2017 10:46, Tim Deegan wrote:
> At 10:33 +0100 on 27 Jun (1498559600), Julien Grall wrote:
>> The current implementation of {G,M}FN_INVALID cannot be used to
>> initialize global variable because the initializer element is not a
>> constant.
>>
>> Due to a bug in GCC 4.9 and older ([1]), it is not easy to find a common
>> value to initialize a variable and directly passed as an argument.
>>
>> Introduce 2 news define {G,M}FN_INVALID_INITIALIZER to be used for
>> initializing a variable.
>>
>> [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64856
>>
>> Signed-off-by: Julien Grall <julien.grall@arm.com>
>
> Acked-by: Tim Deegan <tim@xen.org> (and Ack for the revert too)
> but please choose either { ~0UL } or {~0UL} and use it for both.

Whoops. Sorry for that. I will stick to {~0UL} unless someone prefer the 
{ ~0UL }.

Cheers,

>
>> ---
>>     Build tested it with:
>>         * ARM: GCC 4.9.4, 5.1, 4.3, 6.1.1, 7.1.0
>>         * x86: Clang 3.5.0, 3.6.0, 3.6.2, 3.8.0, 3.9.0, 4.0.0
>>
>>     With introducing a dummy global variable common/mm.c:
>>
>>     mfn_t foo = INVALID_MFN_INITIALIZER
>> ---
>>  xen/include/xen/mm.h | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
>> index 0050fba498..251db4ffa1 100644
>> --- a/xen/include/xen/mm.h
>> +++ b/xen/include/xen/mm.h
>> @@ -57,6 +57,11 @@
>>  TYPE_SAFE(unsigned long, mfn);
>>  #define PRI_mfn          "05lx"
>>  #define INVALID_MFN      _mfn(~0UL)
>> +/*
>> + * To be used for global variable initialization. This workaround a bug
>> + * in GCC < 5.0.
>> + */
>> +#define INVALID_MFN_INITIALIZER {~0UL}
>>
>>  #ifndef mfn_t
>>  #define mfn_t /* Grep fodder: mfn_t, _mfn() and mfn_x() are defined above */
>> @@ -90,6 +95,11 @@ static inline bool_t mfn_eq(mfn_t x, mfn_t y)
>>  TYPE_SAFE(unsigned long, gfn);
>>  #define PRI_gfn          "05lx"
>>  #define INVALID_GFN      _gfn(~0UL)
>> +/*
>> + * To be used for global variable initialization. This workaround a bug
>> + * in GCC < 5.0 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64856
>> + */
>> +#define INVALID_GFN_INITIALIZER { ~0UL }
>>
>>  #ifndef gfn_t
>>  #define gfn_t /* Grep fodder: gfn_t, _gfn() and gfn_x() are defined above */
>> --
>> 2.11.0
>>
Julien Grall June 29, 2017, 4:30 p.m. UTC | #3
Hi,

On 06/27/2017 10:47 AM, Julien Grall wrote:
> 
> 
> On 27/06/2017 10:46, Tim Deegan wrote:
>> At 10:33 +0100 on 27 Jun (1498559600), Julien Grall wrote:
>>> The current implementation of {G,M}FN_INVALID cannot be used to
>>> initialize global variable because the initializer element is not a
>>> constant.
>>>
>>> Due to a bug in GCC 4.9 and older ([1]), it is not easy to find a common
>>> value to initialize a variable and directly passed as an argument.
>>>
>>> Introduce 2 news define {G,M}FN_INVALID_INITIALIZER to be used for
>>> initializing a variable.
>>>
>>> [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64856
>>>
>>> Signed-off-by: Julien Grall <julien.grall@arm.com>
>>
>> Acked-by: Tim Deegan <tim@xen.org> (and Ack for the revert too)
>> but please choose either { ~0UL } or {~0UL} and use it for both.
> 
> Whoops. Sorry for that. I will stick to {~0UL} unless someone prefer the 
> { ~0UL }.

Also, shall I resent a patch with this change? Or will it get taken care 
on commit? (assuming there are no other changes).

Cheers,
Andrew Cooper June 29, 2017, 4:43 p.m. UTC | #4
On 29/06/17 17:30, Julien Grall wrote:
> Hi,
>
> On 06/27/2017 10:47 AM, Julien Grall wrote:
>>
>>
>> On 27/06/2017 10:46, Tim Deegan wrote:
>>> At 10:33 +0100 on 27 Jun (1498559600), Julien Grall wrote:
>>>> The current implementation of {G,M}FN_INVALID cannot be used to
>>>> initialize global variable because the initializer element is not a
>>>> constant.
>>>>
>>>> Due to a bug in GCC 4.9 and older ([1]), it is not easy to find a
>>>> common
>>>> value to initialize a variable and directly passed as an argument.
>>>>
>>>> Introduce 2 news define {G,M}FN_INVALID_INITIALIZER to be used for
>>>> initializing a variable.
>>>>
>>>> [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64856
>>>>
>>>> Signed-off-by: Julien Grall <julien.grall@arm.com>
>>>
>>> Acked-by: Tim Deegan <tim@xen.org> (and Ack for the revert too)
>>> but please choose either { ~0UL } or {~0UL} and use it for both.
>>
>> Whoops. Sorry for that. I will stick to {~0UL} unless someone prefer
>> the { ~0UL }.
>
> Also, shall I resent a patch with this change? Or will it get taken
> care on commit? (assuming there are no other changes).

I'll sort it out on commit, when I do my next sweep (if someone doesn't
beat me to it).

~Andrew
diff mbox series

Patch

diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index 0050fba498..251db4ffa1 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -57,6 +57,11 @@ 
 TYPE_SAFE(unsigned long, mfn);
 #define PRI_mfn          "05lx"
 #define INVALID_MFN      _mfn(~0UL)
+/*
+ * To be used for global variable initialization. This workaround a bug
+ * in GCC < 5.0.
+ */
+#define INVALID_MFN_INITIALIZER {~0UL}
 
 #ifndef mfn_t
 #define mfn_t /* Grep fodder: mfn_t, _mfn() and mfn_x() are defined above */
@@ -90,6 +95,11 @@  static inline bool_t mfn_eq(mfn_t x, mfn_t y)
 TYPE_SAFE(unsigned long, gfn);
 #define PRI_gfn          "05lx"
 #define INVALID_GFN      _gfn(~0UL)
+/*
+ * To be used for global variable initialization. This workaround a bug
+ * in GCC < 5.0 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64856
+ */
+#define INVALID_GFN_INITIALIZER { ~0UL }
 
 #ifndef gfn_t
 #define gfn_t /* Grep fodder: gfn_t, _gfn() and gfn_x() are defined above */