On Tue, Jul 10, 2012 at 10:12:17AM -0700, Tony Luck wrote:
> On Tue, Jul 10, 2012 at 8:44 AM, Borislav Petkov <b...@amd64.org> wrote:
> > Acked-by: Borislav Petkov <borislav.pet...@amd.com>
>
> Thanks for the Ack.
>
> + doit = !!PageDirty(ppage) || !!(flags & MF_MUST_KILL);
>
> Thinking about this some more, the "!!" are redundant
> and are an impediment to readability. We started with
> !!PageDirty(ppage) when we were passing an argument
> directly to kill_procs() and wanted to make sure we had a
> nice boolean value. But the result of the "||" is boolean,
> so we don't need to double negate.
>
> I'm going to change it to:
> doit = PageDirty(ppage) || (flags & MF_MUST_KILL);
Ok, so out of curiosity I took a look at compiler output and both
versions are identical:
> + doit = !!PageDirty(ppage) || !!(flags & MF_MUST_KILL);
.loc 1 974 0
movl $1, %edx #, doit
testb $16, %al #, D.28886
jne .L196 #,
movl -196(%rbp), %ecx # %sfp,
xorl %edx, %edx # doit
testl %ecx, %ecx #
setne %dl #, doit
> doit = PageDirty(ppage) || (flags & MF_MUST_KILL);
.loc 1 974 0
movl $1, %edx #, doit
testb $16, %al #, D.28886
jne .L196 #,
movl -196(%rbp), %ecx # %sfp,
xorl %edx, %edx # doit
testl %ecx, %ecx #
setne %dl #, doit
In both cases you get your standard shortcutting OR logic:
.loc 1 974 0
movl $1, %edx #, doit <--- preset doit
testb $16, %al #, D.28886 <--- PG_dirty
jne .L196 #, <--- ZF=0, shortcut out to
kill_procs
movl -196(%rbp), %ecx # %sfp, <--- get value of (flags &
MF_MUST_KILL) which we computed earlier in the function and saved on stack
xorl %edx, %edx # doit <--- clear doit
testl %ecx, %ecx # <--- check above (flags &
MF_MUST_KILL) is not 0
setne %dl #, doit <--- prep doit for kill_procs
so the compiler is pretty smart already.
You could go another step further by declaring
bool doit;
and changing kill_procs() argument to bool too so that the booleanness
is really explicit.
This saves you the xor:
.loc 1 975 0
movl $1, %edx #, prephitmp.124
testb $16, %al #, D.28891
jne .L196 #,
movl -196(%rbp), %ecx # %sfp,
testl %ecx, %ecx #
setne %dl #, prephitmp.124
because we're using explicitly bools which are u8s, apparently, in this
case and we're reusing the 1 we wrote into edx at the beginning of the
block.
Oh well, enough fun.
--
Regards/Gruss,
Boris.
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
