This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
[PATCH] opcodes/cgen: Rework calculation of shift when inserting fields
- From: Andrew Burgess <andrew dot burgess at embecosm dot com>
- To: binutils at sourceware dot org
- Cc: Andrew Burgess <andrew dot burgess at embecosm dot com>
- Date: Mon, 1 Feb 2016 23:17:51 +0000
- Subject: [PATCH] opcodes/cgen: Rework calculation of shift when inserting fields
- Authentication-results: sourceware.org; auth=none
The calculation of the shift amount, used to insert fields into the
instruction buffer, is not correct when the following conditions are all
true:
- CGEN_INT_INSN_P is defined, and true.
- CGEN_INSN_LSB0_P is true
- Total instruction length is greater than the length of a single
instruction word (the instruction is made of multiple words)
- The word offset is non-zero (the field is outside the first word)
When the above conditions are all true, the calculated shift fails to
take account of the total instruction length.
After this commit the calculation of the shift amount is split into two
parts, first we calculate the shift required to get to BIT0 of the word
in which the field lives, then we calculate the shift required to place
the field within the instruction word.
The change in this commit only effects the CGEN_INT_INSN_P defined true
case, but changes the code for both CGEN_INSN_LSB0_P true, and false.
In the case of CGEN_INSN_LSB0_P being false, the code used to say:
shift = total_length - (word_offset + start + length);
Now it says:
shift_to_word = total_length - (word_offset + word_length);
shift_within_word = word_length - start - length;
shift = shift_to_word + shift_within_word;
>From which we can see that in all cases the computed shift value remains
unchanged after this commit.
In the case of CGEN_INSN_LSB0_P being true, the code used to say:
shift = (word_offset + start + 1) - length;
Now it says:
shift_to_word = total_length - (word_offset + word_length);
shift_within_word = start + 1 - length;
shift = shift_to_word + shift_within_word;
In the case where 'total_length == word_length' AND 'word_offset ==
0' (which indicates an instruction of a single word), we see that the
computed shift value will be unchanged. However, when the total_length
and word_length are different, and the word_offset is non-zero then the
computed shift value will be different (and now correct).
None of the existing targets currently use the previously broken
configuration, which is why the bug was not found earlier.
Tested against all CGEN targets that make use of the effected code with
no regressions.
opcodes/ChangeLog:
* cgen-ibld.in (insert_normal): Rework calculation of shift.
* epiphany-ibld.c: Regenerate.
* fr30-ibld.c: Regenerate.
* frv-ibld.c: Regenerate.
* ip2k-ibld.c: Regenerate.
* iq2000-ibld.c: Regenerate.
* lm32-ibld.c: Regenerate.
* m32c-ibld.c: Regenerate.
* m32r-ibld.c: Regenerate.
* mep-ibld.c: Regenerate.
* mt-ibld.c: Regenerate.
* or1k-ibld.c: Regenerate.
* xc16x-ibld.c: Regenerate.
* xstormy16-ibld.c: Regenerate.
---
opcodes/ChangeLog | 17 +++++++++++++++++
opcodes/cgen-ibld.in | 13 ++++++++++---
opcodes/epiphany-ibld.c | 13 ++++++++++---
opcodes/fr30-ibld.c | 13 ++++++++++---
opcodes/frv-ibld.c | 13 ++++++++++---
opcodes/ip2k-ibld.c | 13 ++++++++++---
opcodes/iq2000-ibld.c | 13 ++++++++++---
opcodes/lm32-ibld.c | 13 ++++++++++---
opcodes/m32c-ibld.c | 13 ++++++++++---
opcodes/m32r-ibld.c | 13 ++++++++++---
opcodes/mep-ibld.c | 13 ++++++++++---
opcodes/mt-ibld.c | 13 ++++++++++---
opcodes/or1k-ibld.c | 13 ++++++++++---
opcodes/xc16x-ibld.c | 13 ++++++++++---
opcodes/xstormy16-ibld.c | 13 ++++++++++---
15 files changed, 157 insertions(+), 42 deletions(-)
diff --git a/opcodes/ChangeLog b/opcodes/ChangeLog
index 505b5e7..427034a 100644
--- a/opcodes/ChangeLog
+++ b/opcodes/ChangeLog
@@ -1,3 +1,20 @@
+2016-01-31 Andrew Burgess <andrew.burgess@embecosm.com>
+
+ * cgen-ibld.in (insert_normal): Rework calculation of shift.
+ * epiphany-ibld.c: Regenerate.
+ * fr30-ibld.c: Regenerate.
+ * frv-ibld.c: Regenerate.
+ * ip2k-ibld.c: Regenerate.
+ * iq2000-ibld.c: Regenerate.
+ * lm32-ibld.c: Regenerate.
+ * m32c-ibld.c: Regenerate.
+ * m32r-ibld.c: Regenerate.
+ * mep-ibld.c: Regenerate.
+ * mt-ibld.c: Regenerate.
+ * or1k-ibld.c: Regenerate.
+ * xc16x-ibld.c: Regenerate.
+ * xstormy16-ibld.c: Regenerate.
+
2016-02-01 Michael McConville <mmcco@mykolab.com>
* cgen-dis.c (count_decodable_bits): Use unsigned value for mask
diff --git a/opcodes/cgen-ibld.in b/opcodes/cgen-ibld.in
index 4730ff4..cd6fffa 100644
--- a/opcodes/cgen-ibld.in
+++ b/opcodes/cgen-ibld.in
@@ -207,12 +207,19 @@ insert_normal (CGEN_CPU_DESC cd,
#if CGEN_INT_INSN_P
{
- int shift;
+ int shift_within_word, shift_to_word, shift;
+ /* How to shift the value to BIT0 of the word. */
+ shift_to_word = total_length - (word_offset + word_length);
+
+ /* How to shift the value to the field within the word. */
if (CGEN_INSN_LSB0_P)
- shift = (word_offset + start + 1) - length;
+ shift_within_word = start + 1 - length;
else
- shift = total_length - (word_offset + start + length);
+ shift_within_word = word_length - start - length;
+
+ /* The total SHIFT, then mask in the value. */
+ shift = shift_to_word + shift_within_word;
*buffer = (*buffer & ~(mask << shift)) | ((value & mask) << shift);
}
diff --git a/opcodes/epiphany-ibld.c b/opcodes/epiphany-ibld.c
index c412169..f978d46 100644
--- a/opcodes/epiphany-ibld.c
+++ b/opcodes/epiphany-ibld.c
@@ -207,12 +207,19 @@ insert_normal (CGEN_CPU_DESC cd,
#if CGEN_INT_INSN_P
{
- int shift;
+ int shift_within_word, shift_to_word, shift;
+ /* How to shift the value to BIT0 of the word. */
+ shift_to_word = total_length - (word_offset + word_length);
+
+ /* How to shift the value to the field within the word. */
if (CGEN_INSN_LSB0_P)
- shift = (word_offset + start + 1) - length;
+ shift_within_word = start + 1 - length;
else
- shift = total_length - (word_offset + start + length);
+ shift_within_word = word_length - start - length;
+
+ /* The total SHIFT, then mask in the value. */
+ shift = shift_to_word + shift_within_word;
*buffer = (*buffer & ~(mask << shift)) | ((value & mask) << shift);
}
diff --git a/opcodes/fr30-ibld.c b/opcodes/fr30-ibld.c
index 12b1733..1293e73 100644
--- a/opcodes/fr30-ibld.c
+++ b/opcodes/fr30-ibld.c
@@ -207,12 +207,19 @@ insert_normal (CGEN_CPU_DESC cd,
#if CGEN_INT_INSN_P
{
- int shift;
+ int shift_within_word, shift_to_word, shift;
+ /* How to shift the value to BIT0 of the word. */
+ shift_to_word = total_length - (word_offset + word_length);
+
+ /* How to shift the value to the field within the word. */
if (CGEN_INSN_LSB0_P)
- shift = (word_offset + start + 1) - length;
+ shift_within_word = start + 1 - length;
else
- shift = total_length - (word_offset + start + length);
+ shift_within_word = word_length - start - length;
+
+ /* The total SHIFT, then mask in the value. */
+ shift = shift_to_word + shift_within_word;
*buffer = (*buffer & ~(mask << shift)) | ((value & mask) << shift);
}
diff --git a/opcodes/frv-ibld.c b/opcodes/frv-ibld.c
index bbcec67..dba36aa 100644
--- a/opcodes/frv-ibld.c
+++ b/opcodes/frv-ibld.c
@@ -207,12 +207,19 @@ insert_normal (CGEN_CPU_DESC cd,
#if CGEN_INT_INSN_P
{
- int shift;
+ int shift_within_word, shift_to_word, shift;
+ /* How to shift the value to BIT0 of the word. */
+ shift_to_word = total_length - (word_offset + word_length);
+
+ /* How to shift the value to the field within the word. */
if (CGEN_INSN_LSB0_P)
- shift = (word_offset + start + 1) - length;
+ shift_within_word = start + 1 - length;
else
- shift = total_length - (word_offset + start + length);
+ shift_within_word = word_length - start - length;
+
+ /* The total SHIFT, then mask in the value. */
+ shift = shift_to_word + shift_within_word;
*buffer = (*buffer & ~(mask << shift)) | ((value & mask) << shift);
}
diff --git a/opcodes/ip2k-ibld.c b/opcodes/ip2k-ibld.c
index a46e34d..fa6a736 100644
--- a/opcodes/ip2k-ibld.c
+++ b/opcodes/ip2k-ibld.c
@@ -207,12 +207,19 @@ insert_normal (CGEN_CPU_DESC cd,
#if CGEN_INT_INSN_P
{
- int shift;
+ int shift_within_word, shift_to_word, shift;
+ /* How to shift the value to BIT0 of the word. */
+ shift_to_word = total_length - (word_offset + word_length);
+
+ /* How to shift the value to the field within the word. */
if (CGEN_INSN_LSB0_P)
- shift = (word_offset + start + 1) - length;
+ shift_within_word = start + 1 - length;
else
- shift = total_length - (word_offset + start + length);
+ shift_within_word = word_length - start - length;
+
+ /* The total SHIFT, then mask in the value. */
+ shift = shift_to_word + shift_within_word;
*buffer = (*buffer & ~(mask << shift)) | ((value & mask) << shift);
}
diff --git a/opcodes/iq2000-ibld.c b/opcodes/iq2000-ibld.c
index 74aa4da..96faa38 100644
--- a/opcodes/iq2000-ibld.c
+++ b/opcodes/iq2000-ibld.c
@@ -207,12 +207,19 @@ insert_normal (CGEN_CPU_DESC cd,
#if CGEN_INT_INSN_P
{
- int shift;
+ int shift_within_word, shift_to_word, shift;
+ /* How to shift the value to BIT0 of the word. */
+ shift_to_word = total_length - (word_offset + word_length);
+
+ /* How to shift the value to the field within the word. */
if (CGEN_INSN_LSB0_P)
- shift = (word_offset + start + 1) - length;
+ shift_within_word = start + 1 - length;
else
- shift = total_length - (word_offset + start + length);
+ shift_within_word = word_length - start - length;
+
+ /* The total SHIFT, then mask in the value. */
+ shift = shift_to_word + shift_within_word;
*buffer = (*buffer & ~(mask << shift)) | ((value & mask) << shift);
}
diff --git a/opcodes/lm32-ibld.c b/opcodes/lm32-ibld.c
index 6a923c7..784acc0 100644
--- a/opcodes/lm32-ibld.c
+++ b/opcodes/lm32-ibld.c
@@ -207,12 +207,19 @@ insert_normal (CGEN_CPU_DESC cd,
#if CGEN_INT_INSN_P
{
- int shift;
+ int shift_within_word, shift_to_word, shift;
+ /* How to shift the value to BIT0 of the word. */
+ shift_to_word = total_length - (word_offset + word_length);
+
+ /* How to shift the value to the field within the word. */
if (CGEN_INSN_LSB0_P)
- shift = (word_offset + start + 1) - length;
+ shift_within_word = start + 1 - length;
else
- shift = total_length - (word_offset + start + length);
+ shift_within_word = word_length - start - length;
+
+ /* The total SHIFT, then mask in the value. */
+ shift = shift_to_word + shift_within_word;
*buffer = (*buffer & ~(mask << shift)) | ((value & mask) << shift);
}
diff --git a/opcodes/m32c-ibld.c b/opcodes/m32c-ibld.c
index b93de26..7066bbf 100644
--- a/opcodes/m32c-ibld.c
+++ b/opcodes/m32c-ibld.c
@@ -207,12 +207,19 @@ insert_normal (CGEN_CPU_DESC cd,
#if CGEN_INT_INSN_P
{
- int shift;
+ int shift_within_word, shift_to_word, shift;
+ /* How to shift the value to BIT0 of the word. */
+ shift_to_word = total_length - (word_offset + word_length);
+
+ /* How to shift the value to the field within the word. */
if (CGEN_INSN_LSB0_P)
- shift = (word_offset + start + 1) - length;
+ shift_within_word = start + 1 - length;
else
- shift = total_length - (word_offset + start + length);
+ shift_within_word = word_length - start - length;
+
+ /* The total SHIFT, then mask in the value. */
+ shift = shift_to_word + shift_within_word;
*buffer = (*buffer & ~(mask << shift)) | ((value & mask) << shift);
}
diff --git a/opcodes/m32r-ibld.c b/opcodes/m32r-ibld.c
index a1bbcef..2b9e93a 100644
--- a/opcodes/m32r-ibld.c
+++ b/opcodes/m32r-ibld.c
@@ -207,12 +207,19 @@ insert_normal (CGEN_CPU_DESC cd,
#if CGEN_INT_INSN_P
{
- int shift;
+ int shift_within_word, shift_to_word, shift;
+ /* How to shift the value to BIT0 of the word. */
+ shift_to_word = total_length - (word_offset + word_length);
+
+ /* How to shift the value to the field within the word. */
if (CGEN_INSN_LSB0_P)
- shift = (word_offset + start + 1) - length;
+ shift_within_word = start + 1 - length;
else
- shift = total_length - (word_offset + start + length);
+ shift_within_word = word_length - start - length;
+
+ /* The total SHIFT, then mask in the value. */
+ shift = shift_to_word + shift_within_word;
*buffer = (*buffer & ~(mask << shift)) | ((value & mask) << shift);
}
diff --git a/opcodes/mep-ibld.c b/opcodes/mep-ibld.c
index d1e33fa..0faf948 100644
--- a/opcodes/mep-ibld.c
+++ b/opcodes/mep-ibld.c
@@ -207,12 +207,19 @@ insert_normal (CGEN_CPU_DESC cd,
#if CGEN_INT_INSN_P
{
- int shift;
+ int shift_within_word, shift_to_word, shift;
+ /* How to shift the value to BIT0 of the word. */
+ shift_to_word = total_length - (word_offset + word_length);
+
+ /* How to shift the value to the field within the word. */
if (CGEN_INSN_LSB0_P)
- shift = (word_offset + start + 1) - length;
+ shift_within_word = start + 1 - length;
else
- shift = total_length - (word_offset + start + length);
+ shift_within_word = word_length - start - length;
+
+ /* The total SHIFT, then mask in the value. */
+ shift = shift_to_word + shift_within_word;
*buffer = (*buffer & ~(mask << shift)) | ((value & mask) << shift);
}
diff --git a/opcodes/mt-ibld.c b/opcodes/mt-ibld.c
index ffc7910..c678e99 100644
--- a/opcodes/mt-ibld.c
+++ b/opcodes/mt-ibld.c
@@ -207,12 +207,19 @@ insert_normal (CGEN_CPU_DESC cd,
#if CGEN_INT_INSN_P
{
- int shift;
+ int shift_within_word, shift_to_word, shift;
+ /* How to shift the value to BIT0 of the word. */
+ shift_to_word = total_length - (word_offset + word_length);
+
+ /* How to shift the value to the field within the word. */
if (CGEN_INSN_LSB0_P)
- shift = (word_offset + start + 1) - length;
+ shift_within_word = start + 1 - length;
else
- shift = total_length - (word_offset + start + length);
+ shift_within_word = word_length - start - length;
+
+ /* The total SHIFT, then mask in the value. */
+ shift = shift_to_word + shift_within_word;
*buffer = (*buffer & ~(mask << shift)) | ((value & mask) << shift);
}
diff --git a/opcodes/or1k-ibld.c b/opcodes/or1k-ibld.c
index 040bd00..b934ce1 100644
--- a/opcodes/or1k-ibld.c
+++ b/opcodes/or1k-ibld.c
@@ -207,12 +207,19 @@ insert_normal (CGEN_CPU_DESC cd,
#if CGEN_INT_INSN_P
{
- int shift;
+ int shift_within_word, shift_to_word, shift;
+ /* How to shift the value to BIT0 of the word. */
+ shift_to_word = total_length - (word_offset + word_length);
+
+ /* How to shift the value to the field within the word. */
if (CGEN_INSN_LSB0_P)
- shift = (word_offset + start + 1) - length;
+ shift_within_word = start + 1 - length;
else
- shift = total_length - (word_offset + start + length);
+ shift_within_word = word_length - start - length;
+
+ /* The total SHIFT, then mask in the value. */
+ shift = shift_to_word + shift_within_word;
*buffer = (*buffer & ~(mask << shift)) | ((value & mask) << shift);
}
diff --git a/opcodes/xc16x-ibld.c b/opcodes/xc16x-ibld.c
index 3deb3b0..43c7a16 100644
--- a/opcodes/xc16x-ibld.c
+++ b/opcodes/xc16x-ibld.c
@@ -207,12 +207,19 @@ insert_normal (CGEN_CPU_DESC cd,
#if CGEN_INT_INSN_P
{
- int shift;
+ int shift_within_word, shift_to_word, shift;
+ /* How to shift the value to BIT0 of the word. */
+ shift_to_word = total_length - (word_offset + word_length);
+
+ /* How to shift the value to the field within the word. */
if (CGEN_INSN_LSB0_P)
- shift = (word_offset + start + 1) - length;
+ shift_within_word = start + 1 - length;
else
- shift = total_length - (word_offset + start + length);
+ shift_within_word = word_length - start - length;
+
+ /* The total SHIFT, then mask in the value. */
+ shift = shift_to_word + shift_within_word;
*buffer = (*buffer & ~(mask << shift)) | ((value & mask) << shift);
}
diff --git a/opcodes/xstormy16-ibld.c b/opcodes/xstormy16-ibld.c
index 4d96394..2d05229 100644
--- a/opcodes/xstormy16-ibld.c
+++ b/opcodes/xstormy16-ibld.c
@@ -207,12 +207,19 @@ insert_normal (CGEN_CPU_DESC cd,
#if CGEN_INT_INSN_P
{
- int shift;
+ int shift_within_word, shift_to_word, shift;
+ /* How to shift the value to BIT0 of the word. */
+ shift_to_word = total_length - (word_offset + word_length);
+
+ /* How to shift the value to the field within the word. */
if (CGEN_INSN_LSB0_P)
- shift = (word_offset + start + 1) - length;
+ shift_within_word = start + 1 - length;
else
- shift = total_length - (word_offset + start + length);
+ shift_within_word = word_length - start - length;
+
+ /* The total SHIFT, then mask in the value. */
+ shift = shift_to_word + shift_within_word;
*buffer = (*buffer & ~(mask << shift)) | ((value & mask) << shift);
}
--
2.6.4