From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25398 invoked by alias); 17 Dec 2014 13:24:28 -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 25384 invoked by uid 89); 17 Dec 2014 13:24:27 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 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 (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 17 Dec 2014 13:24:26 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id sBHDOO4J011140 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Wed, 17 Dec 2014 08:24:24 -0500 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id sBHDOMld005944; Wed, 17 Dec 2014 08:24:23 -0500 Message-ID: <54918406.9050005@redhat.com> Date: Wed, 17 Dec 2014 13:24:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.3.0 MIME-Version: 1.0 To: Yao Qi , gdb-patches@sourceware.org Subject: Re: [PATCH 2/3] Refactor gdbarch method print_float_info References: <1418798765-23198-1-git-send-email-yao@codesourcery.com> <1418798765-23198-3-git-send-email-yao@codesourcery.com> In-Reply-To: <1418798765-23198-3-git-send-email-yao@codesourcery.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2014-12/txt/msg00481.txt.bz2 Thanks! I took another look and I have a few further comments. Sorry I didn't send them the first time around. On 12/17/2014 06:46 AM, Yao Qi wrote: > 2. we want to simply the caller of print_float_info, "simplify" > +void > +default_print_float_info (struct gdbarch *gdbarch, struct ui_file *file, > + struct frame_info *frame, const char *args) Could you add an "See inferior.h." comment? > +{ > + int regnum; > + int printed_something = 0; > + > + for (regnum = 0; > + regnum < gdbarch_num_regs (gdbarch) > + + gdbarch_num_pseudo_regs (gdbarch); > + regnum++) > + { > + if (gdbarch_register_reggroup_p (gdbarch, regnum, float_reggroup)) > + { > + printed_something = 1; > + gdbarch_print_registers_info (gdbarch, file, frame, regnum, 1); > + } > + } > + if (!printed_something) > + fprintf_filtered (file, "No floating-point info " > + "available for this processor.\n"); > +} > + > static void > print_float_info (struct ui_file *file, > struct frame_info *frame, const char *args) > { > struct gdbarch *gdbarch = get_frame_arch (frame); > > - if (gdbarch_print_float_info_p (gdbarch)) > - gdbarch_print_float_info (gdbarch, file, frame, args); > - else > - { > - int regnum; > - int printed_something = 0; > + gdbarch_print_float_info (gdbarch, file, frame, args); I think there's only one caller of print_float_info, so we could inline this there and eliminate print_float_info. > index eebc034..e10197a 100644 > --- a/gdb/inferior.h > +++ b/gdb/inferior.h > @@ -106,6 +106,11 @@ extern void default_print_registers_info (struct gdbarch *gdbarch, > struct frame_info *frame, > int regnum, int all); > > +extern void default_print_float_info (struct gdbarch *gdbarch, > + struct ui_file *file, > + struct frame_info *frame, > + const char *args); > + Could you add an intro comment? Something like: /* Default implementation of gdbarch_print_float_info. Prints the values of all floating point registers. */ > extern void child_terminal_info (struct target_ops *self, const char *, int); > > extern void term_info (char *, int); > > } elseif [istarget "mips*-*-*"] then { > - gdb_test "info float" "f0:.*flt:.*dbl:.*" "info float" > + gdb_test_multiple "info float" "info float" { > + -re "fpu type: none*" { > + pass "info float (without FPU)" > + } > + -re "fpu type:.*cause.*mask.*flags.*round.*flush.*" { > + pass "info float (with FPU)" > + } Missing "$gdb_prompt $" at the end of those regexs. Also, with v2 we still print the register values. So ISTM that it'd be good if the "f0:.*flt:.*dbl:.*" regex is still part of the expected output? And if in the none case we _shouldn't see those, then the test could make sure that's how things work? Thanks, Pedro Alves