eat your own dog food?

eat your own dog food?

[jamal]

Hey Arnaldo,

You ever tasted what you are cooking?;->

---
hadi@lilsol:~/git-trees/arnaldo/jamal/build$ pahole ./pahole| grep XXX
        /* XXX last struct has 1 byte of padding */
        /* XXX 1 bit hole, try to pack */
        /* XXX 1 byte hole, try to pack */
        /* XXX 2 bytes hole, try to pack */
        /* XXX 2 bits hole, try to pack */
        /* XXX 1 byte hole, try to pack */
        /* XXX last struct has 2 bytes of padding */
        /* XXX 1 byte hole, try to pack */
hadi@lilsol:~/git-trees/arnaldo/jamal/build$
---

On a serious note, feature request:

I am trying to put words to what i am looking for and it's a little hard
without providing a lot of boring context. For brevity, lets say i have
an inherited 500K lines of code. Assume i compiled a program for 32-bit
target and now i have to move it to a 128-bit playstation 3 with
whatever strange alignment rules. 
I want to do achieve 2 and a half things:

a) 
i) check the structure for holes for X-bit access alignment where X is
16 (old 680xx), 64, 128. I want to do this without compiling the program
for any of those targets i.e i want to do this on my x86 32-bit
laptop ;-> i.e call this cross-pahole

ii) If i can do the above; i should then be able to say "optimize" (what
you call --reorganize) for X-bit

b) I want to do a multiple of these example "optimize for 32-bit and
64-bit" or "check if it is safe for 128-bit and 16-bit alignemnt"

Note, clearly if i can make it read config files with rules for
something like the MIPS EABI in the first part of this email:
 http://sourceware.org/ml/binutils/2003-06/msg00436.html
then i can say "check if it safe for mips 32 bit and x86_64" ;->
And i can make sure my apps will run on that new cpu i dont have access
to.

I could ask for more, but let me start/stop here incase you are going to
lynch me;->

cheers,
jamal

-
To unsubscribe from this list: send the line "unsubscribe dwarves" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: eat your own dog food?

[Arnaldo Carvalho de Melo]

Em Thu, Jan 10, 2008 at 07:42:48AM -0500, jamal escreveu:
> 
> Hey Arnaldo,
> 
> You ever tasted what you are cooking?;->
> 
> ---
> hadi@lilsol:~/git-trees/arnaldo/jamal/build$ pahole ./pahole| grep XXX
>         /* XXX last struct has 1 byte of padding */
>         /* XXX 1 bit hole, try to pack */
>         /* XXX 1 byte hole, try to pack */
>         /* XXX 2 bytes hole, try to pack */
>         /* XXX 2 bits hole, try to pack */
>         /* XXX 1 byte hole, try to pack */
>         /* XXX last struct has 2 bytes of padding */
>         /* XXX 1 byte hole, try to pack */
> hadi@lilsol:~/git-trees/arnaldo/jamal/build$
> ---

:-) I'll check that later :-P But some holes are inevitable, the best
thing is to try:

[acme@doppio pahole]$ pahole --packable build/pahole
argp_child      32      24      8
argp_option     48      40      8
_IO_FILE        216     208     8
[acme@doppio pahole]$

Which is kinda interesting, huh? Only glibc stuff :-)

--packable means: run --reorganize in all structs and compare the result
with the original struct, if we have savings, tell us (original size,
reorganized size, savings).

> On a serious note, feature request:
> 
> I am trying to put words to what i am looking for and it's a little hard
> without providing a lot of boring context. For brevity, lets say i have
> an inherited 500K lines of code. Assume i compiled a program for 32-bit
> target and now i have to move it to a 128-bit playstation 3 with
> whatever strange alignment rules. 
> I want to do achieve 2 and a half things:
> 
> a) 
> i) check the structure for holes for X-bit access alignment where X is
> 16 (old 680xx), 64, 128. I want to do this without compiling the program
> for any of those targets i.e i want to do this on my x86 32-bit
> laptop ;-> i.e call this cross-pahole
> 
> ii) If i can do the above; i should then be able to say "optimize" (what
> you call --reorganize) for X-bit
> 
> b) I want to do a multiple of these example "optimize for 32-bit and
> 64-bit" or "check if it is safe for 128-bit and 16-bit alignemnt"
> 
> Note, clearly if i can make it read config files with rules for
> something like the MIPS EABI in the first part of this email:
>  http://sourceware.org/ml/binutils/2003-06/msg00436.html
> then i can say "check if it safe for mips 32 bit and x86_64" ;->
> And i can make sure my apps will run on that new cpu i dont have access
> to.
> 
> I could ask for more, but let me start/stop here incase you are going to
> lynch me;->

No need for lynching this time. What we have to do is:

. make the addr_size configurable and when the user specifies an
addr_size that is different than the one in the DWARF info being
processed, fixup it in all the data structures. The --reorganize code
already does many kinds of fixups, it would be one more.

. moreover, specify alignment rules, that way the reorganization code
would process all the structs, applying this rule.

IOW: its possible, should be fairly easy to implement the addr_size and
from that the alignment rules.

I'll try to work on the addr_size first, perhaps this weekend as I've
been promising to do way too many things in many fronts :-\

- Arnaldo
-
To unsubscribe from this list: send the line "unsubscribe dwarves" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: eat your own dog food?

[jamal]

On Thu, 2008-10-01 at 11:39 -0200, Arnaldo Carvalho de Melo wrote:
> Em Thu, Jan 10, 2008 at 07:42:48AM -0500, jamal escreveu:

> :-) I'll check that later :-P But some holes are inevitable, the best
> thing is to try:
> 
> [acme@doppio pahole]$ pahole --packable build/pahole
> argp_child      32      24      8
> argp_option     48      40      8
> _IO_FILE        216     208     8
> [acme@doppio pahole]$
> 
> Which is kinda interesting, huh? Only glibc stuff :-)

hehe;-> damn those dog-food cans.

> No need for lynching this time. 

ok;-> 

> What we have to do is:
> 
> . make the addr_size configurable and when the user specifies an
> addr_size that is different than the one in the DWARF info being
> processed, fixup it in all the data structures. The --reorganize code
> already does many kinds of fixups, it would be one more.
> 
> . moreover, specify alignment rules, that way the reorganization code
> would process all the structs, applying this rule.
> 
> IOW: its possible, should be fairly easy to implement the addr_size and
> from that the alignment rules.
> 
> I'll try to work on the addr_size first, perhaps this weekend as I've
> been promising to do way too many things in many fronts :-\

ok, cool - so i should be able say something like "--add_size 64-bit"
and it will follow something like the MIPS EABI i posted earlier? [I
think x86_64 will work the same way too if i am not mistaken].

cheers,
jamal

-
To unsubscribe from this list: send the line "unsubscribe dwarves" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: eat your own dog food?

[Arnaldo Carvalho de Melo]

