lkml.org 
[lkml]   [2009]   [Feb]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 5/6] ide: remove ide_execute_pkt_cmd()
Hello.

Borislav Petkov wrote:

>>>>> or similar instead of wasting stack space?
>>>>>
>>>>>
>>>> It doesn't necessarily waste stack space. Haven't you heard about
>>>> compiler
>>>> putting local vairables into registers?
>>>>
>>>>
>>> Yes, have you heard of unnecessary register spilling?
>>>
>>>
>> No -- only about stack spilling on CPUs "caching" the top of stack in their
>> register file (like SPARC).
>> Linux runs not only on x86 and many RISCs can store several local variables
>> in the dedicated registers -- it's the part of say MIPS ABIs...
>>

Oh, it's really the same on x86, only there are only 3 registers
dedicated to that. RISCs typically have more, however x86_64 wiuth 16
its registers is close to that already. However, you're right -- these
registers need saving at the function entry, so they effectively take
the stack space.

>>>>> It'll also read better in the if() check:
>>>>>
>>>>> if (drv_can_irq_interrupt(drive)) { ...
>>>>>
>>>>>
>>>>>
>>>> It's faster to checj a local variable than to dereference drv several
>>>> times
>>>> -- unless gcc optimizes that away (by creating an implicit local variable
>>>> :-).
>>>>
>
>
> Let's look at an example:
>
> In ide-cd.c:cdrom_newpc_intr() you have the following code snippet:
>
> <ide-cd.c>
> 799 thislen = blk_fs_request(rq) ? len : rq->data_len;
> 800 if (thislen > len)
> 801 thislen = len;
> 802
> 803 ide_debug_log(IDE_DBG_PC, "%s: DRQ: stat: 0x%x, thislen: %d\n",
> 804 __func__, stat, thislen);
> 805
> 806 /* If DRQ is clear, the command has completed. */
> 807 if ((stat & ATA_DRQ) == 0) {
> 808 if (blk_fs_request(rq)) {
> </ide-cd.c>
>
> Now watch the blk_fs_request() thing.
>
> Here's what my gcc¹ spits out:
>

Thsi code is somewhat confused. Also, I was of a better opinion of gcc...

> <ide-cd.s>
> .LVL174:
> .loc 1 799 0
> movl 76(%r12), %ecx # <variable>.cmd_type, prephitmp.1128
> cmpl $1, %ecx #, prephitmp.1128
> movl %ecx, %r8d # prephitmp.1128, prephitmp.1047
> je .L225 #,
>

Now where is that label?

> .LVL175:
> .loc 1 800 0
> movzwl -44(%rbp), %r15d # len, thislen
>

Oh, that AT&T syntax... it took me a while to realize that it's a
movzx insn. :-)

> .LVL176:
> .loc 1 799 0
> movl 280(%r12), %edx # <variable>.data_len, thislen.1129
> .LVL177:
> .loc 1 800 0
> cmpl %r15d, %edx # thislen, thislen.1129
> movl %r15d, %ebx # thislen, thislen.1163
> jg .L145 #,
> .LVL178:
> movl %edx, %r15d # thislen.1129, thislen
>

I wonder why it doesn't generate cmovng ISO jg and mov...

> .LVL179:
> .L145:
> .loc 1 807 0
> testb $8, -64(%rbp) #, stat
> jne .L147 #,
> .LVL180:
> .loc 1 808 0
> cmpl $1, %ecx #, prephitmp.1128
> je .L226 #,
> .loc 1 825 0
> cmpl $2, %ecx #, prephitmp.1128
> .p2align 4,,3
> .p2align 3
> je .L152 #,
> .LBB408:
> </ide-cd.s>
>
> and at label .LVL174 you see the blk_fs_request() check from line
> 799 above. Later, at label .LVL180 you see the next blk_fs_request() check from
> line 808 and this is cached in %ecx so gcc is smart enough to do that. So,
>

Yes, CSE optimization does work...

> actually you get the same thing/or even better with variables in registers
> instead of on stack

I still don't undestand why you assume that such variable will be
alloceted on stack -- gcc has 3 registers available for local variables
(which doesn't have to save across function calls). However, the
register variables have to take stack space indeed as they need to be
saved on funciton entry... though I'm not sure that gcc will necessary
put such variable in one of those 3 registers if it figures out that
there are no function calls going to happen during its life time.

> and the code is more readable. A win-win situation, I'd say :).
>

You haven't presented the code which gets generated when the local
variable is used, so it's impossible to compare.

MBR, Sergei


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2009-02-15 13:27    [from the cache]
©2003-2011 Jasper Spaans