[PULL 0/4] Hexagon (target/hexagon) bug fixes

[PULL 0/4] Hexagon (target/hexagon) bug fixes

[Taylor Simpson]

The following changes since commit 3ccf6cd0e3e1dfd663814640b3b18b55715d7a75:

  Merge remote-tracking branch 'remotes/kraxel/tags/audio-20210617-pull-request' into staging (2021-06-18 09:54:42 +0100)

are available in the git repository at:

  https://github.com/quic/qemu tags/pull-hex-20210618

for you to fetch changes up to 13ce2ae03000137e1de8d40ff7ceae46fcb34cd5:

  Hexagon (target/hexagon) remove unused TCG variables (2021-06-18 13:26:07 -0500)

----------------------------------------------------------------
Fixes for bugs found by inspection and internal testing
Tests added to tests/tcg/hexagon/misc.c

----------------------------------------------------------------
Taylor Simpson (4):
      Hexagon (target/hexagon) fix bug in fLSBNEW*
      Hexagon (target/hexagon) fix l2fetch instructions
      Hexagon (target/hexagon) cleanup gen_store_conditional[48] functions
      Hexagon (target/hexagon) remove unused TCG variables

 target/hexagon/gen_tcg.h              | 15 +++++++++--
 target/hexagon/macros.h               | 29 ++++++++-------------
 target/hexagon/genptr.c               | 16 +++---------
 target/hexagon/op_helper.c            |  5 ----
 target/hexagon/translate.c            | 11 ++------
 tests/tcg/hexagon/misc.c              | 48 ++++++++++++++++++++++++++++++++++-
 target/hexagon/imported/encode_pp.def |  3 +++
 7 files changed, 80 insertions(+), 47 deletions(-)

[PULL 1/4] Hexagon (target/hexagon) fix bug in fLSBNEW*

[Taylor Simpson]

Change fLSBNEW/fLSBNEW0/fLSBNEW1 from copy to "x & 1"
Remove gen_logical_not function
Clean up fLSBNEWNOT to use andi-1 followed by xori-1

Test cases added to tests/tcg/hexagon/misc.c

Signed-off-by: Taylor Simpson <tsimpson@quicinc.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <1622589584-22571-2-git-send-email-tsimpson@quicinc.com>
---
 target/hexagon/macros.h    | 27 ++++++++++-----------------
 target/hexagon/op_helper.c |  5 -----
 tests/tcg/hexagon/misc.c   | 39 ++++++++++++++++++++++++++++++++++++++-
 3 files changed, 48 insertions(+), 23 deletions(-)

diff --git a/target/hexagon/macros.h b/target/hexagon/macros.h
index b726c3b..2b208f3 100644
--- a/target/hexagon/macros.h
+++ b/target/hexagon/macros.h
@@ -239,33 +239,26 @@ static inline void gen_pred_cancel(TCGv pred, int slot_num)
 #endif
 
 #ifdef QEMU_GENERATE
-#define fLSBNEW(PVAL)   tcg_gen_mov_tl(LSB, (PVAL))
-#define fLSBNEW0        tcg_gen_mov_tl(LSB, hex_new_pred_value[0])
-#define fLSBNEW1        tcg_gen_mov_tl(LSB, hex_new_pred_value[1])
+#define fLSBNEW(PVAL)   tcg_gen_andi_tl(LSB, (PVAL), 1)
+#define fLSBNEW0        tcg_gen_andi_tl(LSB, hex_new_pred_value[0], 1)
+#define fLSBNEW1        tcg_gen_andi_tl(LSB, hex_new_pred_value[1], 1)
 #else
-#define fLSBNEW(PVAL)   (PVAL)
-#define fLSBNEW0        new_pred_value(env, 0)
-#define fLSBNEW1        new_pred_value(env, 1)
+#define fLSBNEW(PVAL)   ((PVAL) & 1)
+#define fLSBNEW0        (env->new_pred_value[0] & 1)
+#define fLSBNEW1        (env->new_pred_value[1] & 1)
 #endif
 
 #ifdef QEMU_GENERATE
