a GCC question

a GCC question

[Takashi Oe]

Hi,

I have modified the code in arch/ppc/kernel/prom.c to add support for
various depths and it looks like this:

static void
draw_byte(unsigned char c, long locX, long locY)
{
	unsigned long offset	= reloc_offset();
	boot_infos_t* bi	= PTRRELOC(RELOC(disp_bi));
	unsigned char *base	= calc_base(bi, locX << 3, locY << 4);
	unsigned char *font	= &RELOC(vga_font)[((unsigned long)c) *
16];
	int rb			= bi->dispDeviceRowBytes;

	switch (bi->dispDeviceDepth) {
	case 32:
		draw_byte_32(font, (unsigned long *)base, rb);
		break;
	case 16:
		draw_byte_16(font, (unsigned long *)base, rb);
		break;
	case 8:
		draw_byte_8(font, (unsigned long *)base, rb);
		break;
	case 4:
		draw_byte_4(font, (unsigned long *)base, rb);
		break;
	case 2:
		draw_byte_2(font, (unsigned short *)base, rb);
		break;
	case 1:
		draw_byte_1(font, (unsigned char *)base, rb);
		break;
	}
}

At the early boot time when this function is called inside kernel, it is
not at its linked location.  When I have only top three cases (32, 16, 8),
it works just fine.  However, when there are more cases, it doesn't work.
Here is the objdump of the function:

000004e0 <draw_byte>:
     4e0:       94 21 ff d0     stwu    r1,-48(r1)
     4e4:       7c 08 02 a6     mflr    r0
     4e8:       bf 21 00 14     stmw    r25,20(r1)
     4ec:       90 01 00 34     stw     r0,52(r1)
     4f0:       7c 7c 1b 78     mr      r28,r3
     4f4:       7c 9a 23 78     mr      r26,r4
     4f8:       7c b9 2b 78     mr      r25,r5
     4fc:       48 00 00 01     bl      4fc <draw_byte+0x1c>
                        4fc: R_PPC_REL24        reloc_offset
     500:       3d 20 00 00     lis     r9,0
                        502: R_PPC_ADDR16_HA    disp_bi
     504:       7c 7b 1b 78     mr      r27,r3
     508:       39 29 00 00     addi    r9,r9,0
                        50a: R_PPC_ADDR16_LO    disp_bi
     50c:       7f bb 48 2e     lwzx    r29,r27,r9
     510:       57 44 18 38     rlwinm  r4,r26,3,0,28
     514:       7f bd da 14     add     r29,r29,r27
     518:       57 25 20 36     rlwinm  r5,r25,4,0,27
     51c:       7f a3 eb 78     mr      r3,r29
     520:       4b ff fa e1     bl      0 <calc_base>
     524:       81 7d 00 2c     lwz     r11,44(r29)
     528:       3d 20 00 00     lis     r9,0
                        52a: R_PPC_ADDR16_HA    vga_font
     52c:       38 0b ff ff     addi    r0,r11,-1
     530:       57 9c 20 36     rlwinm  r28,r28,4,0,27
     534:       39 29 00 00     addi    r9,r9,0
                        536: R_PPC_ADDR16_LO    vga_font
     538:       28 00 00 1f     cmplwi  r0,31
     53c:       7c 64 1b 78     mr      r4,r3
     540:       7f 9c 4a 14     add     r28,r28,r9
     544:       80 bd 00 34     lwz     r5,52(r29)
     548:       7c 7b e2 14     add     r3,r27,r28
     54c:       41 81 00 54     bgt     5a0 <draw_byte+0xc0>
     550:       3d 60 00 00     lis     r11,0
                        552: R_PPC_ADDR16_HA    .rodata+0x770
     554:       39 6b 07 70     addi    r11,r11,1904
                        556: R_PPC_ADDR16_LO    .rodata+0x770
     558:       54 00 10 3a     rlwinm  r0,r0,2,0,29
     55c:       7d 4b 00 2e     lwzx    r10,r11,r0
     560:       3d 20 00 00     lis     r9,0
                        562: R_PPC_ADDR16_HA    .rodata+0x770
     564:       39 29 07 70     addi    r9,r9,1904
                        566: R_PPC_ADDR16_LO    .rodata+0x770
     568:       7d 4a 4a 14     add     r10,r10,r9
     56c:       7d 49 03 a6     mtctr   r10
     570:       4e 80 04 20     bctr
     574:       48 00 00 91     bl      604 <draw_byte_32>
     578:       48 00 00 28     b       5a0 <draw_byte+0xc0>
     57c:       48 00 01 6d     bl      6e8 <draw_byte_16>
     580:       48 00 00 20     b       5a0 <draw_byte+0xc0>
     584:       48 00 02 31     bl      7b4 <draw_byte_8>
     588:       48 00 00 18     b       5a0 <draw_byte+0xc0>
     58c:       48 00 03 25     bl      8b0 <draw_byte_4>
     590:       48 00 00 10     b       5a0 <draw_byte+0xc0>
     594:       48 00 03 ad     bl      940 <draw_byte_2>
     598:       48 00 00 08     b       5a0 <draw_byte+0xc0>
     59c:       48 00 04 39     bl      9d4 <draw_byte_1>
     5a0:       80 01 00 34     lwz     r0,52(r1)
     5a4:       7c 08 03 a6     mtlr    r0
     5a8:       bb 21 00 14     lmw     r25,20(r1)
     5ac:       38 21 00 30     addi    r1,r1,48
     5b0:       4e 80 00 20     blr

