lkml.org 
[lkml]   [2016]   [Jan]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] fs: dcache: Use bool return value instead of int
On 1/12/16 06:51, Al Viro wrote:
> On Tue, Jan 12, 2016 at 05:30:45AM +0800, chengang@emindsoft.com.cn wrote:
>> From: Chen Gang <gang.chen.5i5j@gmail.com>
>>
>> Use bool type for all functions which return boolean value. It will not
>> only let code clearer, but also sometimes let gcc produce better code.
>
> What's the point of this chunk?
>

I'll explain it below this mail, please check.

>> static enum d_walk_ret check_mount(void *data, struct dentry *dentry)
>> {
>> - int *ret = data;
>> + bool *ret = data;
>> if (d_mountpoint(dentry)) {
>> - *ret = 1;
>> + *ret = true;
>> return D_WALK_QUIT;
>> }
>> return D_WALK_CONTINUE;
>
> You are replacing a 1-word store with 1-byte store; if anything, that's more
> likely to yield _worse_ code, not better one.
>

For me, it really generates a little better code:

- Both 1-word store and 1-byte store are 1 instruction, normally, they
have the same execution speed (although it is not quite precise).

- But 1-byte store instruction has short length under CISC archs, which
can generate a little better code globally.

- For most of archs, 1-word store can process bytes nonalignment cases,
for check_mount() individually, the parameter data may be not word
alignment, which may cause the 1-word store slower than 1-byte store.

The related objdump is below:

origin:

00000000 <check_mount>:
0: 8b 12 mov (%edx),%edx
2: 81 e2 00 00 01 00 and $0x10000,%edx
8: 74 16 je 20 <check_mount+0x20>
a: c7 00 01 00 00 00 movl $0x1,(%eax)
10: b8 01 00 00 00 mov $0x1,%eax
15: c3 ret
16: 8d 76 00 lea 0x0(%esi),%esi
19: 8d bc 27 00 00 00 00 lea 0x0(%edi,%eiz,1),%edi
20: 31 c0 xor %eax,%eax
22: c3 ret
23: 8d b6 00 00 00 00 lea 0x0(%esi),%esi
29: 8d bc 27 00 00 00 00 lea 0x0(%edi,%eiz,1),%edi

new:

00000000 <check_mount>:
0: 8b 12 mov (%edx),%edx
2: 81 e2 00 00 01 00 and $0x10000,%edx
8: 74 0e je 18 <check_mount+0x18>
a: c6 00 01 movb $0x1,(%eax)
d: b8 01 00 00 00 mov $0x1,%eax
12: c3 ret
13: 90 nop
14: 8d 74 26 00 lea 0x0(%esi,%eiz,1),%esi
18: 31 c0 xor %eax,%eax
1a: c3 ret
1b: 90 nop
1c: 8d 74 26 00 lea 0x0(%esi,%eiz,1),%esi

[root@localhost fs]# gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/local/libexec/gcc/x86_64-pc-linux-gnu/6.0.0/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: ../gcc-ana/configure
Thread model: posix
gcc version 6.0.0 20151121 (experimental) (GCC)



>> -static inline int d_unhashed(const struct dentry *dentry)
>> +static inline bool d_unhashed(const struct dentry *dentry)
>> {
>> return hlist_bl_unhashed(&dentry->d_hash);
>> }
>>
>> -static inline int d_unlinked(const struct dentry *dentry)
>> +static inline bool d_unlinked(const struct dentry *dentry)
>> {
>> return d_unhashed(dentry) && !IS_ROOT(dentry);
>> }
>
>> -static inline int simple_positive(struct dentry *dentry)
>> +static inline bool simple_positive(struct dentry *dentry)
>> {
>> return d_really_is_positive(dentry) && !d_unhashed(dentry);
>> }
>
> And these three are harmless, but completely pointless...
>

For performance, please check the original reply above this mail.

For me, bool can make the code a little simpler and clearer:

- int can express more things: error code, handler, count ... So if we
really only use one boolean variable, bool type is more clearer (it
is only for boolean).

- The old ANSI C compiler may not support bool type, so we have to use
int type instead of. But if one header/source file has already used
bool type in some part, the whole file need use bool type too.


Thanks.
--
Chen Gang (陈刚)

Open, share, and attitude like air, water, and life which God blessed

\
 
 \ /
  Last update: 2016-01-12 23:21    [W:1.147 / U:0.000 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site