[Qemu-devel] [PULL 0/2] Fixes for x86 host

[Qemu-devel] [PULL 0/2] Fixes for x86 host

[Richard Henderson]

Fixing a regression reported by Eduardo.


r~


The following changes since commit 23eb9e6b6d5315171cc15969bbc755f258004df0:

  Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2017-01-16' into staging (2017-01-17 13:53:50 +0000)

are available in the git repository at:

  git://github.com/rth7680/qemu.git tags/pull-tcg-20170117

for you to fetch changes up to 39f099ec9d6d420b6fe6f7f4f8ed80ae29c65ff2:

  tcg/i386: Always use TZCNT when available (2017-01-17 12:02:08 -0800)

----------------------------------------------------------------
tcg/i386 fixes

----------------------------------------------------------------
Richard Henderson (2):
      Revert "tcg/i386: Rely on undefined/undocumented behaviour of BSF/BSR"
      tcg/i386: Always use TZCNT when available

 tcg/i386/tcg-target.inc.c | 45 ++++++++++++++++++++-------------------------
 1 file changed, 20 insertions(+), 25 deletions(-)

[Qemu-devel] [PULL 1/2] Revert "tcg/i386: Rely on undefined/undocumented behaviour of BSF/BSR"

[Richard Henderson]

This reverts commit 4ac76910734209dab83ddd3795f08fc7889ef463.

This fixes
  http://lists.nongnu.org/archive/html/qemu-devel/2017-01/msg03062.html

While I think we could get away with relying on the undocumented
behaviour, the tcg constraint system isn't powerful enough to
properly describe the required (non-)overlap conditions.

Reported-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 tcg/i386/tcg-target.inc.c | 35 +++++++++++++----------------------
 1 file changed, 13 insertions(+), 22 deletions(-)

diff --git a/tcg/i386/tcg-target.inc.c b/tcg/i386/tcg-target.inc.c
index 01177a9..6489b73 100644
--- a/tcg/i386/tcg-target.inc.c
+++ b/tcg/i386/tcg-target.inc.c
@@ -1148,12 +1148,9 @@ static void tcg_out_ctz(TCGContext *s, int rexw, TCGReg dest, TCGReg arg1,
         tcg_debug_assert(arg2 == (rexw ? 64 : 32));
         tcg_out_modrm(s, OPC_TZCNT + rexw, dest, arg1);
     } else {
-        /* ??? The manual says that the output is undefined when the
-           input is zero, but real hardware leaves it unchanged.  As
-           noted in target-i386/translate.c, real programs depend on
-           this -- now we are one more of those.  */
-        tcg_debug_assert(dest == arg2);
+        tcg_debug_assert(dest != arg2);
         tcg_out_modrm(s, OPC_BSF + rexw, dest, arg1);
+        tcg_out_cmov(s, TCG_COND_EQ, rexw, dest, arg2);
     }
 }
 
@@ -1166,26 +1163,20 @@ static void tcg_out_clz(TCGContext *s, int rexw, TCGReg dest, TCGReg arg1,
             tcg_debug_assert(arg2 == (rexw ? 64 : 32));
         } else {
             tcg_debug_assert(dest != arg2);
-            /* LZCNT sets C if the input was zero.  */
             tcg_out_cmov(s, TCG_COND_LTU, rexw, dest, arg2);
         }
     } else {
-        TCGType type = rexw ? TCG_TYPE_I64: TCG_TYPE_I32;
-        TCGArg rev = rexw ? 63 : 31;
+        tcg_debug_assert(!const_a2);
+        tcg_debug_assert(dest != arg1);
+        tcg_debug_assert(dest != arg2);
 
-        /* Recall that the output of BSR is the index not the count.
-           Therefore we must adjust the result by ^ (SIZE-1).  In some
-           cases below, we prefer an extra XOR to a JMP.  */
-        /* ??? See the comment in tcg_out_ctz re BSF.  */
-        if (const_a2) {
-            tcg_debug_assert(dest != arg1);
-            tcg_out_movi(s, type, dest, arg2 ^ rev);
-        } else {
-            tcg_debug_assert(dest == arg2);
-            tgen_arithi(s, ARITH_XOR + rexw, dest, rev, 0);
-        }
+        /* Recall that the output of BSR is the index not the count.  */
         tcg_out_modrm(s, OPC_BSR + rexw, dest, arg1);
-        tgen_arithi(s, ARITH_XOR + rexw, dest, rev, 0);
+        tgen_arithi(s, ARITH_XOR + rexw, dest, rexw ? 63 : 31, 0);
+
+        /* Since we have destroyed the flags from BSR, we have to re-test.  */
+        tcg_out_cmp(s, arg1, 0, 1, rexw);
+        tcg_out_cmov(s, TCG_COND_EQ, rexw, dest, arg2);
     }
 }
 
