All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] Support saturation with shift=0.
@ 2011-01-19 16:10 Christophe Lyon
  2011-01-19 16:51 ` Peter Maydell
  2011-01-26 13:33 ` Aurelien Jarno
  0 siblings, 2 replies; 5+ messages in thread
From: Christophe Lyon @ 2011-01-19 16:10 UTC (permalink / raw)
  To: qemu-devel


This patch fixes corner-case saturations, when the target range is
zero. It merely removes the guard against (sh == 0), and makes:
__ssat(0x87654321, 1) return 0xffffffff and set the saturation flag
__usat(0x87654321, 0) return 0 and set the saturation flag

Signed-off-by: Christophe Lyon <christophe.lyon@st.com>
---
 target-arm/translate.c |   28 ++++++++++++----------------
 1 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/target-arm/translate.c b/target-arm/translate.c
index 721bada..41cbb96 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -6896,27 +6896,23 @@ static void disas_arm_insn(CPUState * env, DisasContext *s)
                             tcg_gen_shli_i32(tmp, tmp, shift);
                         }
                         sh = (insn >> 16) & 0x1f;
-                        if (sh != 0) {
-                            tmp2 = tcg_const_i32(sh);
-                            if (insn & (1 << 22))
-                                gen_helper_usat(tmp, tmp, tmp2);
-                            else
-                                gen_helper_ssat(tmp, tmp, tmp2);
-                            tcg_temp_free_i32(tmp2);
-                        }
+                        tmp2 = tcg_const_i32(sh);
+                        if (insn & (1 << 22))
+                          gen_helper_usat(tmp, tmp, tmp2);
+                        else
+                          gen_helper_ssat(tmp, tmp, tmp2);
+                        tcg_temp_free_i32(tmp2);
                         store_reg(s, rd, tmp);
                     } else if ((insn & 0x00300fe0) == 0x00200f20) {
                         /* [us]sat16 */
                         tmp = load_reg(s, rm);
                         sh = (insn >> 16) & 0x1f;
-                        if (sh != 0) {
-                            tmp2 = tcg_const_i32(sh);
-                            if (insn & (1 << 22))
-                                gen_helper_usat16(tmp, tmp, tmp2);
-                            else
-                                gen_helper_ssat16(tmp, tmp, tmp2);
-                            tcg_temp_free_i32(tmp2);
-                        }
+                        tmp2 = tcg_const_i32(sh);
+                        if (insn & (1 << 22))
+                          gen_helper_usat16(tmp, tmp, tmp2);
+                        else
+                          gen_helper_ssat16(tmp, tmp, tmp2);
+                        tcg_temp_free_i32(tmp2);
                         store_reg(s, rd, tmp);
                     } else if ((insn & 0x00700fe0) == 0x00000fa0) {
                         /* Select bytes.  */
-- 
1.7.2.3

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] Support saturation with shift=0.
  2011-01-19 16:10 [Qemu-devel] [PATCH] Support saturation with shift=0 Christophe Lyon
@ 2011-01-19 16:51 ` Peter Maydell
  2011-01-20 12:06   ` Christophe Lyon
  2011-01-26 13:33 ` Aurelien Jarno
  1 sibling, 1 reply; 5+ messages in thread
From: Peter Maydell @ 2011-01-19 16:51 UTC (permalink / raw)
  To: Christophe Lyon; +Cc: qemu-devel

On 19 January 2011 16:10, Christophe Lyon <christophe.lyon@st.com> wrote:
>
> This patch fixes corner-case saturations, when the target range is
> zero. It merely removes the guard against (sh == 0), and makes:
> __ssat(0x87654321, 1) return 0xffffffff and set the saturation flag

did you mean __ssat(0x87654321, 0) here? (they give the same
result, of course, but it's the sh==0 case the patch is changing...)

> __usat(0x87654321, 0) return 0 and set the saturation flag
>
> Signed-off-by: Christophe Lyon <christophe.lyon@st.com>

Checked against the ARM ARM and tested by
random-instruction-sequence generation.

(We could have taken the opportunity of adding braces to
conform to the coding style while touching these lines, but
since the patch isn't changing them, just reindenting, I'm
happy not to worry about this.)

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
(for the content of the patch if not the commit message, anyway).

-- PMM

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] Support saturation with shift=0.
  2011-01-19 16:51 ` Peter Maydell
@ 2011-01-20 12:06   ` Christophe Lyon
  2011-01-20 12:15     ` Peter Maydell
  0 siblings, 1 reply; 5+ messages in thread
From: Christophe Lyon @ 2011-01-20 12:06 UTC (permalink / raw)
  To: Peter Maydell; +Cc: qemu-devel

On 19.01.2011 17:51, Peter Maydell wrote:
> On 19 January 2011 16:10, Christophe Lyon <christophe.lyon@st.com> wrote:
>>
>> This patch fixes corner-case saturations, when the target range is
>> zero. It merely removes the guard against (sh == 0), and makes:
>> __ssat(0x87654321, 1) return 0xffffffff and set the saturation flag
> 
> did you mean __ssat(0x87654321, 0) here? (they give the same
> result, of course, but it's the sh==0 case the patch is changing...)

Well... the ARM ARM says that the position for saturation is in the range 1 to 32, so I think the assembler encodes 1 less than what the user actually wrote. Hence at user level we use '1', but '0' is encoded and then parsed by qemu. Am I wrong?

Obviously, I can rephrase the commit message :-)

