| Message ID | 1343396239-19272-5-git-send-email-aliguori@us.ibm.com |
|---|---|
| State | New |
| Headers | show |
On 27 July 2012 14:37, Anthony Liguori <aliguori@us.ibm.com> wrote: > --- a/compiler.h > +++ b/compiler.h > @@ -45,6 +45,7 @@ > # define GCC_ATTR __attribute__((__unused__, format(gnu_printf, 1, 2))) > # define GCC_FMT_ATTR(n, m) __attribute__((format(gnu_printf, n, m))) > # endif > +#define GCC_WEAK __attribute__((weak)) > #else > #define GCC_ATTR /**/ > #define GCC_FMT_ATTR(n, m) The GCC manual says "Weak symbols are supported for ELF targets, and also for a.out targets when using the GNU assembler and linker". Have you tested this on Windows and MacOSX ? (Also, no version of the macro in the not-GCC branch of the #if.) -- PMM
Peter Maydell <peter.maydell@linaro.org> writes: > On 27 July 2012 14:37, Anthony Liguori <aliguori@us.ibm.com> wrote: >> --- a/compiler.h >> +++ b/compiler.h >> @@ -45,6 +45,7 @@ >> # define GCC_ATTR __attribute__((__unused__, format(gnu_printf, 1, 2))) >> # define GCC_FMT_ATTR(n, m) __attribute__((format(gnu_printf, n, m))) >> # endif >> +#define GCC_WEAK __attribute__((weak)) >> #else >> #define GCC_ATTR /**/ >> #define GCC_FMT_ATTR(n, m) > > The GCC manual says "Weak symbols are supported for ELF targets, > and also for a.out targets when using the GNU assembler and linker". > Have you tested this on Windows and MacOSX ? Weak symbols are supposed to be supported by mingw32. I have no idea about MacOS X. I have no way to develop or test for MacOS X using free software so I honestly don't care about it. Regards, Anthony Liguori > > (Also, no version of the macro in the not-GCC branch of the #if.) > > -- PMM
On 27 July 2012 15:27, Anthony Liguori <aliguori@us.ibm.com> wrote: > Peter Maydell <peter.maydell@linaro.org> writes: >> The GCC manual says "Weak symbols are supported for ELF targets, >> and also for a.out targets when using the GNU assembler and linker". >> Have you tested this on Windows and MacOSX ? > > Weak symbols are supposed to be supported by mingw32. > > I have no idea about MacOS X. > > I have no way to develop or test for MacOS X using free software so I > honestly don't care about it. My approach to this is to avoid non-standard things -- if I write a patch which is pretty much standard C then it's the platform's problem if it mishandles it. If I write a patch that uses a compiler-specific or OS-specific thing then I have to also provide the relevant alternatives...so I try to avoid doing that :-) -- PMM
Peter Maydell <peter.maydell@linaro.org> writes: > On 27 July 2012 15:27, Anthony Liguori <aliguori@us.ibm.com> wrote: >> Peter Maydell <peter.maydell@linaro.org> writes: >>> The GCC manual says "Weak symbols are supported for ELF targets, >>> and also for a.out targets when using the GNU assembler and linker". >>> Have you tested this on Windows and MacOSX ? >> >> Weak symbols are supposed to be supported by mingw32. >> >> I have no idea about MacOS X. >> >> I have no way to develop or test for MacOS X using free software so I >> honestly don't care about it. > > My approach to this is to avoid non-standard things http://en.wikipedia.org/wiki/C99#Implementations So unless you plan on compiling QEMU with xlc, pgi, or icc, I don't think relying on "standard things" really helps. QEMU doesn't support C99, it supports GCC. There's no point in debating about whether we should rely on extensions or not. We already do--extensively. Regards, Anthony Liguori > -- if I > write a patch which is pretty much standard C then it's the > platform's problem if it mishandles it. If I write a patch > that uses a compiler-specific or OS-specific thing then I > have to also provide the relevant alternatives...so I try > to avoid doing that :-) > > -- PMM
Peter Maydell <peter.maydell@linaro.org> writes: > On 27 July 2012 14:37, Anthony Liguori <aliguori@us.ibm.com> wrote: >> --- a/compiler.h >> +++ b/compiler.h >> @@ -45,6 +45,7 @@ >> # define GCC_ATTR __attribute__((__unused__, format(gnu_printf, 1, 2))) >> # define GCC_FMT_ATTR(n, m) __attribute__((format(gnu_printf, n, m))) >> # endif >> +#define GCC_WEAK __attribute__((weak)) >> #else >> #define GCC_ATTR /**/ >> #define GCC_FMT_ATTR(n, m) > > The GCC manual says "Weak symbols are supported for ELF targets, > and also for a.out targets when using the GNU assembler and linker". > Have you tested this on Windows and MacOSX ? > > (Also, no version of the macro in the not-GCC branch of the #if.) (We don't support any compiler other than GCC). Not really sure why it is even in a branch. Regards, Anthony Liguori > > -- PMM
On Fri, Jul 27, 2012 at 3:31 PM, Anthony Liguori <aliguori@us.ibm.com> wrote: > Peter Maydell <peter.maydell@linaro.org> writes: > >> On 27 July 2012 15:27, Anthony Liguori <aliguori@us.ibm.com> wrote: >>> Peter Maydell <peter.maydell@linaro.org> writes: >>>> The GCC manual says "Weak symbols are supported for ELF targets, >>>> and also for a.out targets when using the GNU assembler and linker". >>>> Have you tested this on Windows and MacOSX ? >>> >>> Weak symbols are supposed to be supported by mingw32. >>> >>> I have no idea about MacOS X. >>> >>> I have no way to develop or test for MacOS X using free software so I >>> honestly don't care about it. >> >> My approach to this is to avoid non-standard things > > http://en.wikipedia.org/wiki/C99#Implementations > > So unless you plan on compiling QEMU with xlc, pgi, or icc, I don't > think relying on "standard things" really helps. LLVM/Clang should definitely be in the plan. > > QEMU doesn't support C99, it supports GCC. There's no point in > debating about whether we should rely on extensions or not. We already > do--extensively. Not so extensively. There are a few extensions for which there is no simple alternative (like QEMU_PACKED) but other compilers likely need similar extensions. Then there are other extensions (like :? without middle expression) which can be easily avoided. We should avoid to use the non-standard extensions whenever possible. > > Regards, > > Anthony Liguori > > >> -- if I >> write a patch which is pretty much standard C then it's the >> platform's problem if it mishandles it. If I write a patch >> that uses a compiler-specific or OS-specific thing then I >> have to also provide the relevant alternatives...so I try >> to avoid doing that :-) >> >> -- PMM > >
Blue Swirl <blauwirbel@gmail.com> writes: > On Fri, Jul 27, 2012 at 3:31 PM, Anthony Liguori <aliguori@us.ibm.com> wrote: >> Peter Maydell <peter.maydell@linaro.org> writes: >> >>> On 27 July 2012 15:27, Anthony Liguori <aliguori@us.ibm.com> wrote: >>>> Peter Maydell <peter.maydell@linaro.org> writes: >>>>> The GCC manual says "Weak symbols are supported for ELF targets, >>>>> and also for a.out targets when using the GNU assembler and linker". >>>>> Have you tested this on Windows and MacOSX ? >>>> >>>> Weak symbols are supposed to be supported by mingw32. >>>> >>>> I have no idea about MacOS X. >>>> >>>> I have no way to develop or test for MacOS X using free software so I >>>> honestly don't care about it. >>> >>> My approach to this is to avoid non-standard things >> >> http://en.wikipedia.org/wiki/C99#Implementations >> >> So unless you plan on compiling QEMU with xlc, pgi, or icc, I don't >> think relying on "standard things" really helps. > > LLVM/Clang should definitely be in the plan. weak symbols are supported by clang. >> QEMU doesn't support C99, it supports GCC. There's no point in >> debating about whether we should rely on extensions or not. We already >> do--extensively. > > Not so extensively. There are a few extensions for which there is no > simple alternative (like QEMU_PACKED) but other compilers likely need > similar extensions. Then there are other extensions (like :? without > middle expression) which can be easily avoided. We should avoid to use > the non-standard extensions whenever possible. I disagree. I don't see a point to it. QEMU has never been routinely built on anything other than GCC. Why go to a lot of trouble to support a user base that doesn't exist? If someone comes along and actively maintains support for another compiler, we can revisit. But otherwise, there's no practical reason to avoid extensions. Regards, Anthony Liguori > >> >> Regards, >> >> Anthony Liguori >> >> >>> -- if I >>> write a patch which is pretty much standard C then it's the >>> platform's problem if it mishandles it. If I write a patch >>> that uses a compiler-specific or OS-specific thing then I >>> have to also provide the relevant alternatives...so I try >>> to avoid doing that :-) >>> >>> -- PMM >> >>
On Fri, Jul 27, 2012 at 8:51 PM, Anthony Liguori <aliguori@us.ibm.com> wrote: > Blue Swirl <blauwirbel@gmail.com> writes: > >> On Fri, Jul 27, 2012 at 3:31 PM, Anthony Liguori <aliguori@us.ibm.com> wrote: >>> Peter Maydell <peter.maydell@linaro.org> writes: >>> >>>> On 27 July 2012 15:27, Anthony Liguori <aliguori@us.ibm.com> wrote: >>>>> Peter Maydell <peter.maydell@linaro.org> writes: >>>>>> The GCC manual says "Weak symbols are supported for ELF targets, >>>>>> and also for a.out targets when using the GNU assembler and linker". >>>>>> Have you tested this on Windows and MacOSX ? >>>>> >>>>> Weak symbols are supposed to be supported by mingw32. >>>>> >>>>> I have no idea about MacOS X. >>>>> >>>>> I have no way to develop or test for MacOS X using free software so I >>>>> honestly don't care about it. >>>> >>>> My approach to this is to avoid non-standard things >>> >>> http://en.wikipedia.org/wiki/C99#Implementations >>> >>> So unless you plan on compiling QEMU with xlc, pgi, or icc, I don't >>> think relying on "standard things" really helps. >> >> LLVM/Clang should definitely be in the plan. > > weak symbols are supported by clang. > >>> QEMU doesn't support C99, it supports GCC. There's no point in >>> debating about whether we should rely on extensions or not. We already >>> do--extensively. >> >> Not so extensively. There are a few extensions for which there is no >> simple alternative (like QEMU_PACKED) but other compilers likely need >> similar extensions. Then there are other extensions (like :? without >> middle expression) which can be easily avoided. We should avoid to use >> the non-standard extensions whenever possible. > > I disagree. I don't see a point to it. QEMU has never been routinely > built on anything other than GCC. Why go to a lot of trouble to support > a user base that doesn't exist? > > If someone comes along and actively maintains support for another > compiler, we can revisit. But otherwise, there's no practical reason to > avoid extensions. Because it's more compliant to standards. There's also very little benefit from using the nonessential extensions. > > Regards, > > Anthony Liguori > >> >>> >>> Regards, >>> >>> Anthony Liguori >>> >>> >>>> -- if I >>>> write a patch which is pretty much standard C then it's the >>>> platform's problem if it mishandles it. If I write a patch >>>> that uses a compiler-specific or OS-specific thing then I >>>> have to also provide the relevant alternatives...so I try >>>> to avoid doing that :-) >>>> >>>> -- PMM >>> >>> >
Blue Swirl <blauwirbel@gmail.com> writes: > On Fri, Jul 27, 2012 at 8:51 PM, Anthony Liguori <aliguori@us.ibm.com> wrote: >> If someone comes along and actively maintains support for another >> compiler, we can revisit. But otherwise, there's no practical reason to >> avoid extensions. > > Because it's more compliant to standards. That is not a benefit. There is no practical advantage to sticking to only C99. If we stuck to C99 only, QEMU would look very different. We wouldn't have type_init() so we wouldn't have the module system we use today. It literally took me months to get those patches merged because Paul made exactly the same argument you're making now. And it's really been one of the better cleanups in QEMU that we've ever done (using modules to define devices). > There's also very little benefit from using the nonessential > extensions. Using weak symbols eliminates #ifdefs and makes the code base a lot cleaner. It makes it easier to introduce architecture specific hooks in a clean way. Considering how messy a lot of our target-specific code is, I think there could be quite a significant benefit down the road. Regards, Anthony Liguori > >> >> Regards, >> >> Anthony Liguori >> >>> >>>> >>>> Regards, >>>> >>>> Anthony Liguori >>>> >>>> >>>>> -- if I >>>>> write a patch which is pretty much standard C then it's the >>>>> platform's problem if it mishandles it. If I write a patch >>>>> that uses a compiler-specific or OS-specific thing then I >>>>> have to also provide the relevant alternatives...so I try >>>>> to avoid doing that :-) >>>>> >>>>> -- PMM >>>> >>>> >>
Anthony Liguori <aliguori@us.ibm.com> writes: > Blue Swirl <blauwirbel@gmail.com> writes: > >> On Fri, Jul 27, 2012 at 8:51 PM, Anthony Liguori <aliguori@us.ibm.com> wrote: >>> If someone comes along and actively maintains support for another >>> compiler, we can revisit. But otherwise, there's no practical reason to >>> avoid extensions. Exactly. >> Because it's more compliant to standards. > > That is not a benefit. There is no practical advantage to sticking to > only C99. Agree. "Compliance to X" is means, not ends. Specifically, compliance to C99 is not a benefit by itself. Portability to desirable targets is one. Right now, the code is portable enough for our current needs. If you find a target you wish to support where it doesn't work, patches are welcome. > If we stuck to C99 only, QEMU would look very different. We wouldn't > have type_init() so we wouldn't have the module system we use today. > It literally took me months to get those patches merged because Paul > made exactly the same argument you're making now. > > And it's really been one of the better cleanups in QEMU that we've ever > done (using modules to define devices). Developing is hard enough without tying ourselves into knots to avoid useful tools just because they haven't been blessed by the right standards committee. Some pragmatism is in order. >> There's also very little benefit from using the nonessential >> extensions. Blanket statement, needs evidence. > Using weak symbols eliminates #ifdefs and makes the code base a lot > cleaner. It makes it easier to introduce architecture specific hooks in > a clean way. > > Considering how messy a lot of our target-specific code is, I think > there could be quite a significant benefit down the road. I don't have an informed opinion on this particular case. All I can say here is patches are evidence.
On 27 July 2012 16:31, Anthony Liguori <aliguori@us.ibm.com> wrote: > Peter Maydell <peter.maydell@linaro.org> writes: >> My approach to this is to avoid non-standard things > > http://en.wikipedia.org/wiki/C99#Implementations > > So unless you plan on compiling QEMU with xlc, pgi, or icc, I don't > think relying on "standard things" really helps. > > QEMU doesn't support C99, it supports GCC. OK, you could perhaps rephrase that as 'mainstream' rather than 'standards-compliant'. I don't think we need to be strict C99; I do think we have more than one working host OS and that patches that use functionality that's documented not to work on all gcc targets ought to come attached to a statement that they've been tested. (MacOSX isn't actually in MAINTAINERS as a host so is a bit of a red herring. Windows is listed.) So if you really like weak symbols, go ahead. I'm just saying you're imposing a bigger testing burden on yourself than if you handled this some other way. (I do think it would be nice to care about building with CLANG, because there are some static analysis tools that we would then be able to run. That doesn't mean dropping all GCC extensions, though, because CLANG does support a lot of them.) -- PMM
On Fri, Jul 27, 2012 at 10:40 PM, Anthony Liguori <aliguori@us.ibm.com> wrote: > Blue Swirl <blauwirbel@gmail.com> writes: > >> On Fri, Jul 27, 2012 at 8:51 PM, Anthony Liguori <aliguori@us.ibm.com> wrote: >>> If someone comes along and actively maintains support for another >>> compiler, we can revisit. But otherwise, there's no practical reason to >>> avoid extensions. >> >> Because it's more compliant to standards. > > That is not a benefit. There is no practical advantage to sticking to > only C99. > > If we stuck to C99 only, QEMU would look very different. We wouldn't > have type_init() so we wouldn't have the module system we use today. > It literally took me months to get those patches merged because Paul > made exactly the same argument you're making now. > > And it's really been one of the better cleanups in QEMU that we've ever > done (using modules to define devices). Yes, it's a very nice cleanup. But I'm not saying things like that would have to be changed much or at all. As an example for type_init(), a script could grep and process those declarations (like the recently posted IDL stuff) and generate type tables in a .c file for the non-GCC compiler. Then an initialization function would just call the functions in the table in sequence. That way GCC can use initializers but with clever hacks, non-GCC compilers can be used too. > >> There's also very little benefit from using the nonessential >> extensions. > > Using weak symbols eliminates #ifdefs and makes the code base a lot > cleaner. It makes it easier to introduce architecture specific hooks in > a clean way. > > Considering how messy a lot of our target-specific code is, I think > there could be quite a significant benefit down the road. I don't have anything specifically against weak symbols, probably with macros like QEMU_WEAK() they can also be accommodated later if ever needed. Just offhand, the macro could also take a symbol prefix which is ignored for GCC, but used in non-GCC case to create a table which a symbol resolver (not compiled in for GCC) would use. > > Regards, > > Anthony Liguori > >> >>> >>> Regards, >>> >>> Anthony Liguori >>> >>>> >>>>> >>>>> Regards, >>>>> >>>>> Anthony Liguori >>>>> >>>>> >>>>>> -- if I >>>>>> write a patch which is pretty much standard C then it's the >>>>>> platform's problem if it mishandles it. If I write a patch >>>>>> that uses a compiler-specific or OS-specific thing then I >>>>>> have to also provide the relevant alternatives...so I try >>>>>> to avoid doing that :-) >>>>>> >>>>>> -- PMM >>>>> >>>>> >>> >
On Sat, Jul 28, 2012 at 6:25 AM, Markus Armbruster <armbru@redhat.com> wrote: > Anthony Liguori <aliguori@us.ibm.com> writes: > >> Blue Swirl <blauwirbel@gmail.com> writes: >> >>> On Fri, Jul 27, 2012 at 8:51 PM, Anthony Liguori <aliguori@us.ibm.com> wrote: >>>> If someone comes along and actively maintains support for another >>>> compiler, we can revisit. But otherwise, there's no practical reason to >>>> avoid extensions. > > Exactly. > >>> Because it's more compliant to standards. >> >> That is not a benefit. There is no practical advantage to sticking to >> only C99. > > Agree. "Compliance to X" is means, not ends. Specifically, compliance > to C99 is not a benefit by itself. Portability to desirable targets is > one. > > Right now, the code is portable enough for our current needs. If you > find a target you wish to support where it doesn't work, patches are > welcome. > >> If we stuck to C99 only, QEMU would look very different. We wouldn't >> have type_init() so we wouldn't have the module system we use today. >> It literally took me months to get those patches merged because Paul >> made exactly the same argument you're making now. >> >> And it's really been one of the better cleanups in QEMU that we've ever >> done (using modules to define devices). > > Developing is hard enough without tying ourselves into knots to avoid > useful tools just because they haven't been blessed by the right > standards committee. Some pragmatism is in order. IMHO it's just laziness to use easily avoidable constructs like ?: without middle expression. In most cases (as can be seen in my patch), you save typing and reviewing whopping 10-20 characters. We are in seconds range of effort. > >>> There's also very little benefit from using the nonessential >>> extensions. > > Blanket statement, needs evidence. > >> Using weak symbols eliminates #ifdefs and makes the code base a lot >> cleaner. It makes it easier to introduce architecture specific hooks in >> a clean way. >> >> Considering how messy a lot of our target-specific code is, I think >> there could be quite a significant benefit down the road. > > I don't have an informed opinion on this particular case. All I can say > here is patches are evidence.
On Sat, Jul 28, 2012 at 6:50 AM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 27 July 2012 16:31, Anthony Liguori <aliguori@us.ibm.com> wrote: >> Peter Maydell <peter.maydell@linaro.org> writes: >>> My approach to this is to avoid non-standard things >> >> http://en.wikipedia.org/wiki/C99#Implementations >> >> So unless you plan on compiling QEMU with xlc, pgi, or icc, I don't >> think relying on "standard things" really helps. >> >> QEMU doesn't support C99, it supports GCC. > > OK, you could perhaps rephrase that as 'mainstream' rather than > 'standards-compliant'. I don't think we need to be strict C99; > I do think we have more than one working host OS and that patches > that use functionality that's documented not to work on all gcc > targets ought to come attached to a statement that they've been > tested. (MacOSX isn't actually in MAINTAINERS as a host so is > a bit of a red herring. Windows is listed.) I'd also like to avoid a world where everything only targets GCC on x86_64 on Linux with KVM. "Embrace and extend" may also be seen to apply to GCC extensions. > > So if you really like weak symbols, go ahead. I'm just saying > you're imposing a bigger testing burden on yourself than if > you handled this some other way. > > (I do think it would be nice to care about building with CLANG, > because there are some static analysis tools that we would > then be able to run. That doesn't mean dropping all GCC > extensions, though, because CLANG does support a lot of them.) > > -- PMM >
diff --git a/compiler.h b/compiler.h index 736e770..f76921e 100644 --- a/compiler.h +++ b/compiler.h @@ -45,6 +45,7 @@ # define GCC_ATTR __attribute__((__unused__, format(gnu_printf, 1, 2))) # define GCC_FMT_ATTR(n, m) __attribute__((format(gnu_printf, n, m))) # endif +#define GCC_WEAK __attribute__((weak)) #else #define GCC_ATTR /**/ #define GCC_FMT_ATTR(n, m)
This lets us provide a default implementation of a symbol which targets can override. Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> --- compiler.h | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)