Em Thu, Jan 10, 2008 at 08:55:41AM -0500, jamal escreveu:
> On Thu, 2008-10-01 at 11:39 -0200, Arnaldo Carvalho de Melo wrote:
> > Em Thu, Jan 10, 2008 at 07:42:48AM -0500, jamal escreveu:
> 
> > :-) I'll check that later :-P But some holes are inevitable, the best
> > thing is to try:
> > 
> > [acme@doppio pahole]$ pahole --packable build/pahole
> > argp_child      32      24      8
> > argp_option     48      40      8
> > _IO_FILE        216     208     8
> > [acme@doppio pahole]$
> > 
> > Which is kinda interesting, huh? Only glibc stuff :-)
> 
> hehe;-> damn those dog-food cans.
> 
> > No need for lynching this time. 
> 
> ok;-> 
> 
> > What we have to do is:
> > 
> > . make the addr_size configurable and when the user specifies an
> > addr_size that is different than the one in the DWARF info being
> > processed, fixup it in all the data structures. The --reorganize code
> > already does many kinds of fixups, it would be one more.
> > 
> > . moreover, specify alignment rules, that way the reorganization code
> > would process all the structs, applying this rule.
> > 
> > IOW: its possible, should be fairly easy to implement the addr_size and
> > from that the alignment rules.
> > 
> > I'll try to work on the addr_size first, perhaps this weekend as I've
> > been promising to do way too many things in many fronts :-\
> 
> ok, cool - so i should be able say something like "--add_size 64-bit"
> and it will follow something like the MIPS EABI i posted earlier? [I
> think x86_64 will work the same way too if i am not mistaken].

Exactly, you will be able to change the addr_size, I think that there is
even code that can be right away used for that, lemme check.

- Arnaldo
-
To unsubscribe from this list: send the line "unsubscribe dwarves" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: eat your own dog food?

[Arnaldo Carvalho de Melo]

Em Thu, Jan 10, 2008 at 12:06:35PM -0200, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Jan 10, 2008 at 08:55:41AM -0500, jamal escreveu:
> > ok, cool - so i should be able say something like "--add_size 64-bit"
> > and it will follow something like the MIPS EABI i posted earlier? [I
> > think x86_64 will work the same way too if i am not mistaken].
> 
> Exactly, you will be able to change the addr_size, I think that there is
> even code that can be right away used for that, lemme check.

First stab, doesn't handles unions shrinking because the largest member
shrunk, and handles only binaries built for 64 bits (or bigger) having
it addr_size (word_size) shrinking.

So:

[acme@doppio pahole]$ file build/pahole
build/pahole: ELF 64-bit LSB executable, x86-64, version 1 (SYSV),
dynamically linked (uses shared libs), for GNU/Linux 2.6.9, not stripped

A 64 bit app:

[acme@doppio pahole]$ build/pahole -C structure build/pahole
struct structure {
        struct list_head  node;                 /*     0    16 */
        struct class *    class;                /*    16     8 */
        const struct cu  *cu;                   /*    24     8 */
        uint32_t          nr_files;             /*    32     4 */
        uint32_t          nr_methods;           /*    36     4 */

        /* size: 40, cachelines: 1 */
        /* last cacheline: 40 bytes */
};
[acme@doppio pahole]$

Lets change the word_size from 8 to 4 (64 bits -> 32 bits)

[acme@doppio pahole]$ build/pahole -w 4 -C structure build/pahole
struct structure {
	struct list_head  node;                 /*     0     8 */
	struct class *    class;                /*     8     4 */
	const struct cu  *cu;                   /*    12     4 */
	uint32_t          nr_files;             /*    16     4 */
	uint32_t          nr_methods;           /*    20     4 */

	/* size: 24, cachelines: 1 */
	/* last cacheline: 24 bytes */
};

See that it recursively notices structs shrinking, because...

[acme@doppio pahole]$ build/pahole -C list_head build/pahole
struct list_head {
	struct list_head *next;                 /*     0     8 */
	struct list_head *prev;                 /*     8     8 */

	/* size: 16, cachelines: 1 */
	/* last cacheline: 16 bytes */
};

When using a 4 bytes word_size:

[acme@doppio pahole]$ build/pahole -w 4 -C list_head build/pahole
struct list_head {
	struct list_head *next;                 /*     0     4 */
	struct list_head *prev;                 /*     4     4 */

	/* size: 8, cachelines: 1 */
	/* last cacheline: 8 bytes */
};

Lets go all the way back to the 8088:

[acme@doppio pahole]$ build/pahole -w 1 -C structure build/pahole
struct structure {
	struct list_head           node;                 /*     0     2 */
	struct class *             class;                /*     2     1 */
	const struct cu  *         cu;                   /*     3     1 */
	uint32_t                   nr_files;             /*     4     4 */
	uint32_t                   nr_methods;           /*     8     4 */

	/* size: 12, cachelines: 1 */
	/* last cacheline: 12 bytes */
};

Not really because we're only change 'long' from the original word size
to the one specified.

Please test and find the missing bits for me :-)

- Arnaldo

diff --git a/dwarves.h b/dwarves.h
index f5edb65..526335f 100644
--- a/dwarves.h
+++ b/dwarves.h
@@ -107,6 +107,7 @@ struct type {
 	struct list_head node;
 	Dwarf_Off	 specification;
 	size_t		 size;
+	size_t		 size_diff; /* use when changing cu->addr_size */
 	uint16_t	 nr_members;
 	uint8_t		 declaration; /* only one bit used */
 	uint8_t		 definition_emitted:1;
diff --git a/pahole.c b/pahole.c
index 4de68f4..c4f0c12 100644
--- a/pahole.c
+++ b/pahole.c
@@ -21,6 +21,7 @@
 
 static uint8_t class__include_anonymous;
 static uint8_t class__include_nested_anonymous;
+static uint8_t word_size, original_word_size;
 
 static char *class__exclude_prefix;
 static size_t class__exclude_prefix_len;
@@ -387,6 +388,83 @@ static int cu_unique_iterator(struct cu *cu, void *cookie)
 	return cu__for_each_tag(cu, unique_iterator, cookie, tag__filter);
 }
 
