[Qemu-devel] [PATCH 0/2] Provide sextract32() and sextract64()

[Qemu-devel] [PATCH 0/2] Provide sextract32() and sextract64()

[Peter Maydell]

A common operation in instruction decoding is to take a field
from an instruction that represents a signed integer in some
arbitrary number of bits, and sign extend it into a C signed
integer type for manipulation. Provide new functions sextract32()
and sextract64() which perform this operation; they are like
the existing extract32() and extract64() except that the field
is sign-extended into the returned result.

I agree with Markus that sign-extending versions of our
extract functions are more flexible than the simple sext()
I proposed in an earlier RFC, hence the switch in this patch.
The other change is patch 2, which provides some simple test
cases. (Will be handy if we ever decide to revamp to avoid
the dependency on shift-left-signed behaviour.)

Peter Maydell (2):
  bitops: Provide sextract32() and sextract64()
  tests: Add test-bitops.c with some sextract tests

 include/qemu/bitops.h |   50 +++++++++++++++++++++++++++++++++
 tests/Makefile        |    2 ++
 tests/test-bitops.c   |   75 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 127 insertions(+)
 create mode 100644 tests/test-bitops.c

-- 
1.7.9.5

[Qemu-devel] [PATCH 1/2] bitops: Provide sextract32() and sextract64()

[Peter Maydell]

