All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] disas/libvixl/a64/instructions-a64.h: Remove useless varialbe to avoid building break with '-Werror'
@ 2014-10-09 14:00 Chen Gang
  2014-10-09 14:34 ` Peter Maydell
                   ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: Chen Gang @ 2014-10-09 14:00 UTC (permalink / raw)
  To: peter.maydell, rth; +Cc: qemu-trivial, qemu-devel

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


Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
---
 disas/libvixl/a64/instructions-a64.h | 24 ------------------------
 1 file changed, 24 deletions(-)

diff --git a/disas/libvixl/a64/instructions-a64.h b/disas/libvixl/a64/instructions-a64.h
index d5b90c5..1eea851 100644
--- a/disas/libvixl/a64/instructions-a64.h
+++ b/disas/libvixl/a64/instructions-a64.h
@@ -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,
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH] disas/libvixl/a64/instructions-a64.h: Remove useless varialbe to avoid building break with '-Werror'
  2014-10-09 14:00 [Qemu-devel] [PATCH] disas/libvixl/a64/instructions-a64.h: Remove useless varialbe to avoid building break with '-Werror' Chen Gang
@ 2014-10-09 14:34 ` Peter Maydell
  2014-10-10  1:54   ` Chen Gang
  2014-10-09 14:54 ` Eric Blake
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 16+ messages in thread
From: Peter Maydell @ 2014-10-09 14:34 UTC (permalink / raw)
  To: Chen Gang; +Cc: QEMU Trivial, qemu-devel, rth

On 9 October 2014 15:00, Chen Gang <gang.chen.5i5j@gmail.com> wrote:
> 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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH] disas/libvixl/a64/instructions-a64.h: Remove useless varialbe to avoid building break with '-Werror'
  2014-10-09 14:00 [Qemu-devel] [PATCH] disas/libvixl/a64/instructions-a64.h: Remove useless varialbe to avoid building break with '-Werror' Chen Gang
  2014-10-09 14:34 ` Peter Maydell
@ 2014-10-09 14:54 ` Eric Blake
  2014-10-10  1:28   ` Chen Gang
  2014-10-21 15:50 ` Peter Maydell
  2014-10-23  8:09 ` Michael Tokarev
  3 siblings, 1 reply; 16+ messages in thread
From: Eric Blake @ 2014-10-09 14:54 UTC (permalink / raw)
  To: Chen Gang, peter.maydell, rth; +Cc: qemu-trivial, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 501 bytes --]

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.

> The related variables are useless, need be removed, or can not pass
> microblaze building, after fix it, can build microblaze, successfully.
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH] disas/libvixl/a64/instructions-a64.h: Remove useless varialbe to avoid building break with '-Werror'
  2014-10-09 14:54 ` Eric Blake
@ 2014-10-10  1:28   ` Chen Gang
  0 siblings, 0 replies; 16+ messages in thread
From: Chen Gang @ 2014-10-10  1:28 UTC (permalink / raw)
  To: Eric Blake, peter.maydell, rth; +Cc: qemu-trivial, qemu-devel

On 10/9/14 22:54, Eric Blake wrote:
> 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).

>             Also, s/varialbe/variable/ in the subject.
> 
>> 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.
-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH] disas/libvixl/a64/instructions-a64.h: Remove useless varialbe to avoid building break with '-Werror'
  2014-10-09 14:34 ` Peter Maydell
@ 2014-10-10  1:54   ` Chen Gang
  2014-10-10  7:37     ` Peter Maydell
  0 siblings, 1 reply; 16+ messages in thread
From: Chen Gang @ 2014-10-10  1:54 UTC (permalink / raw)
  To: Peter Maydell; +Cc: QEMU Trivial, qemu-devel, rth

On 10/9/14 22:34, Peter Maydell wrote:
> On 9 October 2014 15:00, Chen Gang <gang.chen.5i5j@gmail.com> wrote:
>> 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?
> 

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).

> 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.
-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH] disas/libvixl/a64/instructions-a64.h: Remove useless varialbe to avoid building break with '-Werror'
  2014-10-10  1:54   ` Chen Gang
@ 2014-10-10  7:37     ` Peter Maydell
  2014-10-10  8:53       ` Chen Gang
  0 siblings, 1 reply; 16+ messages in thread
From: Peter Maydell @ 2014-10-10  7:37 UTC (permalink / raw)
  To: Chen Gang; +Cc: QEMU Trivial, qemu-devel, rth

On 10 October 2014 02:54, Chen Gang <gang.chen.5i5j@gmail.com> wrote:
> 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.

> 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.

>> 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.

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH] disas/libvixl/a64/instructions-a64.h: Remove useless varialbe to avoid building break with '-Werror'
  2014-10-10  7:37     ` Peter Maydell