-static inline void gen_logical_not(TCGv dest, TCGv src)
-{
-    TCGv one = tcg_const_tl(1);
-    TCGv zero = tcg_const_tl(0);
-
-    tcg_gen_movcond_tl(TCG_COND_NE, dest, src, zero, zero, one);
-
-    tcg_temp_free(one);
-    tcg_temp_free(zero);
-}
 #define fLSBOLDNOT(VAL) \
     do { \
         tcg_gen_andi_tl(LSB, (VAL), 1); \
         tcg_gen_xori_tl(LSB, LSB, 1); \
     } while (0)
 #define fLSBNEWNOT(PNUM) \
-    gen_logical_not(LSB, (PNUM))
+    do { \
+        tcg_gen_andi_tl(LSB, (PNUM), 1); \
+        tcg_gen_xori_tl(LSB, LSB, 1); \
+    } while (0)
 #else
 #define fLSBNEWNOT(PNUM) (!fLSBNEW(PNUM))
 #define fLSBOLDNOT(VAL) (!fLSBOLD(VAL))
diff --git a/target/hexagon/op_helper.c b/target/hexagon/op_helper.c
index 63dd685..4595559 100644
--- a/target/hexagon/op_helper.c
+++ b/target/hexagon/op_helper.c
@@ -128,11 +128,6 @@ void HELPER(debug_start_packet)(CPUHexagonState *env)
     }
 }
 
-static int32_t new_pred_value(CPUHexagonState *env, int pnum)
-{
-    return env->new_pred_value[pnum];
-}
-
 /* Checks for bookkeeping errors between disassembly context and runtime */
 void HELPER(debug_check_store_width)(CPUHexagonState *env, int slot, int check)
 {
diff --git a/tests/tcg/hexagon/misc.c b/tests/tcg/hexagon/misc.c
index 17c3919..9e139f3 100644
--- a/tests/tcg/hexagon/misc.c
+++ b/tests/tcg/hexagon/misc.c
@@ -181,6 +181,19 @@ static inline void S4_storeirifnew_io(void *p, int pred)
                : "p0", "memory");
 }
 
+static int L2_ploadrifnew_pi(void *p, int pred)
+{
+  int result;
+  asm volatile("%0 = #31\n\t"
+               "{\n\t"
+               "    p0 = cmp.eq(%1, #1)\n\t"
+               "    if (!p0.new) %0 = memw(%2++#4)\n\t"
+               "}\n\t"
+               : "=r"(result) : "r"(pred), "r"(p)
+               : "p0");
+  return result;
+}
+
 /*
  * Test that compound-compare-jump is executed in 2 parts
  * First we have to do all the compares in the packet and
@@ -298,8 +311,24 @@ static int auto_and(void)
     return retval;
 }
 
+void test_lsbnew(void)
+{
+    int result;
+
+    asm("r0 = #2\n\t"
+        "r1 = #5\n\t"
+        "{\n\t"
+        "    p0 = r0\n\t"
+        "    if (p0.new) r1 = #3\n\t"
+        "}\n\t"
+        "%0 = r1\n\t"
+        : "=r"(result) :: "r0", "r1", "p0");
+    check(result, 5);
+}
+
 int main()
 {
+    int res;
     long long res64;
     int pred;
 
@@ -394,6 +423,12 @@ int main()
     S4_storeirifnew_io(&array[8], 1);
     check(array[9], 9);
 
+    memcpy(array, init, sizeof(array));
+    res = L2_ploadrifnew_pi(&array[6], 0);
+    check(res, 6);
+    res = L2_ploadrifnew_pi(&array[7], 1);
+    check(res, 31);
+
     int x = cmpnd_cmp_jump();
     check(x, 12);
 
@@ -406,7 +441,7 @@ int main()
     check((int)pair, 5);
     check((int)(pair >> 32), 7);
 
-    int res = test_clrtnew(1, 7);
+    res = test_clrtnew(1, 7);
     check(res, 0);
     res = test_clrtnew(2, 7);
     check(res, 7);
@@ -422,6 +457,8 @@ int main()
     res = auto_and();
     check(res, 0);
 
+    test_lsbnew();
+
     puts(err ? "FAIL" : "PASS");
     return err;
 }
-- 
2.7.4

[PULL 2/4] Hexagon (target/hexagon) fix l2fetch instructions

[Taylor Simpson]

Y4_l2fetch == l2fetch(Rs32, Rt32)
Y5_l2fetch == l2fetch(Rs32, Rtt32)

The semantics for these instructions are present, but the encodings
are missing.

Note that these are treated as nops in qemu, so we add overrides.

Test case added to tests/tcg/hexagon/misc.c

Signed-off-by: Taylor Simpson <tsimpson@quicinc.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <1622589584-22571-3-git-send-email-tsimpson@quicinc.com>
---
 target/hexagon/gen_tcg.h              | 11 +++++++++++
 tests/tcg/hexagon/misc.c              |  9 +++++++++
 target/hexagon/imported/encode_pp.def |  3 +++
 3 files changed, 23 insertions(+)

diff --git a/target/hexagon/gen_tcg.h b/target/hexagon/gen_tcg.h
index 18fcdbc..a375d6a 100644
--- a/target/hexagon/gen_tcg.h
+++ b/target/hexagon/gen_tcg.h
@@ -734,4 +734,15 @@
 #define fGEN_TCG_F2_dfmpyhh(SHORTCODE) \
     gen_helper_dfmpyhh(RxxV, cpu_env, RxxV, RssV, RttV)
 
+/* Nothing to do for these in qemu, need to suppress compiler warnings */
+#define fGEN_TCG_Y4_l2fetch(SHORTCODE) \
+    do { \
+        RsV = RsV; \
+        RtV = RtV; \
+    } while (0)
+#define fGEN_TCG_Y5_l2fetch(SHORTCODE) \
+    do { \
+        RsV = RsV; \
+    } while (0)
+
 #endif
