On 24/09/2019 22:46, Richard Henderson wrote: > On 9/24/19 8:35 AM, Mark Cave-Ayland wrote: >> Switch over all accesses to the decimal numbers held in struct PPC_DFP from >> using HI_IDX and LO_IDX to using the VsrD() macro instead. Not only does this >> allow the compiler to ensure that the various dfp_* functions are being >> passed >> a ppc_vsr_t rather than an arbitrary uint64_t pointer, but also allows the >> host endian-specific HI_IDX and LO_IDX to be completely removed from >> dfp_helper.c. >> >> Signed-off-by: Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk> >> --- >> target/ppc/dfp_helper.c | 70 ++++++++++++++++++----------------------- >> 1 file changed, 31 insertions(+), 39 deletions(-) > > Ho hum, vs patch 5 that was me not realizing how many different places really > want to manipulate a 128-bit value. Do go ahead and keep ppc_vsr_t for now.
Yes, it's a little bit confusing in places as some operations are done on the decNumber whilst others are done on the decimal representation. After trying a few different approaches, using ppc_vsr_t seemed to be the easiest and most readable solution here. I see now that you've given R-b tags for patches 3-7, and having slept on it I'm inclined to leave patches 1-2 as they are now, i.e. no code changes other than introducing the get/set helpers to help keep the patchset as mechanical as possible. Do you think that seems a reasonable approach? > It does look like we might be well served by using Int128 at some point, so > that these operations can expand to int128_t on appropriate hw so that the > compiler can DTRT with these. Certainly ppc_vsr_t already has __uint128_t and Int128 elements but the impression I got from the #ifdef is that not all compilers would support it? Although having said that, making such a change is not something that's really on my radar. ATB, Mark.
