libvixl: a64: Skip "-Wunused-variable" for gcc 5.0.0 or higher
PATCH v2 1/1 libvixl: a64: Skip "-Wunused-variable" for gcc 5.0.0 or higher
'instructions-a64.h' has unused variables for qemu which can be found by gcc 5.0.0 or higher. and qemu needs "-Werror", and cause building break. But they may be used by another projects (not qemu).
So for gcc 5.0.0 or higher, need still keep them, but ignore diagnostic (still print warning, but not break building). The related warnings:
CXX disas/arm-a64.o
In file included from /upstream/qemu/disas/libvixl/a64/disasm-a64.h:32:0,
from disas/arm-a64.cc:20:
disas/libvixl/a64/instructions-a64.h:98:13: error: 'vixl::kFP32PositiveInfinity' defined but not used [-Werror=unused-variable]
const float kFP32PositiveInfinity = rawbits_to_float(0x7f800000);
^
disas/libvixl/a64/instructions-a64.h:99:13: error: 'vixl::kFP32NegativeInfinity' defined but not used [-Werror=unused-variable]
const float kFP32NegativeInfinity = rawbits_to_float(0xff800000);
^
disas/libvixl/a64/instructions-a64.h:100:14: error: 'vixl::kFP64PositiveInfinity' defined but not used [-Werror=unused-variable]
const double kFP64PositiveInfinity =
^
disas/libvixl/a64/instructions-a64.h:102:14: error: 'vixl::kFP64NegativeInfinity' defined but not used [-Werror=unused-variable]
const double kFP64NegativeInfinity =
^
disas/libvixl/a64/instructions-a64.h:107:21: error: 'vixl::kFP64SignallingNaN' defined but not used [-Werror=unused-variable]
static const double kFP64SignallingNaN =
^
disas/libvixl/a64/instructions-a64.h:109:20: error: 'vixl::kFP32SignallingNaN' defined but not used [-Werror=unused-variable]
static const float kFP32SignallingNaN = rawbits_to_float(0x7f800001);
^
disas/libvixl/a64/instructions-a64.h:112:21: error: 'vixl::kFP64QuietNaN' defined but not used [-Werror=unused-variable]
static const double kFP64QuietNaN =
^
disas/libvixl/a64/instructions-a64.h:114:20: error: 'vixl::kFP32QuietNaN' defined but not used [-Werror=unused-variable]
static const float kFP32QuietNaN = rawbits_to_float(0x7fc00001);
^
disas/libvixl/a64/instructions-a64.h:117:21: error: 'vixl::kFP64DefaultNaN' defined but not used [-Werror=unused-variable]
static const double kFP64DefaultNaN =
^
disas/libvixl/a64/instructions-a64.h:119:20: error: 'vixl::kFP32DefaultNaN' defined but not used [-Werror=unused-variable]
static const float kFP32DefaultNaN = rawbits_to_float(0x7fc00000);
^ cc1plus: all warnings being treated as errors
make: *** [disas/arm-a64.o] Error 1
After this patch, can pass upstream gcc 5.0.0 building (print warning, but not break building), and fedora 20 gcc 4.8.1 building (not find warnings).
--- disas/libvixl/a64/instructions-a64.h | 9 +++++++++ 1 file changed, 9 insertions(+)
disas/libvixl/a64/instructions-a64.h +9 -0
@@ -95,6 +95,12 @@ const unsigned kDoubleExponentBits = 11;const unsigned kFloatMantissaBits =23 ;const unsigned kFloatExponentBits =8 ; +/* For QEMU, gcc 5.0.0 or higher finds unused variables, so ignore diagnostic */ +#if __GNUC__ >=5 +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wunused-variable" +#endif +const float kFP32PositiveInfinity =rawbits_to_float (0x7f800000 );const float kFP32NegativeInfinity =rawbits_to_float (0xff800000 ); const double kFP64PositiveInfinity =
@@ -118,6 +124,9 @@ static const double kFP64DefaultNaN =rawbits_to_double (UINT64_C (0x7ff8000000000000 ));static const float kFP32DefaultNaN =rawbits_to_float (0x7fc00000 ); +#if __GNUC__ >=5 +#pragma GCC diagnostic pop +#endif enum LSDataSize { LSByte =0 ,
Signature · 1 line
1.9.3
Quoted 40 lines · Chen Gang
'
instructions-a64.h' has unused variables for qemu which can be found by gcc 5.0.0 or higher. and qemu needs "-Werror", and cause building break. But they may be used by another projects (not qemu).So for gcc 5.0.0 or higher, need still keep them, but ignore diagnostic (still print warning, but not break building). The related warnings:
CXX
disas/arm-a64.o
In file included from /upstream/qemu/disas/libvixl/a64/disasm-a64.h:32:0,from disas/arm-a64.cc:20: disas/libvixl/a64/instructions-a64.h:98:13: error: 'vixl::kFP32PositiveInfinity' defined but not used [-Werror=unused-variable] const float kFP32PositiveInfinity = rawbits_to_float(0x7f800000); ^ disas/libvixl/a64/instructions-a64.h:99:13: error: 'vixl::kFP32NegativeInfinity' defined but not used [-Werror=unused-variable] const float kFP32NegativeInfinity = rawbits_to_float(0xff800000); ^ disas/libvixl/a64/instructions-a64.h:100:14: error: 'vixl::kFP64PositiveInfinity' defined but not used [-Werror=unused-variable] const double kFP64PositiveInfinity = ^ disas/libvixl/a64/instructions-a64.h:102:14: error: 'vixl::kFP64NegativeInfinity' defined but not used [-Werror=unused-variable] const double kFP64NegativeInfinity = ^ disas/libvixl/a64/instructions-a64.h:107:21: error: 'vixl::kFP64SignallingNaN' defined but not used [-Werror=unused-variable] static const double kFP64SignallingNaN = ^ disas/libvixl/a64/instructions-a64.h:109:20: error: 'vixl::kFP32SignallingNaN' defined but not used [-Werror=unused-variable] static const float kFP32SignallingNaN = rawbits_to_float(0x7f800001); ^ disas/libvixl/a64/instructions-a64.h:112:21: error: 'vixl::kFP64QuietNaN' defined but not used [-Werror=unused-variable] static const double kFP64QuietNaN = ^ disas/libvixl/a64/instructions-a64.h:114:20: error: 'vixl::kFP32QuietNaN' defined but not used [-Werror=unused-variable] static const float kFP32QuietNaN = rawbits_to_float(0x7fc00001); ^ disas/libvixl/a64/instructions-a64.h:117:21: error: 'vixl::kFP64DefaultNaN' defined but not used [-Werror=unused-variable] static const double kFP64DefaultNaN = ^ disas/libvixl/a64/instructions-a64.h:119:20: error: 'vixl::kFP32DefaultNaN' defined but not used [-Werror=unused-variable] static const float kFP32DefaultNaN = rawbits_to_float(0x7fc00000); ^
Quoted 5 lines
cc1plus: all warnings being treated as errors
make: *** [disas/arm-a64.o] Error 1After this patch, can pass upstream gcc 5.0.0 building (print warning, but not break building), and fedora 20 gcc 4.8.1 building (not find
Oh, sorry, under fedora 20, it is "gcc version 4.8.3 20140624 (Red Hat 4.8.3-1) (GCC)".
And the detail gcc 5.0.0 is "gcc version 5.0.0 20141003 (experimental) (GCC)"
Quoted 35 lines
warnings).
Signed-off-by Chen Gang <[email protected]>--- disas/libvixl/a64/instructions-a64.h | 9 +++++++++ 1 file changed, 9 insertions(+)disas/libvixl/a64/instructions-a64.h +9
-0@@ -95,6 +95,12 @@ const unsigned kDoubleExponentBits = 11;const unsigned kFloatMantissaBits =23 ;const unsigned kFloatExponentBits =8 ; +/* For QEMU, gcc 5.0.0 or higher finds unused variables, so ignore diagnostic */ +#if __GNUC__ >=5 +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wunused-variable" +#endif +const float kFP32PositiveInfinity =rawbits_to_float (0x7f800000 );const float kFP32NegativeInfinity =rawbits_to_float (0xff800000 ); const double kFP64PositiveInfinity =@@ -118,6 +124,9 @@ static const double kFP64DefaultNaN =rawbits_to_double (UINT64_C (0x7ff8000000000000 ));static const float kFP32DefaultNaN =rawbits_to_float (0x7fc00000 ); +#if __GNUC__ >=5 +#pragma GCC diagnostic pop +#endif enum LSDataSize { LSByte =0 ,
Signature · 3 lines
Chen Gang Open, share, and attitude like air, water, and life which God blessed
On 11 October 2014 15:13, Chen Gang <[email protected]> wrote:
MJT: please don't put this in -trivial, it will clash with the update to libvixl 1.6 currently on the list.
Quoted 8 lines · Chen Gang
Quoted 7 lines
'
instructions-a64.h' has unused variables for qemu which can be found by gcc 5.0.0 or higher. and qemu needs "-Werror", and cause building break. But they may be used by another projects (not qemu).So for gcc 5.0.0 or higher, need still keep them, but ignore diagnostic (still print warning, but not break building). The related warnings:
Quoted 7 lines
Quoted 7 lines
CXX
disas/arm-a64.o
In file included from /upstream/qemu/disas/libvixl/a64/disasm-a64.h:32:0,from disas/arm-a64.cc:20: disas/libvixl/a64/instructions-a64.h:98:13: error: 'vixl::kFP32PositiveInfinity' defined but not used [-Werror=unused-variable] const float kFP32PositiveInfinity = rawbits_to_float(0x7f800000); ^
disas/libvixl/a64/instructions-a64.h:99:13: error: 'vixl::kFP32NegativeInfinity' defined but not used [-Werror=unused-variable]
So, several questions here:
(1) Does anybody know how the header in question should be providing constant floats and doubles to its users in a way that doesn't cause gcc to complain? If so, I can pass the answer along so at least the next version of the library is OK...
(2) Bonus question, anybody know why gcc is perfectly happy with the unused integer constants in the .h file but doesn't like the float and doubles?
(3) Does gcc 5 emit these warnings for all the .cc files that pull in this header, or only for the arm-a64.cc one?
Some other approaches to this that would confine the fix to the makefiles rather than requiring us to modify the vixl source itself:
a) add a -Wno- option for the affected .o files
b) use -isystem rather than -I to include the libvixl directory on the include path
(a)'s probably better I guess.
thanks
-- PMM
Quoted 5 lines · Peter Maydell
On 11 October 2014 15:13, Chen Gang <[email protected]> wrote:
MJT: please don't put this in -trivial, it will clash with the update to libvixl 1.6 currently on the list.
OK thanks (also remove -trivial from replying mailing list).
Quoted 17 lines
Quoted 9 lines · Chen Gang
Quoted 7 lines
'
instructions-a64.h' has unused variables for qemu which can be found by gcc 5.0.0 or higher. and qemu needs "-Werror", and cause building break. But they may be used by another projects (not qemu).So for gcc 5.0.0 or higher, need still keep them, but ignore diagnostic (still print warning, but not break building). The related warnings:
Quoted 7 lines
Quoted 7 lines
CXX
disas/arm-a64.o
In file included from /upstream/qemu/disas/libvixl/a64/disasm-a64.h:32:0,from disas/arm-a64.cc:20: disas/libvixl/a64/instructions-a64.h:98:13: error: 'vixl::kFP32PositiveInfinity' defined but not used [-Werror=unused-variable] const float kFP32PositiveInfinity = rawbits_to_float(0x7f800000); ^
disas/libvixl/a64/instructions-a64.h:99:13: error: 'vixl::kFP32NegativeInfinity' defined but not used [-Werror=unused-variable]
Quoted 8 lines
So, several questions here:
(1) Does anybody know how the header in question should be providing constant floats and doubles to its users in a way that doesn't cause gcc to complain? If so, I can pass the answer along so at least the next version of the library is OK...
Hmm... may try to simplify rawbits_to_float() and rawbits_to_double() to let gcc know about it is only for constant? (it seems original authors intended to do it in function, I shall try to know why)
I shall try to get a conclusion within this month.
Quoted 4 lines
(2) Bonus question, anybody know why gcc is perfectly happy with the unused integer constants in the .h file but doesn't like the float and doubles?
For (2), they use a real function rawbits_to_float(), is it gcc own issue? I shall try to analyze it (at present, I am also upstream gcc members, and make a patch for it per month).
I shall try to finish analyzing within this month.
Quoted 3 lines
(3) Does gcc 5 emit these warnings for all the .cc files that pull in this header, or only for the arm-a64.cc one?
For (3), yes, it is, so I have to let it in header again.
Quoted 4 lines
Some other approaches to this that would confine the fix to the makefiles rather than requiring us to modify the vixl source itself:
a) add a -Wno- option for the affected .o files
It is one way, but may have effect with gcc 4 version, and also it is effect with the whole file which is wider than current way.
Quoted 3 lines
b) use -isystem rather than -I to include the libvixl directory on the include path
It sounds good to me, although for me, it is not related with current issue.
Thanks
Signature · 3 lines
Chen Gang Open, share, and attitude like air, water, and life which God blessed
Quoted 9 lines · Chen Gang
Quoted 4 lines · Peter Maydell
Some other approaches to this that would confine the fix to the makefiles rather than requiring us to modify the vixl source itself:
a) add a -Wno- option for the affected .o filesIt is one way, but may have effect with gcc 4 version, and also it is effect with the whole file which is wider than current way.
Quoted 6 lines
Quoted 3 lines
b) use -isystem rather than -I to include the libvixl directory on the include path
It sounds good to me, although for me, it is not related with current issue.
-isystem disables a bunch of gcc warnings automatically, which is why I suggested it. I'm not overall sure it's a great idea though.
-- PMM
Quoted 14 lines · Peter Maydell
Quoted 13 lines · Chen Gang
Quoted 4 lines · Peter Maydell
Some other approaches to this that would confine the fix to the makefiles rather than requiring us to modify the vixl source itself:
a) add a -Wno- option for the affected .o filesIt is one way, but may have effect with gcc 4 version, and also it is effect with the whole file which is wider than current way.
Quoted 3 lines
b) use -isystem rather than -I to include the libvixl directory on the include path
Quoted 7 lines
Quoted 2 lines
It sounds good to me, although for me, it is not related with current issue.
-isystem disables a bunch of gcc warnings automatically, which is why I suggested it. I'm not overall sure it's a great idea though.
OK, thanks. "-isystem" really can skip this warning, originally, I am not notice about it. :-)
But unlucky, other files within 'libvix' which also include this header file, also report this warning. So for me, it is not a good idea to let '-isystem' for the 'libvix' own source files.
Next, I shall firstly confirm whether it is a gcc 5.0 (g++) issue or not in gcc upstream mailing list.
- if it is gcc 5.0 issue, I shall try to fix it within this month.
- else, for me, this patch v2 can still continue.
Thanks.
Signature · 3 lines
Chen Gang Open, share, and attitude like air, water, and life which God blessed
Quoted 14 lines · Peter Maydell
Quoted 13 lines · Chen Gang
Quoted 4 lines · Peter Maydell
Some other approaches to this that would confine the fix to the makefiles rather than requiring us to modify the vixl source itself:
a) add a -Wno- option for the affected .o filesIt is one way, but may have effect with gcc 4 version, and also it is effect with the whole file which is wider than current way.
Quoted 3 lines
b) use -isystem rather than -I to include the libvixl directory on the include path
Quoted 6 lines
Quoted 2 lines
It sounds good to me, although for me, it is not related with current issue.
-isystem disables a bunch of gcc warnings automatically, which is why I suggested it. I'm not overall sure it's a great idea though.
-isystem is a heavy hammer, affecting the entire compilation. Better might be just marking the ONE header as being a system header (silence various warnings caused by just that header, while still letting the rest of the compilation warn). If the header comes from third-party sources, this is probably the best approach. It is done by adding:
#if __GNUC__ >= 3
#pragma GCC system_header
#endif
to the header that would otherwise trigger warnings.
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Quoted 11 lines · Eric Blake
-isystem is a heavy hammer, affecting the entire compilation. Better might be just marking the ONE header as being a system header (silence various warnings caused by just that header, while still letting the rest of the compilation warn). If the header comes from third-party sources, this is probably the best approach. It is done by adding:
#if
__GNUC__>= 3
#pragma GCCsystem_header
#endifto the header that would otherwise trigger warnings.
If we're going to modify the header then we might as well just disable the specific warning that's being triggered. The only benefit of -isystem is you can do it in the makefile and leave the source file untouched.
-- PMM
Quoted 4 lines · Peter Maydell
On 11 October 2014 15:13, Chen Gang <[email protected]> wrote:
MJT: please don't put this in -trivial, it will clash with the update to libvixl 1.6 currently on the list.
Actually I'd not put that to anywhere anyway. Because to me, *especially* in a case like this when the code is imported from some other place and will be updated later (so we should not modify it), this issue can be more easily dealt with externally, by adding a compiler option in a Makefile. Provided, ofcourse, that it is not fixed upstream properly to start with - and if it is (and this is the very sane way to go really, to fix it upstream), that fix can be pulled to qemu as well, so no clashes with further upstream changes will happen.
And aso because really, this prob should be fixed properly, not worked around like this..
[]
Quoted 8 lines
Some other approaches to this that would confine the fix to the makefiles rather than requiring us to modify the vixl source itself:
a) add a -Wno- option for the affected .o files
b) use -isystem rather than -I to include the libvixl directory on the include path(a)'s probably better I guess.
That's what I'm after too (after trying to fix it properly). And no, at this time I dont know how gcc5 handles this.
Thanks,
/mjt
Quoted 16 lines · Michael Tokarev
Quoted 4 lines · Peter Maydell
On 11 October 2014 15:13, Chen Gang <[email protected]> wrote:
MJT: please don't put this in -trivial, it will clash with the update to libvixl 1.6 currently on the list.
Actually I'd not put that to anywhere anyway. Because to me, *especially* in a case like this when the code is imported from some other place and will be updated later (so we should not modify it), this issue can be more easily dealt with externally, by adding a compiler option in a
Makefile. Provided, ofcourse, that it is not fixed upstream properly to start with - and if it is (and this is the very sane way to go really, to fix it upstream), that fix can be pulled to qemu as well, so no clashes with further upstream changes will happen.
Quoted 3 lines
And aso because really, this prob should be fixed properly, not worked around like this..
Within qemu, what you said sounds reasonable, but if we consider both libvixl and qemu together, for me, I'like to fix it in related source code.
Maybe I need send the related patch to libvixl firstly, then Cc to qemu. If the patch can not pass libvixl's checking, but qemu still need, we have to do in the way like what you said above.
By the way, originally, I misunderstood "Makefile.objs" under libvixl:
I thought they belong to libvixl, but in fact, they belong to qemu.
Quoted 8 lines
[]
Quoted 7 lines
Some other approaches to this that would confine the fix to the makefiles rather than requiring us to modify the vixl source itself:
a) add a -Wno- option for the affected .o files
b) use -isystem rather than -I to include the libvixl directory on the include path
Quoted 5 lines
Quoted 1 line
(a)'s probably better I guess.
That's what I'm after too (after trying to fix it properly). And no, at this time I dont know how gcc5 handles this.
At present, I have sent the related information to gcc upstream mailing list for gcc5, we are just discussing about it.
- Some gcc members stick to what gcc5 has done is correct (need still report warning).
- But for me, I am just trying to get another gcc members' confirmation. I am not quite familiar with C++, for me it is a complex language, so I need additional confirmation by another gcc members, at least.
Thanks.
Signature · 3 lines
Chen Gang Open share and attitude like air water and life which God blessed
Quoted 9 lines · Chen Gang
Quoted 4 lines · Michael Tokarev
That's what I'm after too (after trying to fix it properly). And no, at this time I dont know how gcc5 handles this.
At present, I have sent the related information to gcc upstream mailing list for gcc5, we are just discussing about it.
Quoted 7 lines
- Some gcc members stick to what gcc5 has done is correct (need still report warning).
- But for me, I am just trying to get another gcc members' confirmation. I am not quite familiar with C++, for me it is a complex language, so I need additional confirmation by another gcc members, at least.
After consult the gcc related members, we are sure what gcc has done is correct, and need process this warning in our qemu or 'libvixl'.
The related mail is below:
-------- Forwarded Message --------
Subject: Re: [Consult] g++: About "-Wunused-variable" for constant variable in header file
Date: Wed, 15 Oct 2014 10:18:44 +0100
From: Jonathan Wakely <[email protected]>
To: Chen Gang <[email protected]>
CC: gcc-help <[email protected]>, Jeff Law <[email protected]>, Peter Maydell <[email protected]>
Quoted 6 lines · Chen Gang
Hello All:
At present, I met one warning issue about gcc 5.0.0.
- For "const float a = 3.4 - 2.1 / 3;", if it is unused, gcc5 will not report warning.
Because there is no cost to initializing the variable.
Quoted 2 lines
- "const char
n(){return 1;}; const a =n();", if 'a' is unused, gcc5 will report warning.
Because it requires dynamic initialization, running a function at startup, which has a cost. If you don't use the variable then you might not want to run the initialization code at startup, so you get a warning.
Quoted 2 lines
For gcc old version (e.g. gcc4), it will not report warning. Is it the new feature for gcc5, or just a gcc5's bug?
I think this behaviour is intended and is not a bug.
Signature · 3 lines
Chen Gang Open, share, and attitude like air, water, and life which God blessed