diff --git a/tests/tcg/hexagon/misc.c b/tests/tcg/hexagon/misc.c
index 9e139f3..f0b1947 100644
--- a/tests/tcg/hexagon/misc.c
+++ b/tests/tcg/hexagon/misc.c
@@ -326,6 +326,13 @@ void test_lsbnew(void)
     check(result, 5);
 }
 
+void test_l2fetch(void)
+{
+    /* These don't do anything in qemu, just make sure they don't assert */
+    asm volatile ("l2fetch(r0, r1)\n\t"
+                  "l2fetch(r0, r3:2)\n\t");
+}
+
 int main()
 {
     int res;
@@ -459,6 +466,8 @@ int main()
 
     test_lsbnew();
 
+    test_l2fetch();
+
     puts(err ? "FAIL" : "PASS");
     return err;
 }
diff --git a/target/hexagon/imported/encode_pp.def b/target/hexagon/imported/encode_pp.def
index 35ae3d2..939c6fc 100644
--- a/target/hexagon/imported/encode_pp.def
+++ b/target/hexagon/imported/encode_pp.def
@@ -493,6 +493,9 @@ DEF_ENC32(Y2_dccleana,     ICLASS_ST" 000 00 00sssss PP------ --------")
 DEF_ENC32(Y2_dcinva,       ICLASS_ST" 000 00 01sssss PP------ --------")
 DEF_ENC32(Y2_dccleaninva,  ICLASS_ST" 000 00 10sssss PP------ --------")
 
+DEF_ENC32(Y4_l2fetch,      ICLASS_ST" 011 00 00sssss PP-ttttt 000-----")
+DEF_ENC32(Y5_l2fetch,      ICLASS_ST" 011 01 00sssss PP-ttttt --------")
+
 /*******************************/
 /*                             */
 /*                             */
-- 
2.7.4

[PULL 3/4] Hexagon (target/hexagon) cleanup gen_store_conditional[48] functions

[Taylor Simpson]

Previously the store-conditional code was writing to hex_pred[prednum].
Then, the fGEN_TCG override was reading from there to the destination
variable so that the packet commit logic would handle it properly.

The correct implementation is to write to the destination variable
and don't have the extra read in the override.

Remove the unused arguments from gen_store_conditional[48]

Signed-off-by: Taylor Simpson <tsimpson@quicinc.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <1622589584-22571-4-git-send-email-tsimpson@quicinc.com>
---
 target/hexagon/gen_tcg.h |  4 ++--
 target/hexagon/macros.h  |  2 +-
 target/hexagon/genptr.c  | 10 ++++------
 3 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/target/hexagon/gen_tcg.h b/target/hexagon/gen_tcg.h
index a375d6a..ee94c90 100644
--- a/target/hexagon/gen_tcg.h
+++ b/target/hexagon/gen_tcg.h
@@ -424,9 +424,9 @@
 #define fGEN_TCG_L4_loadd_locked(SHORTCODE) \
     SHORTCODE
 #define fGEN_TCG_S2_storew_locked(SHORTCODE) \