A common operation in instruction decoding is to take a field
from an instruction that represents a signed integer in some
arbitrary number of bits, and sign extend it into a C signed
integer type for manipulation. Provide new functions sextract32()
and sextract64() which perform this operation; they are like
the existing extract32() and extract64() except that the field
is sign-extended into the returned result.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/qemu/bitops.h |   50 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h
index affcc96..06e2e6f 100644
--- a/include/qemu/bitops.h
+++ b/include/qemu/bitops.h
@@ -222,6 +222,56 @@ static inline uint64_t extract64(uint64_t value, int start, int length)
 }
 
 /**
+ * sextract32:
+ * @value: the value to extract the bit field from
+ * @start: the lowest bit in the bit field (numbered from 0)
+ * @length: the length of the bit field
+ *
+ * Extract from the 32 bit input @value the bit field specified by the
+ * @start and @length parameters, and return it, sign extended to
+ * an int32_t (ie with the most significant bit of the field propagated
+ * to all the upper bits of the return value). The bit field must lie
+ * entirely within the 32 bit word. It is valid to request that
+ * all 32 bits are returned (ie @length 32 and @start 0).
+ *
+ * Returns: the sign extended value of the bit field extracted from the
+ * input value.
+ */
+static inline int32_t sextract32(uint32_t value, int start, int length)
+{
+    assert(start >= 0 && length > 0 && length <= 32 - start);
+    /* Note that this implementation relies on right shift of signed
+     * integers being an arithmetic shift.
+     */
+    return ((int32_t)(value << (32 - length - start))) >> (32 - length);
+}
+
+/**
+ * sextract64:
+ * @value: the value to extract the bit field from
+ * @start: the lowest bit in the bit field (numbered from 0)
+ * @length: the length of the bit field
+ *
+ * Extract from the 64 bit input @value the bit field specified by the
+ * @start and @length parameters, and return it, sign extended to
+ * an int64_t (ie with the most significant bit of the field propagated
+ * to all the upper bits of the return value). The bit field must lie
+ * entirely within the 64 bit word. It is valid to request that
+ * all 64 bits are returned (ie @length 64 and @start 0).
+ *
+ * Returns: the sign extended value of the bit field extracted from the
+ * input value.
+ */
+static inline uint64_t sextract64(uint64_t value, int start, int length)
+{
+    assert(start >= 0 && length > 0 && length <= 64 - start);
+    /* Note that this implementation relies on right shift of signed
+     * integers being an arithmetic shift.
+     */
+    return ((int64_t)(value << (64 - length - start))) >> (64 - length);
+}
+
+/**
  * deposit32:
  * @value: initial value to insert bit field into
  * @start: the lowest bit in the bit field (numbered from 0)
-- 
1.7.9.5

[Qemu-devel] [PATCH 2/2] tests: Add test-bitops.c with some sextract tests

[Peter Maydell]

Add some simple test cases for the new sextract32
and sextract64 functions.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 tests/Makefile      |    2 ++
 tests/test-bitops.c |   75 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 77 insertions(+)
 create mode 100644 tests/test-bitops.c

diff --git a/tests/Makefile b/tests/Makefile
index c107489..73a3c81 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -44,6 +44,7 @@ check-unit-y += tests/test-cutils$(EXESUF)
 gcov-files-test-cutils-y += util/cutils.c
 check-unit-y += tests/test-mul64$(EXESUF)
 gcov-files-test-mul64-y = util/host-utils.c
+check-unit-y += tests/test-bitops$(EXESUF)
 
 check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh
 
@@ -118,6 +119,7 @@ tests/test-qmp-commands$(EXESUF): tests/test-qmp-commands.o tests/test-qmp-marsh
 tests/test-visitor-serialization$(EXESUF): tests/test-visitor-serialization.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a
 
 tests/test-mul64$(EXESUF): tests/test-mul64.o libqemuutil.a
+tests/test-bitops$(EXESUF): tests/test-bitops.o libqemuutil.a
 
 libqos-obj-y = tests/libqos/pci.o tests/libqos/fw_cfg.o
 libqos-obj-y += tests/libqos/i2c.o
diff --git a/tests/test-bitops.c b/tests/test-bitops.c
new file mode 100644
index 0000000..4e713e4
--- /dev/null
+++ b/tests/test-bitops.c
@@ -0,0 +1,75 @@
+/*
+ * Test bitops routines
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#include <glib.h>
+#include <stdint.h>
+#include "qemu/bitops.h"
+
+typedef struct {
+    uint32_t value;
+    int start;
+    int length;
+    int32_t result;
+} S32Test;
+
+typedef struct {
+    uint64_t value;
+    int start;
+    int length;
+    int64_t result;
+} S64Test;
+
+static const S32Test test_s32_data[] = {
+    { 0x38463983, 4, 4, -8 },
+    { 0x38463983, 12, 8, 0x63 },
+    { 0x38463983, 0, 32, 0x38463983 },
+};
+
+static const S64Test test_s64_data[] = {
+    { 0x8459826734967223, 60, 4, -8 },
+    { 0x8459826734967223, 0, 64, 0x8459826734967223 },
+};
+
+static void test_sextract32(void)
+{
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(test_s32_data); i++) {
+        const S32Test *test = &test_s32_data[i];
+        int32_t r = sextract32(test->value, test->start, test->length);
+
+        g_assert_cmpint(r, ==, test->result);
+    }
+}
+
+static void test_sextract64(void)
+{
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(test_s32_data); i++) {
+        const S32Test *test = &test_s32_data[i];
+        int64_t r = sextract64(test->value, test->start, test->length);
+
+        g_assert_cmpint(r, ==, test->result);
+    }
+
+    for (i = 0; i < ARRAY_SIZE(test_s64_data); i++) {
+        const S64Test *test = &test_s64_data[i];
+        int64_t r = sextract64(test->value, test->start, test->length);
+
+        g_assert_cmpint(r, ==, test->result);
+    }
+}
+
+int main(int argc, char **argv)
+{
+    g_test_init(&argc, &argv, NULL);
+    g_test_add_func("/bitops/sextract32", test_sextract32);
+    g_test_add_func("/bitops/sextract64", test_sextract64);
+    return g_test_run();
+}
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH 0/2] Provide sextract32() and sextract64()

[Richard Henderson]

On 06/28/2013 04:40 AM, Peter Maydell wrote:
> Peter Maydell (2):
>   bitops: Provide sextract32() and sextract64()
>   tests: Add test-bitops.c with some sextract tests
> 
>  include/qemu/bitops.h |   50 +++++++++++++++++++++++++++++++++
>  tests/Makefile        |    2 ++
>  tests/test-bitops.c   |   75 +++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 127 insertions(+)
>  create mode 100644 tests/test-bitops.c

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

Re: [Qemu-devel] [PATCH 0/2] Provide sextract32() and sextract64()

[Peter Maydell]

Ping!

thanks
-- PMM

On 28 June 2013 12:40, Peter Maydell <peter.maydell@linaro.org> wrote:
> A common operation in instruction decoding is to take a field
> from an instruction that represents a signed integer in some
> arbitrary number of bits, and sign extend it into a C signed
> integer type for manipulation. Provide new functions sextract32()
> and sextract64() which perform this operation; they are like
> the existing extract32() and extract64() except that the field
> is sign-extended into the returned result.
>
> I agree with Markus that sign-extending versions of our
> extract functions are more flexible than the simple sext()
> I proposed in an earlier RFC, hence the switch in this patch.
> The other change is patch 2, which provides some simple test
> cases. (Will be handy if we ever decide to revamp to avoid
> the dependency on shift-left-signed behaviour.)
>
> Peter Maydell (2):
>   bitops: Provide sextract32() and sextract64()
>   tests: Add test-bitops.c with some sextract tests
>
>  include/qemu/bitops.h |   50 +++++++++++++++++++++++++++++++++
>  tests/Makefile        |    2 ++
>  tests/test-bitops.c   |   75 +++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 127 insertions(+)
>  create mode 100644 tests/test-bitops.c
>

Re: [Qemu-devel] [PATCH 0/2] Provide sextract32() and sextract64()

[Peter Maydell]

Ping^2 !

thanks
-- PMM

On 12 July 2013 16:05, Peter Maydell <peter.maydell@linaro.org> wrote:
> Ping!
>
> thanks
> -- PMM
>
> On 28 June 2013 12:40, Peter Maydell <peter.maydell@linaro.org> wrote:
>> A common operation in instruction decoding is to take a field
>> from an instruction that represents a signed integer in some
>> arbitrary number of bits, and sign extend it into a C signed
>> integer type for manipulation. Provide new functions sextract32()
>> and sextract64() which perform this operation; they are like
>> the existing extract32() and extract64() except that the field
>> is sign-extended into the returned result.
>>
>> I agree with Markus that sign-extending versions of our
>> extract functions are more flexible than the simple sext()
>> I proposed in an earlier RFC, hence the switch in this patch.
>> The other change is patch 2, which provides some simple test
>> cases. (Will be handy if we ever decide to revamp to avoid
>> the dependency on shift-left-signed behaviour.)
>>
>> Peter Maydell (2):
>>   bitops: Provide sextract32() and sextract64()
>>   tests: Add test-bitops.c with some sextract tests
>>
>>  include/qemu/bitops.h |   50 +++++++++++++++++++++++++++++++++
>>  tests/Makefile        |    2 ++
>>  tests/test-bitops.c   |   75 +++++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 127 insertions(+)
>>  create mode 100644 tests/test-bitops.c
>>

Re: [Qemu-devel] [PATCH 0/2] Provide sextract32() and sextract64()

[Anthony Liguori]

Applied.  Thanks.

Regards,

Anthony Liguori