[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] target/ppc: fix vbpermd in big endian hosts

From: Mark Cave-Ayland
Subject: Re: [PATCH] target/ppc: fix vbpermd in big endian hosts
Date: Thu, 2 Jun 2022 09:57:11 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.0

On 01/06/2022 15:21, Philippe Mathieu-Daudé via wrote:

+Mark for commit ef96e3ae96.

On 1/6/22 14:53, matheus.ferst@eldorado.org.br wrote:
From: Matheus Ferst <matheus.ferst@eldorado.org.br>

The extract64 arguments are not endian dependent as they are only used
for bitwise operations. The current behavior in little-endian hosts is
correct; since the indexes in VRB are in PowerISA-ordering, we should
always invert the value before calling extract64. Also, using the VsrD
macro, we can have a single EXTRACT_BIT definition for big and
little-endian with the correct behavior.

Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
Found this bug while refactoring VECTOR_FOR_INORDER_I uses. The
complete patch series will also use Vsr[DB] instead of VBPERM[DQ]_INDEX,
but it will need more testing. For now, we're just changing what is
necessary to fix the instruction.
  target/ppc/int_helper.c | 5 ++---
  1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/target/ppc/int_helper.c b/target/ppc/int_helper.c
index 105b626d1b..4c5d3f03f8 100644
--- a/target/ppc/int_helper.c
+++ b/target/ppc/int_helper.c
@@ -1307,14 +1307,13 @@ XXGENPCV(XXGENPCVDM, 8)
  #define VBPERMQ_INDEX(avr, i) ((avr)->u8[(i)])
  #define VBPERMD_INDEX(i) (i)
  #define VBPERMQ_DW(index) (((index) & 0x40) != 0)
-#define EXTRACT_BIT(avr, i, index) (extract64((avr)->u64[i], index, 1))
  #define VBPERMQ_INDEX(avr, i) ((avr)->u8[15 - (i)])
  #define VBPERMD_INDEX(i) (1 - i)
  #define VBPERMQ_DW(index) (((index) & 0x40) == 0)
-#define EXTRACT_BIT(avr, i, index) \
-        (extract64((avr)->u64[1 - i], 63 - index, 1))
+#define EXTRACT_BIT(avr, i, index) \
+        (extract64((avr)->VsrD(i), 63 - index, 1))
  void helper_vbpermd(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b)

I'm not too familiar with vbpermd, however in general the use of the VsrX() macros is the right way to ensure things work correctly on both big-endian and little-endian hosts, so it looks fine to me.

FWIW with all the great improvements being done in this area, I think that Matheus and Daniel have picked things up really quickly and have a much better test setup than the G4 Mac Mini I used to do the original gvec work. If I happen to spot something on the mailing list then I'll likely reply, but otherwise I'm happy to allow things to progress without requiring an explicit Ack from me (these days my testing is mostly confined to checking that MacOS 9/X boot okay).



reply via email to

[Prev in Thread] Current Thread [Next in Thread]