On 19/09/23 2:36 pm, Xi Ruoyao wrote:
> On Tue, 2023-09-19 at 14:29 +0530, Ajit Agarwal wrote:
>> This new version of patch 7 use improve ree pass for rs6000 target
>> using defined ABI interfaces.
>> Bootstrapped and regtested on power64-linux-gnu.
> 
> You should drop the "4/4" in subject if it does not depends on other
> non-committed patches.  Otherwise you should send all the non-committed
> dependencies in a series.
> 
> If you just have the 4/4 there without {1..3}/4, people will believe
> this is an incomplete patch submission and likely ignore it.
> 

There are 4 oatches that are already under review and all the provious
patches are under review.

I will send 3/4 patches again for review.

Thanks & Regards
Ajit
>> Review comments incorporated.
>>
>> Thanks & Regards
>> Ajit
>>
>> ree: 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-09-19  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                                    | 148
>> +++++++++++++++++-
>>  .../g++.target/powerpc/zext-elim-3.C          |  13 ++
>>  2 files changed, 158 insertions(+), 3 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..79fc54f38a3 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,122 @@ 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
>> +   a 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 (targetm.calls.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 dst_mode = GET_MODE (SET_DEST (set));
>> +  rtx orig_src = XEXP (SET_SRC (set), 0);
>> +
>> +  if (!FUNCTION_ARG_REGNO_P (REGNO (orig_src))
>> +      || abi_extension_candidate_return_reg_p (insn, REGNO
>> (orig_src)))
>> +    return false;
>> +
>> +  /* Mode of destination and source of zero_extend should be
>> different.  */
>> +  if (dst_mode == GET_MODE (orig_src))
>> +    return false;
>> +
>> +  /* REGNO of source and destination of zero_extend should be same. 
>> */
>> +  if (REGNO (SET_DEST (set)) != REGNO (orig_src))
>> +    return false;
>> +
>> +  return true;
>> +}
>> +
>> +/* 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 && code != SIGN_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 +887,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 +1158,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,6 +1243,7 @@ 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
>> @@ -1131,9 +1263,14 @@ add_removable_extension (const_rtx expr,
>> rtx_insn *insn,
>>      }
>>  
>>        /* Second, make sure we can get all the reaching definitions. 
>> */
>> -      defs = get_defs (insn, reg, NULL);
>>        if (!defs)
>>      {
>> +      if (abi_extension_candidate_argno_p (code, REGNO (reg)))
>> +        {
>> +          ext_cand e = {expr, code, mode, insn};
>> +          insn_list->safe_push (e);
>> +          return;
>> +        }
>>        if (dump_file)
>>          {
>>            fprintf (dump_file, "Cannot eliminate extension:\n");
>> @@ -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;
>> @@ -1345,6 +1483,10 @@ find_and_remove_re (void)
>>    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 ())
>> +    continue;
>> +
>>        rtx_insn *def_insn = reinsn_copy_list[i + 1];
>>  
>>        /* Use the mode of the destination of the defining 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" } } */
> 

Reply via email to