[Qemu-devel] [PATCH 1/2] cutils: remove one unnecessary pointer operation

Wei Yang posted 2 patches 6 years, 8 months ago
Maintainers: "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Juan Quintela <quintela@redhat.com>
[Qemu-devel] [PATCH 1/2] cutils: remove one unnecessary pointer operation
Posted by Wei Yang 6 years, 8 months ago
Since we will not operate on the next address pointed by out, it is not
necessary to do addition on it.

After removing the operation, the function size reduced 16/18 bytes.

Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
---
 util/cutils.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/util/cutils.c b/util/cutils.c
index 9aacc422ca..1933a68da5 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -754,11 +754,11 @@ int uleb128_encode_small(uint8_t *out, uint32_t n)
 {
     g_assert(n <= 0x3fff);
     if (n < 0x80) {
-        *out++ = n;
+        *out = n;
         return 1;
     } else {
         *out++ = (n & 0x7f) | 0x80;
-        *out++ = n >> 7;
+        *out = n >> 7;
         return 2;
     }
 }
@@ -766,7 +766,7 @@ int uleb128_encode_small(uint8_t *out, uint32_t n)
 int uleb128_decode_small(const uint8_t *in, uint32_t *n)
 {
     if (!(*in & 0x80)) {
-        *n = *in++;
+        *n = *in;
         return 1;
     } else {
         *n = *in++ & 0x7f;
@@ -774,7 +774,7 @@ int uleb128_decode_small(const uint8_t *in, uint32_t *n)
         if (*in & 0x80) {
             return -1;
         }
-        *n |= *in++ << 7;
+        *n |= *in << 7;
         return 2;
     }
 }
-- 
2.19.1


Re: [Qemu-devel] [PATCH 1/2] cutils: remove one unnecessary pointer operation
Posted by Dr. David Alan Gilbert 6 years, 8 months ago
* Wei Yang (richardw.yang@linux.intel.com) wrote:
> Since we will not operate on the next address pointed by out, it is not
> necessary to do addition on it.
> 
> After removing the operation, the function size reduced 16/18 bytes.

For me with a -O3 it didn't make any difference - the compiler was
already smart enough to spot it, but it is correct.


Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
> ---
>  util/cutils.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/util/cutils.c b/util/cutils.c
> index 9aacc422ca..1933a68da5 100644
> --- a/util/cutils.c
> +++ b/util/cutils.c
> @@ -754,11 +754,11 @@ int uleb128_encode_small(uint8_t *out, uint32_t n)
>  {
>      g_assert(n <= 0x3fff);
>      if (n < 0x80) {
> -        *out++ = n;
> +        *out = n;
>          return 1;
>      } else {
>          *out++ = (n & 0x7f) | 0x80;
> -        *out++ = n >> 7;
> +        *out = n >> 7;
>          return 2;
>      }
>  }
> @@ -766,7 +766,7 @@ int uleb128_encode_small(uint8_t *out, uint32_t n)
>  int uleb128_decode_small(const uint8_t *in, uint32_t *n)
>  {
>      if (!(*in & 0x80)) {
> -        *n = *in++;
> +        *n = *in;
>          return 1;
>      } else {
>          *n = *in++ & 0x7f;
> @@ -774,7 +774,7 @@ int uleb128_decode_small(const uint8_t *in, uint32_t *n)
>          if (*in & 0x80) {
>              return -1;
>          }
> -        *n |= *in++ << 7;
> +        *n |= *in << 7;
>          return 2;
>      }
>  }
> -- 
> 2.19.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH 1/2] cutils: remove one unnecessary pointer operation
Posted by Wei Yang 6 years, 8 months ago
On Tue, Jun 11, 2019 at 06:49:54PM +0100, Dr. David Alan Gilbert wrote:
>* Wei Yang (richardw.yang@linux.intel.com) wrote:
>> Since we will not operate on the next address pointed by out, it is not
>> necessary to do addition on it.
>> 
>> After removing the operation, the function size reduced 16/18 bytes.
>
>For me with a -O3 it didn't make any difference - the compiler was
>already smart enough to spot it, but it is correct.
>

Ah, you are right.

>
>Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>
>> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
>> ---
>>  util/cutils.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>> 
>> diff --git a/util/cutils.c b/util/cutils.c
>> index 9aacc422ca..1933a68da5 100644
>> --- a/util/cutils.c
>> +++ b/util/cutils.c
>> @@ -754,11 +754,11 @@ int uleb128_encode_small(uint8_t *out, uint32_t n)
>>  {
>>      g_assert(n <= 0x3fff);
>>      if (n < 0x80) {
>> -        *out++ = n;
>> +        *out = n;
>>          return 1;
>>      } else {
>>          *out++ = (n & 0x7f) | 0x80;
>> -        *out++ = n >> 7;
>> +        *out = n >> 7;
>>          return 2;
>>      }
>>  }
>> @@ -766,7 +766,7 @@ int uleb128_encode_small(uint8_t *out, uint32_t n)
>>  int uleb128_decode_small(const uint8_t *in, uint32_t *n)
>>  {
>>      if (!(*in & 0x80)) {
>> -        *n = *in++;
>> +        *n = *in;
>>          return 1;
>>      } else {
>>          *n = *in++ & 0x7f;
>> @@ -774,7 +774,7 @@ int uleb128_decode_small(const uint8_t *in, uint32_t *n)
>>          if (*in & 0x80) {
>>              return -1;
>>          }
>> -        *n |= *in++ << 7;
>> +        *n |= *in << 7;
>>          return 2;
>>      }
>>  }
>> -- 
>> 2.19.1
>> 
>--
>Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

-- 
Wei Yang
Help you, Help me

Re: [Qemu-devel] [PATCH 1/2] cutils: remove one unnecessary pointer operation
Posted by Juan Quintela 6 years, 8 months ago
Wei Yang <richardw.yang@linux.intel.com> wrote:
> Since we will not operate on the next address pointed by out, it is not
> necessary to do addition on it.
>
> After removing the operation, the function size reduced 16/18 bytes.
>
> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>

Included.  I agree with David that the compiler should detect that, but
also agreed that it shouldn't be there in the first place.