diff mbox series

[04/11] rtl-ssa: Support inferring uses of mem in change_insns

Message ID ZS7y4oIFl/ju3DZu@arm.com
State Accepted
Headers
Series aarch64: Add new load/store pair fusion pass |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Alex Coplan Oct. 17, 2023, 8:47 p.m. UTC 
  Currently, rtl_ssa::change_insns requires all new uses and defs to be
specified explicitly.  This turns out to be rather inconvenient for
forming load pairs in the new aarch64 load pair pass, as the pass has to
determine which mem def the final load pair consumes, and then obtain or
create a suitable use (i.e. significant bookkeeping, just to keep the
RTL-SSA IR consistent).  It turns out to be much more convenient to
allow change_insns to infer which def is consumed and create a suitable
use of mem itself.  This patch does that.

Bootstrapped/regtested as a series on aarch64-linux-gnu, OK for trunk?

gcc/ChangeLog:

	* rtl-ssa/changes.cc (function_info::finalize_new_accesses): Add new
	parameter to give final insn position, infer use of mem if it isn't
	specified explicitly.
	(function_info::change_insns): Pass down final insn position to
	finalize_new_accesses.
	* rtl-ssa/functions.h: Add parameter to finalize_new_accesses.
---
 gcc/rtl-ssa/changes.cc  | 31 ++++++++++++++++++++++++++++---
 gcc/rtl-ssa/functions.h |  2 +-
 2 files changed, 29 insertions(+), 4 deletions(-)

Comments

Richard Sandiford Oct. 18, 2023, 6 p.m. UTC | #1 
Alex Coplan <alex.coplan@arm.com> writes:
> Currently, rtl_ssa::change_insns requires all new uses and defs to be
> specified explicitly.  This turns out to be rather inconvenient for
> forming load pairs in the new aarch64 load pair pass, as the pass has to
> determine which mem def the final load pair consumes, and then obtain or
> create a suitable use (i.e. significant bookkeeping, just to keep the
> RTL-SSA IR consistent).  It turns out to be much more convenient to
> allow change_insns to infer which def is consumed and create a suitable
> use of mem itself.  This patch does that.
>
> Bootstrapped/regtested as a series on aarch64-linux-gnu, OK for trunk?
>
> gcc/ChangeLog:
>
> 	* rtl-ssa/changes.cc (function_info::finalize_new_accesses): Add new
> 	parameter to give final insn position, infer use of mem if it isn't
> 	specified explicitly.
> 	(function_info::change_insns): Pass down final insn position to
> 	finalize_new_accesses.
> 	* rtl-ssa/functions.h: Add parameter to finalize_new_accesses.

OK, thanks.

Richard

