From patchwork Fri Oct 10 11:27:57 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chen Gang X-Patchwork-Id: 3185 Received: (qmail 5149 invoked by alias); 10 Oct 2014 11:22:25 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 5131 invoked by uid 89); 10 Oct 2014 11:22:24 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-Spam-User: qpsmtpd, 2 recipients X-HELO: mail-pa0-f53.google.com Received: from mail-pa0-f53.google.com (HELO mail-pa0-f53.google.com) (209.85.220.53) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Fri, 10 Oct 2014 11:22:23 +0000 Received: by mail-pa0-f53.google.com with SMTP id kq14so1599061pab.12 for ; Fri, 10 Oct 2014 04:22:22 -0700 (PDT) X-Received: by 10.70.125.197 with SMTP id ms5mr1557030pdb.166.1412940141979; Fri, 10 Oct 2014 04:22:21 -0700 (PDT) Received: from [192.168.2.113] ([124.127.118.42]) by mx.google.com with ESMTPSA id dh14sm3272143pac.17.2014.10.10.04.22.05 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 10 Oct 2014 04:22:20 -0700 (PDT) Message-ID: <5437C2BD.6010403@gmail.com> Date: Fri, 10 Oct 2014 19:27:57 +0800 From: Chen Gang User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:31.0) Gecko/20100101 Thunderbird/31.1.2 MIME-Version: 1.0 To: Pedro Alves , Joel Brobecker CC: Mark Kettenis , amodra@gmail.com, gbenson@redhat.com, michael.sturm@intel.com, walfred.tedeschi@intel.com, binutils@sourceware.org, gdb-patches@sourceware.org Subject: Re: [PATCH] gdb/i387-tdep.c: Avoid warning for "-Werror=strict-overflow" References: <542EC11C.3020406@gmail.com> <201410031546.s93FknOM002165@glazunov.sibelius.xs4all.nl> <542EC9FC.8050107@gmail.com> <20141003164420.GK6927@adacore.com> <542EE1BF.7060203@redhat.com> <542F831D.1000502@gmail.com> <543255B6.7060509@redhat.com> <54329A9A.9090303@gmail.com> In-Reply-To: <54329A9A.9090303@gmail.com> On 10/6/14 21:35, Chen Gang wrote: > On 10/6/14 16:41, Pedro Alves wrote: >> On 10/04/2014 06:18 AM, Chen Gang wrote: >>> >>> OK, thanks. It is really one way, it is a little better than my original >>> way. But for me, it is still not a good idea: it introduces a new macro >>> and a new variable for each area (originally, it is only one statement). >> >> I see no problem with adding the new macro. We already have a ton >> of similar macros, see i386-tdep.h and i387-tdep.h. Looks >> like the existing I387_NUM_REGS is what we'd need here? >> >> BTC, OOC, did you try Joel's idea with the local variable? >> In case Mark prefers that, it'd be good to know whether it works. >> I can't seem to get my gcc to emit that warning. >> >> Combining both ideas, for clarity, we end up with something >> like: >> >> int end; >> >> end = I387_ST0_REGNUM (tdep) + I387_NUM_REGS; >> for (i = I387_ST0_REGNUM (tdep); i < end; i++) >> >> ... >> >> end = I387_XMM0_REGNUM (tdep) + I387_NUM_XMM_REGS (tdep); >> for (i = I387_XMM0_REGNUM (tdep); i < end; i++) >> >> >> That's way clearer to me than the existing: >> > > That's way not quite bad to me than the existing: > > - It is easier understanding, although a little complex than origin. > > - For compiler, 'end' is simple enough to be sure to be optimized. > > - And I guess, compiler will understand, and will not worry about it. > > >> for (i = I387_ST0_REGNUM (tdep); i < I387_XMM0_REGNUM (tdep); i++) >> ... >> for (i = I387_XMM0_REGNUM (tdep); i < I387_MXCSR_REGNUM (tdep); i++) >> >> anyway, which assumes the reader knows register numbers are >> ordered like st -> xmm -> mxcrsr. >> >> If this works, I think it's my preference. >> > > OK, thanks, at least, what you said is acceptable to me. If no any > additional reply within this week (within 2014-10-12), I shall send > patch v2 for it. > After try, it seems still a little strange for human being: it is too 'clear' to need be combined (so I have to give related comment for it). The related diff may like below, it can pass compiling without related warnings, if no any objections within 2 days, I shall send patch v2 for it. -------------------------- diff begin ---------------------------------- -------------------------- diff end ------------------------------------ Thanks. diff --git a/gdb/i387-tdep.c b/gdb/i387-tdep.c index d66ac6a..4617bdd 100644 --- a/gdb/i387-tdep.c +++ b/gdb/i387-tdep.c @@ -450,11 +450,12 @@ i387_supply_fsave (struct regcache *regcache, int regnum, const void *fsave) struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch); enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); const gdb_byte *regs = fsave; - int i; + int i, end; gdb_assert (tdep->st0_regnum >= I386_ST0_REGNUM); - for (i = I387_ST0_REGNUM (tdep); i < I387_XMM0_REGNUM (tdep); i++) + end = I387_XMM0_REGNUM (tdep); /* let compiler don't worry about it */ + for (i = I387_ST0_REGNUM (tdep); i < end; i++) if (regnum == -1 || regnum == i) { if (fsave == NULL) @@ -503,11 +504,12 @@ i387_collect_fsave (const struct regcache *regcache, int regnum, void *fsave) { struct gdbarch_tdep *tdep = gdbarch_tdep (get_regcache_arch (regcache)); gdb_byte *regs = fsave; - int i; + int i, end; gdb_assert (tdep->st0_regnum >= I386_ST0_REGNUM); - for (i = I387_ST0_REGNUM (tdep); i < I387_XMM0_REGNUM (tdep); i++) + end = I387_XMM0_REGNUM (tdep); /* let compiler don't worry about it */ + for (i = I387_ST0_REGNUM (tdep); i < end; i++) if (regnum == -1 || regnum == i) { /* Most of the FPU control registers occupy only 16 bits in