disas/libvixl/a64/instructions-a64.h: Remove useless varialbe to avoid building break with '-Werror'
PATCH 1/1 disas/libvixl/a64/instructions-a64.h: Remove useless varialbe to avoid building break with '-Werror'
The related variables are useless, need be removed, or can not pass microblaze building, after fix it, can build microblaze, successfully.
The related configuration:
./configure --target-list="arm-softmmu,microblazeel-softmmu" --enable-fdt --disable-kvm
The related compiling error:
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
--- disas/libvixl/a64/instructions-a64.h | 24 ------------------------ 1 file changed, 24 deletions(-)
disas/libvixl/a64/instructions-a64.h +0 -24
@@ -95,30 +95,6 @@ const unsigned kDoubleExponentBits = 11;const unsigned kFloatMantissaBits =23 ;const unsigned kFloatExponentBits =8 ; -const float kFP32PositiveInfinity =rawbits_to_float (0x7f800000 ); -const float kFP32NegativeInfinity =rawbits_to_float (0xff800000 ); -const double kFP64PositiveInfinity = -rawbits_to_double (UINT64_C (0x7ff0000000000000 )); -const double kFP64NegativeInfinity = -rawbits_to_double (UINT64_C (0xfff0000000000000 )); - -// This value is a signalling NaN as both a double and as a float (taking the -// least-significant word). -static const double kFP64SignallingNaN = -rawbits_to_double (UINT64_C (0x7ff000007f800001 )); -static const float kFP32SignallingNaN =rawbits_to_float (0x7f800001 ); - -// A similar value, but as a quiet NaN. -static const double kFP64QuietNaN = -rawbits_to_double (UINT64_C (0x7ff800007fc00001 )); -static const float kFP32QuietNaN =rawbits_to_float (0x7fc00001 ); - -// The default NaN values (for FPCR.DN=1). -static const double kFP64DefaultNaN = -rawbits_to_double (UINT64_C (0x7ff8000000000000 )); -static const float kFP32DefaultNaN =rawbits_to_float (0x7fc00000 ); - -enum LSDataSize { LSByte =0 , LSHalfword =1 ,
Signature · 1 line
1.9.3
Quoted 8 lines · Chen Gang
The related variables are useless, need be removed, or can not pass microblaze building, after fix it, can build microblaze, successfully.
The related configuration:
./configure --target-list="arm-softmmu,microblazeel-softmmu" --enable-fdt --disable-kvm
The related compiling error:
I build this code with both these targets enabled without any problems.
There is an odd compiler thing where if you have any *other* compilation issues then these warnings will also be emitted, but once you've fixed that other compiler error then these warnings are no longer produced. Maybe you ran into that?
The reason I'm reluctant to make changes to these files is that they're pulled in from a different upstream project (libvixl) so we should only fix critical problems in them, or it makes new versions harder to update to.
thanks
-- PMM
On 10/09/2014 08:00 AM, Chen Gang wrote:
That's a very long subject line. Try to keep subjects around 60 characters or so ('git shortlog -30' can give you an idea of reasonable subjects). Also, s/varialbe/variable/ in the subject.
Quoted 3 lines
The related variables are useless, need be removed, or can not pass microblaze building, after fix it, can build microblaze, successfully.
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Quoted 5 lines · Eric Blake
On 10/09/2014 08:00 AM, Chen Gang wrote:
That's a very long subject line. Try to keep subjects around 60 characters or so ('git shortlog -30' can give you an idea of reasonable subjects).
OK, thanks, I shall notice about it next (also let 'git shortlog -30' as reference).
Quoted 3 lines
Also, s/varialbe/variable/ in the subject.
Quoted 1 line
The related variables are useless, need be removed, or can not pass
OK, thanks. If this patch can still continue (pass checking), I shall send patch v2 for it.
Thanks.
Signature · 3 lines
Chen Gang Open, share, and attitude like air, water, and life which God blessed
Quoted 10 lines · Peter Maydell
Quoted 8 lines · Chen Gang
The related variables are useless, need be removed, or can not pass microblaze building, after fix it, can build microblaze, successfully.
The related configuration:
./configure --target-list="arm-softmmu,microblazeel-softmmu" --enable-fdt --disable-kvm
The related compiling error:
Quoted 8 lines
I build this code with both these targets enabled without any problems.
There is an odd compiler thing where if you have any *other* compilation issues then these warnings will also be emitted, but once you've fixed that other compiler error then these warnings are no longer produced. Maybe you ran into that?
I use the latest upstream gcc (which pulled from master in 2014-10-0?). In my memory (not quite sure), the older version gcc may not notice about this warning.
But for me, the warning (compiler worries about) sounds reasonable, and it's harmless to be fixed (after have a look, for me, they are declared, but never be used).
Quoted 5 lines
The reason I'm reluctant to make changes to these files is that they're pulled in from a different upstream project (libvixl) so we should only fix critical problems in them, or it makes new versions harder to update to.
Originally, I first try the Xilinx branch (Xilinx-master from Xilinx github), yesterday, and found this issue, then I try upstream main branch, found the same issue.
For me, when add the related patch (which will use these variables in 'libvixl'), then declare and set them in the related headers, again. That will let other reviewers and readers easier understanding.
- removing them at present, is easy understanding.
- add them again when really need them, is also easy understanding.
So for me, I still prefer to remove these declarations firstly.
Thanks.
Signature · 3 lines
Chen Gang Open, share, and attitude like air, water, and life which God blessed
Quoted 3 lines · Chen Gang
I use the latest upstream gcc (which pulled from master in 2014-10-0?). In my memory (not quite sure), the older version gcc may not notice about this warning.
Hmm. I'll see if I can test with that gcc.
Quoted 3 lines
But for me, the warning (compiler worries about) sounds reasonable, and it's harmless to be fixed (after have a look, for me, they are declared, but never be used).
It's a library. Other users of this code upstream will use these constants; it's just that we don't happen to.
Quoted 10 lines
Quoted 5 lines
The reason I'm reluctant to make changes to these files is that they're pulled in from a different upstream project (libvixl) so we should only fix critical problems in them, or it makes new versions harder to update to.
Originally, I first try the Xilinx branch (Xilinx-master from Xilinx github), yesterday, and found this issue, then I try upstream main branch, found the same issue.
Quoted 7 lines
For me, when add the related patch (which will use these variables in 'libvixl'), then declare and set them in the related headers, again. That will let other reviewers and readers easier understanding.
- removing them at present, is easy understanding.
- add them again when really need them, is also easy understanding.
But it's all changes which we would have to carry locally and then re-make every time we updated to a new libvixl. I definitely don't want to do that unless it's absolutely required.
thanks
-- PMM
Quoted 7 lines · Peter Maydell
Quoted 3 lines · Chen Gang
I use the latest upstream gcc (which pulled from master in 2014-10-0?). In my memory (not quite sure), the older version gcc may not notice about this warning.
Hmm. I'll see if I can test with that gcc.
It is not quite difficult to do that: get upstream source code, follow the related document to build, then use it, it should be OK (I just do like that).
Quoted 21 lines
Quoted 3 lines
But for me, the warning (compiler worries about) sounds reasonable, and it's harmless to be fixed (after have a look, for me, they are declared, but never be used).
It's a library. Other users of this code upstream will use these constants; it's just that we don't happen to.
Quoted 14 lines
Quoted 5 lines
The reason I'm reluctant to make changes to these files is that they're pulled in from a different upstream project (libvixl) so we should only fix critical problems in them, or it makes new versions harder to update to.
Originally, I first try the Xilinx branch (Xilinx-master from Xilinx github), yesterday, and found this issue, then I try upstream main branch, found the same issue.
For me, when add the related patch (which will use these variables in 'libvixl'), then declare and set them in the related headers, again. That will let other reviewers and readers easier understanding.
Quoted 9 lines
Quoted 3 lines
- removing them at present, is easy understanding.
- add them again when really need them, is also easy understanding.
But it's all changes which we would have to carry locally and then re-make every time we updated to a new libvixl. I definitely don't want to do that unless it's absolutely required.
It is really a little complex, we almost can not touch this header file, sorry for my original misunderstanding.
And I guess, "disas/arm-64.cc" is our own file (only for qemu, not from libvixl upstream project). If really it is, we may do something in it to avoid this warning, e.g.
"#pragma GCC diagnostic ignored -Wunused-variable" (almost like "include/ui/qemu-pixman.h" have done).
Thanks.
Signature · 3 lines
Chen Gang Open, share, and attitude like air, water, and life which God blessed
Quoted 25 lines · Chen Gang
Quoted 14 lines · Peter Maydell
Quoted 14 lines
Quoted 5 lines
The reason I'm reluctant to make changes to these files is that they're pulled in from a different upstream project (libvixl) so we should only fix critical problems in them, or it makes new versions harder to update to.
Originally, I first try the Xilinx branch (Xilinx-master from Xilinx github), yesterday, and found this issue, then I try upstream main branch, found the same issue.
For me, when add the related patch (which will use these variables in 'libvixl'), then declare and set them in the related headers, again. That will let other reviewers and readers easier understanding.
Quoted 9 lines
Quoted 3 lines
- removing them at present, is easy understanding.
- add them again when really need them, is also easy understanding.
But it's all changes which we would have to carry locally and then re-make every time we updated to a new libvixl. I definitely don't want to do that unless it's absolutely required.
Quoted 9 lines
It is really a little complex, we almost can not touch this header file, sorry for my original misunderstanding.
And I guess, "disas/arm-64.cc" is our own file (only for qemu, not from libvixl upstream project). If really it is, we may do something in it to avoid this warning, e.g.
"#pragma GCC diagnostic ignored -Wunused-variable" (almost like "
include/ui/qemu-pixman.h" have done).
Oh, may need "-Wunused-but-set-variable" instead of "-Wunused-variable", and also need check CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE (I guess the latest gcc should be OK).
If what I guess is OK, I shall try to test it under both the latest gcc for x86_64 and the host gcc under Fedora 20 x86_64. If they are all OK, I shall send patch v2 for it.
Thanks.
Signature · 3 lines
Chen Gang Open, share, and attitude like air, water, and life which God blessed
Quoted 73 lines · Chen Gang
The related variables are useless, need be removed, or can not pass microblaze building, after fix it, can build microblaze, successfully.
The related configuration:
./configure --target-list="arm-softmmu,microblazeel-softmmu" --enable-fdt --disable-kvm
The related compiling error:
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 1Signed-off-by Chen Gang <[email protected]>--- disas/libvixl/a64/instructions-a64.h | 24 ------------------------ 1 file changed, 24 deletions(-)disas/libvixl/a64/instructions-a64.h +0
-14@@ -95,30 +95,6 @@ const unsigned kDoubleExponentBits = 11;const unsigned kFloatMantissaBits =23 ;const unsigned kFloatExponentBits =8 ; -const float kFP32PositiveInfinity =rawbits_to_float (0x7f800000 ); -const float kFP32NegativeInfinity =rawbits_to_float (0xff800000 ); -const double kFP64PositiveInfinity = -rawbits_to_double (UINT64_C (0x7ff0000000000000 )); -const double kFP64NegativeInfinity = -rawbits_to_double (UINT64_C (0xfff0000000000000 )); - -// This value is a signalling NaN as both a double and as a float (taking the -// least-significant word). -static const double kFP64SignallingNaN = -rawbits_to_double (UINT64_C (0x7ff000007f800001 )); -static const float kFP32SignallingNaN =rawbits_to_float (0x7f800001 ); - -// A similar value, but as a quiet NaN.
disas/libvixl/a64/instructions-a64.h const unsigned kDoubleExponentBits = 11; · line 121
Quoted 10 lines
disas/libvixl/a64/instructions-a64.h +0
-10-static const double kFP64QuietNaN = -rawbits_to_double (UINT64_C (0x7ff800007fc00001 )); -static const float kFP32QuietNaN =rawbits_to_float (0x7fc00001 ); - -// The default NaN values (for FPCR.DN=1). -static const double kFP64DefaultNaN = -rawbits_to_double (UINT64_C (0x7ff8000000000000 )); -static const float kFP32DefaultNaN =rawbits_to_float (0x7fc00000 ); - -
Upstream's plan for fixing this is to turn these into 'extern const double foo' in the header with the definition in an appropriate .cc file. Given that, I think I'm happy for us to take this patch in QEMU for the moment, with the expectation that the next libvixl drop will just obsolete it.
-- PMM
Quoted 1 line · Peter Maydell
On 9 October 2014 15:00, Chen Gang <[email protected]> wrote:
[]
Quoted 8 lines
Quoted 8 lines
disas/libvixl/a64/instructions-a64.h +0
-2@@ -95,30 +95,6 @@ const unsigned kDoubleExponentBits = 11;const unsigned kFloatMantissaBits =23 ;const unsigned kFloatExponentBits =8 ; -const float kFP32PositiveInfinity =rawbits_to_float (0x7f800000 ); -const float kFP32NegativeInfinity =rawbits_to_float (0xff800000 );
...
Quoted 5 lines
Upstream's plan for fixing this is to turn these into 'extern const double foo' in the header with the definition in an appropriate .cc file. Given that, I think I'm happy for us to take this patch in QEMU for the moment, with the expectation that the next libvixl drop will just obsolete it.
If upstream already have a patch for this, why not take the upstream's solution now, so we wont need to revert our solution before applying next drop from upstream?
BTW, how the "upstream drop" is done, anyway? If it is something like git merge, we may need to care, but if it is just cp -a upstream/* . when there's no reason to... ;)
Thanks,
/mjt
Quoted 3 lines · Michael Tokarev
If upstream already have a patch for this, why not take the upstream's solution now, so we wont need to revert our solution before applying next drop from upstream?
Upstream haven't released it yet.
Quoted 3 lines
BTW, how the "upstream drop" is done, anyway? If it is something like git merge, we may need to care, but if it is just cp -a upstream/* . when there's no reason to... ;)
It's a copy. As you say, since it's just a copy we don't need to worry about merge conflicts so we can just fix it in a convenient way for us for the moment.
thanks
-- PMM
Michael Tokarev
AppliedQuoted 2 lines · Chen Gang
The related variables are useless, need be removed, or can not pass microblaze building, after fix it, can build microblaze, successfully.
...
Applied to trivial (with subject fixup), thanks!
/mjt
Quoted 6 lines · Michael Tokarev
Quoted 2 lines · Chen Gang
The related variables are useless, need be removed, or can not pass microblaze building, after fix it, can build microblaze, successfully.
...
Applied to trivial (with subject fixup), thanks!
Please don't -- as I said, I have a libvixl update in my queue, and these will conflict. I'll apply an appropriate variation of this patch to my target-arm tree.
thanks
-- PMM
Quoted 6 lines · Peter Maydell
Quoted 1 line · Michael Tokarev
Applied to trivial (with subject fixup), thanks!
Please don't -- as I said, I have a libvixl update in my queue, and these will conflict. I'll apply an appropriate variation of this patch to my target-arm tree.
...or maybe I just *thought* I'd said that, since I can't find a mail in this thread where I did; sorry.
-- PMM
Quoted 10 lines · Peter Maydell
Quoted 6 lines · Peter Maydell
Quoted 1 line · Michael Tokarev
Applied to trivial (with subject fixup), thanks!
Please don't -- as I said, I have a libvixl update in my queue, and these will conflict. I'll apply an appropriate variation of this patch to my target-arm tree.
...or maybe I just *thought* I'd said that, since I can't find a mail in this thread where I did; sorry.
You did, in a "v2" version. Which I overlooked really, and tried to apply the original v1 (which has you reviewed-by as a temp. workaround). Oh well.. :)
There's quite a lot of noize about these things recently.
Not applying it.
Thanks,
/mjt\
Quoted 9 lines · Michael Tokarev
Quoted 8 lines · Peter Maydell
Quoted 6 lines · Peter Maydell
Quoted 1 line · Michael Tokarev
Applied to trivial (with subject fixup), thanks!
Please don't -- as I said, I have a libvixl update in my queue, and these will conflict. I'll apply an appropriate variation of this patch to my target-arm tree.
Quoted 6 lines
Quoted 2 lines
...or maybe I just *thought* I'd said that, since I can't find a mail in this thread where I did; sorry.
You did, in a "v2" version. Which I overlooked really, and tried to apply the original v1 (which has you reviewed-by as a temp. workaround). Oh well.. :)
Oh yes. I actually prefer v1, so I've applied it (with a tweaked commit message) to target-arm.next. When libvixl 1.7 appears we can just drop this change.
-- PMM