+void class__realign_LP(struct tag *tag, struct cu *cu)
+{
+	struct tag *tag_pos;
+	struct class *self = tag__class(tag);
+	size_t word_size_diff;
+	size_t orig_size = self->type.size;
+
+	if (original_word_size > word_size)
+		word_size_diff = original_word_size - word_size;
+	else
+		word_size_diff = word_size - original_word_size;
+
+	type__for_each_tag(tag__type(tag), tag_pos) {
+		struct tag *type;
+		size_t diff = 0;
+
+		/* we want only data members, i.e. with byte_offset attr */
+		if (tag_pos->tag != DW_TAG_member &&
+		    tag_pos->tag != DW_TAG_inheritance)
+		    	continue;
+
+		type = cu__find_tag_by_id(cu, tag_pos->type);
+		switch (type->tag) {
+		case DW_TAG_base_type: {
+			struct base_type *bt = tag__base_type(tag_pos);
+
+			if (strcmp(bt->name, "long int") != 0)
+				break;
+			/* fallthru */
+		}
+		case DW_TAG_pointer_type:
+			diff = word_size_diff;
+			break;
+		case DW_TAG_structure_type:
+			diff = tag__type(type)->size_diff;
+			break;
+		}
+
+		if (diff != 0 &&
+		    original_word_size > word_size) {
+			self->type.size -= diff;
+			class__subtract_offsets_from(self, cu,
+						     tag__class_member(tag_pos), diff);
+		}
+	}
+
+	if (original_word_size > word_size)
+		self->type.size_diff = orig_size - self->type.size;
+	else
+		self->type.size_diff = self->type.size - orig_size;
+}
+
+static int tag_fixup_word_size_iterator(struct tag *tag, struct cu *cu,
+					void *cookie __unused)
+{
+	switch (tag->tag) {
+	case DW_TAG_base_type: {
+		struct base_type *bt = tag__base_type(tag);
+
+		if (strcmp(bt->name, "long int") == 0)
+			bt->size = word_size;
+	}
+		break;
+	case DW_TAG_structure_type:
+		class__realign_LP(tag, cu);
+		break;
+	}
+	return 0;
+}
+
+static int cu_fixup_word_size_iterator(struct cu *cu, void *cookie)
+{
+	original_word_size = cu->addr_size;
+	cu->addr_size = word_size;
+	return cu__for_each_tag(cu, tag_fixup_word_size_iterator, cookie, NULL);
+}
+
 static struct tag *nr_methods__filter(struct tag *tag, struct cu *cu __unused,
 				      void *cookie __unused)
 {
@@ -652,6 +730,12 @@ static const struct argp_option pahole__options[] = {
 		.doc  = "be verbose",
 	},
 	{
+		.name = "word_size",
+		.key  = 'w',
+		.arg  = "WORD_SIZE",
+		.doc  = "change the arch word size to WORD_SIZE"
+	},
+	{
 		.name = NULL,
 	}
 };