-    do { SHORTCODE; READ_PREG(PdV, PdN); } while (0)
+    SHORTCODE
 #define fGEN_TCG_S4_stored_locked(SHORTCODE) \
-    do { SHORTCODE; READ_PREG(PdV, PdN); } while (0)
+    SHORTCODE
 
 #define fGEN_TCG_STORE(SHORTCODE) \
     do { \
diff --git a/target/hexagon/macros.h b/target/hexagon/macros.h
index 2b208f3..84fa687 100644
--- a/target/hexagon/macros.h
+++ b/target/hexagon/macros.h
@@ -591,7 +591,7 @@ static inline TCGv gen_read_ireg(TCGv result, TCGv val, int shift)
 
 #ifdef QEMU_GENERATE
 #define fSTORE_LOCKED(NUM, SIZE, EA, SRC, PRED) \
-    gen_store_conditional##SIZE(env, ctx, PdN, PRED, EA, SRC);
+    gen_store_conditional##SIZE(ctx, PRED, EA, SRC);
 #endif
 
 #ifdef QEMU_GENERATE
diff --git a/target/hexagon/genptr.c b/target/hexagon/genptr.c
index 797a6c0..bd18cb1 100644
--- a/target/hexagon/genptr.c
+++ b/target/hexagon/genptr.c
@@ -334,8 +334,7 @@ static inline void gen_load_locked8u(TCGv_i64 dest, TCGv vaddr, int mem_index)
     tcg_gen_mov_i64(hex_llsc_val_i64, dest);
 }
 