@@ -2459,7 +2450,7 @@ static const TCGTargetOpDef *tcg_target_op_def(TCGOpcode op)
     case INDEX_op_ctz_i64:
         {
             static const TCGTargetOpDef ctz[2] = {
-                { .args_ct_str = { "r", "r", "0" } },
+                { .args_ct_str = { "&r", "r", "r" } },
                 { .args_ct_str = { "&r", "r", "rW" } },
             };
             return &ctz[have_bmi1];
@@ -2468,7 +2459,7 @@ static const TCGTargetOpDef *tcg_target_op_def(TCGOpcode op)
     case INDEX_op_clz_i64:
         {
             static const TCGTargetOpDef clz[2] = {
-                { .args_ct_str = { "&r", "r", "0i" } },
+                { .args_ct_str = { "&r", "r", "r" } },
                 { .args_ct_str = { "&r", "r", "rW" } },
             };
             return &clz[have_lzcnt];
-- 
2.9.3

[Qemu-devel] [PULL 2/2] tcg/i386: Always use TZCNT when available

[Richard Henderson]

I think this is cleaner than sometimes using BSF.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 tcg/i386/tcg-target.inc.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/tcg/i386/tcg-target.inc.c b/tcg/i386/tcg-target.inc.c
index 6489b73..5918008 100644
--- a/tcg/i386/tcg-target.inc.c
+++ b/tcg/i386/tcg-target.inc.c
@@ -1143,10 +1143,14 @@ static void tcg_out_movcond64(TCGContext *s, TCGCond cond, TCGReg dest,
 static void tcg_out_ctz(TCGContext *s, int rexw, TCGReg dest, TCGReg arg1,
                         TCGArg arg2, bool const_a2)
 {
-    if (const_a2) {
-        tcg_debug_assert(have_bmi1);
-        tcg_debug_assert(arg2 == (rexw ? 64 : 32));
+    if (have_bmi1) {
         tcg_out_modrm(s, OPC_TZCNT + rexw, dest, arg1);
+        if (const_a2) {
+            tcg_debug_assert(arg2 == (rexw ? 64 : 32));
+        } else {
+            tcg_debug_assert(dest != arg2);
+            tcg_out_cmov(s, TCG_COND_LTU, rexw, dest, arg2);
+        }
     } else {
         tcg_debug_assert(dest != arg2);
         tcg_out_modrm(s, OPC_BSF + rexw, dest, arg1);
-- 
2.9.3

Re: [Qemu-devel] [PULL 0/2] Fixes for x86 host

[Peter Maydell]

On 17 January 2017 at 23:03, Richard Henderson <rth@twiddle.net> wrote:
> Fixing a regression reported by Eduardo.
>
>
> r~
>
>
> The following changes since commit 23eb9e6b6d5315171cc15969bbc755f258004df0:
>
>   Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2017-01-16' into staging (2017-01-17 13:53:50 +0000)
>
> are available in the git repository at:
>
>   git://github.com/rth7680/qemu.git tags/pull-tcg-20170117
>
> for you to fetch changes up to 39f099ec9d6d420b6fe6f7f4f8ed80ae29c65ff2:
>
>   tcg/i386: Always use TZCNT when available (2017-01-17 12:02:08 -0800)
>
> ----------------------------------------------------------------
> tcg/i386 fixes
>

Applied, thanks.

-- PMM