As you can see, the switch statement is causing offsets fetch from
.rodata, and, since .rodata section is also not at its linked location,
the value fetched is total garbage or, in the case of my machine, it
causes a machine check.

Is there a gcc flag which will prevent generation of codes like this?  Or
any suggestion for better C codes?


Takashi Oe


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: a GCC question

[David Edelsohn]

	I would guess that for smaller numbers of cases, GCC is deciding
that it is more efficient to use a series of "if" statements for a
decision tree instead of a switch statement.  You can explicitly code it
as such, e.g.,

if (bi->dispDeviceDepth == 32)
...
else if (bi-dispDeviceDepth == 16)
...
else if
...
else abort();

(BTW you do not provide a default case for some illegal value in your
switch statement.)

David

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: a GCC question

[Takashi Oe]

On Sat, 5 Aug 2000, David Edelsohn wrote:

> 	I would guess that for smaller numbers of cases, GCC is deciding
> that it is more efficient to use a series of "if" statements for a
> decision tree instead of a switch statement.  You can explicitly code it
> as such, e.g.,
>
> if (bi->dispDeviceDepth == 32)
> ...
> else if (bi-dispDeviceDepth == 16)
> ...
> else if
> ...
> else abort();

Ah, that works, though the code doesn't work for some other unknown
reasons now, probably due to something along the line of things Olaf was
talking about recently.  Thanks!

Is there other kind of C constructs besides a switch statement which uses
.rodata or some such internally that you know of?  If the use of .rodata
is explicit like for the case of printk("I'm in .rodata."), we can fix the
reference dynamically pretty easily, but not the internal one like for the
case of a switch statement.

> (BTW you do not provide a default case for some illegal value in your
> switch statement.)

The default action is to do nothing, so there is no default case in there.


Takashi Oe


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: a GCC question

[David Edelsohn]

	I do not know SVR4 that well to know what else might end up in
rodata other than symbols.  Switch jump tables are the only thing that
comes to mind.

	Even if the default is to do nothing, one always should provide a
default action or just break so that the density / coverage of the switch
statement cases is apparent.

David

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: a GCC question

[Takashi Oe]

On Sat, 5 Aug 2000, David Edelsohn wrote:

