From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10386 invoked by alias); 12 Jan 2017 12:36:25 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 10367 invoked by uid 89); 12 Jan 2017 12:36:24 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-5.1 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=sk:read_me, sk:branch_, *branch_delay_insns, 33,48 X-Spam-User: qpsmtpd, 2 recipients X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 12 Jan 2017 12:36:23 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 167FD8123F; Thu, 12 Jan 2017 12:36:23 +0000 (UTC) Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.phx2.redhat.com [10.5.9.4]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v0CCaLKJ008531; Thu, 12 Jan 2017 07:36:22 -0500 Subject: Re: [PATCH 1/8] Refactor disassembly code To: Yao Qi , Simon Marchi References: <1484051178-16013-1-git-send-email-yao.qi@linaro.org> <1484051178-16013-2-git-send-email-yao.qi@linaro.org> <87fd30a805c696a677c56289e7b7b511@polymtl.ca> <20170112121909.GC31406@E107787-LIN> Cc: binutils@sourceware.org, gdb-patches@sourceware.org From: Pedro Alves Message-ID: <12986d60-ddb1-c5e3-4c6a-4fc3d6816bed@redhat.com> Date: Thu, 12 Jan 2017 12:36:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <20170112121909.GC31406@E107787-LIN> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2017-01/txt/msg00219.txt.bz2 On 01/12/2017 12:19 PM, Yao Qi wrote: >>> diff --git a/gdb/disasm.h b/gdb/disasm.h >>> index 4c6fd54..5592cdb 100644 >>> --- a/gdb/disasm.h >>> +++ b/gdb/disasm.h >>> @@ -33,6 +33,48 @@ struct gdbarch; >>> struct ui_out; >>> struct ui_file; >>> >>> +class gdb_disassembler >>> +{ >>> + using di_read_memory_ftype = decltype >>> (disassemble_info::read_memory_func); >>> + >>> +public: >>> + gdb_disassembler (struct gdbarch *gdbarch, struct ui_file *file) >>> + : gdb_disassembler (gdbarch, file, dis_asm_read_memory) >>> + {} >>> + >>> + int print_insn (CORE_ADDR memaddr); >>> + int print_insn (CORE_ADDR memaddr, int *branch_delay_insns); >> >> Not very important, but since print_insn(CORE_ADDR) is trivial, you >> could merge those two methods and provide a default parameter value >> of NULL for branch_delay_insns. > > OK. Fixed them locally. I had written it that way originally because a default parameter forces the compiler to pass down an extra parameter (adding to register pressure) to all call sites, when only a few places actually need the extra output parameter. It's like a double-optional -- i.e., the parameter can be NULL, so merging doesn't simplify that much, given that the version with the single argument does not need to check the parameter. I.e., one function can be built on top of the other. I see it as a different case from when a parameter is optional such that the passed in value always need to be taken in consideration by the method implementation, like when passing a flags argument, with the default being some flag value (or zero). But this is not really performance critical code, so if you want to change it, I don't mind. Thanks, Pedro Alves