[05/11] rtl-ssa: Support for inserting new insns

Message ID ZS7y+bf7KQk28SIH@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
  The upcoming aarch64 load pair pass needs to form store pairs, and can
re-order stores over loads when alias analysis determines this is safe.
In the case that both mem defs have uses in the RTL-SSA IR, and both
stores require re-ordering over their uses, we represent that as
(tentative) deletion of the original store insns and creation of a new
insn, to prevent requiring repeated re-parenting of uses during the
pass.  We then update all mem uses that require re-parenting in one go
at the end of the pass.

To support this, RTL-SSA needs to handle inserting new insns (rather
than just changing existing ones), so this patch adds support for that.

New insns (and new accesses) are temporaries, allocated above a temporary
obstack_watermark, such that the user can easily back out of a change without
awkward bookkeeping.

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

gcc/ChangeLog:

	* rtl-ssa/accesses.cc (function_info::create_set): New.
	* rtl-ssa/accesses.h (access_info::is_temporary): New.
	* rtl-ssa/changes.cc (move_insn): Handle new (temporary) insns.
	(function_info::finalize_new_accesses): Handle new/temporary
	user-created accesses.
	(function_info::apply_changes_to_insn): Ensure m_is_temp flag
	on new insns gets cleared.
	(function_info::change_insns): Handle new/temporary insns.
	(function_info::create_insn): New.
	* rtl-ssa/changes.h (class insn_change): Make function_info a
	friend class.
	* rtl-ssa/functions.h (function_info): Declare new entry points:
	create_set, create_insn.  Declare new change_alloc helper.
	* rtl-ssa/insns.cc (insn_info::print_full): Identify temporary insns in
	dump.
	* rtl-ssa/insns.h (insn_info): Add new m_is_temp flag and accompanying
	is_temporary accessor.
	* rtl-ssa/internals.inl (insn_info::insn_info): Initialize m_is_temp to
	false.
	* rtl-ssa/member-fns.inl (function_info::change_alloc): New.
	* rtl-ssa/movement.h (restrict_movement_for_defs_ignoring): Add
	handling for temporary defs.
---
 gcc/rtl-ssa/accesses.cc    | 10 ++++++
 gcc/rtl-ssa/accesses.h     |  4 +++
 gcc/rtl-ssa/changes.cc     | 73 +++++++++++++++++++++++++++++++-------
 gcc/rtl-ssa/changes.h      |  2 ++
 gcc/rtl-ssa/functions.h    | 14 ++++++++
 gcc/rtl-ssa/insns.cc       |  5 +++
 gcc/rtl-ssa/insns.h        |  7 +++-
 gcc/rtl-ssa/internals.inl  |  1 +
 gcc/rtl-ssa/member-fns.inl | 12 +++++++
 gcc/rtl-ssa/movement.h     |  8 ++++-
 10 files changed, 122 insertions(+), 14 deletions(-)
  

Patch

diff --git a/gcc/rtl-ssa/accesses.cc b/gcc/rtl-ssa/accesses.cc
index 774ab9d99ee..edf8b75f4d6 100644
--- a/gcc/rtl-ssa/accesses.cc
+++ b/gcc/rtl-ssa/accesses.cc
@@ -1398,6 +1398,16 @@  function_info::make_uses_available (obstack_watermark &watermark,
   return use_array (new_uses, num_uses);
 }
 
+set_info *
+function_info::create_set (obstack_watermark &watermark,
+			   insn_info *insn,
+			   resource_info resource)
+{
+  auto set = change_alloc<set_info> (watermark, insn, resource);
+  set->m_is_temp = true;
+  return set;
+}
+
 // Return true if ACCESS1 can represent ACCESS2 and if ACCESS2 can
 // represent ACCESS1.
 static bool