@ 2014-10-10  8:53       ` Chen Gang
  2014-10-10  9:03         ` Chen Gang
  0 siblings, 1 reply; 16+ messages in thread
From: Chen Gang @ 2014-10-10  8:53 UTC (permalink / raw)
  To: Peter Maydell; +Cc: QEMU Trivial, qemu-devel, rth

On 10/10/14 15:37, Peter Maydell wrote:
> On 10 October 2014 02:54, Chen Gang <gang.chen.5i5j@gmail.com> wrote:
>> 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).

>> 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.
> 
>>> 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.
> 
> 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.
-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH] disas/libvixl/a64/instructions-a64.h: Remove useless varialbe to avoid building break with '-Werror'
  2014-10-10  8:53       ` Chen Gang
@ 2014-10-10  9:03         ` Chen Gang
  0 siblings, 0 replies; 16+ messages in thread
From: Chen Gang @ 2014-10-10  9:03 UTC (permalink / raw)
  To: Peter Maydell; +Cc: QEMU Trivial, qemu-devel, rth

On 10/10/14 16:53, Chen Gang wrote:
> On 10/10/14 15:37, Peter Maydell wrote:
>>>> 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.
>>
>> 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).
> 

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.
-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH] disas/libvixl/a64/instructions-a64.h: Remove useless varialbe to avoid building break with '-Werror'
  2014-10-09 14:00 [Qemu-devel] [PATCH] disas/libvixl/a64/instructions-a64.h: Remove useless varialbe to avoid building break with '-Werror' Chen Gang
  2014-10-09 14:34 ` Peter Maydell
  2014-10-09 14:54 ` Eric Blake
@ 2014-10-21 15:50 ` Peter Maydell
  2014-10-23  6:49   ` [Qemu-devel] [Qemu-trivial] " Michael Tokarev
  2014-10-23  8:09 ` Michael Tokarev
  3 siblings, 1 reply; 16+ messages in thread
From: Peter Maydell @ 2014-10-21 15:50 UTC (permalink / raw)
  To: Chen Gang; +Cc: QEMU Trivial, qemu-devel, rth

On 9 October 2014 15:00, Chen Gang <gang.chen.5i5j@gmail.com> wrote:
> 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
>
>
> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
> ---
>  disas/libvixl/a64/instructions-a64.h | 24 ------------------------
>  1 file changed, 24 deletions(-)
>
> diff --git a/disas/libvixl/a64/instructions-a64.h b/disas/libvixl/a64/instructions-a64.h
> index d5b90c5..1eea851 100644
> --- a/disas/libvixl/a64/instructions-a64.h
> +++ b/disas/libvixl/a64/instructions-a64.h
> @@ -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);
> -
> -

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.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

-- PMM

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [Qemu-trivial] [PATCH] disas/libvixl/a64/instructions-a64.h: Remove useless varialbe to avoid building break with '-Werror'
  2014-10-21 15:50 ` Peter Maydell
@ 2014-10-23  6:49   ` Michael Tokarev
  2014-10-23  7:14     ` Peter Maydell
  0 siblings, 1 reply; 16+ messages in thread
From: Michael Tokarev @ 2014-10-23  6:49 UTC (permalink / raw)
  To: Peter Maydell, Chen Gang; +Cc: QEMU Trivial, qemu-devel, rth

On 10/21/2014 07:50 PM, Peter Maydell wrote:
> On 9 October 2014 15:00, Chen Gang <gang.chen.5i5j@gmail.com> wrote:
[]
>> --- a/disas/libvixl/a64/instructions-a64.h
>> +++ b/disas/libvixl/a64/instructions-a64.h
>> @@ -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);
...

> 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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [Qemu-trivial] [PATCH] disas/libvixl/a64/instructions-a64.h: Remove useless varialbe to avoid building break with '-Werror'
  2014-10-23  6:49   ` [Qemu-devel] [Qemu-trivial] " Michael Tokarev
@ 2014-10-23  7:14     ` Peter Maydell
  0 siblings, 0 replies; 16+ messages in thread
From: Peter Maydell @ 2014-10-23  7:14 UTC (permalink / raw)
  To: Michael Tokarev; +Cc: QEMU Trivial, rth, Chen Gang, qemu-devel

On 23 October 2014 07:49, Michael Tokarev <mjt@tls.msk.ru> wrote:
> 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.

> 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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [Qemu-trivial] [PATCH] disas/libvixl/a64/instructions-a64.h: Remove useless varialbe to avoid building break with '-Werror'
  2014-10-09 14:00 [Qemu-devel] [PATCH] disas/libvixl/a64/instructions-a64.h: Remove useless varialbe to avoid building break with '-Werror' Chen Gang
                   ` (2 preceding siblings ...)
  2014-10-21 15:50 ` Peter Maydell