@@ -696,6 +780,7 @@ static error_t pahole__options_parser(int key, char *arg,
 	case 'T': formatter = nr_definitions_formatter;	break;
 	case 't': separator = arg[0];			break;
 	case 'V': global_verbose = 1;			break;
+	case 'w': word_size = atoi(arg);		break;
 	case 'X': cu__exclude_prefix = arg;
 		  cu__exclude_prefix_len = strlen(cu__exclude_prefix);
 							break;
@@ -738,6 +823,9 @@ int main(int argc, char *argv[])
 
 	dwarves__init(cacheline_size);
 
+	if (word_size != 0)
+		cus__for_each_cu(cus, cu_fixup_word_size_iterator, NULL, NULL);
+
 	if (class_dwarf_offset != 0) {
 		struct cu *cu;
 		struct tag *tag = cus__find_tag_by_id(cus, &cu,
-
To unsubscribe from this list: send the line "unsubscribe dwarves" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: eat your own dog food?

[jamal]

On Thu, 2008-10-01 at 14:39 -0200, Arnaldo Carvalho de Melo wrote:

> First stab, doesn't handles unions shrinking because the largest member
> shrunk, and handles only binaries built for 64 bits (or bigger) having
> it addr_size (word_size) shrinking.
> 

Only for 64-bit machines? 
My laptop is still 32-bit - from the patch i cant see anything that'll
stop me from testing for 64 bit from a 32-bit compilation.

[..]

> 
> Lets go all the way back to the 8088:
> 

;->

> [acme@doppio pahole]$ build/pahole -w 1 -C structure build/pahole
> struct structure {
> 	struct list_head           node;                 /*     0     2 */
> 	struct class *             class;                /*     2     1 */
> 	const struct cu  *         cu;                   /*     3     1 */
> 	uint32_t                   nr_files;             /*     4     4 */
> 	uint32_t                   nr_methods;           /*     8     4 */
> 
> 	/* size: 12, cachelines: 1 */
> 	/* last cacheline: 12 bytes */
> };
> 
> Not really because we're only change 'long' from the original word size
> to the one specified.
> 
> Please test and find the missing bits for me :-)


Will do later today.


cheers,
jamal

-
To unsubscribe from this list: send the line "unsubscribe dwarves" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: eat your own dog food?

[Arnaldo Carvalho de Melo]

Em Thu, Jan 10, 2008 at 04:37:14PM -0500, jamal escreveu:
> On Thu, 2008-10-01 at 14:39 -0200, Arnaldo Carvalho de Melo wrote:
> 
> > First stab, doesn't handles unions shrinking because the largest member
> > shrunk, and handles only binaries built for 64 bits (or bigger) having
> > it addr_size (word_size) shrinking.
> > 
> 
> Only for 64-bit machines? 
> My laptop is still 32-bit - from the patch i cant see anything that'll
> stop me from testing for 64 bit from a 32-bit compilation.

for 32 bit binaries we'll need to add to the offsets after pointers and
long ints that will be 64 bits when the compiler marked them as 32.

good data point from you, that your laptop is 32 bits, I'll implement
the small -> big transition now.
 
- Arnaldo
-
To unsubscribe from this list: send the line "unsubscribe dwarves" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: eat your own dog food?

[jamal]

On Thu, 2008-10-01 at 19:44 -0200, Arnaldo Carvalho de Melo wrote:


> good data point from you, that your laptop is 32 bits, I'll implement
> the small -> big transition now.

I'll take either incremental or full patch including one you sent
earlier.
 
Btw, thinking about this a little more, it may be trickier than i
thought initially and will have to including eabi considerations even
in the case of two machines of equivalent bit width. Tell me if i am bad
crack:

Consider ia32 and MIPS 32 bit (as per eabi defined here
http://sourceware.org/ml/binutils/2003-06/msg00436.html) 

consider this:

typedef unsigned int __u32;
typedef unsigned long long __u64;
struct tc_stats
{
  __u64 bytes; 
  __u32 packets;
  __u32 drops;                 
  __u32 overlimits; 
  __u32 bps;        
  __u32 pps;
  __u32 qlen;
  __u32 backlog;
};

On mips:
sizeof (struct tc_stats) == 40 and it is aligned 8 

On my laptop:
sizeof (struct tc_stats) == 36 aligned at 4

The mips alignment explains the padding because long long is
8-byte aligned.

i.e eventually you may need to have an eabi file per architecture

cheers,
jamal

-
To unsubscribe from this list: send the line "unsubscribe dwarves" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: eat your own dog food?

[jamal]

On Thu, 2008-10-01 at 19:45 -0500, jamal wrote:

> Btw, thinking about this a little more, it may be trickier than i
> thought initially and will have to including eabi considerations even
> in the case of two machines of equivalent bit width. Tell me if i am bad
> crack:
> 

I guess i am not on bad crack - heres ia32 data represantation:
http://www.linux-foundation.org/spec/book/ELF-IA32/ELF-IA32/lowlevel.html#DATAREPRESENTATION

That would explain it.

cheers,
jamal

-
To unsubscribe from this list: send the line "unsubscribe dwarves" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: eat your own dog food?

[Arnaldo Carvalho de Melo]

Em Thu, Jan 10, 2008 at 02:39:59PM -0200, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Jan 10, 2008 at 12:06:35PM -0200, Arnaldo Carvalho de Melo escreveu:
> > Em Thu, Jan 10, 2008 at 08:55:41AM -0500, jamal escreveu:
> > > ok, cool - so i should be able say something like "--add_size 64-bit"
> > > and it will follow something like the MIPS EABI i posted earlier? [I
> > > think x86_64 will work the same way too if i am not mistaken].
> > 
> > Exactly, you will be able to change the addr_size, I think that there is
> > even code that can be right away used for that, lemme check.
> 
> First stab, doesn't handles unions shrinking because the largest member
> shrunk, and handles only binaries built for 64 bits (or bigger) having
> it addr_size (word_size) shrinking.

Working now on unions, etc, have you had the chance of testing this
patch?

- Arnaldo
-
To unsubscribe from this list: send the line "unsubscribe dwarves" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: eat your own dog food?

[jamal]

On Thu, 2008-10-01 at 19:39 -0200, Arnaldo Carvalho de Melo wrote:


> Working now on unions, etc, have you had the chance of testing this
> patch?

I should be able to test it in a couple of hours. If you have the unions
change before then, send me a unified patch - else an incremental
version.

cheers,
jamal

-
To unsubscribe from this list: send the line "unsubscribe dwarves" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: eat your own dog food?

[Arnaldo Carvalho de Melo]

Em Thu, Jan 10, 2008 at 04:41:16PM -0500, jamal escreveu:
> On Thu, 2008-10-01 at 19:39 -0200, Arnaldo Carvalho de Melo wrote:
> 
> 
> > Working now on unions, etc, have you had the chance of testing this
> > patch?
> 
> I should be able to test it in a couple of hours. If you have the unions
> change before then, send me a unified patch - else an incremental
> version.

Remove the non-commited patches and do a git-pull, basic work is done,
reusing code I had in ctracer to do some simple alignment fixups.

Please let me know about any bugs you may find.

- Arnaldo

commit f6eb9ed92dafbd8cac082737fe0352c0ff6e7fdc
Author: Arnaldo Carvalho de Melo <acme-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Date:   Sat Jan 12 13:14:40 2008 -0200

    [PAHOLE]: Allow changing the architecture word-size
    
    Using a x86_64 binary:
    
    [acme@doppio pahole]$ build/pahole -C restart_block /usr/lib/debug/lib/modules/2.6.21-65.el5rt/kernel/drivers/net/e1000/e1000.ko.debug
    struct restart_block {
    	long int          (*fn)(struct restart_block *); /*     0     8 */
    	union {
    		struct {
    			long unsigned int arg0;          /*     8     8 */
    			long unsigned int arg1;          /*    16     8 */
    			long unsigned int arg2;          /*    24     8 */
    			long unsigned int arg3;          /*    32     8 */
    		};                                       /*          32 */
    		struct {
    			u32 *      uaddr;                /*     8     8 */
    			u32        val;                  /*    16     4 */
    			u32        flags;                /*    20     4 */
    			u64        time;                 /*    24     8 */
    		} fu;                                    /*          24 */
    	};                                               /*     8    32 */
    
    	/* size: 40, cachelines: 1 */
    	/* last cacheline: 40 bytes */
    };
    
    Changing the word-size from 8 to 4 bytes:
    
    [acme@doppio pahole]$ build/pahole -w 4 -C restart_block /usr/lib/debug/lib/modules/2.6.21-65.el5rt/kernel/drivers/net/e1000/e1000.ko.debug
    struct restart_block {
    	long int       (*fn)(struct restart_block *); /*     0     4 */
    	union {
    		struct {
    			long unsigned int arg0;          /*     4     4 */
    			long unsigned int arg1;          /*     8     4 */
    			long unsigned int arg2;          /*    12     4 */
    			long unsigned int arg3;          /*    16     4 */
    		};                                       /*          16 */
    		struct {
    			u32 *      uaddr;                /*     4     4 */
    			u32        val;                  /*     8     4 */
    			u32        flags;                /*    12     4 */
    			u64        time;                 /*    16     8 */
    		} fu;                                    /*          20 */
    	};                                               /*     4    20 */
    
    	/* size: 24, cachelines: 1 */
    	/* last cacheline: 24 bytes */
    };
    
    And from 8 to 16:
    
    [acme@doppio pahole]$ build/pahole -w 16 -C restart_block /usr/lib/debug/lib/modules/2.6.21-65.el5rt/kernel/drivers/net/e1000/e1000.ko.debug
    struct restart_block {
    	long int          (*fn)(struct restart_block *); /*     0    16 */
    	union {
    		struct {
    			long unsigned int arg0;          /*    16    16 */
    			long unsigned int arg1;          /*    32    16 */
    			long unsigned int arg2;          /*    48    16 */
    			long unsigned int arg3;          /*    64    16 */
    			/* --- cacheline 1 boundary (64 bytes) --- */
    		};                                       /*          64 */
    		struct {
    			u32 *      uaddr;                /*    16    16 */
    			u32        val;                  /*    32     4 */
    			u32        flags;                /*    36     4 */
    			u64        time;                 /*    40     8 */
    		} fu;                                    /*          32 */
    	};                                               /*    16    64 */
    	/* --- cacheline 1 boundary (64 bytes) was 16 bytes ago --- */
    
    	/* size: 80, cachelines: 2 */
    	/* last cacheline: 16 bytes */
    };
    
    More work is required to specify different alignment rules.
    
    Signed-off-by: Arnaldo Carvalho de Melo <acme-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
-
To unsubscribe from this list: send the line "unsubscribe dwarves" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Changing the word-size was: Re: eat your own dog food?

[Arnaldo Carvalho de Melo]

Em Sat, Jan 12, 2008 at 01:21:09PM -0200, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Jan 10, 2008 at 04:41:16PM -0500, jamal escreveu:
> > On Thu, 2008-10-01 at 19:39 -0200, Arnaldo Carvalho de Melo wrote:
> > 
> > 
> > > Working now on unions, etc, have you had the chance of testing this
> > > patch?
> > 
> > I should be able to test it in a couple of hours. If you have the unions
> > change before then, send me a unified patch - else an incremental
> > version.
> 
> Remove the non-commited patches and do a git-pull, basic work is done,
> reusing code I had in ctracer to do some simple alignment fixups.
> 
> Please let me know about any bugs you may find.

There are still some problems:

[acme@doppio pahole]$ build/pahole -C e1000_opt_list \
/usr/lib/debug/lib/modules/2.6.21-65.el5rt/kernel/drivers/net/e1000/e1000.ko.debug

struct e1000_opt_list {
	int                        i;                    /*     0     4 */

	/* XXX 4 bytes hole, try to pack */

	char *                     str;                  /*     8     8 */

	/* size: 16, cachelines: 1 */
	/* sum members: 12, holes: 1, sum holes: 4 */
	/* last cacheline: 16 bytes */
};

So on 64 bits we have a hole because pointers are aligned at word-size
boundaries, changing it to 4 bytes:

[acme@doppio pahole]$ build/pahole -w 4 -C e1000_opt_list \
/usr/lib/debug/lib/modules/2.6.21-65.el5rt/kernel/drivers/net/e1000/e1000.ko.debug
struct e1000_opt_list {
	int                        i;                    /*     0     4 */

	/* XXX 4 bytes hole, try to pack */

	char *                     str;                  /*     8     4 */

	/* size: 12, cachelines: 1 */
	/* sum members: 8, holes: 1, sum holes: 4 */
	/* last cacheline: 12 bytes */
};

The pointer size was correctly changed to 4 and the size reduced to 12,
but we should have subtracted 4 bytes from 'str' offset and the hole
would disappear and the size becomes 8.

Working on it.

This new feature should be more exploited on an upcoming cool feature,
one where we'll be able to ask pahole if a struct has optimal layout in
a range of architectures.

That should be a piece of cake to implement as we'll just clone each
struct, change the word size of it and check if with the new word-size
there are holes where previously there wasn't.

With full fledged alignment rules in description files for each
architecture we will be able to check on all arches if a struct is The
Holy Grail :-)

Ah, and if we find out that on one arch it is unholy, i.e. has
non-optimal layout, we can ask pahole to reorganize it and check again.

Humm, the reorganize (that jamal calls 'optimize') machinery will have
to take into account the rules for all the architectures... That should
be fun! :-P

- Arnaldo
-
To unsubscribe from this list: send the line "unsubscribe dwarves" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Changing the word-size was: Re: eat your own dog food?

[Arnaldo Carvalho de Melo]

Em Sat, Jan 12, 2008 at 01:54:04PM -0200, Arnaldo Carvalho de Melo escreveu:
> Em Sat, Jan 12, 2008 at 01:21:09PM -0200, Arnaldo Carvalho de Melo escreveu:
> > Em Thu, Jan 10, 2008 at 04:41:16PM -0500, jamal escreveu:
> > > On Thu, 2008-10-01 at 19:39 -0200, Arnaldo Carvalho de Melo wrote:
> > > 
> > > 
> > > > Working now on unions, etc, have you had the chance of testing this
> > > > patch?
> > > 
> > > I should be able to test it in a couple of hours. If you have the unions
> > > change before then, send me a unified patch - else an incremental
> > > version.
> > 
> > Remove the non-commited patches and do a git-pull, basic work is done,
> > reusing code I had in ctracer to do some simple alignment fixups.
> > 
> > Please let me know about any bugs you may find.
> 
> There are still some problems:
> 
> [acme@doppio pahole]$ build/pahole -C e1000_opt_list \
> /usr/lib/debug/lib/modules/2.6.21-65.el5rt/kernel/drivers/net/e1000/e1000.ko.debug
> 
> struct e1000_opt_list {
> 	int                        i;                    /*     0     4 */
> 
> 	/* XXX 4 bytes hole, try to pack */
> 
> 	char *                     str;                  /*     8     8 */
> 
> 	/* size: 16, cachelines: 1 */
> 	/* sum members: 12, holes: 1, sum holes: 4 */
> 	/* last cacheline: 16 bytes */
> };
> 
> So on 64 bits we have a hole because pointers are aligned at word-size
> boundaries, changing it to 4 bytes:
> 
> [acme@doppio pahole]$ build/pahole -w 4 -C e1000_opt_list \
> /usr/lib/debug/lib/modules/2.6.21-65.el5rt/kernel/drivers/net/e1000/e1000.ko.debug
> struct e1000_opt_list {
> 	int                        i;                    /*     0     4 */
> 
> 	/* XXX 4 bytes hole, try to pack */
> 
> 	char *                     str;                  /*     8     4 */
> 
> 	/* size: 12, cachelines: 1 */
> 	/* sum members: 8, holes: 1, sum holes: 4 */
> 	/* last cacheline: 12 bytes */
> };
> 
> The pointer size was correctly changed to 4 and the size reduced to 12,
> but we should have subtracted 4 bytes from 'str' offset and the hole
> would disappear and the size becomes 8.
> 
> Working on it.

Partially fixed, which says a lot about how bad class__fixup_alignment
is :-\ Only holes after unions are not being fixed up, still scratching
my head on why this is the case.

Anyway:

[acme@doppio pahole]$ build/pahole -w 4 -C e1000_opt_list \
/usr/lib/debug/lib/modules/2.6.21-65.el5rt/kernel/drivers/net/e1000/e1000.ko.debug
struct e1000_opt_list {
	int                        i;                    /*     0     4 */
	char *                     str;                  /*     4     4 */

	/* size: 8, cachelines: 1 */
	/* last cacheline: 8 bytes */
};

- Arnaldo
-
To unsubscribe from this list: send the line "unsubscribe dwarves" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Changing the word-size was: Re: eat your own dog food?

[Arnaldo Carvalho de Melo]

Em Sat, Jan 12, 2008 at 02:28:08PM -0200, Arnaldo Carvalho de Melo escreveu:
> Em Sat, Jan 12, 2008 at 01:54:04PM -0200, Arnaldo Carvalho de Melo escreveu:
> > Em Sat, Jan 12, 2008 at 01:21:09PM -0200, Arnaldo Carvalho de Melo escreveu:
> > > Em Thu, Jan 10, 2008 at 04:41:16PM -0500, jamal escreveu:
> > > > On Thu, 2008-10-01 at 19:39 -0200, Arnaldo Carvalho de Melo wrote:
> > > > 
> > > > 
> > > > > Working now on unions, etc, have you had the chance of testing this
> > > > > patch?
> > > > 
> > > > I should be able to test it in a couple of hours. If you have the unions
> > > > change before then, send me a unified patch - else an incremental
> > > > version.
> > > 
> > > Remove the non-commited patches and do a git-pull, basic work is done,
> > > reusing code I had in ctracer to do some simple alignment fixups.
> > > 
> > > Please let me know about any bugs you may find.
> > 
> > There are still some problems:
> > 
> > [acme@doppio pahole]$ build/pahole -C e1000_opt_list \
> > /usr/lib/debug/lib/modules/2.6.21-65.el5rt/kernel/drivers/net/e1000/e1000.ko.debug
> > 
> > struct e1000_opt_list {
> > 	int                        i;                    /*     0     4 */
> > 
> > 	/* XXX 4 bytes hole, try to pack */
> > 
> > 	char *                     str;                  /*     8     8 */
> > 
> > 	/* size: 16, cachelines: 1 */
> > 	/* sum members: 12, holes: 1, sum holes: 4 */
> > 	/* last cacheline: 16 bytes */
> > };
> > 
> > So on 64 bits we have a hole because pointers are aligned at word-size
> > boundaries, changing it to 4 bytes:
> > 
> > [acme@doppio pahole]$ build/pahole -w 4 -C e1000_opt_list \
> > /usr/lib/debug/lib/modules/2.6.21-65.el5rt/kernel/drivers/net/e1000/e1000.ko.debug
> > struct e1000_opt_list {
> > 	int                        i;                    /*     0     4 */
> > 
> > 	/* XXX 4 bytes hole, try to pack */
> > 
> > 	char *                     str;                  /*     8     4 */
> > 
> > 	/* size: 12, cachelines: 1 */
> > 	/* sum members: 8, holes: 1, sum holes: 4 */
> > 	/* last cacheline: 12 bytes */
> > };
> > 
> > The pointer size was correctly changed to 4 and the size reduced to 12,
> > but we should have subtracted 4 bytes from 'str' offset and the hole
> > would disappear and the size becomes 8.
> > 
> > Working on it.
> 
> Partially fixed, which says a lot about how bad class__fixup_alignment
> is :-\ Only holes after unions are not being fixed up, still scratching
> my head on why this is the case.

Problem wasn't in class__fixup_alignment, we have to check if the type
of members were already resized when resizing a struct/union.
Fixed'n'pushed.

Now what seems to be the last thing to do is to follow typedefs and find
unnamed structs, think about spinlock_t, socket_lock_t, etc.

- Arnaldo
-
To unsubscribe from this list: send the line "unsubscribe dwarves" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Changing the word-size was: Re: eat your own dog food?

[jamal]

[-- Attachment #1: Type: text/plain, Size: 931 bytes --]

On Sat, 2008-12-01 at 15:28 -0200, Arnaldo Carvalho de Melo wrote:

> Now what seems to be the last thing to do is to follow typedefs and find
> unnamed structs, think about spinlock_t, socket_lock_t, etc.
> 

Good stuff Arnaldo - updated local tree. 
I should be able to test later today.

Dont know much about cmake, is it possible to check for elfutils version
and not depend on build_id if version < xxx?
This way i dont have to go out of my way to maintain my own versions of
elf utilities. I have to do the patch you sent earlier manually now. I
will remove it if you will fix cmake.

BTW, while perusing archs i managed i think to gather sufficiently
accurate info (;->) on the different alignments; i got lazy at the end,
but quiet a few are in the attached txt file incase you find them
useful. Double check sparc for example. I think most 64-bit hardware
looks consistent; the funnies are in 32-bit archs.

cheers,
jamal

[-- Attachment #2: eabis --]
[-- Type: text/plain, Size: 2823 bytes --]

		MIPS EABI
			=========

Sizes and alignments
--------------------

	Type		Size (bytes)	Alignment (bytes)

	char		1		1
	short		2		2
	int		4		4
	unsigned	4		4
	long		4		4 (32-bit mode)
			8		8 (64-bit mode)
	long long	8		8
	float		4		4
	double		8		8
	pointers	4		4 (32-bit mode)
			8		8 (64-bit mode)

* alignment within aggregates (structs and unions) is as above, with
  padding added if needed
* aggregates have alignment equal to that of their most aligned
  member
* aggregates have sizes which are a multiple of their alignment

ia32 Sizes and alignments
--------------------

	Type		Size (bytes)	Alignment (bytes)

	char		1		1
	short		2		2
	int		4		4
	unsigned	4		4
	long		4		4 
	long long	8		4
	float		4		4
	double		8		4
	pointers	4		4 

* alignment within aggregates (structs and unions) is as above, with
  padding added if needed
* aggregates have alignment equal to that of their most aligned
  member
* aggregates have sizes which are a multiple of their alignment
  This may result in tail padding.

ia64 Sizes and alignments
--------------------

	Type		Size (bytes)	Alignment (bytes)

	char		1		1
	short		2		2
	int		4		4
	unsigned	4		4
	long		8		8 
	long long	8		8
	float		4		4
	double		8		8
	pointers	8		8 

    * An entire structure or union object shall be aligned on the same 
      boundary as its most strictly aligned member.
    * Each member shall be assigned to the lowest available offset with the 
      appropriate alignment. This may require internal padding, depending 
      on the previous member.
    * A structure's size shall be increased, if necessary, to make it a 
      multiple of the alignment. This may require tail padding, depending 
      on the last member. 

AMD 64
-------


	Type		Size (bytes)	Alignment (bytes)

	char		1		1
	short		2		2
	int		4		4
	unsigned	4		4
	long		4		4 
			8		8 (64-bit mode)
	long long	8		4
			8		8 (64-bit mode)
	float		4		4
	double		8		4
	pointers	4		4 
			8		8 (64-bit mode)

powerpc - 32bit

	Type		Size (bytes)	Alignment (bytes)

	char		1		1
	short		2		2
	int		4		4
	unsigned	4		4
	long	        4		4
	long long	8		8
	float		4		4
	double		8		8
	long double	16		16
	pointers	4		4 

powerpc - 64bit
	Type		Size (bytes)	Alignment (bytes)

	char		1		1
	short		2		2
	int		4		4
	unsigned	4		4
	long	        8		8
	long long	8		8
	int128  	16		16
	float		4		4
	double		8		8
	long double	16		16
	pointers	8		8 


sparc:
	Type		Size (bytes)	Alignment (bytes)

	char		1		1
	short		2		2
	int		4		4
	unsigned	4		4
	long	        4		4
	long long	8		8
	float		4		4
	double		8		8
	long double	16		8
	pointers	4		4 


s390:
------
	Type		Size (bytes)	Alignment (bytes)

	char		1		1
	short		2		2
	int		4		4
	unsigned	4		4
	long	        4		4
	long long	8		8
	float		4		4
	double		8		8
	long double	16		16
	pointers	4		4 

s390-zseries
---------

Re: Changing the word-size was: Re: eat your own dog food?

[Arnaldo Carvalho de Melo]

Em Sat, Jan 12, 2008 at 01:08:41PM -0500, jamal escreveu:
> On Sat, 2008-12-01 at 15:28 -0200, Arnaldo Carvalho de Melo wrote:
> 
> > Now what seems to be the last thing to do is to follow typedefs and find
> > unnamed structs, think about spinlock_t, socket_lock_t, etc.
> > 
> 
> Good stuff Arnaldo - updated local tree. 
> I should be able to test later today.
> 
> Dont know much about cmake, is it possible to check for elfutils version
> and not depend on build_id if version < xxx?
> This way i dont have to go out of my way to maintain my own versions of
> elf utilities. I have to do the patch you sent earlier manually now. I
> will remove it if you will fix cmake.

As I said in the cset commit, I'm no cmake/autoconf guy, but I'll check
that, after all this is a prime area where these otherwise distracting
things should help.
 
> BTW, while perusing archs i managed i think to gather sufficiently
> accurate info (;->) on the different alignments; i got lazy at the end,
> but quiet a few are in the attached txt file incase you find them
> useful. Double check sparc for example. I think most 64-bit hardware
> looks consistent; the funnies are in 32-bit archs.

I'll check that, thanks for the research, that may well be the starting
point for arch descriptions, that if libebl from elfutils doesn't
already provide that to us, something I haven't checked.

- Arnaldo
-
To unsubscribe from this list: send the line "unsubscribe dwarves" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Changing the word-size was: Re: eat your own dog food?

[jamal]

On Sat, 2008-12-01 at 16:42 -0200, Arnaldo Carvalho de Melo wrote:

> As I said in the cset commit, I'm no cmake/autoconf guy, but I'll check
> that, after all this is a prime area where these otherwise distracting
> things should help.


Ok, sorry - didnt see that until now; dont worry about it for now.
The fix is good enough for me as is; i just changed #if 1 to 0.

> I'll check that, thanks for the research, that may well be the starting
> point for arch descriptions, that if libebl from elfutils doesn't
> already provide that to us, something I haven't checked.

youre more than likely right; i think that libel will have all the
details somewhere ... and reading from it would be the better option
since you dont have to maintain that.

I am going to pull your latest change and test.

cheers,
jamal

-
To unsubscribe from this list: send the line "unsubscribe dwarves" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Changing the word-size was: Re: eat your own dog food?

[jamal]

On Sat, 2008-12-01 at 19:42 -0500, jamal wrote:

> I am going to pull your latest change and test.

Just one bug to report: you dont adjust your tail padding when the size
changes from native; example:

------
hadi@lilsol:~/pahole-tests$ pahole -C s4 ./a.out
struct s4 {
        char                       a;                 /*     0     1 */

        /* XXX 3 bytes hole, try to pack */

        double                     b;                /*     4     8 */
        char                       c;                /*    12     1 */

        /* size: 16, cachelines: 1 */
        /* sum members: 10, holes: 1, sum holes: 3 */
        /* padding: 3 */
        /* last cacheline: 16 bytes */
};
hadi@lilsol:~/pahole-tests$ pahole -w 8 -C s4 ./a.out
struct s4 {
        char                       a;                 /*     0     1 */

        /* XXX 7 bytes hole, try to pack */

        double                     b;                /*     8     8 */
        char                       c;                /*    16     1 */

        /* size: 20, cachelines: 1 */
        /* sum members: 10, holes: 1, sum holes: 7 */
        /* padding: 3 */
        /* last cacheline: 20 bytes */
};
hadi@lilsol:~/pahole-tests$

---------------

In this case the tail padding should be 7.

The rules say:
1) aggregates have alignment equal to that of their most aligned
  member - which happens to be 8 above
2) aggregates have sizes which are a multiple of their alignment
  This may result in tail padding - the tail contains a char.

So the padding above should be 7.

Heres something that initially puzzled me:

---
hadi@lilsol:~/pahole-tests$ pahole -C s1 ./a.out
struct s1 {
        short int                  a1;               /*     0     2 */
        short int                  a2;               /*     2     2 */
        short int                  a3;               /*     4     2 */

        /* size: 6, cachelines: 1 */
        /* last cacheline: 6 bytes */
};
hadi@lilsol:~/pahole-tests$ pahole -C s1 -w 8 ./a.out
struct s1 {
        short int                  a1;               /*     0     2 */
        short int                  a2;               /*     2     2 */
        short int                  a3;              /*     4     2 */

        /* size: 6, cachelines: 1 */
        /* last cacheline: 6 bytes */
};
------------

It puzzled me initially because i thought we'd need a tail padding of
some sort for both 32 bit and 64 bit - but it turns out from rule #1
that the most aligned member was 2 bytes, so no padding was needed for
either. So whatever was done here to follow the rules is key.


cheers,
jamal

-
To unsubscribe from this list: send the line "unsubscribe dwarves" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Changing the word-size was: Re: eat your own dog food?

[Arnaldo Carvalho de Melo]

Em Sun, Jan 13, 2008 at 02:31:06PM -0200, Arnaldo Carvalho de Melo escreveu:
> Em Sat, Jan 12, 2008 at 08:15:55PM -0500, jamal escreveu:
> > On Sat, 2008-12-01 at 19:42 -0500, jamal wrote:
> > 
> > In this case the tail padding should be 7.
> > 
> > The rules say:
> > 1) aggregates have alignment equal to that of their most aligned
> >   member - which happens to be 8 above
> > 2) aggregates have sizes which are a multiple of their alignment
> >   This may result in tail padding - the tail contains a char.
> > 
> > So the padding above should be 7.
> 
> Working on that, implementing type__most_aligned_member(), that will
> recursively find the member that is most aligned, but then how to deal
> with packed structs?

