Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1471830 > unrolled thread

Re: [PATCH] arch: all: include: asm: bitops: Use bool instead of int for all bit test functions

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] arch: all: include: asm: bitops: Use bool instead of int for all bit test functions Arnd Bergmann <arnd@arndb.de> - 2016-08-29 15:20 +0200
    Re: [PATCH] arch: all: include: asm: bitops: Use bool instead of int  for all bit test functions Peter Zijlstra <peterz@infradead.org> - 2016-08-29 15:50 +0200
      Re: [PATCH] arch: all: include: asm: bitops: Use bool instead of int  for all bit test functions Geert Uytterhoeven <geert@linux-m68k.org> - 2016-08-29 16:00 +0200
    Re: [PATCH] arch: all: include: asm: bitops: Use bool instead of int  for all bit test functions Vineet Gupta <vgupta@synopsys.com> - 2016-08-29 18:50 +0200
      Re: [PATCH] arch: all: include: asm: bitops: Use bool instead of  int for all bit test functions Chen Gang <chengang@emindsoft.com.cn> - 2016-08-29 23:50 +0200
        cmsg newgroup alt.sex.fetish.bool (was Re: [PATCH] arch: all:  include: asm: bitops: Use bool instead of int for all bit test functions) Al Viro <viro@ZenIV.linux.org.uk> - 2016-09-01 23:10 +0200
          Re: cmsg newgroup alt.sex.fetish.bool (was Re: [PATCH] arch: all:  include: asm: bitops: Use bool instead of int for all bit test functions) Chen Gang <chengang@emindsoft.com.cn> - 2016-09-03 01:40 +0200
            Re: cmsg newgroup alt.sex.fetish.bool (was Re: [PATCH] arch: all:  include: asm: bitops: Use bool instead of int for all bit test functions) Vineet Gupta <Vineet.Gupta1@synopsys.com> - 2016-09-03 02:10 +0200
              Re: cmsg newgroup alt.sex.fetish.bool (was Re: [PATCH] arch: all:  include: asm: bitops: Use bool instead of int for all bit test functions) Chen Gang <chengang@emindsoft.com.cn> - 2016-09-04 00:40 +0200
                Re: cmsg newgroup alt.sex.fetish.bool (was Re: [PATCH] arch: all:  include: asm: bitops: Use bool instead of int for all bit test functions) Al Viro <viro@ZenIV.linux.org.uk> - 2016-09-04 07:10 +0200
                  Re: cmsg newgroup alt.sex.fetish.bool (was Re: [PATCH] arch: all:  include: asm: bitops: Use bool instead of int for all bit test functions) Chen Gang <chengang@emindsoft.com.cn> - 2016-09-07 18:00 +0200
    Re: [PATCH] arch: all: include: asm: bitops: Use bool instead of  int for all bit test functions Chen Gang <chengang@emindsoft.com.cn> - 2016-08-30 00:30 +0200

#1471830 — Re: [PATCH] arch: all: include: asm: bitops: Use bool instead of int for all bit test functions

On Sunday 28 August 2016, chengang@emindsoft.com.cn wrote:
> From: Chen Gang <chengang@emindsoft.com.cn>
> 
> Also use the same changing to asm-generic, and also use bool variable
> instead of int variable for mips, mn10300, parisc and tile related
> functions, and also avoid checkpatch.pl to report ERROR.
> 
> Originally, except powerpc and xtensa, all another architectures intend
> to return 0 or 1. After this patch, also let powerpc and xtensa return 0
> or 1.
> 
> The patch passes cross building for mips and parisc with default config.
> All related contents are found by "grep test_bit, grep test_and" under
> arch sub-directory.
> 
> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>

This seems like a good idea overall, and I'm fine with the asm-generic
contents. If there is consensus on changing this, we probably also want
to do some other steps:

- Change the Documentation/atomic_ops.txt file accordingly
- split up the series per architecture (I don't think there are any
  interdependencies)