diff --git a/gcc/rtl-ssa/accesses.h b/gcc/rtl-ssa/accesses.h
index fce31d46717..7e7a90ece97 100644
--- a/gcc/rtl-ssa/accesses.h
+++ b/gcc/rtl-ssa/accesses.h
@@ -204,6 +204,10 @@  public:
   // in the main instruction pattern.
   bool only_occurs_in_notes () const { return m_only_occurs_in_notes; }
 
+  // Return true if this is a temporary access, e.g. one created for
+  // an insn that is about to be inserted.
+  bool is_temporary () const { return m_is_temp; }
+
 protected:
   access_info (resource_info, access_kind);
 
diff --git a/gcc/rtl-ssa/changes.cc b/gcc/rtl-ssa/changes.cc
index 523ad60d7d8..b11a88e0919 100644
--- a/gcc/rtl-ssa/changes.cc
+++ b/gcc/rtl-ssa/changes.cc
@@ -345,14 +345,20 @@  move_insn (insn_change &change, insn_info *after)
   // At the moment we don't support moving instructions between EBBs,
   // but this would be worth adding if it's useful.
   insn_info *insn = change.insn ();
-  gcc_assert (after->ebb () == insn->ebb ());
+
   bb_info *bb = after->bb ();
   basic_block cfg_bb = bb->cfg_bb ();
 
-  if (insn->bb () != bb)
-    // Force DF to mark the old block as dirty.
-    df_insn_delete (rtl);
-  ::remove_insn (rtl);
+  if (!insn->is_temporary ())
+    {
+      gcc_assert (after->ebb () == insn->ebb ());
+
+      if (insn->bb () != bb)
+	// Force DF to mark the old block as dirty.
+	df_insn_delete (rtl);
+      ::remove_insn (rtl);
+    }
+
   ::add_insn_after (rtl, after_rtl, cfg_bb);
 }
 
@@ -390,10 +396,15 @@  function_info::finalize_new_accesses (insn_change &change, insn_info *pos)
 	gcc_assert (def);
 	if (def->m_is_temp)
 	  {
-	    // At present, the only temporary instruction definitions we
-	    // create are clobbers, such as those added during recog.
-	    gcc_assert (is_a<clobber_info *> (def));
-	    def = allocate<clobber_info> (change.insn (), ref.regno);
+	    if (is_a<clobber_info *> (def))
+	      def = allocate<clobber_info> (change.insn (), ref.regno);
+	    else if (is_a<set_info *> (def))
+	      {
+		def->m_is_temp = false;
+		def = allocate<set_info> (change.insn (), def->resource ());
+	      }
+	    else
+	      gcc_unreachable ();
 	  }
 	else if (!def->m_has_been_superceded)
 	  {
@@ -452,7 +463,9 @@  function_info::finalize_new_accesses (insn_change &change, insn_info *pos)
   unsigned int i = 0;
   for (use_info *use : change.new_uses)
     {
-      if (!use->m_has_been_superceded)
+      if (use->m_is_temp)
+	use->m_has_been_superceded = true;
+      else if (!use->m_has_been_superceded)
 	{
 	  use = allocate_temp<use_info> (insn, use->resource (), use->def ());
 	  use->m_has_been_superceded = true;
@@ -588,6 +601,7 @@  function_info::apply_changes_to_insn (insn_change &change)
     }
 
   add_reg_unused_notes (insn);
+  insn->m_is_temp = false;
 }
 
 // Add a temporary placeholder instruction after AFTER.
@@ -620,7 +634,8 @@  function_info::change_insns (array_slice<insn_change *> changes)
       if (!change->is_deletion ())
 	{
 	  // Remove any notes that are no longer relevant.
-	  update_notes (change->rtl ());
+	  if (!change->insn ()->m_is_temp)
+	    update_notes (change->rtl ());
 
 	  // Make sure that the placement of this instruction would still
 	  // leave room for previous instructions.
@@ -629,6 +644,17 @@  function_info::change_insns (array_slice<insn_change *> changes)
 	    // verify_insn_changes is supposed to make sure that this holds.
 	    gcc_unreachable ();
 	  min_insn = later_insn (min_insn, change->move_range.first);
+
+	  if (change->insn ()->m_is_temp)
+	    {
+	      change->m_insn = allocate<insn_info> (change->insn ()->bb (),
+						    change->rtl (),
+						    change->insn_uid ());
+
+	      // Set the flag again so subsequent logic is aware.
+	      // It will be cleared later on.
+	      change->m_insn->m_is_temp = true;
+	    }
 	}
     }
 
