[PATCH 2/3] aarch64: Re-parent trailing nondebug base reg uses [PR113089]

[PATCH 2/3] aarch64: Re-parent trailing nondebug base reg uses [PR113089]

[Alex Coplan]

[-- Attachment #1: Type: text/plain, Size: 626 bytes --]

While working on PR113089, I realised we where missing code to re-parent
trailing nondebug uses of the base register in the case of cancelling
writeback in the load/store pair pass.  This patch fixes that.

Bootstrapped/regtested as a series on aarch64-linux-gnu (with/without
the pass enabled), OK for trunk?

Thanks,
Alex

gcc/ChangeLog:

	PR target/113089
	* config/aarch64/aarch64-ldp-fusion.cc (ldp_bb_info::fuse_pair):
	Update trailing nondebug uses of the base register in the case
	of cancelling writeback.
---
 gcc/config/aarch64/aarch64-ldp-fusion.cc | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)


[-- Attachment #2: 0002-aarch64-Re-parent-trailing-nondebug-base-reg-uses-PR.patch --]
[-- Type: text/x-patch, Size: 1360 bytes --]

diff --git a/gcc/config/aarch64/aarch64-ldp-fusion.cc b/gcc/config/aarch64/aarch64-ldp-fusion.cc
index 70b75c668ce..4d7fd72c6b1 100644
--- a/gcc/config/aarch64/aarch64-ldp-fusion.cc
+++ b/gcc/config/aarch64/aarch64-ldp-fusion.cc
@@ -1693,6 +1693,30 @@ ldp_bb_info::fuse_pair (bool load_p,
 
   if (trailing_add)
     changes.safe_push (make_delete (trailing_add));
+  else if ((writeback & 2) && !writeback_effect)
+    {
+      // The second insn initially had writeback but now the pair does not,
+      // need to update any nondebug uses of the base register def in the
+      // second insn.  We'll take care of debug uses later.
+      auto def = find_access (insns[1]->defs (), base_regno);
+      gcc_assert (def);
+      auto set = dyn_cast<set_info *> (def);
+      if (set && set->has_nondebug_uses ())
+	{
+	  auto orig_use = find_access (insns[0]->uses (), base_regno);
+	  for (auto use : set->nondebug_insn_uses ())
+	    {
+	      auto change = make_change (use->insn ());
+	      change->new_uses = check_remove_regno_access (attempt,
+							    change->new_uses,
+							    base_regno);
+	      change->new_uses = insert_access (attempt,
+						orig_use,
+						change->new_uses);
+	      changes.safe_push (change);
+	    }
+	}
+    }
 
   auto is_changing = insn_is_changing (changes);
   for (unsigned i = 0; i < changes.length (); i++)

Re: [PATCH 2/3] aarch64: Re-parent trailing nondebug base reg uses [PR113089]

[Richard Sandiford]

Alex Coplan <alex.coplan@arm.com> writes:
> While working on PR113089, I realised we where missing code to re-parent
> trailing nondebug uses of the base register in the case of cancelling
> writeback in the load/store pair pass.  This patch fixes that.
>
> Bootstrapped/regtested as a series on aarch64-linux-gnu (with/without
> the pass enabled), OK for trunk?
>
> Thanks,
> Alex
>
> gcc/ChangeLog:
>
> 	PR target/113089
> 	* config/aarch64/aarch64-ldp-fusion.cc (ldp_bb_info::fuse_pair):
> 	Update trailing nondebug uses of the base register in the case
> 	of cancelling writeback.

OK, thanks.  I suppose in future it would be good to have a way of
stitching together the use lists of two consecutive defs, since that
should be possible in constant time.  But that's just a possible future
enhancement.  The update as it stands should still be linear, since all
updated uses would come before the next writeback candidate involving
the same base register.

Richard

> ---
>  gcc/config/aarch64/aarch64-ldp-fusion.cc | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
>
> diff --git a/gcc/config/aarch64/aarch64-ldp-fusion.cc b/gcc/config/aarch64/aarch64-ldp-fusion.cc
> index 70b75c668ce..4d7fd72c6b1 100644
> --- a/gcc/config/aarch64/aarch64-ldp-fusion.cc
> +++ b/gcc/config/aarch64/aarch64-ldp-fusion.cc
> @@ -1693,6 +1693,30 @@ ldp_bb_info::fuse_pair (bool load_p,
>  
>    if (trailing_add)
>      changes.safe_push (make_delete (trailing_add));
> +  else if ((writeback & 2) && !writeback_effect)
> +    {
> +      // The second insn initially had writeback but now the pair does not,
> +      // need to update any nondebug uses of the base register def in the
> +      // second insn.  We'll take care of debug uses later.
> +      auto def = find_access (insns[1]->defs (), base_regno);
> +      gcc_assert (def);
> +      auto set = dyn_cast<set_info *> (def);
> +      if (set && set->has_nondebug_uses ())
> +	{
> +	  auto orig_use = find_access (insns[0]->uses (), base_regno);
> +	  for (auto use : set->nondebug_insn_uses ())
> +	    {
> +	      auto change = make_change (use->insn ());
> +	      change->new_uses = check_remove_regno_access (attempt,
> +							    change->new_uses,
> +							    base_regno);
> +	      change->new_uses = insert_access (attempt,
> +						orig_use,
> +						change->new_uses);
> +	      changes.safe_push (change);
> +	    }
> +	}
> +    }
>  
>    auto is_changing = insn_is_changing (changes);
>    for (unsigned i = 0; i < changes.length (); i++)