> 	I do not know SVR4 that well to know what else might end up in
> rodata other than symbols.  Switch jump tables are the only thing that
> comes to mind.

Good!  That's one less thing to worry about.

> 	Even if the default is to do nothing, one always should provide a
> default action or just break so that the density / coverage of the switch
> statement cases is apparent.

I don't know what the density is, but, if having a default action for
nothing is always prefered, I'll fix it (or rather I'll ask Ben to fix
it.)  Is it in gcc documentation or C standard somewhere?


Thanks,

Takashi Oe


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: a GCC question

[David Edelsohn]

>>>>> Takashi Oe writes:

>> Even if the default is to do nothing, one always should provide a
>> default action or just break so that the density / coverage of the switch
>> statement cases is apparent.

Takashi> I don't know what the density is, but, if having a default action for
Takashi> nothing is always prefered, I'll fix it (or rather I'll ask Ben to fix
Takashi> it.)  Is it in gcc documentation or C standard somewhere?

	It's not in the documentation or in the standard.  It is good, but
not a requirements.  It helps document that the programmer knows what is
expected and sometimes helps the compiler generate better code by knowing
what cases need be considered.

David

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: a GCC question

[Takashi Oe]

Hi,

On Sun, 6 Aug 2000, David Edelsohn wrote:

> >>>>> Takashi Oe writes:
>
> >> Even if the default is to do nothing, one always should provide a
> >> default action or just break so that the density / coverage of the switch
> >> statement cases is apparent.
>
> Takashi> I don't know what the density is, but, if having a default action for
> Takashi> nothing is always prefered, I'll fix it (or rather I'll ask Ben to fix
> Takashi> it.)  Is it in gcc documentation or C standard somewhere?
>
> 	It's not in the documentation or in the standard.  It is good, but
> not a requirements.  It helps document that the programmer knows what is
> expected and sometimes helps the compiler generate better code by knowing
> what cases need be considered.

Thank you for the explanation.  Actually, I used to put a default action
always with a switch statement because that's how I was taught when I
first learned to code in C, well, until I came across a code in Linux
kernel which had a switch statement without a default action.  At first I
was skeptical, but I realized I can save two lines of typing that way when
there is nothing to do for default, and I got accustomed to code similarly
rather quickly.  But, if it helps the compiler, I can type the extra two
lines as I used to.  No problem.

By the way, has anyone noticed compiling arch/ppc/mm/init.c takes very
long time?  On my machine, it takes as long as about two minutes to just
compile it, while compiling other files are almost instantaneous.  Well,
it's one of the biggest C files, but, still, I wonder if we can organize
the code a bit to help the compiler somehow.  Perhaps, my machine is too
slow?


Takashi Oe


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: a GCC question

[Timothy A. Seufert]

At 1:01 AM -0500 8/6/00, Takashi Oe wrote:

>By the way, has anyone noticed compiling arch/ppc/mm/init.c takes very
>long time?  On my machine, it takes as long as about two minutes to just
>compile it, while compiling other files are almost instantaneous.  Well,
>it's one of the biggest C files, but, still, I wonder if we can organize
>the code a bit to help the compiler somehow.  Perhaps, my machine is too
>slow?

Something odd is going on.  2 minutes is way too long.  I just did:

rm arch/ppc/mm/init.o
make

on a 2.2.17pre13 tree which was otherwise fully compiled and watched
for make to start compiling init.c.  It took less than a second to
compile.

My computer is very fast, but it can't be that much faster than
yours!  Perhaps you are consistently hitting some VM thrashing at
that point in a full kernel build?

   Tim Seufert

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: a GCC question

[Takashi Oe]

On Sun, 6 Aug 2000, Timothy A. Seufert wrote:

