[PATCH] drivers/android: remove redundant ret variable

[PATCH] drivers/android: remove redundant ret variable

[cgel.zte]

From: Minghao Chi <chi.minghao@zte.com.cn>

Return value from list_lru_count() directly instead
of taking this in another redundant variable.

Reported-by: Zeal Robot <zealci@zte.com.cn>
Signed-off-by: Minghao Chi <chi.minghao@zte.com.cn>
Signed-off-by: CGEL ZTE <cgel.zte@gmail.com>
---
 drivers/android/binder_alloc.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index 340515f54498..4f221d1839f4 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -1049,18 +1049,14 @@ enum lru_status binder_alloc_free_page(struct list_head *item,
 static unsigned long
 binder_shrink_count(struct shrinker *shrink, struct shrink_control *sc)
 {
-	unsigned long ret = list_lru_count(&binder_alloc_lru);
-	return ret;
+	return list_lru_count(&binder_alloc_lru);
 }
 
 static unsigned long
 binder_shrink_scan(struct shrinker *shrink, struct shrink_control *sc)
 {
-	unsigned long ret;
-
-	ret = list_lru_walk(&binder_alloc_lru, binder_alloc_free_page,
+	return list_lru_walk(&binder_alloc_lru, binder_alloc_free_page,
 			    NULL, sc->nr_to_scan);
-	return ret;
 }
 
 static struct shrinker binder_shrinker = {
-- 
2.25.1

Re: [PATCH] drivers/android: remove redundant ret variable

[Greg KH]

On Tue, Jan 04, 2022 at 11:35:00AM +0000, cgel.zte@gmail.com wrote:
> From: Minghao Chi <chi.minghao@zte.com.cn>
> 
> Return value from list_lru_count() directly instead
> of taking this in another redundant variable.
> 
> Reported-by: Zeal Robot <zealci@zte.com.cn>
> Signed-off-by: Minghao Chi <chi.minghao@zte.com.cn>
> Signed-off-by: CGEL ZTE <cgel.zte@gmail.com>
> ---
>  drivers/android/binder_alloc.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
> index 340515f54498..4f221d1839f4 100644
> --- a/drivers/android/binder_alloc.c
> +++ b/drivers/android/binder_alloc.c
> @@ -1049,18 +1049,14 @@ enum lru_status binder_alloc_free_page(struct list_head *item,
>  static unsigned long
>  binder_shrink_count(struct shrinker *shrink, struct shrink_control *sc)
>  {
> -	unsigned long ret = list_lru_count(&binder_alloc_lru);
> -	return ret;
> +	return list_lru_count(&binder_alloc_lru);
>  }
>  
>  static unsigned long
>  binder_shrink_scan(struct shrinker *shrink, struct shrink_control *sc)
>  {
> -	unsigned long ret;
> -
> -	ret = list_lru_walk(&binder_alloc_lru, binder_alloc_free_page,
> +	return list_lru_walk(&binder_alloc_lru, binder_alloc_free_page,
>  			    NULL, sc->nr_to_scan);
> -	return ret;

Is the compiler output any different?

If not, why make this change?

What did you use to test this change is equivalent?

thanks,

greg k-h

'Re: [PATCH] drivers/android: remove redundant ret variable'

[cgel.zte]

I found that there are indeed some optimizations
after removing the redundancy.

For example:

int foo(int a, int b)
{
        int ret;
        ret = a>b ? a: b;
        return ret;
}
int main()
{
        int val;
        val = foo(7,2);
        return 0;
}

and

int foo(int a, int b)
{
        return a>b ? a: b;
}
int main()
{
        int val;
        val = foo(7,2);
        return 0;
}

The corresponding disassembly code is as follows：

    1129:       f3 0f 1e fa             endbr64 
    112d:       55                      push   %rbp
    112e:       48 89 e5                mov    %rsp,%rbp
    1131:       89 7d fc                mov    %edi,-0x4(%rbp)
    1134:       89 75 f8                mov    %esi,-0x8(%rbp)
    1137:       8b 45 fc                mov    -0x4(%rbp),%eax
    113a:       39 45 f8                cmp    %eax,-0x8(%rbp)
    113d:       0f 4d 45 f8             cmovge -0x8(%rbp),%eax
    1141:       5d                      pop    %rbp
    1142:       c3                      retq

and

    1129:       f3 0f 1e fa             endbr64 
    112d:       55                      push   %rbp
    112e:       48 89 e5                mov    %rsp,%rbp
    1131:       89 7d ec                mov    %edi,-0x14(%rbp)
    1134:       89 75 e8                mov    %esi,-0x18(%rbp)
    1137:       8b 45 ec                mov    -0x14(%rbp),%eax
    113a:       39 45 e8                cmp    %eax,-0x18(%rbp)
    113d:       0f 4d 45 e8             cmovge -0x18(%rbp),%eax
    1141:       89 45 fc                mov    %eax,-0x4(%rbp)
    1144:       8b 45 fc                mov    -0x4(%rbp),%eax
    1147:       5d                      pop    %rbp
    1148:       c3                      retq

After removing the redundancy, the compiler does
have some optimizations

Re: 'Re: [PATCH] drivers/android: remove redundant ret variable'

[Greg KH]

On Wed, Jan 05, 2022 at 09:19:42AM +0000, cgel.zte@gmail.com wrote:
> I found that there are indeed some optimizations
> after removing the redundancy.

You stripped off all context, making this impossible to remember what
you are responding to :(

> 
> For example:
> 
> int foo(int a, int b)
> {
>         int ret;
>         ret = a>b ? a: b;
>         return ret;
> }
> int main()
> {
>         int val;
>         val = foo(7,2);
>         return 0;
> }
> 
> and
> 
> int foo(int a, int b)
> {
>         return a>b ? a: b;
> }
> int main()
> {
>         int val;
>         val = foo(7,2);
>         return 0;
> }
> 
> The corresponding disassembly code is as follows：
> 
>     1129:       f3 0f 1e fa             endbr64 
>     112d:       55                      push   %rbp
>     112e:       48 89 e5                mov    %rsp,%rbp
>     1131:       89 7d fc                mov    %edi,-0x4(%rbp)
>     1134:       89 75 f8                mov    %esi,-0x8(%rbp)
>     1137:       8b 45 fc                mov    -0x4(%rbp),%eax
>     113a:       39 45 f8                cmp    %eax,-0x8(%rbp)
>     113d:       0f 4d 45 f8             cmovge -0x8(%rbp),%eax
>     1141:       5d                      pop    %rbp
>     1142:       c3                      retq
> 
> and
> 
>     1129:       f3 0f 1e fa             endbr64 
>     112d:       55                      push   %rbp
>     112e:       48 89 e5                mov    %rsp,%rbp
>     1131:       89 7d ec                mov    %edi,-0x14(%rbp)
>     1134:       89 75 e8                mov    %esi,-0x18(%rbp)
>     1137:       8b 45 ec                mov    -0x14(%rbp),%eax
>     113a:       39 45 e8                cmp    %eax,-0x18(%rbp)
>     113d:       0f 4d 45 e8             cmovge -0x18(%rbp),%eax
>     1141:       89 45 fc                mov    %eax,-0x4(%rbp)
>     1144:       8b 45 fc                mov    -0x4(%rbp),%eax
>     1147:       5d                      pop    %rbp
>     1148:       c3                      retq
> 
> After removing the redundancy, the compiler does
> have some optimizations

As I said on your other email, look at the kernel built files please,
they should be much different from your tiny example above.

thanks,

greg k-h