-static inline void gen_store_conditional4(CPUHexagonState *env,
-                                          DisasContext *ctx, int prednum,
+static inline void gen_store_conditional4(DisasContext *ctx,
                                           TCGv pred, TCGv vaddr, TCGv src)
 {
     TCGLabel *fail = gen_new_label();
@@ -349,7 +348,7 @@ static inline void gen_store_conditional4(CPUHexagonState *env,
     tmp = tcg_temp_new();
     tcg_gen_atomic_cmpxchg_tl(tmp, hex_llsc_addr, hex_llsc_val, src,
                               ctx->mem_idx, MO_32);
-    tcg_gen_movcond_tl(TCG_COND_EQ, hex_pred[prednum], tmp, hex_llsc_val,
+    tcg_gen_movcond_tl(TCG_COND_EQ, pred, tmp, hex_llsc_val,
                        one, zero);
     tcg_temp_free(one);
     tcg_temp_free(zero);
@@ -363,8 +362,7 @@ static inline void gen_store_conditional4(CPUHexagonState *env,
     tcg_gen_movi_tl(hex_llsc_addr, ~0);
 }
 
-static inline void gen_store_conditional8(CPUHexagonState *env,
-                                          DisasContext *ctx, int prednum,
+static inline void gen_store_conditional8(DisasContext *ctx,
                                           TCGv pred, TCGv vaddr, TCGv_i64 src)
 {
     TCGLabel *fail = gen_new_label();
@@ -380,7 +378,7 @@ static inline void gen_store_conditional8(CPUHexagonState *env,
                                ctx->mem_idx, MO_64);
     tcg_gen_movcond_i64(TCG_COND_EQ, tmp, tmp, hex_llsc_val_i64,
                         one, zero);
-    tcg_gen_extrl_i64_i32(hex_pred[prednum], tmp);
+    tcg_gen_extrl_i64_i32(pred, tmp);
     tcg_temp_free_i64(one);
     tcg_temp_free_i64(zero);
     tcg_temp_free_i64(tmp);
-- 
2.7.4

[PULL 4/4] Hexagon (target/hexagon) remove unused TCG variables

[Taylor Simpson]

Signed-off-by: Taylor Simpson <tsimpson@quicinc.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <1622589584-22571-5-git-send-email-tsimpson@quicinc.com>
---
 target/hexagon/genptr.c    |  6 ------
 target/hexagon/translate.c | 11 ++---------
 2 files changed, 2 insertions(+), 15 deletions(-)

diff --git a/target/hexagon/genptr.c b/target/hexagon/genptr.c
index bd18cb1..5dbabe0 100644
--- a/target/hexagon/genptr.c
+++ b/target/hexagon/genptr.c
@@ -121,10 +121,7 @@ static void gen_log_reg_write_pair(int rnum, TCGv_i64 val)
 
 static inline void gen_log_pred_write(DisasContext *ctx, int pnum, TCGv val)
 {
-    TCGv zero = tcg_const_tl(0);
     TCGv base_val = tcg_temp_new();
-    TCGv and_val = tcg_temp_new();
-    TCGv pred_written = tcg_temp_new();
 
     tcg_gen_andi_tl(base_val, val, 0xff);
 
@@ -143,10 +140,7 @@ static inline void gen_log_pred_write(DisasContext *ctx, int pnum, TCGv val)
     }
     tcg_gen_ori_tl(hex_pred_written, hex_pred_written, 1 << pnum);
 
-    tcg_temp_free(zero);
     tcg_temp_free(base_val);
-    tcg_temp_free(and_val);
-    tcg_temp_free(pred_written);
 }
 
 static inline void gen_read_p3_0(TCGv control_reg)
diff --git a/target/hexagon/translate.c b/target/hexagon/translate.c
index 9a37644..b23d36a 100644
--- a/target/hexagon/translate.c
+++ b/target/hexagon/translate.c
@@ -273,7 +273,6 @@ static void gen_reg_writes(DisasContext *ctx)
 
 static void gen_pred_writes(DisasContext *ctx, Packet *pkt)
 {
-    TCGv zero, control_reg, pval;
     int i;
 
     /* Early exit if the log is empty */
@@ -281,10 +280,6 @@ static void gen_pred_writes(DisasContext *ctx, Packet *pkt)
         return;
     }
 
-    zero = tcg_const_tl(0);
-    control_reg = tcg_temp_new();
-    pval = tcg_temp_new();
-
     /*
      * Only endloop instructions will conditionally
      * write a predicate.  If there are no endloop
@@ -292,6 +287,7 @@ static void gen_pred_writes(DisasContext *ctx, Packet *pkt)
      * write of the predicates.
      */
     if (pkt->pkt_has_endloop) {
+        TCGv zero = tcg_const_tl(0);
         TCGv pred_written = tcg_temp_new();
         for (i = 0; i < ctx->preg_log_idx; i++) {
             int pred_num = ctx->preg_log[i];
@@ -302,6 +298,7 @@ static void gen_pred_writes(DisasContext *ctx, Packet *pkt)
                                hex_new_pred_value[pred_num],
                                hex_pred[pred_num]);
         }
+        tcg_temp_free(zero);
         tcg_temp_free(pred_written);
     } else {
         for (i = 0; i < ctx->preg_log_idx; i++) {
@@ -314,10 +311,6 @@ static void gen_pred_writes(DisasContext *ctx, Packet *pkt)
             }
         }
     }
-
-    tcg_temp_free(zero);
-    tcg_temp_free(control_reg);
-    tcg_temp_free(pval);
 }
 
 static void gen_check_store_width(DisasContext *ctx, int slot_num)
-- 
2.7.4

RE: [PULL 0/4] Hexagon (target/hexagon) bug fixes

[Taylor Simpson]

Adding Peter to the CC list ...

> -----Original Message-----
> From: Taylor Simpson <tsimpson@quicinc.com>
> Sent: Friday, June 18, 2021 1:07 PM
> To: qemu-devel@nongnu.org
> Cc: Taylor Simpson <tsimpson@quicinc.com>; richard.henderson@linaro.org;
> philmd@redhat.com; ale@rev.ng; Brian Cain <bcain@quicinc.com>
> Subject: [PULL 0/4] Hexagon (target/hexagon) bug fixes
> 
> The following changes since commit
> 3ccf6cd0e3e1dfd663814640b3b18b55715d7a75:
> 
>   Merge remote-tracking branch 'remotes/kraxel/tags/audio-20210617-pull-
> request' into staging (2021-06-18 09:54:42 +0100)
> 
> are available in the git repository at:
> 
>   https://github.com/quic/qemu tags/pull-hex-20210618
> 
> for you to fetch changes up to 13ce2ae03000137e1de8d40ff7ceae46fcb34cd5:
> 
>   Hexagon (target/hexagon) remove unused TCG variables (2021-06-18
> 13:26:07 -0500)
> 
> ----------------------------------------------------------------
> Fixes for bugs found by inspection and internal testing Tests added to
> tests/tcg/hexagon/misc.c
> 
> ----------------------------------------------------------------
> Taylor Simpson (4):
>       Hexagon (target/hexagon) fix bug in fLSBNEW*
>       Hexagon (target/hexagon) fix l2fetch instructions
>       Hexagon (target/hexagon) cleanup gen_store_conditional[48] functions
>       Hexagon (target/hexagon) remove unused TCG variables
> 
>  target/hexagon/gen_tcg.h              | 15 +++++++++--
>  target/hexagon/macros.h               | 29 ++++++++-------------
>  target/hexagon/genptr.c               | 16 +++---------
>  target/hexagon/op_helper.c            |  5 ----
>  target/hexagon/translate.c            | 11 ++------
>  tests/tcg/hexagon/misc.c              | 48
> ++++++++++++++++++++++++++++++++++-
>  target/hexagon/imported/encode_pp.def |  3 +++
>  7 files changed, 80 insertions(+), 47 deletions(-)

Re: [PULL 0/4] Hexagon (target/hexagon) bug fixes

[Peter Maydell]

On Wed, 23 Jun 2021 at 15:31, Taylor Simpson <tsimpson@quicinc.com> wrote:
>
> Adding Peter to the CC list ...

This is on my to-process list (I catch pull requests by
looking for particular text in the email), but it's from
a git repository I haven't dealt with before, so it went
onto my "deal with when I have some time spare" list.

-- PMM

Re: [PULL 0/4] Hexagon (target/hexagon) bug fixes

[Peter Maydell]

On Fri, 18 Jun 2021 at 20:08, Taylor Simpson <tsimpson@quicinc.com> wrote:
>
> The following changes since commit 3ccf6cd0e3e1dfd663814640b3b18b55715d7a75:
>
>   Merge remote-tracking branch 'remotes/kraxel/tags/audio-20210617-pull-request' into staging (2021-06-18 09:54:42 +0100)
>
> are available in the git repository at:
>
>   https://github.com/quic/qemu tags/pull-hex-20210618
>
> for you to fetch changes up to 13ce2ae03000137e1de8d40ff7ceae46fcb34cd5:
>
>   Hexagon (target/hexagon) remove unused TCG variables (2021-06-18 13:26:07 -0500)
>
> ----------------------------------------------------------------
> Fixes for bugs found by inspection and internal testing
> Tests added to tests/tcg/hexagon/misc.c
>

Could you upload the key you used to sign this to
keyserver.ubuntu.com, please ? (The gpg keyserver
infra seems to be having a problem currently since
keys.gnupg.net has been taken down, but the Ubuntu
keyserver works...)

thanks
-- PMM

Re: [PULL 0/4] Hexagon (target/hexagon) bug fixes

[Peter Maydell]

On Fri, 18 Jun 2021 at 20:08, Taylor Simpson <tsimpson@quicinc.com> wrote:
>
> The following changes since commit 3ccf6cd0e3e1dfd663814640b3b18b55715d7a75:
>
>   Merge remote-tracking branch 'remotes/kraxel/tags/audio-20210617-pull-request' into staging (2021-06-18 09:54:42 +0100)
>
> are available in the git repository at:
>
>   https://github.com/quic/qemu tags/pull-hex-20210618
>
> for you to fetch changes up to 13ce2ae03000137e1de8d40ff7ceae46fcb34cd5:
>
>   Hexagon (target/hexagon) remove unused TCG variables (2021-06-18 13:26:07 -0500)
>
> ----------------------------------------------------------------
> Fixes for bugs found by inspection and internal testing
> Tests added to tests/tcg/hexagon/misc.c
>

(Philippe kindly reuploaded your gpg key to a keyserver that I could
download it from, so that part is now sorted).

This fails the "clang-user" job on gitlab CI:
https://gitlab.com/qemu-project/qemu/-/jobs/1385267038


../target/hexagon/genptr.c:30:20: error: unused function
'gen_read_preg' [-Werror,-Wunused-function]
static inline TCGv gen_read_preg(TCGv pred, uint8_t num)
^

(Clang is pickier than gcc about not having unused static inline functions.)

thanks
-- PMM

RE: [PULL 0/4] Hexagon (target/hexagon) bug fixes

[Taylor Simpson]

> -----Original Message-----
> From: Peter Maydell <peter.maydell@linaro.org>
> Sent: Tuesday, June 29, 2021 6:18 AM
> To: Taylor Simpson <tsimpson@quicinc.com>
> Cc: QEMU Developers <qemu-devel@nongnu.org>; Alessandro Di Federico
> <ale@rev.ng>; Philippe Mathieu-Daudé <philmd@redhat.com>; Richard
> Henderson <richard.henderson@linaro.org>; Brian Cain
> <bcain@quicinc.com>
> Subject: Re: [PULL 0/4] Hexagon (target/hexagon) bug fixes
> 
> 
> (Philippe kindly reuploaded your gpg key to a keyserver that I could
> download it from, so that part is now sorted).

Thanks Philippe!!

> This fails the "clang-user" job on gitlab CI:
> https://gitlab.com/qemu-project/qemu/-/jobs/1385267038
> 
> ../target/hexagon/genptr.c:30:20: error: unused function 'gen_read_preg' [-
> Werror,-Wunused-function] static inline TCGv gen_read_preg(TCGv pred,
> uint8_t num) ^
> 
> (Clang is pickier than gcc about not having unused static inline functions.)

I will fix this and resubmit the pull request.

Thanks,
Taylor

Re: [PULL 1/4] Hexagon (target/hexagon) fix bug in fLSBNEW*

[Thomas Huth]

On 17/06/2021 23.52, Taylor Simpson wrote:
> Change fLSBNEW/fLSBNEW0/fLSBNEW1 from copy to "x & 1"
> Remove gen_logical_not function
> Clean up fLSBNEWNOT to use andi-1 followed by xori-1
> 
> Test cases added to tests/tcg/hexagon/misc.c
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Taylor Simpson <tsimpson@quicinc.com>
> ---
>   target/hexagon/macros.h    | 27 ++++++++++-----------------
>   target/hexagon/op_helper.c |  5 -----
>   tests/tcg/hexagon/misc.c   | 39 ++++++++++++++++++++++++++++++++++++++-
>   3 files changed, 48 insertions(+), 23 deletions(-)

  Hi!

Looks like you've missed to add a cover letter with the PULL information to 
this series (at least I did not receive one and the archive on 
https://lists.gnu.org/archive/html/qemu-devel/2021-06/threads.html doesn't 
show it either)... I guess Peter won't be able to process this PULL request 
that way, so please send again with a proper cover letter added.

  Thomas

[PULL 1/4] Hexagon (target/hexagon) fix bug in fLSBNEW*

[Taylor Simpson]

Change fLSBNEW/fLSBNEW0/fLSBNEW1 from copy to "x & 1"
Remove gen_logical_not function
Clean up fLSBNEWNOT to use andi-1 followed by xori-1

Test cases added to tests/tcg/hexagon/misc.c

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Taylor Simpson <tsimpson@quicinc.com>
---
 target/hexagon/macros.h    | 27 ++++++++++-----------------
 target/hexagon/op_helper.c |  5 -----
 tests/tcg/hexagon/misc.c   | 39 ++++++++++++++++++++++++++++++++++++++-
 3 files changed, 48 insertions(+), 23 deletions(-)

diff --git a/target/hexagon/macros.h b/target/hexagon/macros.h
index b726c3b..2b208f3 100644
--- a/target/hexagon/macros.h
+++ b/target/hexagon/macros.h
@@ -239,33 +239,26 @@ static inline void gen_pred_cancel(TCGv pred, int slot_num)
 #endif
 
 #ifdef QEMU_GENERATE
-#define fLSBNEW(PVAL)   tcg_gen_mov_tl(LSB, (PVAL))
-#define fLSBNEW0        tcg_gen_mov_tl(LSB, hex_new_pred_value[0])
-#define fLSBNEW1        tcg_gen_mov_tl(LSB, hex_new_pred_value[1])
+#define fLSBNEW(PVAL)   tcg_gen_andi_tl(LSB, (PVAL), 1)
+#define fLSBNEW0        tcg_gen_andi_tl(LSB, hex_new_pred_value[0], 1)
+#define fLSBNEW1        tcg_gen_andi_tl(LSB, hex_new_pred_value[1], 1)
 #else
-#define fLSBNEW(PVAL)   (PVAL)
-#define fLSBNEW0        new_pred_value(env, 0)
-#define fLSBNEW1        new_pred_value(env, 1)
+#define fLSBNEW(PVAL)   ((PVAL) & 1)
+#define fLSBNEW0        (env->new_pred_value[0] & 1)
+#define fLSBNEW1        (env->new_pred_value[1] & 1)
 #endif
 
 #ifdef QEMU_GENERATE
-static inline void gen_logical_not(TCGv dest, TCGv src)
-{
-    TCGv one = tcg_const_tl(1);
-    TCGv zero = tcg_const_tl(0);
-
-    tcg_gen_movcond_tl(TCG_COND_NE, dest, src, zero, zero, one);
-
-    tcg_temp_free(one);
-    tcg_temp_free(zero);
-}
 #define fLSBOLDNOT(VAL) \
     do { \
         tcg_gen_andi_tl(LSB, (VAL), 1); \
         tcg_gen_xori_tl(LSB, LSB, 1); \
     } while (0)
 #define fLSBNEWNOT(PNUM) \
-    gen_logical_not(LSB, (PNUM))
+    do { \
+        tcg_gen_andi_tl(LSB, (PNUM), 1); \
+        tcg_gen_xori_tl(LSB, LSB, 1); \
+    } while (0)
 #else
 #define fLSBNEWNOT(PNUM) (!fLSBNEW(PNUM))
 #define fLSBOLDNOT(VAL) (!fLSBOLD(VAL))
diff --git a/target/hexagon/op_helper.c b/target/hexagon/op_helper.c
index 63dd685..4595559 100644
--- a/target/hexagon/op_helper.c
+++ b/target/hexagon/op_helper.c
@@ -128,11 +128,6 @@ void HELPER(debug_start_packet)(CPUHexagonState *env)
     }
 }
 