@ 2014-10-23  8:09 ` Michael Tokarev
  2014-10-23  9:02   ` Peter Maydell
  3 siblings, 1 reply; 16+ messages in thread
From: Michael Tokarev @ 2014-10-23  8:09 UTC (permalink / raw)
  To: Chen Gang, peter.maydell, rth; +Cc: qemu-trivial, qemu-devel

On 10/09/2014 06:00 PM, Chen Gang wrote:
> 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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [Qemu-trivial] [PATCH] disas/libvixl/a64/instructions-a64.h: Remove useless varialbe to avoid building break with '-Werror'
  2014-10-23  8:09 ` Michael Tokarev
@ 2014-10-23  9:02   ` Peter Maydell
  2014-10-23 10:09     ` Peter Maydell
  0 siblings, 1 reply; 16+ messages in thread
From: Peter Maydell @ 2014-10-23  9:02 UTC (permalink / raw)
  To: Michael Tokarev; +Cc: QEMU Trivial, qemu-devel, Chen Gang, rth

On 23 October 2014 09:09, Michael Tokarev <mjt@tls.msk.ru> wrote:
> On 10/09/2014 06:00 PM, Chen Gang wrote:
>> 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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [Qemu-trivial] [PATCH] disas/libvixl/a64/instructions-a64.h: Remove useless varialbe to avoid building break with '-Werror'
  2014-10-23  9:02   ` Peter Maydell
@ 2014-10-23 10:09     ` Peter Maydell
  2014-10-23 10:27       ` Michael Tokarev
  0 siblings, 1 reply; 16+ messages in thread
From: Peter Maydell @ 2014-10-23 10:09 UTC (permalink / raw)
  To: Michael Tokarev; +Cc: QEMU Trivial, qemu-devel, Chen Gang, rth

On 23 October 2014 10:02, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 23 October 2014 09:09, Michael Tokarev <mjt@tls.msk.ru> wrote:
>> 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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [Qemu-trivial] [PATCH] disas/libvixl/a64/instructions-a64.h: Remove useless varialbe to avoid building break with '-Werror'
  2014-10-23 10:09     ` Peter Maydell
@ 2014-10-23 10:27       ` Michael Tokarev
  2014-10-23 11:05         ` Peter Maydell
  0 siblings, 1 reply; 16+ messages in thread
From: Michael Tokarev @ 2014-10-23 10:27 UTC (permalink / raw)
  To: Peter Maydell; +Cc: QEMU Trivial, qemu-devel, Chen Gang, rth

On 10/23/2014 02:09 PM, Peter Maydell wrote:
> On 23 October 2014 10:02, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 23 October 2014 09:09, Michael Tokarev <mjt@tls.msk.ru> wrote:
>>> 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\

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [Qemu-trivial] [PATCH] disas/libvixl/a64/instructions-a64.h: Remove useless varialbe to avoid building break with '-Werror'
  2014-10-23 10:27       ` Michael Tokarev
@ 2014-10-23 11:05         ` Peter Maydell
  0 siblings, 0 replies; 16+ messages in thread
From: Peter Maydell @ 2014-10-23 11:05 UTC (permalink / raw)
  To: Michael Tokarev; +Cc: QEMU Trivial, qemu-devel, Chen Gang, rth

On 23 October 2014 11:27, Michael Tokarev <mjt@tls.msk.ru> wrote:
> On 10/23/2014 02:09 PM, Peter Maydell wrote:
>> On 23 October 2014 10:02, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> On 23 October 2014 09:09, Michael Tokarev <mjt@tls.msk.ru> wrote:
>>>> 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.. :)

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2014-10-23 11:06 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-09 14:00 [Qemu-devel] [PATCH] disas/libvixl/a64/instructions-a64.h: Remove useless varialbe to avoid building break with '-Werror' Chen Gang
2014-10-09 14:34 ` Peter Maydell
2014-10-10  1:54   ` Chen Gang
2014-10-10  7:37     ` Peter Maydell
2014-10-10  8:53       ` Chen Gang
2014-10-10  9:03         ` Chen Gang
2014-10-09 14:54 ` Eric Blake
2014-10-10  1:28   ` Chen Gang
2014-10-21 15:50 ` Peter Maydell
2014-10-23  6:49   ` [Qemu-devel] [Qemu-trivial] " Michael Tokarev
2014-10-23  7:14     ` Peter Maydell
2014-10-23  8:09 ` Michael Tokarev
2014-10-23  9:02   ` Peter Maydell
2014-10-23 10:09     ` Peter Maydell
2014-10-23 10:27       ` Michael Tokarev
2014-10-23 11:05         ` Peter Maydell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.