From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5231 invoked by alias); 9 Oct 2017 16:30:27 -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 5108 invoked by uid 89); 9 Oct 2017 16:30:16 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-11.4 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=that!, scanning, Scanning X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 09 Oct 2017 16:30:13 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 52A25562A3; Mon, 9 Oct 2017 12:30:09 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id l5FawhmEAX1R; Mon, 9 Oct 2017 12:30:09 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 362FB5629F; Mon, 9 Oct 2017 12:30:09 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id B02CC80372; Mon, 9 Oct 2017 12:30:08 -0400 (EDT) Date: Mon, 09 Oct 2017 16:30:00 -0000 From: Joel Brobecker To: Ulrich Weigand Cc: gdb-patches@sourceware.org Subject: Re: [RFC][13/19] Target FP: Perform Ada fixed-point scaling in target format Message-ID: <20171009163008.5svu3sjyeubqaux7@adacore.com> References: <20170905182135.5FF0AD8086F@oc3748833570.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170905182135.5FF0AD8086F@oc3748833570.ibm.com> User-Agent: NeoMutt/20170113 (1.7.2) X-SW-Source: 2017-10/txt/msg00219.txt.bz2 On Tue, Sep 05, 2017 at 08:21:35PM +0200, Ulrich Weigand wrote: > [RFC][13/19] Target FP: Perform Ada fixed-point scaling in target format > > One of the few still remaining uses of DOUBLEST in GDB is the Ada front-end > code that handles scaling of Ada fixed-point types. The target format for > those types is some integer format; to convert those values to standard > floating-point representation, that integer needs to be multiplied by a > rational scale factor, given as a pair of numerator and denominator. > > To avoid having to deal with long integer arithmetic, the current Ada > front-end code currently performs those scaling operations in host > DOUBLEST arithmetic. To eliminate this use of DOUBLEST, this patch > changes the front-end to instead perform those operations in the > *target* floating-point format (chosing to use the target "long double"). > > The implementation is mostly straight-forward, using value_cast and > value_binop to perform the target operations. > > Scanning in the scale numerator and denominator is now done into > a host "long long" instead of a DOUBLEST, which should be large > enough to hold all possible values. (Otherwise, this can be replaced > by target-format target_float_from_string operations as well.) > > Printing fixed-point types and values should be completely unchanges, > using target_float_to_string with the same format strings as current code. > > Bye, > Ulrich > > ChangeLog: > > * ada-lang.c (cast_to_fixed): Reimplement in target arithmetic. > (cast_to_fixed): Likewise. ^^^^^^^^^^^^^ cast_from_fixed :) > (ada_scaling_type): New function. > (ada_delta): Return value instead of DOUBLEST. Perform target > arithmetic instead of host arithmetic. > (scaling_factor): Rename to ... > (ada_scaling_factor) ... this. Return value instead of DOUBLEST. > Perform target arithmetic instead of host arithmetic. Maybe mention that we are making this function non-static too? > (ada_fixed_to_float): Remove. > (ada_float_to_fixed): Remove. > * ada-lang.h (ada_fixed_to_float): Remove. > (ada_float_to_fixed): Remove. > (ada_delta): Return value instead of DOUBLEST. > (ada_scaling_factor): Add prototype. > > * ada-typeprint.c: Include "target-float.h". > (print_fixed_point_type): Perform target arithmetic instead of > host arithmetic. > * ada-valprint.c: Include "target-float.h". > (ada_val_print_num): Perform target arithmetic instead of > host arithmetic for fixed-point types. Generally speaking, the patch looks good to me, and the only remark I have is actually an trivial C++ question which you probably had covered. We have a fixed point tests in the testsuite (gdb.ada/fixed_points.exp), so having it run and pass after your change should be a very good sanity check on its own. Thanks for doing that! [...] > Index: binutils-gdb/gdb/ada-typeprint.c > =================================================================== > --- binutils-gdb.orig/gdb/ada-typeprint.c > +++ binutils-gdb/gdb/ada-typeprint.c [...] > @@ -360,16 +361,23 @@ print_enum_type (struct type *type, stru > static void > print_fixed_point_type (struct type *type, struct ui_file *stream) > { > - DOUBLEST delta = ada_delta (type); > - DOUBLEST small = ada_fixed_to_float (type, 1); > + struct value *delta = ada_delta (type); > + struct value *small = ada_scaling_factor (type); > > - if (delta < 0.0) > + if (delta == nullptr) > fprintf_filtered (stream, "delta ??"); > else > { > - fprintf_filtered (stream, "delta %g", (double) delta); > - if (delta != small) > - fprintf_filtered (stream, " <'small = %g>", (double) small); > + std::string str; > + str = target_float_to_string (value_contents (delta), > + value_type (delta), "%g"); This is a C++ question, really. Does it make any difference if you declare the std::string first, and then only set its contents in a second statement? I can't remember the details, but it has to do with initialization vs assignment. I _really_ hope that the string class is sufficiently well designed that the two are really equivalent in practice? -- Joel