From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6617 invoked by alias); 14 May 2009 11:32:34 -0000 Received: (qmail 6607 invoked by uid 22791); 14 May 2009 11:32:32 -0000 X-SWARE-Spam-Status: No, hits=-1.7 required=5.0 tests=AWL,BAYES_00,MSGID_FROM_MTA_HEADER,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mtagate8.de.ibm.com (HELO mtagate8.de.ibm.com) (195.212.29.157) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 14 May 2009 11:32:26 +0000 Received: from d12nrmr1607.megacenter.de.ibm.com (d12nrmr1607.megacenter.de.ibm.com [9.149.167.49]) by mtagate8.de.ibm.com (8.14.3/8.13.8) with ESMTP id n4EBWNLw464336 for ; Thu, 14 May 2009 11:32:23 GMT Received: from d12av02.megacenter.de.ibm.com (d12av02.megacenter.de.ibm.com [9.149.165.228]) by d12nrmr1607.megacenter.de.ibm.com (8.13.8/8.13.8/NCO v9.2) with ESMTP id n4EBWNgh1216646 for ; Thu, 14 May 2009 13:32:23 +0200 Received: from d12av02.megacenter.de.ibm.com (loopback [127.0.0.1]) by d12av02.megacenter.de.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id n4EBWNbP030137 for ; Thu, 14 May 2009 13:32:23 +0200 Received: from tuxmaker.boeblingen.de.ibm.com (tuxmaker.boeblingen.de.ibm.com [9.152.85.9]) by d12av02.megacenter.de.ibm.com (8.12.11.20060308/8.12.11) with SMTP id n4EBWMrT030112; Thu, 14 May 2009 13:32:22 +0200 Message-Id: <200905141132.n4EBWMrT030112@d12av02.megacenter.de.ibm.com> Received: by tuxmaker.boeblingen.de.ibm.com (sSMTP sendmail emulation); Thu, 14 May 2009 13:32:22 +0200 Subject: [rfc] Eliminate duplicated logic with decimal operations To: gdb-patches@sourceware.org, bauerman@br.ibm.com Date: Thu, 14 May 2009 11:32:00 -0000 From: "Ulrich Weigand" MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit 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 X-SW-Source: 2009-05/txt/msg00292.txt.bz2 Hello, I've noticed that during decimal floating point operations, there seems to be some duplicated decisions regarding the result type. In particular, decimal_binop provides a LEN_RESULT *output* parameter that gets set to the length of the result type (which is always the larger of the two input types). However, the sole caller of the routine, value_binop, in fact simply *ignores* that returned length value. Instead, it uses a duplicated logic to determine the full result type as the larger of the two (promoted, if necessary) input types. Subsequent code then simply assumes the value provided by decimal_binop matches the type determined by value_binop. This happens to be the case today because the two instances of duplicated logic always come to the same results. It seems to me that it would be simpler to avoid that duplication. The patch below only computes the result type in value_binop, and passes the resulting length as *input* to decimal_binop, which will then ensure it returns a value of that length. This makes the rest of the promote_decimal routine superfluous -- its only effect are two extra conversions, which seem unnecessary: 1. input (input_len) --> decNumber 2. decNumber --> temp (output_len) 3. temp (output_len) --> decNumber The patch below removes steps 2 and 3 and only keeps the first conversion to decNumber. This applies to both decimal_binop and decimal_compare. Thiago, any thoughts on this? Do you agree that those conversions are unnecessary? Tested on powerpc64-linux with no regressions. Bye, Ulrich ChangeLog: * dfp.h (decimal_binop): Convert LEN_RESULT to input parameter. * dfp.c (promote_decimal): Remove. (decimal_binop): Convert LEN_RESULT to input parameter. Remove call to decimal_binop. (decimal_compare): Remove call to decimal_binop. * valarith.c (value_binop): Pass desired result type length to decimal_binop. Index: gdb-head/gdb/dfp.c =================================================================== --- gdb-head.orig/gdb/dfp.c +++ gdb-head/gdb/dfp.c @@ -255,38 +255,11 @@ decimal_to_doublest (const gdb_byte *fro return strtod (buffer, NULL); } -/* Check if operands have the same size and convert them to the - biggest of the two if necessary. */ -static int -promote_decimal (gdb_byte *x, int len_x, gdb_byte *y, int len_y) -{ - int len_result; - decNumber number; - - if (len_x < len_y) - { - decimal_to_number (x, len_x, &number); - decimal_from_number (&number, x, len_y); - len_result = len_y; - } - else if (len_x > len_y) - { - decimal_to_number (y, len_y, &number); - decimal_from_number (&number, y, len_x); - len_result = len_x; - } - else - len_result = len_x; - - return len_result; -} - -/* Perform operation OP with operands X and Y and store value in RESULT. - If LEN_X and LEN_Y are not equal, RESULT will have the size of the biggest - of the two, and LEN_RESULT will be set accordingly. */ +/* Perform operation OP with operands X and Y with sizes LEN_X and LEN_Y + and store value in RESULT with size LEN_RESULT. */ void decimal_binop (enum exp_opcode op, const gdb_byte *x, int len_x, - const gdb_byte *y, int len_y, gdb_byte *result, int *len_result) + const gdb_byte *y, int len_y, gdb_byte *result, int len_result) { decContext set; decNumber number1, number2, number3; @@ -295,14 +268,10 @@ decimal_binop (enum exp_opcode op, const match_endianness (x, len_x, dec1); match_endianness (y, len_y, dec2); - *len_result = promote_decimal (dec1, len_x, dec2, len_y); + decimal_to_number (dec1, len_x, &number1); + decimal_to_number (dec2, len_y, &number2); - /* Both operands are of size *len_result from now on. */ - - decimal_to_number (dec1, *len_result, &number1); - decimal_to_number (dec2, *len_result, &number2); - - set_decnumber_context (&set, *len_result); + set_decnumber_context (&set, len_result); switch (op) { @@ -330,9 +299,9 @@ decimal_binop (enum exp_opcode op, const /* Check for errors in the DFP operation. */ decimal_check_errors (&set); - decimal_from_number (&number3, dec3, *len_result); + decimal_from_number (&number3, dec3, len_result); - match_endianness (dec3, *len_result, result); + match_endianness (dec3, len_result, result); } /* Returns true if X (which is LEN bytes wide) is the number zero. */ @@ -362,11 +331,11 @@ decimal_compare (const gdb_byte *x, int match_endianness (x, len_x, dec1); match_endianness (y, len_y, dec2); - len_result = promote_decimal (dec1, len_x, dec2, len_y); - - decimal_to_number (dec1, len_result, &number1); - decimal_to_number (dec2, len_result, &number2); + decimal_to_number (dec1, len_x, &number1); + decimal_to_number (dec2, len_y, &number2); + /* Perform the comparison in the larger of the two sizes. */ + len_result = len_x > len_y ? len_x : len_y; set_decnumber_context (&set, len_result); decNumberCompare (&result, &number1, &number2, &set); Index: gdb-head/gdb/dfp.h =================================================================== --- gdb-head.orig/gdb/dfp.h +++ gdb-head/gdb/dfp.h @@ -35,7 +35,7 @@ extern void decimal_from_integral (struc extern void decimal_from_floating (struct value *from, gdb_byte *to, int len); extern DOUBLEST decimal_to_doublest (const gdb_byte *from, int len); extern void decimal_binop (enum exp_opcode, const gdb_byte *, int, - const gdb_byte *, int, gdb_byte *, int *); + const gdb_byte *, int, gdb_byte *, int); extern int decimal_is_zero (const gdb_byte *x, int len); extern int decimal_compare (const gdb_byte *x, int len_x, const gdb_byte *y, int len_y); extern void decimal_convert (const gdb_byte *from, int len_from, gdb_byte *to, Index: gdb-head/gdb/valarith.c =================================================================== --- gdb-head.orig/gdb/valarith.c +++ gdb-head/gdb/valarith.c @@ -887,6 +887,19 @@ value_binop (struct value *arg1, struct gdb_byte v1[16], v2[16]; gdb_byte v[16]; + /* If only one type is decimal float, use its type. + Otherwise use the bigger type. */ + if (TYPE_CODE (type1) != TYPE_CODE_DECFLOAT) + result_type = type2; + else if (TYPE_CODE (type2) != TYPE_CODE_DECFLOAT) + result_type = type1; + else if (TYPE_LENGTH (type2) > TYPE_LENGTH (type1)) + result_type = type2; + else + result_type = type1; + + len_v = TYPE_LENGTH (result_type); + value_args_as_decimal (arg1, arg2, v1, &len_v1, v2, &len_v2); switch (op) @@ -896,24 +909,13 @@ value_binop (struct value *arg1, struct case BINOP_MUL: case BINOP_DIV: case BINOP_EXP: - decimal_binop (op, v1, len_v1, v2, len_v2, v, &len_v); + decimal_binop (op, v1, len_v1, v2, len_v2, v, len_v); break; default: error (_("Operation not valid for decimal floating point number.")); } - /* If only one type is decimal float, use its type. - Otherwise use the bigger type. */ - if (TYPE_CODE (type1) != TYPE_CODE_DECFLOAT) - result_type = type2; - else if (TYPE_CODE (type2) != TYPE_CODE_DECFLOAT) - result_type = type1; - else if (TYPE_LENGTH (type2) > TYPE_LENGTH (type1)) - result_type = type2; - else - result_type = type1; - val = value_from_decfloat (result_type, v); } else if (TYPE_CODE (type1) == TYPE_CODE_FLT -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE Ulrich.Weigand@de.ibm.com