From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19746 invoked by alias); 21 Dec 2014 00:14:06 -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 19734 invoked by uid 89); 21 Dec 2014 00:14:05 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.0 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 21 Dec 2014 00:14:03 +0000 Received: from nat-ies.mentorg.com ([192.94.31.2] helo=SVR-IES-FEM-01.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1Y2U9n-0006dA-K9 from Maciej_Rozycki@mentor.com ; Sat, 20 Dec 2014 16:13:59 -0800 Received: from localhost (137.202.0.76) by SVR-IES-FEM-01.mgc.mentorg.com (137.202.0.104) with Microsoft SMTP Server (TLS) id 14.3.181.6; Sun, 21 Dec 2014 00:13:57 +0000 Date: Sun, 21 Dec 2014 00:14:00 -0000 From: "Maciej W. Rozycki" To: Pedro Alves CC: Yao Qi , Subject: Re: [PATCH 3/3] MIPS: Provide FPU info and decode FCSR in `info float' In-Reply-To: <5491609E.1080204@redhat.com> Message-ID: References: <1418798765-23198-1-git-send-email-yao@codesourcery.com> <1418798765-23198-4-git-send-email-yao@codesourcery.com> <5491609E.1080204@redhat.com> User-Agent: Alpine 1.10 (DEB 962 2008-03-14) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" X-SW-Source: 2014-12/txt/msg00597.txt.bz2 On Wed, 17 Dec 2014, Pedro Alves wrote: > > +static void > > +mips_print_float_info (struct gdbarch *gdbarch, struct ui_file *file, > > + struct frame_info *frame, const char *args) > > +{ > > + int fcsr = mips_regnum (gdbarch)->fp_control_status; > > + enum mips_fpu_type type = MIPS_FPU_TYPE (gdbarch); > > + ULONGEST fcs = 0; > > + int i; > > + > > + if (fcsr == -1 || !deprecated_frame_register_read (frame, fcsr, NULL)) > > + type = MIPS_FPU_NONE; > > "deprecated" method usage alert. It's better to use methods that return > values and then print "", "" as appropriate, > though in this case you may be able to just use read_frame_register_unsigned > instead. For the record, we use `get_frame_register_unsigned' in such cases elsewhere, e.g. `micromips_bc1_pc' and we don't expect it to fail on the assumption that `fp_control_status' will have been set correctly. I see this change went in already, but I'd like to see it updated with a follow-up change for consistency. Specifically `get_frame_register_unsigned' is unsuitable here (as we want to poke at the actual register file, not the frame state), but I think a failure from `read_frame_register_unsigned' should be signalled as an error (exception being thrown, just as from the former function) rather than being silently treated as if there were no FPU. Also I think the registers should be dumped earlier on, with only generic FPU parameters preceding, as the extra decoded FCSR information is an elaboration of raw register contents, so I find it logical (as far as the reading flow of English text is considered) to be presented later, i.e.: fpu type: ... reg size: ... [registers] cond : ... [...] Or so I think. It would help reviewing it if the updated version of the patch was presented with corresponding output produced accompanying, just as I did with the original version. Otherwise it requires drawing the output in one's head which is not necessarily straightforward. Maciej