* [obv][commit] Replace one/two use variables with their values
@ 2012-09-17 9:02 Siddhesh Poyarekar
2012-09-17 10:33 ` Yao Qi
0 siblings, 1 reply; 4+ messages in thread
From: Siddhesh Poyarekar @ 2012-09-17 9:02 UTC (permalink / raw)
To: gdb-patches
Hi,
I have committed another obvious patch that replaces variables that are
used just once or twice, directly with their values.
Regards,
Siddhesh
http://sourceware.org/ml/gdb-cvs/2012-09/msg00084.html
* m2-valprint.c (m2_print_array_contents): Eliminate variable
ELTLEN and use TYPE_LENGTH directly.
(m2_val_print): Likewise.
* m68k-tdep.c (m68k_svr4_extract_return_value): Eliminate
variable LEN and use TYPE_LENGTH directly.
(m68k_svr4_store_return_value): Likewise.
* mips-tdep.c (mips_o32_push_dummy_call): Eliminate variable
ARGLEN and use TYPE_LENGTH directly.
(mips_o64_push_dummy_call): Likewise.
* s390-tdep (s390_function_arg_pass_by_reference): Eliminate
variable LENGTH and use TYPE_LENGTH directly.
(s390_function_arg_float): Likewise.
(s390_function_arg_integer): Likewise.
(s390_push_dummy_call): Likewise.
(s390_return_value_convention): Likewise.
* spu-tdep.c (spu_push_dummy_call): Eliminate LEN and use
TYPE_LENGTH directly.
===================================================================
RCS file: /cvs/src/src/gdb/m2-valprint.c,v
retrieving revision 1.45
retrieving revision 1.46
diff -u -r1.45 -r1.46
--- src/gdb/m2-valprint.c 2012/08/16 07:36:20 1.45
+++ src/gdb/m2-valprint.c 2012/09/17 08:52:18 1.46
@@ -269,16 +269,14 @@
const struct value_print_options *options,
int len)
{
- int eltlen;
CHECK_TYPEDEF (type);
if (TYPE_LENGTH (type) > 0)
{
- eltlen = TYPE_LENGTH (type);
if (options->prettyprint_arrays)
print_spaces_filtered (2 + 2 * recurse, stream);
/* For an array of chars, print with string syntax. */
- if (eltlen == 1 &&
+ if (TYPE_LENGTH (type) == 1 &&
((TYPE_CODE (type) == TYPE_CODE_INT)
|| ((current_language->la_language == language_m2)
&& (TYPE_CODE (type) == TYPE_CODE_CHAR)))
@@ -320,7 +318,6 @@
unsigned int i = 0; /* Number of characters printed. */
unsigned len;
struct type *elttype;
- unsigned eltlen;
CORE_ADDR addr;
CHECK_TYPEDEF (type);
@@ -330,12 +327,11 @@
if (TYPE_LENGTH (type) > 0 && TYPE_LENGTH (TYPE_TARGET_TYPE
(type)) > 0) {
elttype = check_typedef (TYPE_TARGET_TYPE (type));
- eltlen = TYPE_LENGTH (elttype);
- len = TYPE_LENGTH (type) / eltlen;
+ len = TYPE_LENGTH (type) / TYPE_LENGTH (elttype);
if (options->prettyprint_arrays)
print_spaces_filtered (2 + 2 * recurse, stream);
/* For an array of chars, print with string syntax. */
- if (eltlen == 1 &&
+ if (TYPE_LENGTH (elttype) == 1 &&
((TYPE_CODE (elttype) == TYPE_CODE_INT)
|| ((current_language->la_language == language_m2)
&& (TYPE_CODE (elttype) == TYPE_CODE_CHAR)))
===================================================================
RCS file: /cvs/src/src/gdb/m68k-tdep.c,v
retrieving revision 1.159
retrieving revision 1.160
diff -u -r1.159 -r1.160
--- src/gdb/m68k-tdep.c 2012/07/24 16:37:24 1.159
+++ src/gdb/m68k-tdep.c 2012/09/17 08:52:18 1.160
@@ -315,7 +315,6 @@
m68k_svr4_extract_return_value (struct type *type, struct regcache
*regcache, gdb_byte *valbuf)
{
- int len = TYPE_LENGTH (type);
gdb_byte buf[M68K_MAX_REGISTER_SIZE];
struct gdbarch *gdbarch = get_regcache_arch (regcache);
struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
@@ -326,7 +325,7 @@
regcache_raw_read (regcache, M68K_FP0_REGNUM, buf);
convert_typed_floating (buf, fpreg_type, valbuf, type);
}
- else if (TYPE_CODE (type) == TYPE_CODE_PTR && len == 4)
+ else if (TYPE_CODE (type) == TYPE_CODE_PTR && TYPE_LENGTH (type) ==
4) regcache_raw_read (regcache, M68K_A0_REGNUM, valbuf);
else
m68k_extract_return_value (type, regcache, valbuf);
@@ -357,7 +356,6 @@
m68k_svr4_store_return_value (struct type *type, struct regcache
*regcache, const gdb_byte *valbuf)
{
- int len = TYPE_LENGTH (type);
struct gdbarch *gdbarch = get_regcache_arch (regcache);
struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
@@ -368,7 +366,7 @@
convert_typed_floating (valbuf, type, buf, fpreg_type);
regcache_raw_write (regcache, M68K_FP0_REGNUM, buf);
}
- else if (TYPE_CODE (type) == TYPE_CODE_PTR && len == 4)
+ else if (TYPE_CODE (type) == TYPE_CODE_PTR && TYPE_LENGTH (type) ==
4) {
regcache_raw_write (regcache, M68K_A0_REGNUM, valbuf);
regcache_raw_write (regcache, M68K_D0_REGNUM, valbuf);
===================================================================
RCS file: /cvs/src/src/gdb/mips-tdep.c,v
retrieving revision 1.561
retrieving revision 1.562
diff -u -r1.561 -r1.562
--- src/gdb/mips-tdep.c 2012/08/19 22:22:49 1.561
+++ src/gdb/mips-tdep.c 2012/09/17 08:52:18 1.562
@@ -5174,13 +5174,12 @@
for (argnum = 0; argnum < nargs; argnum++)
{
struct type *arg_type = check_typedef (value_type
(args[argnum]));
- int arglen = TYPE_LENGTH (arg_type);
/* Align to double-word if necessary. */
if (mips_type_needs_double_align (arg_type))
len = align_up (len, MIPS32_REGSIZE * 2);
/* Allocate space on the stack. */
- len += align_up (arglen, MIPS32_REGSIZE);
+ len += align_up (TYPE_LENGTH (arg_type), MIPS32_REGSIZE);
}
sp -= align_up (len, 16);
@@ -5703,10 +5702,9 @@
for (argnum = 0; argnum < nargs; argnum++)
{
struct type *arg_type = check_typedef (value_type
(args[argnum]));
- int arglen = TYPE_LENGTH (arg_type);
/* Allocate space on the stack. */
- len += align_up (arglen, MIPS64_REGSIZE);
+ len += align_up (TYPE_LENGTH (arg_type), MIPS64_REGSIZE);
}
sp -= align_up (len, 16);
===================================================================
RCS file: /cvs/src/src/gdb/s390-tdep.c,v
retrieving revision 1.206
retrieving revision 1.207
diff -u -r1.206 -r1.207
--- src/gdb/s390-tdep.c 2012/05/18 21:02:50 1.206
+++ src/gdb/s390-tdep.c 2012/09/17 08:52:18 1.207
@@ -2489,8 +2489,7 @@
static int
s390_function_arg_pass_by_reference (struct type *type)
{
- unsigned length = TYPE_LENGTH (type);
- if (length > 8)
+ if (TYPE_LENGTH (type) > 8)
return 1;
return (is_struct_like (type) && !is_power_of_two (TYPE_LENGTH
(type))) @@ -2503,8 +2502,7 @@
static int
s390_function_arg_float (struct type *type)
{
- unsigned length = TYPE_LENGTH (type);
- if (length > 8)
+ if (TYPE_LENGTH (type) > 8)
return 0;
return is_float_like (type);
@@ -2515,13 +2513,12 @@
static int
s390_function_arg_integer (struct type *type)
{
- unsigned length = TYPE_LENGTH (type);
- if (length > 8)
+ if (TYPE_LENGTH (type) > 8)
return 0;
return is_integer_like (type)
|| is_pointer_like (type)
- || (is_struct_like (type) && is_power_of_two (length));
+ || (is_struct_like (type) && is_power_of_two (TYPE_LENGTH
(type))); }
/* Return ARG, a `SIMPLE_ARG', sign-extended or zero-extended to a full
@@ -2616,11 +2613,10 @@
{
struct value *arg = args[i];
struct type *type = check_typedef (value_type (arg));
- unsigned length = TYPE_LENGTH (type);
if (s390_function_arg_pass_by_reference (type))
{
- sp -= length;
+ sp -= TYPE_LENGTH (type);
sp = align_down (sp, alignment_of (type));
copy_addr[i] = sp;
}
@@ -2799,8 +2795,7 @@
static enum return_value_convention
s390_return_value_convention (struct gdbarch *gdbarch, struct type
*type) {
- int length = TYPE_LENGTH (type);
- if (length > 8)
+ if (TYPE_LENGTH (type) > 8)
return RETURN_VALUE_STRUCT_CONVENTION;
switch (TYPE_CODE (type))
===================================================================
RCS file: /cvs/src/src/gdb/spu-tdep.c,v
retrieving revision 1.81
retrieving revision 1.82
diff -u -r1.81 -r1.82
--- src/gdb/spu-tdep.c 2012/05/18 21:02:50 1.81
+++ src/gdb/spu-tdep.c 2012/09/17 08:52:18 1.82
@@ -1373,8 +1373,7 @@
struct value *arg = args[i];
struct type *type = check_typedef (value_type (arg));
const gdb_byte *contents = value_contents (arg);
- int len = TYPE_LENGTH (type);
- int n_regs = align_up (len, 16) / 16;
+ int n_regs = align_up (TYPE_LENGTH (type), 16) / 16;
/* If the argument doesn't wholly fit into registers, it and
all subsequent arguments go to the stack. */
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [obv][commit] Replace one/two use variables with their values
2012-09-17 9:02 [obv][commit] Replace one/two use variables with their values Siddhesh Poyarekar
@ 2012-09-17 10:33 ` Yao Qi
2012-09-17 10:46 ` Siddhesh Poyarekar
0 siblings, 1 reply; 4+ messages in thread
From: Yao Qi @ 2012-09-17 10:33 UTC (permalink / raw)
To: Siddhesh Poyarekar; +Cc: gdb-patches
On 09/17/2012 05:01 PM, Siddhesh Poyarekar wrote:
> Hi,
>
> I have committed another obvious patch that replaces variables that are
> used just once or twice, directly with their values.
>
What is your motivation of this change? Does this patch suggest that
we'd better use marcro rather than local variables? I understand most
of it, but ...
> @@ -330,12 +327,11 @@
> if (TYPE_LENGTH (type) > 0 && TYPE_LENGTH (TYPE_TARGET_TYPE
> (type)) > 0) {
> elttype = check_typedef (TYPE_TARGET_TYPE (type));
> - eltlen = TYPE_LENGTH (elttype);
> - len = TYPE_LENGTH (type) / eltlen;
> + len = TYPE_LENGTH (type) / TYPE_LENGTH (elttype);
> if (options->prettyprint_arrays)
> print_spaces_filtered (2 + 2 * recurse, stream);
> /* For an array of chars, print with string syntax. */
> - if (eltlen == 1 &&
> + if (TYPE_LENGTH (elttype) == 1 &&
> ((TYPE_CODE (elttype) == TYPE_CODE_INT)
> || ((current_language->la_language == language_m2)
> && (TYPE_CODE (elttype) == TYPE_CODE_CHAR)))
>
... this part. We define a local variable 'eltlen', and use it twice.
Isn't a common programming practise? I don't object to this patch, but
want to know why do we do this change?
> ===================================================================
> RCS file: /cvs/src/src/gdb/m68k-tdep.c,v
> retrieving revision 1.159
> retrieving revision 1.160
> diff -u -r1.159 -r1.160
> --- src/gdb/m68k-tdep.c 2012/07/24 16:37:24 1.159
> +++ src/gdb/m68k-tdep.c 2012/09/17 08:52:18 1.160
> @@ -315,7 +315,6 @@
> m68k_svr4_extract_return_value (struct type *type, struct regcache
> *regcache, gdb_byte *valbuf)
> {
> - int len = TYPE_LENGTH (type);
> gdb_byte buf[M68K_MAX_REGISTER_SIZE];
> struct gdbarch *gdbarch = get_regcache_arch (regcache);
> struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
> @@ -326,7 +325,7 @@
> regcache_raw_read (regcache, M68K_FP0_REGNUM, buf);
> convert_typed_floating (buf, fpreg_type, valbuf, type);
> }
> - else if (TYPE_CODE (type) == TYPE_CODE_PTR && len == 4)
> + else if (TYPE_CODE (type) == TYPE_CODE_PTR && TYPE_LENGTH (type) ==
> 4) regcache_raw_read (regcache, M68K_A0_REGNUM, valbuf);
b.t.w, looks your mailer wrap your patch incorrectly.
--
Yao
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [obv][commit] Replace one/two use variables with their values
2012-09-17 10:33 ` Yao Qi
@ 2012-09-17 10:46 ` Siddhesh Poyarekar
2012-09-17 11:29 ` Yao Qi
0 siblings, 1 reply; 4+ messages in thread
From: Siddhesh Poyarekar @ 2012-09-17 10:46 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On Mon, 17 Sep 2012 18:32:36 +0800, Yao wrote:
> ... this part. We define a local variable 'eltlen', and use it
> twice. Isn't a common programming practise? I don't object to this
> patch, but want to know why do we do this change?
Sorry, I should have mentioned this in my original email. I have been
working on adding support for 64-bit struct offset and sizes, which
meant expansion of the bitpos offset and the type.length to 64-bit. The
discussion is here:
http://sourceware.org/ml/gdb-patches/2012-08/msg00144.html
These commits are an attempt to push in some of the more trivial fixes
independently so that the final changeset is as small as possible. The
net change size is still currently at about 7k lines, so I've not
succeeded all that much with this patch at my goal of reducing the
patch size, but I'm still trying :)
Without these independent patches, the bitpos patch would have all
these local variables expanded to ULONGEST, which is more of a
mechanical change that can be avoided in the large patch.
> > - else if (TYPE_CODE (type) == TYPE_CODE_PTR && len == 4)
> > + else if (TYPE_CODE (type) == TYPE_CODE_PTR && TYPE_LENGTH (type)
> > == 4) regcache_raw_read (regcache, M68K_A0_REGNUM, valbuf);
>
> b.t.w, looks your mailer wrap your patch incorrectly.
>
Sorry about that; I copy-pasted the patch when I should have attached
it, due to which claws mercilessly wrapped it at 72 lines. Will watch
out for this.
Regards,
Siddhesh
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [obv][commit] Replace one/two use variables with their values
2012-09-17 10:46 ` Siddhesh Poyarekar
@ 2012-09-17 11:29 ` Yao Qi
0 siblings, 0 replies; 4+ messages in thread
From: Yao Qi @ 2012-09-17 11:29 UTC (permalink / raw)
To: Siddhesh Poyarekar; +Cc: gdb-patches
On 09/17/2012 06:45 PM, Siddhesh Poyarekar wrote:
> These commits are an attempt to push in some of the more trivial fixes
> independently so that the final changeset is as small as possible. The
> net change size is still currently at about 7k lines, so I've not
> succeeded all that much with this patch at my goal of reducing the
> patch size, but I'm still trying:)
>
> Without these independent patches, the bitpos patch would have all
> these local variables expanded to ULONGEST, which is more of a
> mechanical change that can be avoided in the large patch.
I got your point. That makes sense to me. Thanks.
--
Yao
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-09-17 11:29 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-09-17 9:02 [obv][commit] Replace one/two use variables with their values Siddhesh Poyarekar
2012-09-17 10:33 ` Yao Qi
2012-09-17 10:46 ` Siddhesh Poyarekar
2012-09-17 11:29 ` Yao Qi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox