All: Ok for trunk. Please review.
Thanks & Regards Ajit On 01/06/23 10:53 am, Ajit Agarwal via Gcc-patches wrote: > Hello All: > > This new version of patch 4 use improve ree pass for rs6000 target using > defined ABI interfaces. > Bootstrapped and regtested on power64-linux-gnu. > > Review comments incorporated. > > Thanks & Regards > Ajit > > Improve ree pass for rs6000 target using defined abi interfaces > > For rs6000 target we see redundant zero and sign > extension and done to improve ree pass to eliminate > such redundant zero and sign extension using defined > ABI interfaces. > > 2023-06-01 Ajit Kumar Agarwal <aagar...@linux.ibm.com> > > gcc/ChangeLog: > > * ree.cc (combine_reaching_defs): Use of zero_extend and sign_extend > defined abi interfaces. > (add_removable_extension): Use of defined abi interfaces for no > reaching defs. > (abi_extension_candidate_return_reg_p): New function. > (abi_extension_candidate_p): New function. > (abi_extension_candidate_argno_p): New function. > (abi_handle_regs_without_defs_p): New function. > (abi_target_promote_function_mode): New function. > > gcc/testsuite/ChangeLog: > > * g++.target/powerpc/zext-elim-3.C > --- > gcc/ree.cc | 199 +++++++++++++++--- > .../g++.target/powerpc/zext-elim-3.C | 13 ++ > 2 files changed, 183 insertions(+), 29 deletions(-) > create mode 100644 gcc/testsuite/g++.target/powerpc/zext-elim-3.C > > diff --git a/gcc/ree.cc b/gcc/ree.cc > index fc04249fa84..2025a7c43da 100644 > --- a/gcc/ree.cc > +++ b/gcc/ree.cc > @@ -514,7 +514,8 @@ get_uses (rtx_insn *insn, rtx reg) > if (REGNO (DF_REF_REG (def)) == REGNO (reg)) > break; > > - gcc_assert (def != NULL); > + if (def == NULL) > + return NULL; > > ref_chain = DF_REF_CHAIN (def); > > @@ -750,6 +751,120 @@ get_extended_src_reg (rtx src) > return src; > } > > +/* Return TRUE if target mode is equal to source mode of zero_extend > + or sign_extend otherwise false. */ > + > +static bool > +abi_target_promote_function_mode (machine_mode mode) > +{ > + int unsignedp; > + machine_mode tgt_mode = > + targetm.calls.promote_function_mode (NULL_TREE, mode, &unsignedp, > + NULL_TREE, 1); > + > + if (tgt_mode == mode) > + return true; > + else > + return false; > +} > + > +/* Return TRUE if the candidate insn is zero extend and regno is > + an return registers. */ > + > +static bool > +abi_extension_candidate_return_reg_p (rtx_insn *insn, int regno) > +{ > + rtx set = single_set (insn); > + > + if (GET_CODE (SET_SRC (set)) != ZERO_EXTEND) > + return false; > + > + if (FUNCTION_VALUE_REGNO_P (regno)) > + return true; > + > + return false; > +} > + > +/* Return TRUE if reg source operand of zero_extend is argument registers > + and not return registers and source and destination operand are same > + and mode of source and destination operand are not same. */ > + > +static bool > +abi_extension_candidate_p (rtx_insn *insn) > +{ > + rtx set = single_set (insn); > + > + if (GET_CODE (SET_SRC (set)) != ZERO_EXTEND) > + return false; > + > + machine_mode ext_dst_mode = GET_MODE (SET_DEST (set)); > + rtx orig_src = XEXP (SET_SRC (set),0); > + > + bool copy_needed > + = (REGNO (SET_DEST (set)) != REGNO (XEXP (SET_SRC (set), 0))); > + > + if (!copy_needed && ext_dst_mode != GET_MODE (orig_src) > + && FUNCTION_ARG_REGNO_P (REGNO (orig_src)) > + && !abi_extension_candidate_return_reg_p (insn, REGNO (orig_src))) > + return true; > + > + return false; > +} > + > +/* Return TRUE if the candidate insn is zero extend and regno is > + an argument registers. */ > + > +static bool > +abi_extension_candidate_argno_p (rtx_code code, int regno) > +{ > + if (code != ZERO_EXTEND) > + return false; > + > + if (FUNCTION_ARG_REGNO_P (regno)) > + return true; > + > + return false; > +} > + > +/* Return TRUE if the candidate insn doesn't have defs and have > + * uses without RTX_BIN_ARITH/RTX_COMM_ARITH/RTX_UNARY rtx class. */ > + > +static bool > +abi_handle_regs_without_defs_p (rtx_insn *insn) > +{ > + if (side_effects_p (PATTERN (insn))) > + return false; > + > + struct df_link *uses > + = get_uses (insn, SET_DEST (PATTERN (insn))); > + > + if (!uses) > + return false; > + > + for (df_link *use = uses; use; use = use->next) > + { > + if (!use->ref) > + return false; > + > + if (BLOCK_FOR_INSN (insn) > + != BLOCK_FOR_INSN (DF_REF_INSN (use->ref))) > + return false; > + > + rtx_insn *use_insn = DF_REF_INSN (use->ref); > + > + if (GET_CODE (PATTERN (use_insn)) == SET) > + { > + rtx_code code = GET_CODE (SET_SRC (PATTERN (use_insn))); > + > + if (GET_RTX_CLASS (code) == RTX_BIN_ARITH > + || GET_RTX_CLASS (code) == RTX_COMM_ARITH > + || GET_RTX_CLASS (code) == RTX_UNARY) > + return false; > + } > + } > + return true; > +} > + > /* This function goes through all reaching defs of the source > of the candidate for elimination (CAND) and tries to combine > the extension with the definition instruction. The changes > @@ -770,6 +885,11 @@ combine_reaching_defs (ext_cand *cand, const_rtx > set_pat, ext_state *state) > > state->defs_list.truncate (0); > state->copies_list.truncate (0); > + rtx orig_src = XEXP (SET_SRC (cand->expr),0); > + > + if (abi_extension_candidate_p (cand->insn) > + && (!get_defs (cand->insn, orig_src, NULL))) > + return abi_handle_regs_without_defs_p (cand->insn); > > outcome = make_defs_and_copies_lists (cand->insn, set_pat, state); > > @@ -1036,6 +1156,15 @@ combine_reaching_defs (ext_cand *cand, const_rtx > set_pat, ext_state *state) > } > } > > + rtx insn_set = single_set (cand->insn); > + > + machine_mode mode = (GET_MODE (XEXP (SET_SRC (insn_set), 0))); > + > + bool promote_p = abi_target_promote_function_mode (mode); > + > + if (promote_p) > + return true; > + > if (merge_successful) > { > /* Commit the changes here if possible > @@ -1112,26 +1241,34 @@ add_removable_extension (const_rtx expr, rtx_insn > *insn, > rtx reg = XEXP (src, 0); > struct df_link *defs, *def; > ext_cand *cand; > + defs = get_defs (insn, reg, NULL); > > /* Zero-extension of an undefined value is partly defined (it's > completely undefined for sign-extension, though). So if there exists > a path from the entry to this zero-extension that leaves this register > uninitialized, removing the extension could change the behavior of > correct programs. So first, check it is not the case. */ > - if (code == ZERO_EXTEND && !bitmap_bit_p (init_regs, REGNO (reg))) > + if (!defs && abi_extension_candidate_argno_p (code, REGNO (reg))) > { > - if (dump_file) > - { > - fprintf (dump_file, "Cannot eliminate extension:\n"); > - print_rtl_single (dump_file, insn); > - fprintf (dump_file, " because it can operate on uninitialized" > - " data\n"); > - } > + ext_cand e = {expr, code, mode, insn}; > + insn_list->safe_push (e); > return; > } > > + if ((code == ZERO_EXTEND > + && !bitmap_bit_p (init_regs, REGNO (reg)))) > + { > + if (dump_file) > + { > + fprintf (dump_file, "Cannot eliminate extension:\n"); > + print_rtl_single (dump_file, insn); > + fprintf (dump_file, " because it can operate on uninitialized" > + " data\n"); > + } > + return; > + } > + > /* Second, make sure we can get all the reaching definitions. */ > - defs = get_defs (insn, reg, NULL); > if (!defs) > { > if (dump_file) > @@ -1321,7 +1458,8 @@ find_and_remove_re (void) > && (REGNO (SET_DEST (set)) != REGNO (XEXP (SET_SRC (set), 0)))) > { > reinsn_copy_list.safe_push (curr_cand->insn); > - reinsn_copy_list.safe_push (state.defs_list[0]); > + if (state.defs_list.length() != 0) > + reinsn_copy_list.safe_push (state.defs_list[0]); > } > reinsn_del_list.safe_push (curr_cand->insn); > state.modified[INSN_UID (curr_cand->insn)].deleted = 1; > @@ -1342,24 +1480,27 @@ find_and_remove_re (void) > Remember that the memory reference will be changed to refer to the > destination of the extention. So we're actually emitting a copy > from the new destination to the old destination. */ > - for (unsigned int i = 0; i < reinsn_copy_list.length (); i += 2) > - { > - rtx_insn *curr_insn = reinsn_copy_list[i]; > - rtx_insn *def_insn = reinsn_copy_list[i + 1]; > - > - /* Use the mode of the destination of the defining insn > - for the mode of the copy. This is necessary if the > - defining insn was used to eliminate a second extension > - that was wider than the first. */ > - rtx sub_rtx = *get_sub_rtx (def_insn); > - rtx set = single_set (curr_insn); > - rtx new_dst = gen_rtx_REG (GET_MODE (SET_DEST (sub_rtx)), > - REGNO (XEXP (SET_SRC (set), 0))); > - rtx new_src = gen_rtx_REG (GET_MODE (SET_DEST (sub_rtx)), > - REGNO (SET_DEST (set))); > - rtx new_set = gen_rtx_SET (new_dst, new_src); > - emit_insn_after (new_set, def_insn); > - } > + for (unsigned int i = 0; i < reinsn_copy_list.length (); i += 2) > + { > + rtx_insn *curr_insn = reinsn_copy_list[i]; > + > + if ((i+1) < reinsn_copy_list.length()) > + { > + rtx_insn *def_insn = reinsn_copy_list[i + 1]; > + /* Use the mode of the destination of the defining insn > + for the mode of the copy. This is necessary if the > + defining insn was used to eliminate a second extension > + that was wider than the first. */ > + rtx sub_rtx = *get_sub_rtx (def_insn); > + rtx set = single_set (curr_insn); > + rtx new_dst = gen_rtx_REG (GET_MODE (SET_DEST (sub_rtx)), > + REGNO (XEXP (SET_SRC (set), 0))); > + rtx new_src = gen_rtx_REG (GET_MODE (SET_DEST (sub_rtx)), > + REGNO (SET_DEST (set))); > + rtx new_set = gen_rtx_SET (new_dst, new_src); > + emit_insn_after (new_set, def_insn); > + } > + } > > /* Delete all useless extensions here in one sweep. */ > FOR_EACH_VEC_ELT (reinsn_del_list, i, curr_insn) > diff --git a/gcc/testsuite/g++.target/powerpc/zext-elim-3.C > b/gcc/testsuite/g++.target/powerpc/zext-elim-3.C > new file mode 100644 > index 00000000000..5a050df06ff > --- /dev/null > +++ b/gcc/testsuite/g++.target/powerpc/zext-elim-3.C > @@ -0,0 +1,13 @@ > +/* { dg-options "-mcpu=power9 -O2" } */ > + > +void *memset(void *b, int c, unsigned long len) > +{ > + unsigned long i; > + > + for (i = 0; i < len; i++) > + ((unsigned char *)b)[i] = c; > + > + return b; > +} > + > +/* { dg-final { scan-assembler-not "\mrlwinm\M" } } */