- For the architectures on which the definition changes (at least
  x86 and ARM), do some more sanity checks and see if there are
  noticeable changes in object code, and if so whether it looks
  better or worse (I'm guessing it will be better if anything)
- See which architectures can still get converted to using the
  asm-generic headers instead of providing their own, I think at
  least for the nonatomic ones, there are a couple.

	Arnd

[toc] | [next] | [standalone]

#1471857 — Re: [PATCH] arch: all: include: asm: bitops: Use bool instead of int for all bit test functions

On Mon, Aug 29, 2016 at 03:03:41PM +0200, Arnd Bergmann wrote:
> - Change the Documentation/atomic_ops.txt file accordingly

Not sure that really matters; that document is so out of date its nearly
useless :-(

Rewriting it is somewhere on the TODO list...

[toc] | [prev] | [next] | [standalone]

#1471864 — Re: [PATCH] arch: all: include: asm: bitops: Use bool instead of int for all bit test functions

On Mon, Aug 29, 2016 at 3:46 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Mon, Aug 29, 2016 at 03:03:41PM +0200, Arnd Bergmann wrote:
>> - Change the Documentation/atomic_ops.txt file accordingly
>
> Not sure that really matters; that document is so out of date its nearly
> useless :-(
>
> Rewriting it is somewhere on the TODO list...

Resphinxing?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[toc] | [prev] | [next] | [standalone]

#1471981 — Re: [PATCH] arch: all: include: asm: bitops: Use bool instead of int for all bit test functions

On 08/29/2016 06:03 AM, Arnd Bergmann wrote:
> On Sunday 28 August 2016, chengang@emindsoft.com.cn wrote:
>> From: Chen Gang <chengang@emindsoft.com.cn>
>>
>> Also use the same changing to asm-generic, and also use bool variable
>> instead of int variable for mips, mn10300, parisc and tile related
>> functions, and also avoid checkpatch.pl to report ERROR.
>>
>> Originally, except powerpc and xtensa, all another architectures intend
>> to return 0 or 1. After this patch, also let powerpc and xtensa return 0
>> or 1.
>>
>> The patch passes cross building for mips and parisc with default config.
>> All related contents are found by "grep test_bit, grep test_and" under
>> arch sub-directory.
>>
>> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
> 
> This seems like a good idea overall, and I'm fine with the asm-generic
> contents. If there is consensus on changing this, we probably also want
> to do some other steps:
> 
> - Change the Documentation/atomic_ops.txt file accordingly
> - split up the series per architecture (I don't think there are any
>   interdependencies)
> - For the architectures on which the definition changes (at least
>   x86 and ARM), do some more sanity checks and see if there are
>   noticeable changes in object code, and if so whether it looks
>   better or worse (I'm guessing it will be better if anything)

For ARC atleast, it will be slightly worse. As bool is promoted to int in various
expressions, gcc generates an additional EXTB (extend byte) instruction.

> - See which architectures can still get converted to using the
>   asm-generic headers instead of providing their own, I think at
>   least for the nonatomic ones, there are a couple.
> 
> 	Arnd
> 

[toc] | [prev] | [next] | [standalone]

#1472130 — Re: [PATCH] arch: all: include: asm: bitops: Use bool instead of int for all bit test functions

On 8/30/16 00:48, Vineet Gupta wrote:
> On 08/29/2016 06:03 AM, Arnd Bergmann wrote:
>> On Sunday 28 August 2016, chengang@emindsoft.com.cn wrote:
>>> From: Chen Gang <chengang@emindsoft.com.cn>
>>>
>>> Also use the same changing to asm-generic, and also use bool variable
>>> instead of int variable for mips, mn10300, parisc and tile related
>>> functions, and also avoid checkpatch.pl to report ERROR.
>>>
>>> Originally, except powerpc and xtensa, all another architectures intend
>>> to return 0 or 1. After this patch, also let powerpc and xtensa return 0
>>> or 1.
>>>
>>> The patch passes cross building for mips and parisc with default config.
>>> All related contents are found by "grep test_bit, grep test_and" under
>>> arch sub-directory.
>>>
>>> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
>>
>> This seems like a good idea overall, and I'm fine with the asm-generic
>> contents. If there is consensus on changing this, we probably also want
>> to do some other steps:
>>
>> - Change the Documentation/atomic_ops.txt file accordingly
>> - split up the series per architecture (I don't think there are any
>>   interdependencies)
>> - For the architectures on which the definition changes (at least
>>   x86 and ARM), do some more sanity checks and see if there are
>>   noticeable changes in object code, and if so whether it looks
>>   better or worse (I'm guessing it will be better if anything)
> 
> For ARC atleast, it will be slightly worse. As bool is promoted to int in various
> expressions, gcc generates an additional EXTB (extend byte) instruction.
> 

Could you provide the related proof?

Or shall I try to analyze about it and get proof?

Thanks.
-- 
Chen Gang (陈刚)

Managing Natural Environments is the Duty of Human Beings.

[toc] | [prev] | [next] | [standalone]

#1474590 — cmsg newgroup alt.sex.fetish.bool (was Re: [PATCH] arch: all: include: asm: bitops: Use bool instead of int for all bit test functions)

On Tue, Aug 30, 2016 at 05:49:05AM +0800, Chen Gang wrote:

> Could you provide the related proof?
> 
> Or shall I try to analyze about it and get proof?

Can you show a proof that it actually improves anything?  He who proposes
a patch gets to defend it, not the other way round...

Al, bloody annoyed

[toc] | [prev] | [next] | [standalone]

#1475502 — Re: cmsg newgroup alt.sex.fetish.bool (was Re: [PATCH] arch: all: include: asm: bitops: Use bool instead of int for all bit test functions)

On 9/2/16 04:43, Al Viro wrote:
> On Tue, Aug 30, 2016 at 05:49:05AM +0800, Chen Gang wrote:
> 
>> Could you provide the related proof?
>>
>> Or shall I try to analyze about it and get proof?
> 
> Can you show a proof that it actually improves anything?  He who proposes
> a patch gets to defend it, not the other way round...
> 
> Al, bloody annoyed
> 

OK, what you said sounds reasonable to me.

It makes the code more readable since they are really pure Boolean
functions, and let the functions are precisely same in all archs. But
really, I shall try to prove that it has no negative effect.

e.g. for arc arch. now, I have built the arc raw compiler to build arc
kernel, but excuse me, I plan to finish proof next week, because during
these days, I have to work, buy house, and focus on my father's health.


Thanks.
-- 
Chen Gang (陈刚)

Managing Natural Environments is the Duty of Human Beings.

[toc] | [prev] | [next] | [standalone]

#1475505 — Re: cmsg newgroup alt.sex.fetish.bool (was Re: [PATCH] arch: all: include: asm: bitops: Use bool instead of int for all bit test functions)

On 09/02/2016 04:33 PM, Chen Gang wrote:
> On 9/2/16 04:43, Al Viro wrote:
>> > On Tue, Aug 30, 2016 at 05:49:05AM +0800, Chen Gang wrote:
>> > 
>>> >> Could you provide the related proof?
>>> >>
>>> >> Or shall I try to analyze about it and get proof?
>> > 
>> > Can you show a proof that it actually improves anything?  He who proposes
>> > a patch gets to defend it, not the other way round...
>> > 
>> > Al, bloody annoyed
>> > 
> OK, what you said sounds reasonable to me.
>
> It makes the code more readable since they are really pure Boolean
> functions, and let the functions are precisely same in all archs. But
> really, I shall try to prove that it has no negative effect.
>
> e.g. for arc arch. now, I have built the arc raw compiler to build arc
> kernel, but excuse me, I plan to finish proof next week, because during
> these days, I have to work, buy house, and focus on my father's health.

Since you seem to be have so much stuff to do I decided to help. I did a quick
compile of kernel with and w/o your changes

bloat-o-meter vmlinux-v4.8rc4-baseline vmlinux-v4.8rc4-bool-in-atomics
add/remove: 0/0 grow/shrink: 1/0 up/down: 6/0 (6)
function                                     old     new   delta
vermagic                                      49      55      +6
Total: Before=5967447, After=5967453, chg 0.000000%

I'm mildly surprised that there is no difference so yeah this change is fine as
far as I'm concerned.

-Vineet

[toc] | [prev] | [next] | [standalone]

#1475801 — Re: cmsg newgroup alt.sex.fetish.bool (was Re: [PATCH] arch: all: include: asm: bitops: Use bool instead of int for all bit test functions)

On 9/3/16 08:07, Vineet Gupta wrote:
> On 09/02/2016 04:33 PM, Chen Gang wrote:
>> On 9/2/16 04:43, Al Viro wrote:
>>>>
>>>> Can you show a proof that it actually improves anything?  He who proposes
>>>> a patch gets to defend it, not the other way round...
>>>>
>>>> Al, bloody annoyed
>>>>
>> OK, what you said sounds reasonable to me.
>>
>> It makes the code more readable since they are really pure Boolean
>> functions, and let the functions are precisely same in all archs. But
>> really, I shall try to prove that it has no negative effect.
>>
>> e.g. for arc arch. now, I have built the arc raw compiler to build arc
>> kernel, but excuse me, I plan to finish proof next week, because during
>> these days, I have to work, buy house, and focus on my father's health.
> 
> Since you seem to be have so much stuff to do I decided to help. I did a quick
> compile of kernel with and w/o your changes
> 
> bloat-o-meter vmlinux-v4.8rc4-baseline vmlinux-v4.8rc4-bool-in-atomics
> add/remove: 0/0 grow/shrink: 1/0 up/down: 6/0 (6)
> function                                     old     new   delta
> vermagic                                      49      55      +6
> Total: Before=5967447, After=5967453, chg 0.000000%
> 
> I'm mildly surprised that there is no difference so yeah this change is fine as
> far as I'm concerned.
> 

Thank you for your reply :-)


And for all: shall I provide the proof for another archs?

For me, Boolean gives additional chance to compiler to improve the code.
If the compiler can not improve the code, it can treat it as int simply.
So theoretically, at least, Boolean should not be worse than int.


Thanks.
-- 
Chen Gang (陈刚)

Managing Natural Environments is the Duty of Human Beings.

[toc] | [prev] | [next] | [standalone]

#1475851 — Re: cmsg newgroup alt.sex.fetish.bool (was Re: [PATCH] arch: all: include: asm: bitops: Use bool instead of int for all bit test functions)

On Sun, Sep 04, 2016 at 06:36:56AM +0800, Chen Gang wrote:

> And for all: shall I provide the proof for another archs?
> 
> For me, Boolean gives additional chance to compiler to improve the code.

Whereas for compiler it gives nothing.  Not in those cases.

> If the compiler can not improve the code, it can treat it as int simply.
> So theoretically, at least, Boolean should not be worse than int.

Except for pointless code churn and pandering to irrational beliefs, that is...
Please, RTFISO9899 and learn the semantics of _Bool; it's not that complicated.
Start with 6.2.5[2,6] and 6.3.1.2, then look through 6.8.4 and 6.8.5 to
figure out the semantics of conditions in if/while/for.  Note also 6.5.8,
6.5.9, 6.5.13 and 6.5.14 and observe that type of (x > 5 && y < 1) is *NOT* 
_Bool; it's int.

If you can show any improvement or loss in code generation in this case
(static inline int converted to static inline bool), I would really like to
see the details.  As in .config/file/function/gcc version/target architecture.
Optimizer bugs happens, but they should be reported when found, and I would
expect _Bool handling to be _less_ exercised than that of normal logical
expressions, so loss is probably more likely.  And yes, it also should be
reported.

[toc] | [prev] | [next] | [standalone]

#1478452 — Re: cmsg newgroup alt.sex.fetish.bool (was Re: [PATCH] arch: all: include: asm: bitops: Use bool instead of int for all bit test functions)

On 9/4/16 09:01, Al Viro wrote:
> On Sun, Sep 04, 2016 at 06:36:56AM +0800, Chen Gang wrote:
> 
>> And for all: shall I provide the proof for another archs?
>>
>> For me, Boolean gives additional chance to compiler to improve the code.
> 
> Whereas for compiler it gives nothing.  Not in those cases.
> 
>> If the compiler can not improve the code, it can treat it as int simply.
>> So theoretically, at least, Boolean should not be worse than int.
> 
> Except for pointless code churn and pandering to irrational beliefs, that is...
> Please, RTFISO9899 and learn the semantics of _Bool; it's not that complicated.
> Start with 6.2.5[2,6] and 6.3.1.2, then look through 6.8.4 and 6.8.5 to
> figure out the semantics of conditions in if/while/for.  Note also 6.5.8,
> 6.5.9, 6.5.13 and 6.5.14 and observe that type of (x > 5 && y < 1) is *NOT* 
> _Bool; it's int.
> 
> If you can show any improvement or loss in code generation in this case
> (static inline int converted to static inline bool), I would really like to
> see the details.  As in .config/file/function/gcc version/target architecture.
> Optimizer bugs happens, but they should be reported when found, and I would
> expect _Bool handling to be _less_ exercised than that of normal logical
> expressions, so loss is probably more likely.  And yes, it also should be
> reported.
> 

Sorry for replying late, and excuse me, I did not read the details more.
During these days I have no enough time on it (working, buying house,
and catching a cold, but lucky enough that my father's health is OK).

I shall try to read the details and analyze it within next weekend (I
guess I can not finish within this week end, sorry again for I really
have no time during these days).

But all together, for me, I guess our discussion can not 'prevent' that
bool return value instead of int return value for pure bool function in
our kernel. :-)


Thanks.
-- 
Chen Gang (陈刚)

Managing Natural Environments is the Duty of Human Beings.

[toc] | [prev] | [next] | [standalone]

#1472138 — Re: [PATCH] arch: all: include: asm: bitops: Use bool instead of int for all bit test functions

On 8/29/16 21:03, Arnd Bergmann wrote:
> On Sunday 28 August 2016, chengang@emindsoft.com.cn wrote:
>> From: Chen Gang <chengang@emindsoft.com.cn>
>>
>> Also use the same changing to asm-generic, and also use bool variable
>> instead of int variable for mips, mn10300, parisc and tile related
>> functions, and also avoid checkpatch.pl to report ERROR.
>>
>> Originally, except powerpc and xtensa, all another architectures intend
>> to return 0 or 1. After this patch, also let powerpc and xtensa return 0
>> or 1.
>>
>> The patch passes cross building for mips and parisc with default config.
>> All related contents are found by "grep test_bit, grep test_and" under
>> arch sub-directory.
>>
>> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
> 
> This seems like a good idea overall, and I'm fine with the asm-generic
> contents. If there is consensus on changing this, we probably also want
> to do some other steps:
> 
> - Change the Documentation/atomic_ops.txt file accordingly
> - split up the series per architecture (I don't think there are any
>   interdependencies)
> - For the architectures on which the definition changes (at least
>   x86 and ARM), do some more sanity checks and see if there are
>   noticeable changes in object code, and if so whether it looks
>   better or worse (I'm guessing it will be better if anything)
> - See which architectures can still get converted to using the
>   asm-generic headers instead of providing their own, I think at
>   least for the nonatomic ones, there are a couple.
> 

Thank you for your ideas, suggestions, and completions.

And I guess, at least for arc, I or another related members will try to
check the object code.


Thanks.
-- 
Chen Gang (陈刚)

Managing Natural Environments is the Duty of Human Beings.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web