On 1/12/16 06:51, Al Viro wrote:
> On Tue, Jan 12, 2016 at 05:30:45AM +0800, chengang@xxxxxxxxxxxxxxxx wrote:
>> From: Chen Gang <gang.chen.5i5j@xxxxxxxxx>
>>
>> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