-static int32_t new_pred_value(CPUHexagonState *env, int pnum)
-{
-    return env->new_pred_value[pnum];
-}
-
 /* Checks for bookkeeping errors between disassembly context and runtime */
 void HELPER(debug_check_store_width)(CPUHexagonState *env, int slot, int check)
 {
diff --git a/tests/tcg/hexagon/misc.c b/tests/tcg/hexagon/misc.c
index 17c3919..9e139f3 100644
--- a/tests/tcg/hexagon/misc.c
+++ b/tests/tcg/hexagon/misc.c
@@ -181,6 +181,19 @@ static inline void S4_storeirifnew_io(void *p, int pred)
                : "p0", "memory");
 }
 
+static int L2_ploadrifnew_pi(void *p, int pred)
+{
+  int result;
+  asm volatile("%0 = #31\n\t"
+               "{\n\t"
+               "    p0 = cmp.eq(%1, #1)\n\t"
+               "    if (!p0.new) %0 = memw(%2++#4)\n\t"
+               "}\n\t"
+               : "=r"(result) : "r"(pred), "r"(p)
+               : "p0");
+  return result;
+}
+
 /*
  * Test that compound-compare-jump is executed in 2 parts
  * First we have to do all the compares in the packet and
@@ -298,8 +311,24 @@ static int auto_and(void)
     return retval;
 }
 
+void test_lsbnew(void)
+{
+    int result;
+
+    asm("r0 = #2\n\t"
+        "r1 = #5\n\t"
+        "{\n\t"
+        "    p0 = r0\n\t"
+        "    if (p0.new) r1 = #3\n\t"
+        "}\n\t"
+        "%0 = r1\n\t"
+        : "=r"(result) :: "r0", "r1", "p0");
+    check(result, 5);
+}
+
 int main()
 {
+    int res;
     long long res64;
     int pred;
 
@@ -394,6 +423,12 @@ int main()
     S4_storeirifnew_io(&array[8], 1);
     check(array[9], 9);
 
+    memcpy(array, init, sizeof(array));
+    res = L2_ploadrifnew_pi(&array[6], 0);
+    check(res, 6);
+    res = L2_ploadrifnew_pi(&array[7], 1);
+    check(res, 31);
+
     int x = cmpnd_cmp_jump();
     check(x, 12);
 
@@ -406,7 +441,7 @@ int main()
     check((int)pair, 5);
     check((int)(pair >> 32), 7);
 
-    int res = test_clrtnew(1, 7);
+    res = test_clrtnew(1, 7);
     check(res, 0);
     res = test_clrtnew(2, 7);
     check(res, 7);
@@ -422,6 +457,8 @@ int main()
     res = auto_and();
     check(res, 0);
 
+    test_lsbnew();
+
     puts(err ? "FAIL" : "PASS");
     return err;
 }
-- 
2.7.4