diff mbox series

[6/6] target/arm: Execute Thumb instructions when their condbits are 0xf

Message ID 20190617175317.27557-7-peter.maydell@linaro.org
State Superseded
Headers show
Series Six minor M-profile bugfixes | expand

Commit Message

Peter Maydell June 17, 2019, 5:53 p.m. UTC
Thumb instructions in an IT block are set up to be conditionally
executed depending on a set of condition bits encoded into the IT
bits of the CPSR/XPSR.  The architecture specifies that if the
condition bits are 0b1111 this means "always execute" (like 0b1110),
not "never execute"; we were treating it as "never execute".  (See
the ConditionHolds() pseudocode in both the A-profile and M-profile
Arm ARM.)

This is a bit of an obscure corner case, because the only legal
way to get to an 0b1111 set of condbits is to do an exception
return which sets the XPSR/CPSR up that way. An IT instruction
which encodes a condition sequence that would include an 0b1111 is
UNPREDICTABLE, and for v8A the CONSTRAINED UNPREDICTABLE choices
for such an IT insn are to NOP, UNDEF, or treat 0b1111 like 0b1110.
Add a comment noting that we take the latter option.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

---
 target/arm/translate.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

-- 
2.20.1

Comments

Richard Henderson June 17, 2019, 8:04 p.m. UTC | #1
On 6/17/19 10:53 AM, Peter Maydell wrote:
> Thumb instructions in an IT block are set up to be conditionally

> executed depending on a set of condition bits encoded into the IT

> bits of the CPSR/XPSR.  The architecture specifies that if the

> condition bits are 0b1111 this means "always execute" (like 0b1110),

> not "never execute"; we were treating it as "never execute".  (See

> the ConditionHolds() pseudocode in both the A-profile and M-profile

> Arm ARM.)

> 

> This is a bit of an obscure corner case, because the only legal

> way to get to an 0b1111 set of condbits is to do an exception

> return which sets the XPSR/CPSR up that way. An IT instruction

> which encodes a condition sequence that would include an 0b1111 is

> UNPREDICTABLE, and for v8A the CONSTRAINED UNPREDICTABLE choices

> for such an IT insn are to NOP, UNDEF, or treat 0b1111 like 0b1110.

> Add a comment noting that we take the latter option.

> 

> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

> ---

>  target/arm/translate.c | 15 +++++++++++++--

>  1 file changed, 13 insertions(+), 2 deletions(-)


Reviewed-by: Richard Henderson <richard.henderson@linaro.org>



r~
diff mbox series

Patch

diff --git a/target/arm/translate.c b/target/arm/translate.c
index 4750b9fa1bb..45ea0a11c7c 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -11595,7 +11595,14 @@  static void disas_thumb_insn(DisasContext *s, uint32_t insn)
                 gen_nop_hint(s, (insn >> 4) & 0xf);
                 break;
             }
-            /* If Then.  */
+            /*
+             * IT (If-Then)
+             *
+             * Combinations of firstcond and mask which set up an 0b1111
+             * condition are UNPREDICTABLE; we take the CONSTRAINED
+             * UNPREDICTABLE choice to treat 0b1111 the same as 0b1110,
+             * i.e. both meaning "execute always".
+             */
             s->condexec_cond = (insn >> 4) & 0xe;
             s->condexec_mask = insn & 0x1f;
             /* No actual code generated for this insn, just setup state.  */
@@ -12129,7 +12136,11 @@  static void thumb_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
     if (dc->condexec_mask && !thumb_insn_is_unconditional(dc, insn)) {
         uint32_t cond = dc->condexec_cond;
 
-        if (cond != 0x0e) {     /* Skip conditional when condition is AL. */
+        /*
+         * Conditionally skip the insn. Note that both 0xe and 0xf mean
+         * "always"; 0xf is not "never".
+         */
+        if (cond < 0x0e) {
             arm_skip_unless(dc, cond);
         }
     }