Richard Henderson <r...@twiddle.net> writes: > On 12/05/2016 03:25 AM, Nikunj A Dadhania wrote: >> +void helper_lxvll(CPUPPCState *env, target_ulong addr, >> + target_ulong xt_num, target_ulong rb) >> +{ >> + ppc_vsr_t xt; >> + >> + getVSR(xt_num, &xt, env); >> + if (unlikely((rb & 0xFF) == 0)) { >> + xt.s128 = int128_make128(0, 0); > > Nit: int128_zero.
Sure. >> + } else { >> + target_ulong end = ((rb & 0xFF) * 8) - 1; >> + if (msr_le) { >> + xt.u64[LO_IDX] = cpu_ldq_data_ra(env, addr, GETPC()); >> + addr = addr_add(env, addr, 8); >> + xt.u64[HI_IDX] = cpu_ldq_data_ra(env, addr, GETPC()); >> + xt.s128 = int128_and(xt.s128, mask_u128(127 - end, 127)); > > The ISA document says that this is a sequence of byte operations. Which means > that END < 127 will not access bytes outside of the length. Which means that > your code will trigger SIGSEGV near page boundaries when real hardware > won't. In that case, I can use I can use cpu_ldub_data_ra() > I also don't see how this does the right thing for little-endian. Needs to be in big-endian order - two things 1) LO/HI swapped 2) No byte swapping AFAIU the example given in ISA, i see the right output in my test. Regards Nikunj