Should be fixed now for non-packed structs, please ACK.

Regards,

- Arnaldo
-
To unsubscribe from this list: send the line "unsubscribe dwarves" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Changing the word-size was: Re: eat your own dog food?

[Arnaldo Carvalho de Melo]

Em Sun, Jan 13, 2008 at 04:07:30PM -0200, Arnaldo Carvalho de Melo escreveu:
> Em Sun, Jan 13, 2008 at 02:31:06PM -0200, Arnaldo Carvalho de Melo escreveu:
> > Em Sat, Jan 12, 2008 at 08:15:55PM -0500, jamal escreveu:
> > > On Sat, 2008-12-01 at 19:42 -0500, jamal wrote:
> > > 
> > > In this case the tail padding should be 7.
> > > 
> > > The rules say:
> > > 1) aggregates have alignment equal to that of their most aligned
> > >   member - which happens to be 8 above
> > > 2) aggregates have sizes which are a multiple of their alignment
> > >   This may result in tail padding - the tail contains a char.
> > > 
> > > So the padding above should be 7.
> > 
> > Working on that, implementing type__most_aligned_member(), that will
> > recursively find the member that is most aligned, but then how to deal
> > with packed structs?
> 
> Should be fixed now for non-packed structs, please ACK.

Oops, I concentrated on the tail padding, have to check how we're
handling rule number 1, but now we infrastructure is there, should be
easy.

