[Bug 1841990] Re: instruction 'denbcdq' misbehaving

[Mark Cave-Ayland]

Right so this looks like a different bug: if you look at helper_bcdadd()
and helper_bcdsub() in target/ppc/int_helper.c then you can see the
problem straight away: the code is accessing the elements of ppc_avr_t
without directly without using the VsrX() macros which correct for host
endian.

Fortunately the fix is really easy - replace the direct access with the
relevant VsrX() macro from target/ppc/cpu.h instead. It does look as if
there are several places in the BCD code that need fixing up though.

The first thing to fix is the #define BCD_DIG_BYTE around line 2055: the
VsrX() macro offsets are in "big-endian" format to match the ISA
specification so VsrD(0) is the MSB and VsrD(1) is the LSB, which means
that during the conversion you generally want the index from within the
#if defined(HOST_WORDS_BIGENDIAN) ... #endif section.

Given that the VsrX() macros invert the array index according to host
endian then you can completely remove everything between #if
defined(HOST_WORDS_BIGENDIAN) ... #endif and replace it with simply:

    #define BCD_DIG_BYTE(n) (15 - ((n) / 2))

Then as an example in the bcd_get_sgn() function below you can change
the switch from:

    switch (bcd->u8[BCD_DIG_BYTE(0)] & 0xF)

to:

    switch (bcd->VsrB(BCD_DIG_BYTE(0)) & 0xF)

etc. and repeat for the remaining bcd helpers down to helper_vsbox()
around line 2766. Note it seems the last few bcd helpers have a #if
defined(HOST_WORDS_BIGENDIAN) ... #endif section towards the start that
might a bit of thought, however once they are written in terms of the
VsrX() macros then everything will "just work" regardless of host
endian.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1841990

Title:
  instruction 'denbcdq' misbehaving

Status in QEMU:
  New

Bug description:
  Instruction 'denbcdq' appears to have no effect.  Test case attached.

  On ppc64le native:
  --
  gcc -g -O -mcpu=power9 bcdcfsq.c test-denbcdq.c -o test-denbcdq
  $ ./test-denbcdq
  0x00000000000000000000000000000000
  0x0000000000000000000000000000000c
  0x22080000000000000000000000000000
  $ ./test-denbcdq 1
  0x00000000000000000000000000000001
  0x0000000000000000000000000000001c
  0x22080000000000000000000000000001
  $ ./test-denbcdq $(seq 0 99)
  0x00000000000000000000000000000064
  0x0000000000000000000000000000100c
  0x22080000000000000000000000000080
  --

  With "qemu-ppc64le -cpu power9"
  --
  $ qemu-ppc64le -cpu power9 -L [...] ./test-denbcdq
  0x00000000000000000000000000000000
  0x0000000000000000000000000000000c
  0x0000000000000000000000000000000c
  $ qemu-ppc64le -cpu power9 -L [...] ./test-denbcdq 1
  0x00000000000000000000000000000001
  0x0000000000000000000000000000001c
  0x0000000000000000000000000000001c
  $ qemu-ppc64le -cpu power9 -L [...] ./test-denbcdq $(seq 100)
  0x00000000000000000000000000000064
  0x0000000000000000000000000000100c
  0x0000000000000000000000000000100c
  --

  I started looking at the code, but I got confused rather quickly.
  Could be related to endianness? I think denbcdq arrived on the scene
  before little-endian was a big deal.  Maybe something to do with
  utilizing implicit floating-point register pairs...  I don't think the
  right data is getting to helper_denbcdq, which would point back to the
  gen_fprp_ptr uses in dfp-impl.inc.c (GEN_DFP_T_FPR_I32_Rc).  (Maybe?)

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1841990/+subscriptions