> ---
>  gcc/rtl-ssa/changes.cc  | 31 ++++++++++++++++++++++++++++---
>  gcc/rtl-ssa/functions.h |  2 +-
>  2 files changed, 29 insertions(+), 4 deletions(-)
>
> diff --git a/gcc/rtl-ssa/changes.cc b/gcc/rtl-ssa/changes.cc
> index c48ddd2463c..523ad60d7d8 100644
> --- a/gcc/rtl-ssa/changes.cc
> +++ b/gcc/rtl-ssa/changes.cc
> @@ -370,8 +370,11 @@ update_insn_in_place (insn_change &change)
>  // Finalize the new list of definitions and uses in CHANGE, removing
>  // any uses and definitions that are no longer needed, and converting
>  // pending clobbers into actual definitions.
> +//
> +// POS gives the final position of INSN, which hasn't yet been moved into
> +// place.
>  void
> -function_info::finalize_new_accesses (insn_change &change)
> +function_info::finalize_new_accesses (insn_change &change, insn_info *pos)
>  {
>    insn_info *insn = change.insn ();
>  
> @@ -462,13 +465,34 @@ function_info::finalize_new_accesses (insn_change &change)
>    // Add (possibly temporary) uses to m_temp_uses for each resource.
>    // If there are multiple references to the same resource, aggregate
>    // information in the modes and flags.
> +  use_info *mem_use = nullptr;
>    for (rtx_obj_reference ref : properties.refs ())
>      if (ref.is_read ())
>        {
>  	unsigned int regno = ref.regno;
>  	machine_mode mode = ref.is_reg () ? ref.mode : BLKmode;
>  	use_info *use = find_access (unshared_uses, ref.regno);
> -	gcc_assert (use);
> +	if (!use)
> +	  {
> +	    // For now, we only support inferring uses of mem.
> +	    gcc_assert (regno == MEM_REGNO);
> +
> +	    if (mem_use)
> +	      {
> +		mem_use->record_reference (ref, false);
> +		continue;
> +	      }
> +
> +	    resource_info resource { mode, regno };
> +	    auto def = find_def (resource, pos).prev_def (pos);
> +	    auto set = safe_dyn_cast <set_info *> (def);
> +	    gcc_assert (set);
> +	    mem_use = allocate<use_info> (insn, resource, set);
> +	    mem_use->record_reference (ref, true);
> +	    m_temp_uses.safe_push (mem_use);
> +	    continue;
> +	  }
> +
>  	if (use->m_has_been_superceded)
>  	  {
>  	    // This is the first reference to the resource.
> @@ -656,7 +680,8 @@ function_info::change_insns (array_slice<insn_change *> changes)
>  
>  	  // Finalize the new list of accesses for the change.  Don't install
>  	  // them yet, so that we still have access to the old lists below.
> -	  finalize_new_accesses (change);
> +	  finalize_new_accesses (change,
> +				 placeholder ? placeholder : insn);
>  	}
>        placeholders[i] = placeholder;
>      }
> diff --git a/gcc/rtl-ssa/functions.h b/gcc/rtl-ssa/functions.h
> index d7da9774213..73690a0e63b 100644
> --- a/gcc/rtl-ssa/functions.h
> +++ b/gcc/rtl-ssa/functions.h
> @@ -265,7 +265,7 @@ private:
>  
>    insn_info *add_placeholder_after (insn_info *);
>    void possibly_queue_changes (insn_change &);
> -  void finalize_new_accesses (insn_change &);
> +  void finalize_new_accesses (insn_change &, insn_info *);
>    void apply_changes_to_insn (insn_change &);
>  
>    void init_function_data ();
diff mbox series

Patch

diff --git a/gcc/rtl-ssa/changes.cc b/gcc/rtl-ssa/changes.cc
index c48ddd2463c..523ad60d7d8 100644
--- a/gcc/rtl-ssa/changes.cc
+++ b/gcc/rtl-ssa/changes.cc
@@ -370,8 +370,11 @@  update_insn_in_place (insn_change &change)
 // Finalize the new list of definitions and uses in CHANGE, removing
 // any uses and definitions that are no longer needed, and converting
 // pending clobbers into actual definitions.
+//
+// POS gives the final position of INSN, which hasn't yet been moved into
+// place.
 void
-function_info::finalize_new_accesses (insn_change &change)
+function_info::finalize_new_accesses (insn_change &change, insn_info *pos)
 {
   insn_info *insn = change.insn ();
 
@@ -462,13 +465,34 @@  function_info::finalize_new_accesses (insn_change &change)
   // Add (possibly temporary) uses to m_temp_uses for each resource.
   // If there are multiple references to the same resource, aggregate
   // information in the modes and flags.
+  use_info *mem_use = nullptr;
   for (rtx_obj_reference ref : properties.refs ())
     if (ref.is_read ())
       {
 	unsigned int regno = ref.regno;
 	machine_mode mode = ref.is_reg () ? ref.mode : BLKmode;
 	use_info *use = find_access (unshared_uses, ref.regno);
-	gcc_assert (use);
+	if (!use)
+	  {
+	    // For now, we only support inferring uses of mem.
+	    gcc_assert (regno == MEM_REGNO);
+
+	    if (mem_use)
+	      {
+		mem_use->record_reference (ref, false);
+		continue;
+	      }
+
+	    resource_info resource { mode, regno };
+	    auto def = find_def (resource, pos).prev_def (pos);
+	    auto set = safe_dyn_cast <set_info *> (def);
+	    gcc_assert (set);
+	    mem_use = allocate<use_info> (insn, resource, set);
+	    mem_use->record_reference (ref, true);
+	    m_temp_uses.safe_push (mem_use);
+	    continue;
+	  }
+
 	if (use->m_has_been_superceded)
 	  {
 	    // This is the first reference to the resource.
@@ -656,7 +680,8 @@  function_info::change_insns (array_slice<insn_change *> changes)
 
 	  // Finalize the new list of accesses for the change.  Don't install
 	  // them yet, so that we still have access to the old lists below.
-	  finalize_new_accesses (change);
+	  finalize_new_accesses (change,
+				 placeholder ? placeholder : insn);
 	}
       placeholders[i] = placeholder;
     }
diff --git a/gcc/rtl-ssa/functions.h b/gcc/rtl-ssa/functions.h
index d7da9774213..73690a0e63b 100644
--- a/gcc/rtl-ssa/functions.h
+++ b/gcc/rtl-ssa/functions.h
@@ -265,7 +265,7 @@  private:
 
   insn_info *add_placeholder_after (insn_info *);
   void possibly_queue_changes (insn_change &);
-  void finalize_new_accesses (insn_change &);
+  void finalize_new_accesses (insn_change &, insn_info *);
   void apply_changes_to_insn (insn_change &);
 
   void init_function_data ();