- Arnaldo
-
To unsubscribe from this list: send the line "unsubscribe dwarves" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Changing the word-size was: Re: eat your own dog food?

[Arnaldo Carvalho de Melo]

Em Sun, Jan 13, 2008 at 04:19:01PM -0200, Arnaldo Carvalho de Melo escreveu:
> Em Sun, Jan 13, 2008 at 04:07:30PM -0200, Arnaldo Carvalho de Melo escreveu:
> > Em Sun, Jan 13, 2008 at 02:31:06PM -0200, Arnaldo Carvalho de Melo escreveu:
> > > Em Sat, Jan 12, 2008 at 08:15:55PM -0500, jamal escreveu:
> > > > On Sat, 2008-12-01 at 19:42 -0500, jamal wrote:
> > > > 
> > > > In this case the tail padding should be 7.
> > > > 
> > > > The rules say:
> > > > 1) aggregates have alignment equal to that of their most aligned
> > > >   member - which happens to be 8 above
> > > > 2) aggregates have sizes which are a multiple of their alignment
> > > >   This may result in tail padding - the tail contains a char.
> > > > 
> > > > So the padding above should be 7.
> > > 
> > > Working on that, implementing type__most_aligned_member(), that will
> > > recursively find the member that is most aligned, but then how to deal
> > > with packed structs?
> > 
> > Should be fixed now for non-packed structs, please ACK.
> 
> Oops, I concentrated on the tail padding, have to check how we're
> handling rule number 1, but now we infrastructure is there, should be
> easy.