@@ -721,7 +747,8 @@  function_info::change_insns (array_slice<insn_change *> changes)
 	      // Remove the placeholder first so that we have a wider range of
 	      // program points when inserting INSN.
 	      insn_info *after = placeholder->prev_any_insn ();
-	      remove_insn (insn);
+	      if (!insn->is_temporary ())
+		remove_insn (insn);
 	      remove_insn (placeholder);
 	      insn->set_bb (after->bb ());
 	      add_insn_after (insn, after);
@@ -1034,6 +1061,28 @@  function_info::perform_pending_updates ()
   return changed_cfg;
 }
 
+insn_info *
+function_info::create_insn (obstack_watermark &watermark,
+			    rtx_code insn_code,
+			    rtx pat)
+{
+  rtx_insn *rti = nullptr;
+
+  // TODO: extend, move in to emit-rtl.cc.
+  switch (insn_code)
+    {
+    case INSN:
+      rti = make_insn_raw (pat);
+      break;
+    default:
+      gcc_unreachable ();
+    }
+
+  auto insn = change_alloc<insn_info> (watermark, nullptr, rti, INSN_UID (rti));
+  insn->m_is_temp = true;
+  return insn;
+}
+
 // Print a description of CHANGE to PP.
 void
 rtl_ssa::pp_insn_change (pretty_printer *pp, const insn_change &change)
diff --git a/gcc/rtl-ssa/changes.h b/gcc/rtl-ssa/changes.h
index d56e3a646e2..d91cf432afe 100644
--- a/gcc/rtl-ssa/changes.h
+++ b/gcc/rtl-ssa/changes.h
@@ -32,6 +32,8 @@  namespace rtl_ssa {
 // something that we might do.
 class insn_change
 {
+  friend class function_info;
+
 public:
   enum delete_action { DELETE };
 
diff --git a/gcc/rtl-ssa/functions.h b/gcc/rtl-ssa/functions.h
index 73690a0e63b..c122fe70e42 100644
--- a/gcc/rtl-ssa/functions.h
+++ b/gcc/rtl-ssa/functions.h
@@ -68,6 +68,16 @@  public:
   // Return the SSA information for CFG_BB.
   bb_info *bb (basic_block cfg_bb) const { return m_bbs[cfg_bb->index]; }
 
+  // Create a temporary def.
+  set_info *create_set (obstack_watermark &watermark,
+			insn_info *insn,
+			resource_info resource);
+
+  // Create a temporary insn with code INSN_CODE and pattern PAT.
+  insn_info *create_insn (obstack_watermark &watermark,
+			  rtx_code insn_code,
+			  rtx pat);
+
   // Return a list of all the instructions in the function, in reverse
   // postorder.  The list includes both real and artificial instructions.
   //
@@ -181,6 +191,10 @@  public:
   // Print the contents of the function to PP.
   void print (pretty_printer *pp) const;
 
+  // Allocate an object of type T above the obstack watermark WM.
+  template<typename T, typename... Ts>
+  T *change_alloc (obstack_watermark &wm, Ts... args);
+
 private:
   class bb_phi_info;
   class build_info;
diff --git a/gcc/rtl-ssa/insns.cc b/gcc/rtl-ssa/insns.cc
index f970375d906..f2898859779 100644
--- a/gcc/rtl-ssa/insns.cc
+++ b/gcc/rtl-ssa/insns.cc
@@ -192,6 +192,11 @@  insn_info::print_full (pretty_printer *pp) const
 	      pp_newline_and_indent (pp, 0);
 	      pp_string (pp, "has volatile refs");
 	    }
+	  if (m_is_temp)
+	    {
+	      pp_newline_and_indent (pp, 0);
+	      pp_string (pp, "temporary");
+	    }
 	}
       pp_indentation (pp) -= 2;
     }