> At 1:01 AM -0500 8/6/00, Takashi Oe wrote:
>
> >By the way, has anyone noticed compiling arch/ppc/mm/init.c takes very
> >long time?  On my machine, it takes as long as about two minutes to just
> >compile it, while compiling other files are almost instantaneous.  Well,
> >it's one of the biggest C files, but, still, I wonder if we can organize
> >the code a bit to help the compiler somehow.  Perhaps, my machine is too
> >slow?
>
> Something odd is going on.  2 minutes is way too long.  I just did:
>
> rm arch/ppc/mm/init.o
> make
>
> on a 2.2.17pre13 tree which was otherwise fully compiled and watched
> for make to start compiling init.c.  It took less than a second to
> compile.

Hmm, that's true.  I mean, on 2.2.x tree, it takes about five seconds even
with my slow machine, which is comparable to your result.  I've noticed
the long compile time for arch/ppc/mm/init.c on 2.4.x tree.

> My computer is very fast, but it can't be that much faster than
> yours!  Perhaps you are consistently hitting some VM thrashing at
> that point in a full kernel build?

I've just tried vmstat during the compilation, but I didn't see
anything...


Takashi Oe


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: a GCC question

[Olaf Hering]

On Sat, Aug 05, Takashi Oe wrote:

>
> On Sat, 5 Aug 2000, David Edelsohn wrote:
>
> > 	I would guess that for smaller numbers of cases, GCC is deciding
> > that it is more efficient to use a series of "if" statements for a
> > decision tree instead of a switch statement.  You can explicitly code it
> > as such, e.g.,
> >
> > if (bi->dispDeviceDepth == 32)
> > ...
> > else if (bi-dispDeviceDepth == 16)
> > ...
> > else if
> > ...
> > else abort();
>
> Ah, that works, though the code doesn't work for some other unknown
> reasons now, probably due to something along the line of things Olaf was
> talking about recently.  Thanks!

Can you send me a diff against the current Benh kernel?
I just inserted these (useless) printk() to a non called function and
now it works so far for me.

find_pci_device_OFnode(unsigned char bus, unsigned char dev_fn)
{
/*      char blubber[60] = ""; */
        struct device_node* np;
        unsigned int *reg;
        int l;
/*      printk("I am just a dummy ...\n");  */
        for (np = allnodes; np != 0; np = np->allnext) {
                int in_macio = 0;
                struct device_node* parent = np->parent;
                while(parent) {
                        char *pname = (char *)get_property(parent, "name", &l);
        printk("I am just another dummy ...\n");
        printk("I am just another dummy ...\n");
                        if (pname && strcmp(pname, "mac-io") == 0) {
                                in_macio = 1;
                                break;
                        }

Bit ugly.


Gruss Olaf

--
 $ man clone

BUGS
       Main feature not yet implemented...

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: a GCC question

[Takashi Oe]

On Sun, 6 Aug 2000, Olaf Hering wrote:

> Can you send me a diff against the current Benh kernel?

You mean his 2.2.x tree?  Or does he have 2.4.x tree somewhere?  I'm only
working with 2.4.x.

> I just inserted these (useless) printk() to a non called function and
> now it works so far for me.
>
> find_pci_device_OFnode(unsigned char bus, unsigned char dev_fn)
> {
> /*      char blubber[60] = ""; */
>         struct device_node* np;
>         unsigned int *reg;
>         int l;
> /*      printk("I am just a dummy ...\n");  */
>         for (np = allnodes; np != 0; np = np->allnext) {
>                 int in_macio = 0;
>                 struct device_node* parent = np->parent;
>                 while(parent) {
>                         char *pname = (char *)get_property(parent, "name", &l);
>         printk("I am just another dummy ...\n");
>         printk("I am just another dummy ...\n");
>                         if (pname && strcmp(pname, "mac-io") == 0) {
>                                 in_macio = 1;
>                                 break;
>                         }
>
> Bit ugly.

I bet you get the same effect (working kernel) by placing the printk()s
elsewhere.  I know something strange (to me) is going on, but I haven't
found anything wrong in objdump of prom.o.


Takashi Oe


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/