Re: [PATCH V14 4/4] ree: Improve ree pass using defined abi interfaces

2023-10-25 Thread Ajit Agarwal 



On 24/10/23 11:47 pm, Vineet Gupta wrote:
> 
> 
> On 10/24/23 10:03, Ajit Agarwal wrote:
>> Hello Vineet, Jeff and Bernhard:
>>
>> This version 14 of the patch uses abi interfaces to remove zero and sign 
>> extension elimination.
>> This fixes aarch64 regressions failures with aggressive CSE.
> 
> Once again, this information belong between the two "---" lines that you 
> added for v6 and stopped updating.
> 
> And it seems the only code difference between v13 and v14 is
> 
> -  return tgt_mode == mode;
> +  if (tgt_mode == mode)
> +    return true;
> +  else
> +    return false;
> 
> How does that make any difference ?

In V14 of the patch I reverted the CSE done v13 of the patch.
This is because I got a mail from Linaro with Linaro regressions fails. 
Then I got a sorry mail saying there were some errands at there end and ask me 
to ignore.

Please review the V13 of the patch with CSE'd and please let me know if this 
okay for trunk.

Thanks & Regards
Ajit


> 
> -Vineet
> 
>>
>> Bootstrapped and regtested on powerpc-linux-gnu.
>>
>> In this version (version 14) of the patch following review comments are 
>> incorporated.
>>
>> a) Removal of hard code zero_extend and sign_extend  in abi interfaces.
>> b) Source and destination with different registers are considered.
>> c) Further enhancements.
>> d) Added sign extension elimination using abi interfaces.
>> d) Addressed remaining review comments from Vineet.
>> e) Addressed review comments from Bernhard.
>> f) Fix aarch64 regressions failure.
>>
>> Please let me know if there is anything missing in this patch.
>>
>> Ok for trunk?
>>
>> Thanks & Regards
>> Ajit
>>
>> ree: Improve ree pass using defined abi interfaces
>>
>> For rs6000 target we see zero and sign extend with missing
>> definitions. Improved to eliminate such zero and sign extension
>> using defined ABI interfaces.
>>
>> 2023-10-24  Ajit Kumar Agarwal  
>>
>> gcc/ChangeLog:
>>
>>  * ree.cc (combine_reaching_defs): Eliminate zero_extend and 
>> sign_extend
>>  using 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): New function.
>>  (abi_target_promote_function_mode): New function.
>>
>> gcc/testsuite/ChangeLog:
>>
>>  * g++.target/powerpc/zext-elim-3.C
>> ---
>> changes since v6:
>>    - Added missing abi interfaces.
>>    - Rearranging and restructuring the code.
>>    - Removal of hard coded zero extend and sign extend in abi interfaces.
>>    - Relaxed different registers with source and destination in abi 
>> interfaces.
>>    - Using CSE in abi interfaces.
>>    - Fix aarch64 regressions.
>>    - Add Sign extension removal in abi interfaces.
>>    - Modified comments as per coding convention.
>>    - Modified code as per coding convention.
>>    - Fix bug bootstrapping RISCV failures.
>> ---
>>   gcc/ree.cc    | 147 +-
>>   .../g++.target/powerpc/zext-elim-3.C  |  13 ++
>>   2 files changed, 154 insertions(+), 6 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..f557b49b366 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, false otherwise.  
>> */
>> +
>> +static bool
>> +abi_target_promote_function_mode (machine_mode mode)
>> +{
>> +  int unsignedp;
>> +  machine_mode tgt_mode
>> +    = targetm.calls.promote_function_mode (NULL_TREE, mode, ,
>> +   NULL_TREE, 1);
>> +
>> +  if (tgt_mode == mode)
>> +    return true;
>> +  else
>> +    return false;
>> +}
>> +
>> +/* Return TRUE if regno is a return register.  */
>> +
>> +static inline bool
>> +abi_extension_candidate_return_reg_p (int regno)
>> +{
>> +  if (targetm.calls.function_value_regno_p (regno))
>> +    return true;
>> +
>> +  return false;
>> +}
>> +
>> +/* Return TRUE if the following conditions are satisfied.
>> +
>> +  a) reg source operand is argument register and not return register.
>> +  b) mode of source and destination operand are different.
>> +  c) if not promoted REGNO of source and destination operand are same.  */
>> +
>> +static bool
>> +abi_extension_candidate_p (rtx_insn *insn)
>> +{
>> +  rtx set = single_set (insn);
>> +  machine_mode dst_mode = GET_MODE (SET_DEST (set));
>> +  rtx orig_src = XEXP (SET_SRC (set), 0);
>> +
>> +  if

Re: [PATCH V14 4/4] ree: Improve ree pass using defined abi interfaces

2023-10-24 Thread Vineet Gupta 




On 10/24/23 10:03, Ajit Agarwal wrote:

Hello Vineet, Jeff and Bernhard:

This version 14 of the patch uses abi interfaces to remove zero and sign 
extension elimination.
This fixes aarch64 regressions failures with aggressive CSE.


Once again, this information belong between the two "---" lines that you 
added for v6 and stopped updating.


And it seems the only code difference between v13 and v14 is

-  return tgt_mode == mode;
+  if (tgt_mode == mode)
+    return true;
+  else
+    return false;

How does that make any difference ?

-Vineet



Bootstrapped and regtested on powerpc-linux-gnu.

In this version (version 14) of the patch following review comments are 
incorporated.

a) Removal of hard code zero_extend and sign_extend  in abi interfaces.
b) Source and destination with different registers are considered.
c) Further enhancements.
d) Added sign extension elimination using abi interfaces.
d) Addressed remaining review comments from Vineet.
e) Addressed review comments from Bernhard.
f) Fix aarch64 regressions failure.

Please let me know if there is anything missing in this patch.

Ok for trunk?

Thanks & Regards
Ajit

ree: Improve ree pass using defined abi interfaces

For rs6000 target we see zero and sign extend with missing
definitions. Improved to eliminate such zero and sign extension
using defined ABI interfaces.

2023-10-24  Ajit Kumar Agarwal  

gcc/ChangeLog:

 * ree.cc (combine_reaching_defs): Eliminate zero_extend and sign_extend
 using 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): New function.
 (abi_target_promote_function_mode): New function.

gcc/testsuite/ChangeLog:

 * g++.target/powerpc/zext-elim-3.C
---
changes since v6:
   - Added missing abi interfaces.
   - Rearranging and restructuring the code.
   - Removal of hard coded zero extend and sign extend in abi interfaces.
   - Relaxed different registers with source and destination in abi interfaces.
   - Using CSE in abi interfaces.
   - Fix aarch64 regressions.
   - Add Sign extension removal in abi interfaces.
   - Modified comments as per coding convention.
   - Modified code as per coding convention.
   - Fix bug bootstrapping RISCV failures.
---
  gcc/ree.cc| 147 +-
  .../g++.target/powerpc/zext-elim-3.C  |  13 ++
  2 files changed, 154 insertions(+), 6 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..f557b49b366 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, false otherwise.  */

+
+static bool
+abi_target_promote_function_mode (machine_mode mode)
+{
+  int unsignedp;
+  machine_mode tgt_mode
+= targetm.calls.promote_function_mode (NULL_TREE, mode, ,
+  NULL_TREE, 1);
+
+  if (tgt_mode == mode)
+return true;
+  else
+return false;
+}
+
+/* Return TRUE if regno is a return register.  */
+
+static inline bool
+abi_extension_candidate_return_reg_p (int regno)
+{
+  if (targetm.calls.function_value_regno_p (regno))
+return true;
+
+  return false;
+}
+
+/* Return TRUE if the following conditions are satisfied.
+
+  a) reg source operand is argument register and not return register.
+  b) mode of source and destination operand are different.
+  c) if not promoted REGNO of source and destination operand are same.  */
+
+static bool
+abi_extension_candidate_p (rtx_insn *insn)
+{
+  rtx set = single_set (insn);
+  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 (REGNO (orig_src)))
+return false;
+
+  /* Return FALSE if mode of destination and source is same.  */
+  if (dst_mode == GET_MODE (orig_src))
+return false;
+
+  machine_mode mode = GET_MODE (XEXP (SET_SRC (set), 0));
+  bool promote_p = abi_target_promote_function_mode (mode);
+
+  /* Return FALSE if promote is false and REGNO of source and destination
+ is different.  */
+  if (!promote_p && REGNO (SET_DEST (set)) != REGNO (orig_src))
+return false;
+
+  return true;
+}
+
+/* Return TRUE if regno is an argument register.  */
+
+static inline bool
+abi_extension_candidate_argno_p (int regno)
+{
+  return FUNCTION_ARG_REGNO_P (regno);
+}
+
+/* Return