diff --git a/gcc/rtl-ssa/insns.h b/gcc/rtl-ssa/insns.h
index a604fe295cd..6d0506706ad 100644
--- a/gcc/rtl-ssa/insns.h
+++ b/gcc/rtl-ssa/insns.h
@@ -306,6 +306,8 @@  public:
   // Print a full description of the instruction.
   void print_full (pretty_printer *) const;
 
+  bool is_temporary () const { return m_is_temp; }
+
 private:
   // The first-order way of representing the order between instructions
   // is to assign "program points", with higher point numbers coming
@@ -414,8 +416,11 @@  private:
   unsigned int m_has_pre_post_modify : 1;
   unsigned int m_has_volatile_refs : 1;
 
+  // Indicates the insn is a temporary / new user-allocated insn.
+  unsigned int m_is_temp : 1;
+
   // For future expansion.
-  unsigned int m_spare : 27;
+  unsigned int m_spare : 26;
 
   // The program point at which the instruction occurs.
   //
diff --git a/gcc/rtl-ssa/internals.inl b/gcc/rtl-ssa/internals.inl
index e49297c12b3..907c4504352 100644
--- a/gcc/rtl-ssa/internals.inl
+++ b/gcc/rtl-ssa/internals.inl
@@ -415,6 +415,7 @@  inline insn_info::insn_info (bb_info *bb, rtx_insn *rtl, int cost_or_uid)
     m_is_asm (false),
     m_has_pre_post_modify (false),
     m_has_volatile_refs (false),
+    m_is_temp (false),
     m_spare (0),
     m_point (0),
     m_cost_or_uid (cost_or_uid),
diff --git a/gcc/rtl-ssa/member-fns.inl b/gcc/rtl-ssa/member-fns.inl
index c127fab8b98..f7185c92751 100644
--- a/gcc/rtl-ssa/member-fns.inl
+++ b/gcc/rtl-ssa/member-fns.inl
@@ -953,4 +953,16 @@  function_info::add_regno_clobber (obstack_watermark &watermark,
   return true;
 }
 
+template<typename T, typename... Ts>
+inline T *
+function_info::change_alloc (obstack_watermark &wm, Ts... args)
+{
+  static_assert (std::is_trivially_destructible<T>::value,
+		 "destructor won't be called");
+  static_assert (alignof (T) <= obstack_alignment,
+		 "too much alignment required");
+  void *addr = XOBNEW (wm, T);
+  return new (addr) T (std::forward<Ts> (args)...);
+}
+
 }
diff --git a/gcc/rtl-ssa/movement.h b/gcc/rtl-ssa/movement.h
index d9945f49172..0e94a9870fc 100644
--- a/gcc/rtl-ssa/movement.h
+++ b/gcc/rtl-ssa/movement.h
@@ -177,6 +177,11 @@  restrict_movement_for_defs_ignoring (insn_range_info &move_range,
 {
   for (def_info *def : defs)
     {
+      // Skip fresh defs that are being inserted, as these shouldn't
+      // constrain movement.
+      if (def->is_temporary ())
+	continue;
+
       // If the definition is a clobber, we can move it with respect
       // to other clobbers.
       //
@@ -242,7 +247,8 @@  restrict_movement_for_defs_ignoring (insn_range_info &move_range,
 
   // Make sure that we don't move stores between basic blocks, since we
   // don't have enough information to tell whether it's safe.
-  if (def_info *def = memory_access (defs))
+  def_info *def = memory_access (defs);
+  if (def && !def->is_temporary ())
     {
       move_range = move_later_than (move_range, def->bb ()->head_insn ());
       move_range = move_earlier_than (move_range, def->bb ()->end_insn ());