Looks OK:

[acme@doppio examples]$ file align
align: ELF 64-bit LSB executable, x86-64, version 1 (SYSV), dynamically
linked (uses shared libs), for GNU/Linux 2.6.9, not stripped
[acme@doppio examples]$ ../build/pahole align
struct foo {
	char                       l;                    /*     0     1 */

	/* XXX 7 bytes hole, try to pack */

	struct {
		unsigned int       b;                    /*     8     4 */
		struct {
			long unsigned int c;             /*    16     8 */
		};                                       /*    16     8 */
	};                                               /*     8    16 */
	char                       a;                    /*    24     1 */

	/* size: 32, cachelines: 1 */
	/* sum members: 18, holes: 1, sum holes: 7 */
	/* padding: 7 */
	/* last cacheline: 32 bytes */
};

[acme@doppio examples]$ ../build/pahole -w4 align
struct foo {
	char                       l;                    /*     0     1 */

	/* XXX 3 bytes hole, try to pack */

	struct {
		unsigned int       b;                    /*     4     4 */
		struct {
			long unsigned int c;             /*     8     4 */
		};                                       /*     8     4 */
	};                                               /*     4     8 */
	char                       a;                    /*    12     1 */

	/* size: 16, cachelines: 1 */
	/* sum members: 10, holes: 1, sum holes: 3 */
	/* padding: 3 */
	/* last cacheline: 16 bytes */
};

