From patchwork Fri Feb 3 10:28:19 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Yao Qi X-Patchwork-Id: 19108 Received: (qmail 78356 invoked by alias); 3 Feb 2017 10:28:41 -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 77757 invoked by uid 89); 3 Feb 2017 10:28:40 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=1.3 required=5.0 tests=BAYES_50, FREEMAIL_FROM, MSGID_FROM_MTA_HEADER, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM, SPF_PASS autolearn=no version=3.3.2 spammy=U*brobecker, i18n, sk:brobeck, 696 X-HELO: mail-wj0-f195.google.com Received: from mail-wj0-f195.google.com (HELO mail-wj0-f195.google.com) (209.85.210.195) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 03 Feb 2017 10:28:30 +0000 Received: by mail-wj0-f195.google.com with SMTP id ip10so405935wjb.1 for ; Fri, 03 Feb 2017 02:28:29 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=PIqVF9mbMtiDS7vkSpFl2qZ14IUNBDZPioZ7jHH3WYw=; b=dZLJsZ0ReEroA0J4xhnVPcCuOhJPuxfzyc9zYyA+ssemiCgVjIJuWOoD3WRnO5BsTS 7lIx5zAhpFtiOvi019opDIvfkS6NyHZKJ0HLaWBlf8+N9Pyzly8s9MtXOp424s5xDk2/ r8pNfU+kbWwxbjzUysBeG5+5P9GUcouS1qGXGuHrjQZ+PDb/z/pgNf2uh2EVxbTDRc+J ukj0QyknGGCb/i2owkz3wsTTvt3AXj4ZkSiEpGpRlAflb63M46zbFHmxACjgCe7NKj1D IpiKbgeC8L+JVmAIRNBTqSrf6oI/OgcmBOvgFCGV/7d8/CcpXcjkevp+l9i6SvQJGOpQ v0tw== X-Gm-Message-State: AIkVDXJecw+6L5IsMjV+cLO9NDiDL4POYMOM5tEpkz2iVqKiAxumXQylPSl1vvNFp4Yt7A== X-Received: by 10.223.178.146 with SMTP id g18mr11881325wrd.124.1486117708064; Fri, 03 Feb 2017 02:28:28 -0800 (PST) Received: from E107787-LIN ([194.214.185.158]) by smtp.gmail.com with ESMTPSA id n10sm44373303wrb.9.2017.02.03.02.28.26 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Fri, 03 Feb 2017 02:28:27 -0800 (PST) Date: Fri, 3 Feb 2017 10:28:19 +0000 From: Yao Qi To: Alan Hayward Cc: Joel Brobecker , Pedro Alves , "gdb-patches@sourceware.org" Subject: Re: [PATCH] Removal of uses of MAX_REGISTER_SIZE Message-ID: <20170203102819.GA11916@E107787-LIN> References: <7CF07197-4FED-4970-BB4B-2FE828E29A63@arm.com> <45e3a5e1-a9aa-1bc0-5d08-526b89fc458e@redhat.com> <20170201124123.GA27498@E107787-LIN> <20170202094012.dge4r6rsl2skdrii@adacore.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Received: by 10.12.140.194 with HTTP; Fri, 3 Feb 2017 02:26:34 -0800 (PST) User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes On Fri, Feb 3, 2017 at 9:59 AM, Alan Hayward wrote: > >> On 2 Feb 2017, at 09:40, Joel Brobecker wrote: >> >>> #2 - Switch to heap allocation >>> >>> Or std::vector or something like that that hides it. It's not clear >>> whether that would have a noticeable performance impact. >> >> I would try that. I think using one of the standard C++ classes >> looks a little more attractive to me, and would only consider >> the lambda functions if we can show a noticeable performance >> impact. Those two are not exclusive, by the way, but in the past, >> we've always frowned on calls to alloca in a loop, and using >> a xmalloc+cleanup combination has never been an issue in my cases. >> I'd imagine that a standard C++ memory management class would be >> fast enough for those same situations. >> >> -- >> Joel > > We're not allocating much space, so I'd hope std::vector didn't cause much > of a slowdown. Also, the allocas with lamdba's approach feels a little > overcomplicated. > > Reworking my patch that adds max_register_size (), I've replaced the allocas > with std::vector. > > This patch ok? If people are happy, I'll then rework the larger patch. I don't think we have to replace all MAX_REGISTER_SIZE with std::vector. MAX_REGISTER_SIZE is mostly used in arch-dependent code (*-tdep.c and *-nat.c), where the register size or max register size is known. For example, MAX_REGISTER_SIZE is used only once in arm-tdep.c, and it can be replaced with FP_REGISTER_SIZE, because 'buf' is to get the contents for FPA register. Similarly, MAX_REGISTER_SIZE is used three times in aarch64-tdep.c, all of them can be repalced by V_REGISTER_SIZE. Also, MAX_REGISTER_SIZE can be replaced by I386_MAX_REGISTER_SIZE in i386-tdep.c. I would like to examine the usages of MAX_REGISTER_SIZE in each target-dependent code, and replace MAX_REGISTER_SIZE with known constants as much as we can. I don't think anyone has objections on replacing one constant MAX_REGISTER_SIZE with other smaller constants :) Then, let us discuss how to remove MAX_REGISTER_SIZE from arch-independent code after all above is done. diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c index 801c03d..1f82187 100644 --- a/gdb/aarch64-tdep.c +++ b/gdb/aarch64-tdep.c @@ -1982,7 +1982,7 @@ aarch64_store_return_value (struct type *type, struct regcache *regs, for (i = 0; i < elements; i++) { int regno = AARCH64_V0_REGNUM + i; - bfd_byte tmpbuf[MAX_REGISTER_SIZE]; + bfd_byte tmpbuf[V_REGISTER_SIZE]; if (aarch64_debug) { diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c index 0ae311f..42a39dc 100644 --- a/gdb/arm-tdep.c +++ b/gdb/arm-tdep.c @@ -69,6 +69,10 @@ #include "features/arm/arm-with-vfpv3.c" #include "features/arm/arm-with-neon.c" +#if GDB_SELF_TEST +#include "selftest.h" +#endif + static int arm_debug; /* Macros for setting and testing a bit in a minimal symbol that marks @@ -4237,6 +4241,23 @@ convert_to_extended (const struct floatformat *fmt, void *dbl, const void *ptr, &d, dbl); } +#if GDB_SELF_TEST + +namespace selftests { + +static void +arm_floatformat_test (void) +{ + SELF_CHECK (floatformat_totalsize_bytes (&floatformat_arm_ext_big) + == FP_REGISTER_SIZE); + SELF_CHECK (floatformat_totalsize_bytes (&floatformat_arm_ext_littlebyte_bigword) + == FP_REGISTER_SIZE); +} + +} // namespace selftests + +#endif /* GDB_SELF_TEST */ + /* Given BUF, which is OLD_LEN bytes ending at ENDADDR, expand the buffer to be NEW_LEN bytes ending at ENDADDR. Return NULL if an error occurs. BUF is freed. */ @@ -8153,11 +8174,10 @@ arm_store_return_value (struct type *type, struct regcache *regs, if (TYPE_CODE (type) == TYPE_CODE_FLT) { - gdb_byte buf[MAX_REGISTER_SIZE]; - switch (gdbarch_tdep (gdbarch)->fp_model) { case ARM_FLOAT_FPA: + gdb_byte buf[FP_REGISTER_SIZE]; convert_to_extended (floatformat_from_type (type), buf, valbuf, gdbarch_byte_order (gdbarch)); @@ -9717,6 +9737,10 @@ vfp - VFP co-processor."), NULL, NULL, /* FIXME: i18n: "ARM debugging is %s. */ &setdebuglist, &showdebuglist); + +#if GDB_SELF_TEST + register_self_test (selftests::arm_floatformat_test); +#endif } /* ARM-reversible process record data structures. */