diff mbox series

[01/11] rtl-ssa: Fix bug in function_info::add_insn_after

Message ID ZS7yiPmkNmgUNDrh@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:46 p.m. UTC 
  In the case that !insn->is_debug_insn () && next->is_debug_insn (), this
function was missing an update of the prev pointer on the first nondebug
insn following the sequence of debug insns starting at next.

This can lead to corruption of the insn chain, in that we end up with:

  insn->next_any_insn ()->prev_any_insn () != insn

in this case.  This patch fixes that.

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

gcc/ChangeLog:

	* rtl-ssa/insns.cc (function_info::add_insn_after): Ensure we
	update the prev pointer on the following nondebug insn in the
	case that !insn->is_debug_insn () && next->is_debug_insn ().
---
 gcc/rtl-ssa/insns.cc | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

Comments

Richard Sandiford Oct. 18, 2023, 5:54 p.m. UTC | #1 
Alex Coplan <alex.coplan@arm.com> writes:
> In the case that !insn->is_debug_insn () && next->is_debug_insn (), this
> function was missing an update of the prev pointer on the first nondebug
> insn following the sequence of debug insns starting at next.
>
> This can lead to corruption of the insn chain, in that we end up with:
>
>   insn->next_any_insn ()->prev_any_insn () != insn
>
> in this case.  This patch fixes that.
>
> Bootstrapped/regtested as a series on aarch64-linux-gnu, OK for trunk?
>
> gcc/ChangeLog:
>
> 	* rtl-ssa/insns.cc (function_info::add_insn_after): Ensure we
> 	update the prev pointer on the following nondebug insn in the
> 	case that !insn->is_debug_insn () && next->is_debug_insn ().

OK, thanks.

Richard

> ---
>  gcc/rtl-ssa/insns.cc | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/gcc/rtl-ssa/insns.cc b/gcc/rtl-ssa/insns.cc
> index a0c2fec2b70..f970375d906 100644
> --- a/gcc/rtl-ssa/insns.cc
> +++ b/gcc/rtl-ssa/insns.cc
> @@ -291,9 +291,17 @@ function_info::add_insn_after (insn_info *insn, insn_info *after)
>  	  first->set_last_debug_insn (insn);
>  	}
>        else // !insn->is_debug_insn () && next->is_debug_insn ()
> -	// At present we don't (need to) support inserting a nondebug
> -	// instruction between two existing debug instructions.
> -	gcc_assert (!after->is_debug_insn ());
> +	{
> +	  // At present we don't (need to) support inserting a nondebug
> +	  // instruction between two existing debug instructions.
> +	  gcc_assert (!after->is_debug_insn ());
> +
> +	  // Find the next nondebug insn and update its previous pointer
> +	  // to point to INSN.
> +	  auto next_nondebug = next->last_debug_insn ()->next_any_insn ();
> +	  gcc_checking_assert (!next_nondebug->is_debug_insn ());
> +	  next_nondebug->set_prev_sametype_insn (insn);
> +	}
>  
>        // If AFTER and NEXT are separated by at least two points, we can
>        // use a unique point number for INSN.  Otherwise INSN will have
diff mbox series

Patch

diff --git a/gcc/rtl-ssa/insns.cc b/gcc/rtl-ssa/insns.cc
index a0c2fec2b70..f970375d906 100644
--- a/gcc/rtl-ssa/insns.cc
+++ b/gcc/rtl-ssa/insns.cc
@@ -291,9 +291,17 @@  function_info::add_insn_after (insn_info *insn, insn_info *after)
 	  first->set_last_debug_insn (insn);
 	}
       else // !insn->is_debug_insn () && next->is_debug_insn ()
-	// At present we don't (need to) support inserting a nondebug
-	// instruction between two existing debug instructions.
-	gcc_assert (!after->is_debug_insn ());
+	{
+	  // At present we don't (need to) support inserting a nondebug
+	  // instruction between two existing debug instructions.
+	  gcc_assert (!after->is_debug_insn ());
+
+	  // Find the next nondebug insn and update its previous pointer
+	  // to point to INSN.
+	  auto next_nondebug = next->last_debug_insn ()->next_any_insn ();
+	  gcc_checking_assert (!next_nondebug->is_debug_insn ());
+	  next_nondebug->set_prev_sametype_insn (insn);
+	}
 
       // If AFTER and NEXT are separated by at least two points, we can
       // use a unique point number for INSN.  Otherwise INSN will have