All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chen Gang <gang.chen.5i5j@gmail.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: QEMU Trivial <qemu-trivial@nongnu.org>,
	qemu-devel <qemu-devel@nongnu.org>,
	"rth@twiddle.net" <rth@twiddle.net>
Subject: Re: [Qemu-trivial] [PATCH] disas/libvixl/a64/instructions-a64.h: Remove useless varialbe to avoid building break with '-Werror'
Date: Fri, 10 Oct 2014 16:53:28 +0800	[thread overview]
Message-ID: <54379E88.4050102@gmail.com> (raw)
In-Reply-To: <CAFEAcA83aUuhBQ+Pu+Ott+W3ar20pKQN7vSmZfLaBoBGSwQ1mg@mail.gmail.com>

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


WARNING: multiple messages have this Message-ID (diff)
From: Chen Gang <gang.chen.5i5j@gmail.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: QEMU Trivial <qemu-trivial@nongnu.org>,
	qemu-devel <qemu-devel@nongnu.org>,
	"rth@twiddle.net" <rth@twiddle.net>
Subject: Re: [Qemu-devel] [PATCH] disas/libvixl/a64/instructions-a64.h: Remove useless varialbe to avoid building break with '-Werror'
Date: Fri, 10 Oct 2014 16:53:28 +0800	[thread overview]
Message-ID: <54379E88.4050102@gmail.com> (raw)
In-Reply-To: <CAFEAcA83aUuhBQ+Pu+Ott+W3ar20pKQN7vSmZfLaBoBGSwQ1mg@mail.gmail.com>

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

  reply	other threads:[~2014-10-10  8:48 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-09 14:00 [Qemu-trivial] [PATCH] disas/libvixl/a64/instructions-a64.h: Remove useless varialbe to avoid building break with '-Werror' Chen Gang
2014-10-09 14:00 ` [Qemu-devel] " Chen Gang
2014-10-09 14:34 ` [Qemu-trivial] " Peter Maydell
2014-10-09 14:34   ` [Qemu-devel] " Peter Maydell
2014-10-10  1:54   ` [Qemu-trivial] " Chen Gang
2014-10-10  1:54     ` [Qemu-devel] " Chen Gang
2014-10-10  7:37     ` [Qemu-trivial] " Peter Maydell
2014-10-10  7:37       ` [Qemu-devel] " Peter Maydell
2014-10-10  8:53       ` Chen Gang [this message]
2014-10-10  8:53         ` Chen Gang
2014-10-10  9:03         ` [Qemu-trivial] " Chen Gang
2014-10-10  9:03           ` [Qemu-devel] " Chen Gang
2014-10-09 14:54 ` [Qemu-trivial] " Eric Blake
2014-10-09 14:54   ` Eric Blake
2014-10-10  1:28   ` [Qemu-trivial] " Chen Gang
2014-10-10  1:28     ` Chen Gang
2014-10-21 15:50 ` [Qemu-trivial] " Peter Maydell
2014-10-21 15:50   ` [Qemu-devel] " Peter Maydell
2014-10-23  6:49   ` [Qemu-trivial] " Michael Tokarev
2014-10-23  6:49     ` [Qemu-devel] " Michael Tokarev
2014-10-23  7:14     ` Peter Maydell
2014-10-23  7:14       ` [Qemu-devel] " Peter Maydell
2014-10-23  8:09 ` Michael Tokarev
2014-10-23  8:09   ` [Qemu-devel] " Michael Tokarev
2014-10-23  9:02   ` Peter Maydell
2014-10-23  9:02     ` [Qemu-devel] " Peter Maydell
2014-10-23 10:09     ` Peter Maydell
2014-10-23 10:09       ` [Qemu-devel] " Peter Maydell
2014-10-23 10:27       ` Michael Tokarev
2014-10-23 10:27         ` [Qemu-devel] " Michael Tokarev
2014-10-23 11:05         ` Peter Maydell
2014-10-23 11:05           ` [Qemu-devel] " Peter Maydell
  -- strict thread matches above, loose matches on Subject: below --
2014-10-21 23:27 Chen Gang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=54379E88.4050102@gmail.com \
    --to=gang.chen.5i5j@gmail.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-trivial@nongnu.org \
    --cc=rth@twiddle.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.