On Wed, Jul 27, 2016 at 12:11:08PM +0530, Nikunj A Dadhania wrote: > David Gibson <[email protected]> writes: > > > [ Unknown signature status ] > > On Wed, Jul 27, 2016 at 11:47:15AM +0530, Nikunj A Dadhania wrote: > >> David Gibson <[email protected]> writes: > >> > >> > [ Unknown signature status ] > >> > On Tue, Jul 26, 2016 at 05:28:30PM +0530, Nikunj A Dadhania wrote: > >> >> While implementing modulo instructions figured out that the > >> >> implementation uses many branches. Change the logic to achieve the > >> >> branch-less code. Undefined value is set to dividend in case of invalid > >> >> input. > >> >> > >> >> Signed-off-by: Nikunj A Dadhania <[email protected]> > >> >> --- > >> >> target-ppc/translate.c | 48 > >> >> +++++++++++++++++++++++------------------------- > >> >> 1 file changed, 23 insertions(+), 25 deletions(-) > >> >> > >> >> diff --git a/target-ppc/translate.c b/target-ppc/translate.c > >> >> index 7c7328f..69d9ae0 100644 > >> >> --- a/target-ppc/translate.c > >> >> +++ b/target-ppc/translate.c > >> >> @@ -1049,41 +1049,39 @@ static void gen_addpcis(DisasContext *ctx) > >> >> static inline void gen_op_arith_divw(DisasContext *ctx, TCGv ret, TCGv > >> >> arg1, > >> >> TCGv arg2, int sign, int > >> >> compute_ov) > >> >> { > >> >> - TCGLabel *l1 = gen_new_label(); > >> >> - TCGLabel *l2 = gen_new_label(); > >> >> - TCGv_i32 t0 = tcg_temp_local_new_i32(); > >> >> - TCGv_i32 t1 = tcg_temp_local_new_i32(); > >> >> + TCGv_i32 t0 = tcg_temp_new_i32(); > >> >> + TCGv_i32 t1 = tcg_temp_new_i32(); > >> >> + TCGv_i32 t2 = tcg_temp_new_i32(); > >> >> + TCGv_i32 t3 = tcg_temp_new_i32(); > >> >> > >> >> tcg_gen_trunc_tl_i32(t0, arg1); > >> >> tcg_gen_trunc_tl_i32(t1, arg2); > >> >> - tcg_gen_brcondi_i32(TCG_COND_EQ, t1, 0, l1); > >> >> - if (sign) { > >> >> - TCGLabel *l3 = gen_new_label(); > >> >> - tcg_gen_brcondi_i32(TCG_COND_NE, t1, -1, l3); > >> >> - tcg_gen_brcondi_i32(TCG_COND_EQ, t0, INT32_MIN, l1); > >> >> - gen_set_label(l3); > >> >> - tcg_gen_div_i32(t0, t0, t1); > >> >> - } else { > >> >> - tcg_gen_divu_i32(t0, t0, t1); > >> >> - } > >> >> - if (compute_ov) { > >> >> - tcg_gen_movi_tl(cpu_ov, 0); > >> >> - } > >> >> - tcg_gen_br(l2); > >> >> - gen_set_label(l1); > >> >> if (sign) { > >> >> - tcg_gen_sari_i32(t0, t0, 31); > >> >> + tcg_gen_setcondi_i32(TCG_COND_EQ, t2, t0, INT_MIN); > >> >> + tcg_gen_setcondi_i32(TCG_COND_EQ, t3, t1, -1); > >> >> + tcg_gen_and_i32(t2, t2, t3); > >> >> + tcg_gen_setcondi_i32(TCG_COND_EQ, t3, t1, 0); > >> >> + tcg_gen_or_i32(t2, t2, t3); > >> >> + tcg_gen_movi_i32(t3, 0); > >> >> + tcg_gen_movcond_i32(TCG_COND_NE, t1, t2, t3, t2, t1); > >> >> + tcg_gen_div_i32(t3, t0, t1); > >> >> + tcg_gen_extu_i32_tl(ret, t3); > >> > > >> > Should this be a signed extend, given it's a signed divide? > >> > >> Don't think so, as the instruction is 32-bit, caller will only look at > >> the 32bit and div_i32 is signed, it will take care of extending sign > >> till 32-boundary. > > > > Hrm. I thought most 32-bit arithmetic operations on Power actually > > set the underlying 64-bit registers to a sign extended version of the > > 32-bit result. > > I think, when I want to operate on it as 64-bit, i will need signed > extension. rth can give more info on this. > > Retained the behaviour as per the previous code as well:
Ah, yes RT[0:31] <- undefined according to the ISA, so I guess this is
ok (though ideally we'd double check what the actual hardware does and
match that, just in case something relies on it).
>
> tcg_gen_trunc_tl_i32(t0, arg1);
> tcg_gen_trunc_tl_i32(t1, arg2);
> tcg_gen_brcondi_i32(TCG_COND_EQ, t1, 0, l1);
> if (sign) {
> TCGLabel *l3 = gen_new_label();
> tcg_gen_brcondi_i32(TCG_COND_NE, t1, -1, l3);
> tcg_gen_brcondi_i32(TCG_COND_EQ, t0, INT32_MIN, l1);
> gen_set_label(l3);
> tcg_gen_div_i32(t0, t0, t1);
> } else {
> tcg_gen_divu_i32(t0, t0, t1);
> }
> if (compute_ov) {
> tcg_gen_movi_tl(cpu_ov, 0);
> }
> tcg_gen_br(l2);
> gen_set_label(l1);
> if (sign) {
> tcg_gen_sari_i32(t0, t0, 31);
> } else {
> tcg_gen_movi_i32(t0, 0);
> }
> if (compute_ov) {
> tcg_gen_movi_tl(cpu_ov, 1);
> tcg_gen_movi_tl(cpu_so, 1);
> }
> gen_set_label(l2);
> tcg_gen_extu_i32_tl(ret, t0); <<<<<<<<<<<<<<<<<<<<<<<
> tcg_temp_free_i32(t0);
> tcg_temp_free_i32(t1);
>
> IMO, thats correct.
>
> Regards
> Nikunj
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