> 
>> __usat(0x87654321, 0) return 0 and set the saturation flag
>>
>> Signed-off-by: Christophe Lyon <christophe.lyon@st.com>
> 
> Checked against the ARM ARM and tested by
> random-instruction-sequence generation.
> 

Thanks.

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] Support saturation with shift=0.
  2011-01-20 12:06   ` Christophe Lyon
@ 2011-01-20 12:15     ` Peter Maydell
  0 siblings, 0 replies; 5+ messages in thread
From: Peter Maydell @ 2011-01-20 12:15 UTC (permalink / raw)
  To: Christophe Lyon; +Cc: qemu-devel

On 20 January 2011 12:06, Christophe Lyon <christophe.lyon@st.com> wrote:
> On 19.01.2011 17:51, Peter Maydell wrote:
>> On 19 January 2011 16:10, Christophe Lyon <christophe.lyon@st.com> wrote:
>>>
>>> This patch fixes corner-case saturations, when the target range is
>>> zero. It merely removes the guard against (sh == 0), and makes:
>>> __ssat(0x87654321, 1) return 0xffffffff and set the saturation flag
>>
>> did you mean __ssat(0x87654321, 0) here? (they give the same
>> result, of course, but it's the sh==0 case the patch is changing...)
>
> Well... the ARM ARM says that the position for saturation is in the
> range 1 to 32, so I think the assembler encodes 1 less than what the
> user actually wrote. Hence at user level we use '1', but '0' is encoded
> and then parsed by qemu. Am I wrong?

No, you're right, there's a "+1" in the SSAT decoding instructions
which I hadn't noticed.

-- PMM

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] Support saturation with shift=0.
  2011-01-19 16:10 [Qemu-devel] [PATCH] Support saturation with shift=0 Christophe Lyon
  2011-01-19 16:51 ` Peter Maydell
@ 2011-01-26 13:33 ` Aurelien Jarno
  1 sibling, 0 replies; 5+ messages in thread
From: Aurelien Jarno @ 2011-01-26 13:33 UTC (permalink / raw)
  To: Christophe Lyon; +Cc: qemu-devel

On Wed, Jan 19, 2011 at 05:10:52PM +0100, Christophe Lyon wrote:
> 
> This patch fixes corner-case saturations, when the target range is
> zero. It merely removes the guard against (sh == 0), and makes:
> __ssat(0x87654321, 1) return 0xffffffff and set the saturation flag
> __usat(0x87654321, 0) return 0 and set the saturation flag
> 
> Signed-off-by: Christophe Lyon <christophe.lyon@st.com>
> ---
>  target-arm/translate.c |   28 ++++++++++++----------------
>  1 files changed, 12 insertions(+), 16 deletions(-)

Thanks, applied.

> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index 721bada..41cbb96 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -6896,27 +6896,23 @@ static void disas_arm_insn(CPUState * env, DisasContext *s)
>                              tcg_gen_shli_i32(tmp, tmp, shift);
>                          }
>                          sh = (insn >> 16) & 0x1f;
> -                        if (sh != 0) {
> -                            tmp2 = tcg_const_i32(sh);
> -                            if (insn & (1 << 22))
> -                                gen_helper_usat(tmp, tmp, tmp2);
> -                            else
> -                                gen_helper_ssat(tmp, tmp, tmp2);
> -                            tcg_temp_free_i32(tmp2);
> -                        }
> +                        tmp2 = tcg_const_i32(sh);
> +                        if (insn & (1 << 22))
> +                          gen_helper_usat(tmp, tmp, tmp2);
> +                        else
> +                          gen_helper_ssat(tmp, tmp, tmp2);
> +                        tcg_temp_free_i32(tmp2);
>                          store_reg(s, rd, tmp);
>                      } else if ((insn & 0x00300fe0) == 0x00200f20) {
>                          /* [us]sat16 */
>                          tmp = load_reg(s, rm);
>                          sh = (insn >> 16) & 0x1f;
> -                        if (sh != 0) {
> -                            tmp2 = tcg_const_i32(sh);
> -                            if (insn & (1 << 22))
> -                                gen_helper_usat16(tmp, tmp, tmp2);
> -                            else
> -                                gen_helper_ssat16(tmp, tmp, tmp2);
> -                            tcg_temp_free_i32(tmp2);
> -                        }
> +                        tmp2 = tcg_const_i32(sh);
> +                        if (insn & (1 << 22))
> +                          gen_helper_usat16(tmp, tmp, tmp2);
> +                        else
> +                          gen_helper_ssat16(tmp, tmp, tmp2);
> +                        tcg_temp_free_i32(tmp2);
>                          store_reg(s, rd, tmp);
>                      } else if ((insn & 0x00700fe0) == 0x00000fa0) {
>                          /* Select bytes.  */
> -- 
> 1.7.2.3
> 
> 
> 

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-01-26 13:34 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-01-19 16:10 [Qemu-devel] [PATCH] Support saturation with shift=0 Christophe Lyon
2011-01-19 16:51 ` Peter Maydell
2011-01-20 12:06   ` Christophe Lyon
2011-01-20 12:15     ` Peter Maydell
2011-01-26 13:33 ` Aurelien Jarno

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.