From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55125) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XcVrr-0006Px-Dw for qemu-devel@nongnu.org; Fri, 10 Oct 2014 04:48:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XcVrk-0005xD-JZ for qemu-devel@nongnu.org; Fri, 10 Oct 2014 04:48:07 -0400 Message-ID: <54379E88.4050102@gmail.com> Date: Fri, 10 Oct 2014 16:53:28 +0800 From: Chen Gang MIME-Version: 1.0 References: <54369514.5000004@gmail.com> <54373C63.1010005@gmail.com> In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] disas/libvixl/a64/instructions-a64.h: Remove useless varialbe to avoid building break with '-Werror' List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: QEMU Trivial , qemu-devel , "rth@twiddle.net" On 10/10/14 15:37, Peter Maydell wrote: > On 10 October 2014 02:54, Chen Gang 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