Looks ok when going from 32-bit to 64-bit

[acme@doppio examples]$ gcc -m32 -g align.c -o align
[acme@doppio examples]$ file align
align: ELF 32-bit LSB executable, Intel 80386, version 1 (SYSV),
dynamically linked (uses shared libs), for GNU/Linux 2.6.9, not stripped

[acme@doppio examples]$ ../build/pahole align
struct foo {
	char                       l;                    /*     0     1 */

	/* XXX 3 bytes hole, try to pack */

	struct {
		unsigned int       b;                    /*     4     4 */
		struct {
			long unsigned int c;             /*     8     4 */
		};                                       /*     8     4 */
	};                                               /*     4     8 */
	char                       a;                    /*    12     1 */

	/* size: 16, cachelines: 1 */
	/* sum members: 10, holes: 1, sum holes: 3 */
	/* padding: 3 */
	/* last cacheline: 16 bytes */
};

[acme@doppio examples]$ ../build/pahole -w8 align
struct foo {
	char                       l;                    /*     0     1 */

	/* XXX 3 bytes hole, try to pack */

	struct {
		unsigned int       b;                    /*     4     4 */

		/* XXX 4 bytes hole, try to pack */

		struct {
			long unsigned int c;             /*    12     8 */
		};                                       /*    12     8 */
	};                                               /*     4    16 */

	/* Bitfield combined with next fields */

	char                       a;                    /*    16     1 */

	/* size: 24, cachelines: 1 */
	/* sum members: 14, holes: 1, sum holes: 3 */
	/* padding: 7 */
	/* last cacheline: 24 bytes */
};

But not when going from 32-bit to 64-bit, sigh :-\

And there is another bug, the bitfield combining code got confused,
because the offset for 'a' is not at 20 as it should even when not
correctly aligning the outernmost unnamed struct.

Will fix.

- Arnaldo
-
To unsubscribe from this list: send the line "unsubscribe dwarves" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Changing the word-size was: Re: eat your own dog food?

[jamal]

On Sun, 2008-13-01 at 16:26 -0200, Arnaldo Carvalho de Melo wrote:

> > > > 
> > > > Working on that, implementing type__most_aligned_member(), that will
> > > > recursively find the member that is most aligned, but then how to deal
> > > > with packed structs?

packed structs _always_ have alignment = 1 afaik.


> Looks OK:

I updated tree and recompiled - retested and all looks ok to me ....

> [acme@doppio examples]$ file align


> [acme@doppio examples]$ ../build/pahole align
> struct foo {
> 	char                       l;                    /*     0     1 */
> 
> 	/* XXX 3 bytes hole, try to pack */
> 
> 	struct {
> 		unsigned int       b;                    /*     4     4 */
> 		struct {
> 			long unsigned int c;             /*     8     4 */
> 		};                                       /*     8     4 */
> 	};                                               /*     4     8 */
> 	char                       a;                    /*    12     1 */
> 
> 	/* size: 16, cachelines: 1 */
> 	/* sum members: 10, holes: 1, sum holes: 3 */
> 	/* padding: 3 */
> 	/* last cacheline: 16 bytes */
> };
> 
> [acme@doppio examples]$ ../build/pahole -w8 align
> struct foo {
> 	char                       l;                    /*     0     1 */
> 
> 	/* XXX 3 bytes hole, try to pack */
> 
> 	struct {
> 		unsigned int       b;                    /*     4     4 */
> 
> 		/* XXX 4 bytes hole, try to pack */
> 
> 		struct {
> 			long unsigned int c;             /*    12     8 */
> 		};                                       /*    12     8 */
> 	};                                               /*     4    16 */
> 
> 	/* Bitfield combined with next fields */
> 
> 	char                       a;                    /*    16     1 */
> 
> 	/* size: 24, cachelines: 1 */
> 	/* sum members: 14, holes: 1, sum holes: 3 */
> 	/* padding: 7 */
> 	/* last cacheline: 24 bytes */
> };
> 
> But not when going from 32-bit to 64-bit, sigh :-\
> And there is another bug, the bitfield combining code got confused,
> because the offset for 'a' is not at 20 as it should even when not
> correctly aligning the outernmost unnamed struct.

got same results here. Other than that annoyance the results look
correct.

Note: So far what you have seems to work well (sans the little bug
above) for say x86 32<->64. The bizare part imo is the alignment
difference with different archs (eg -w 4 for ppc vs ia32 would result in
different holes/paddings).
Q: Wouldnt it be better to just build arrays/tables of all the rules as
per eabi? i.e you have a sizeof and alignment rows and then you can do
table lookup of arch/type where key could infact be a dwarf tag. What
that would mean is to have cli interface for --arch ppc32 or --arch
mips64 etc. If you do that, then allow users to also add new archs for
you.

> Will fix.

Ok, let me know.

cheers,
jamal

-
To unsubscribe from this list: send the line "unsubscribe dwarves" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Changing the word-size was: Re: eat your own dog food?

[Arnaldo Carvalho de Melo]

Em Sat, Jan 12, 2008 at 03:28:56PM -0200, Arnaldo Carvalho de Melo escreveu:
> Now what seems to be the last thing to do is to follow typedefs and find
> unnamed structs, think about spinlock_t, socket_lock_t, etc.

OK, typedefs are being followed now, now we get to arrays, working on that.

- Arnaldo
-
To unsubscribe from this list: send the line "unsubscribe dwarves" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Changing the word-size was: Re: eat your own dog food?

[Arnaldo Carvalho de Melo]

Em Sat, Jan 12, 2008 at 07:36:35PM -0200, Arnaldo Carvalho de Melo escreveu:
> Em Sat, Jan 12, 2008 at 03:28:56PM -0200, Arnaldo Carvalho de Melo escreveu:
> > Now what seems to be the last thing to do is to follow typedefs and find
> > unnamed structs, think about spinlock_t, socket_lock_t, etc.
> 
> OK, typedefs are being followed now, now we get to arrays, working on that.

Fixed'n'pushed, now I'll wait for your comments, I guess everything is
right now when just changing the word-size, and should be OK mostly
apart from unusual alignment rules.

- Arnaldo
-
To unsubscribe from this list: send the line "unsubscribe dwarves" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html