[PATCH] fwprop: Fix single_use_p calculation

[PATCH] fwprop: Fix single_use_p calculation

[Ilya Leoshkevich]

Bootstrapped and regtested on x86_64-redhat-linux, ppc64le-redhat-linux
and s390x-redhat-linux.  Ok for master?



Commit efb6bc55a93a ("fwprop: Allow (subreg (mem)) simplifications")
introduced a check that was supposed to look at the propagated def's
number of uses.  It uses insn_info::num_uses (), which in reality
returns the number of uses def's insn has.  The whole change therefore
works only by accident.

Fix by looking at def_info's uses instead of insn_info's uses.  This
requires passing around def_info instead of insn_info.

gcc/ChangeLog:

2021-03-02  Ilya Leoshkevich  <iii@linux.ibm.com>

	* fwprop.c (def_has_single_use_p): New function.
	(fwprop_propagation::fwprop_propagation): Look at
	def_info's uses.
	(try_fwprop_subst_note): Use def_info instead of insn_info.
	(try_fwprop_subst_pattern): Likewise.
	(try_fwprop_subst_notes): Likewise.
	(try_fwprop_subst): Likewise.
	(forward_propagate_subreg): Likewise.
	(forward_propagate_and_simplify): Likewise.
	(forward_propagate_into): Likewise.
	* iterator-utils.h (single_element_p): New function.
---
 gcc/fwprop.c         | 89 ++++++++++++++++++++++++++------------------
 gcc/iterator-utils.h | 10 +++++
 2 files changed, 62 insertions(+), 37 deletions(-)

diff --git a/gcc/fwprop.c b/gcc/fwprop.c
index 4b8a554e823..478dcdd96cc 100644
--- a/gcc/fwprop.c
+++ b/gcc/fwprop.c
@@ -175,7 +175,7 @@ namespace
     static const uint16_t CONSTANT = FIRST_SPARE_RESULT << 1;
     static const uint16_t PROFITABLE = FIRST_SPARE_RESULT << 2;
 
-    fwprop_propagation (insn_info *, insn_info *, rtx, rtx);
+    fwprop_propagation (insn_info *, def_info *, rtx, rtx);
 
     bool changed_mem_p () const { return result_flags & CHANGED_MEM; }
     bool folded_to_constants_p () const;
@@ -191,13 +191,27 @@ namespace
   };
 }
 
-/* Prepare to replace FROM with TO in INSN.  */
+/* Return true if DEF has a single non-debug non-phi use.  */
+
+static bool
+def_has_single_use_p (def_info *def)
+{
+  if (!is_a<set_info *> (def))
+    return false;
+
+  set_info *set = as_a<set_info *> (def);
+
+  return single_element_p (set->nondebug_insn_uses ())
+	 && !set->has_phi_uses ();
+}
+
+/* Prepare to replace FROM with TO in USE_INSN.  */
 
 fwprop_propagation::fwprop_propagation (insn_info *use_insn,
-					insn_info *def_insn, rtx from, rtx to)
+					def_info *def, rtx from, rtx to)
   : insn_propagation (use_insn->rtl (), from, to),
-    single_use_p (def_insn->num_uses () == 1),
-    single_ebb_p (use_insn->ebb () == def_insn->ebb ())
+    single_use_p (def_has_single_use_p (def)),
+    single_ebb_p (use_insn->ebb () == def->insn ()->ebb ())
 {
   should_check_mems = true;
   should_note_simplifications = true;
@@ -368,9 +382,9 @@ contains_paradoxical_subreg_p (rtx x)
   return false;
 }
 
-/* Try to substitute (set DEST SRC) from DEF_INSN into note NOTE of USE_INSN.
-   Return the number of substitutions on success, otherwise return -1 and
-   leave USE_INSN unchanged.
+/* Try to substitute (set DEST SRC), which defines DEF, into note NOTE of
+   USE_INSN.  Return the number of substitutions on success, otherwise return
+   -1 and leave USE_INSN unchanged.
 
    If REQUIRE_CONSTANT is true, require all substituted occurences of SRC
    to fold to a constant, so that the note does not use any more registers
@@ -379,13 +393,14 @@ contains_paradoxical_subreg_p (rtx x)
    instruction pattern.  */
 
 static int
-try_fwprop_subst_note (insn_info *use_insn, insn_info *def_insn,
+try_fwprop_subst_note (insn_info *use_insn, def_info *def,
 		       rtx note, rtx dest, rtx src, bool require_constant)
 {
   rtx_insn *use_rtl = use_insn->rtl ();
+  insn_info *def_insn = def->insn ();
 
   insn_change_watermark watermark;
-  fwprop_propagation prop (use_insn, def_insn, dest, src);
+  fwprop_propagation prop (use_insn, def, dest, src);
   if (!prop.apply_to_rvalue (&XEXP (note, 0)))
     {
       if (dump_file && (dump_flags & TDF_DETAILS))
@@ -436,19 +451,20 @@ try_fwprop_subst_note (insn_info *use_insn, insn_info *def_insn,
   return prop.num_replacements;
 }
 
-/* Try to substitute (set DEST SRC) from DEF_INSN into location LOC of
+/* Try to substitute (set DEST SRC), which defines DEF, into location LOC of
    USE_INSN's pattern.  Return true on success, otherwise leave USE_INSN
    unchanged.  */
 
 static bool
 try_fwprop_subst_pattern (obstack_watermark &attempt, insn_change &use_change,
-			  insn_info *def_insn, rtx *loc, rtx dest, rtx src)
+			  def_info *def, rtx *loc, rtx dest, rtx src)
 {
   insn_info *use_insn = use_change.insn ();
   rtx_insn *use_rtl = use_insn->rtl ();
+  insn_info *def_insn = def->insn ();
 
   insn_change_watermark watermark;
-  fwprop_propagation prop (use_insn, def_insn, dest, src);
+  fwprop_propagation prop (use_insn, def, dest, src);
   if (!prop.apply_to_pattern (loc))
     {
       if (dump_file && (dump_flags & TDF_DETAILS))
@@ -538,7 +554,7 @@ try_fwprop_subst_pattern (obstack_watermark &attempt, insn_change &use_change,
     {
       if ((REG_NOTE_KIND (note) == REG_EQUAL
 	   || REG_NOTE_KIND (note) == REG_EQUIV)
-	  && try_fwprop_subst_note (use_insn, def_insn, note,
+	  && try_fwprop_subst_note (use_insn, def, note,
 				    dest, src, false) < 0)
 	{
 	  *note_ptr = XEXP (note, 1);
@@ -554,19 +570,19 @@ try_fwprop_subst_pattern (obstack_watermark &attempt, insn_change &use_change,
   return true;
 }
 
-/* Try to substitute (set DEST SRC) from DEF_INSN into USE_INSN's notes,
+/* Try to substitute (set DEST SRC), which defines DEF, into USE_INSN's notes,
    given that it was not possible to do this for USE_INSN's main pattern.
    Return true on success, otherwise leave USE_INSN unchanged.  */
 
 static bool
-try_fwprop_subst_notes (insn_info *use_insn, insn_info *def_insn,
+try_fwprop_subst_notes (insn_info *use_insn, def_info *def,
 			rtx dest, rtx src)
 {
   rtx_insn *use_rtl = use_insn->rtl ();
   for (rtx note = REG_NOTES (use_rtl); note; note = XEXP (note, 1))
     if ((REG_NOTE_KIND (note) == REG_EQUAL
 	 || REG_NOTE_KIND (note) == REG_EQUIV)
-	&& try_fwprop_subst_note (use_insn, def_insn, note,
+	&& try_fwprop_subst_note (use_insn, def, note,
 				  dest, src, true) > 0)
       {
 	confirm_change_group ();
@@ -576,7 +592,7 @@ try_fwprop_subst_notes (insn_info *use_insn, insn_info *def_insn,
   return false;
 }
 
-/* Check whether we could validly substitute (set DEST SRC) from DEF_INSN
+/* Check whether we could validly substitute (set DEST SRC), which defines DEF,
    into USE.  If so, first try performing the substitution in location LOC
    of USE->insn ()'s pattern.  If that fails, try instead to substitute
    into the notes.
@@ -584,10 +600,11 @@ try_fwprop_subst_notes (insn_info *use_insn, insn_info *def_insn,
    Return true on success, otherwise leave USE_INSN unchanged.  */
 
 static bool
-try_fwprop_subst (use_info *use, insn_info *def_insn,
+try_fwprop_subst (use_info *use, def_info *def,
 		  rtx *loc, rtx dest, rtx src)
 {
   insn_info *use_insn = use->insn ();
+  insn_info *def_insn = def->insn ();
 
   auto attempt = crtl->ssa->new_change_attempt ();
   use_array src_uses = remove_note_accesses (attempt, def_insn->uses ());
@@ -622,9 +639,8 @@ try_fwprop_subst (use_info *use, insn_info *def_insn,
   if (!restrict_movement (use_change))
     return false;
 
-  return (try_fwprop_subst_pattern (attempt, use_change, def_insn,
-				    loc, dest, src)
-	  || try_fwprop_subst_notes (use_insn, def_insn, dest, src));
+  return (try_fwprop_subst_pattern (attempt, use_change, def, loc, dest, src)
+	  || try_fwprop_subst_notes (use_insn, def, dest, src));
 }
 
 /* For the given single_set INSN, containing SRC known to be a
@@ -671,7 +687,7 @@ free_load_extend (rtx src, insn_info *insn)
    in REF.  The other parameters are the same.  */
 
 static bool
-forward_propagate_subreg (use_info *use, insn_info *def_insn,
+forward_propagate_subreg (use_info *use, def_info *def,
 			  rtx dest, rtx src, df_ref ref)
 {
   scalar_int_mode int_use_mode, src_mode;
@@ -697,8 +713,7 @@ forward_propagate_subreg (use_info *use, insn_info *def_insn,
 	  && REGNO (SUBREG_REG (src)) >= FIRST_PSEUDO_REGISTER
 	  && GET_MODE (SUBREG_REG (src)) == use_mode
 	  && subreg_lowpart_p (src))
-	return try_fwprop_subst (use, def_insn, loc,
-				 use_reg, SUBREG_REG (src));
+	return try_fwprop_subst (use, def, loc, use_reg, SUBREG_REG (src));
     }
 
   /* If this is a SUBREG of a ZERO_EXTEND or SIGN_EXTEND, and the SUBREG
@@ -725,22 +740,21 @@ forward_propagate_subreg (use_info *use, insn_info *def_insn,
 	  && REG_P (XEXP (src, 0))
 	  && REGNO (XEXP (src, 0)) >= FIRST_PSEUDO_REGISTER
 	  && GET_MODE (XEXP (src, 0)) == use_mode
-	  && !free_load_extend (src, def_insn)
+	  && !free_load_extend (src, def->insn ())
 	  && (targetm.mode_rep_extended (int_use_mode, src_mode)
 	      != (int) GET_CODE (src)))
-	return try_fwprop_subst (use, def_insn, loc, use_reg, XEXP (src, 0));
+	return try_fwprop_subst (use, def, loc, use_reg, XEXP (src, 0));
     }
 
   return false;
 }
 
-/* Try to substitute (set DEST SRC) from DEF_INSN into USE and simplify
+/* Try to substitute (set DEST SRC), which defines DEF, into USE and simplify
    the result, handling cases where DEST is used in a subreg and where
    applying that subreg to SRC results in a useful simplification.  */
 
 static bool
-forward_propagate_subreg (use_info *use, insn_info *def_insn,
-			  rtx dest, rtx src)
+forward_propagate_subreg (use_info *use, def_info *def, rtx dest, rtx src)
 {
   if (!use->includes_subregs () || !REG_P (dest))
     return false;
@@ -755,26 +769,27 @@ forward_propagate_subreg (use_info *use, insn_info *def_insn,
 
   FOR_EACH_INSN_USE (ref, use_rtl)
     if (DF_REF_REGNO (ref) == use->regno ()
-	&& forward_propagate_subreg (use, def_insn, dest, src, ref))
+	&& forward_propagate_subreg (use, def, dest, src, ref))
       return true;
 
   FOR_EACH_INSN_EQ_USE (ref, use_rtl)
     if (DF_REF_REGNO (ref) == use->regno ()
-	&& forward_propagate_subreg (use, def_insn, dest, src, ref))
+	&& forward_propagate_subreg (use, def, dest, src, ref))
       return true;
 
   return false;
 }
 
-/* Try to substitute (set DEST SRC) from DEF_INSN into USE and
+/* Try to substitute (set DEST SRC), which defines DEF, into USE and
    simplify the result.  */
 
 static bool
-forward_propagate_and_simplify (use_info *use, insn_info *def_insn,
+forward_propagate_and_simplify (use_info *use, def_info *def,
 				rtx dest, rtx src)
 {
   insn_info *use_insn = use->insn ();
   rtx_insn *use_rtl = use_insn->rtl ();
+  insn_info *def_insn = def->insn ();
 
   /* ??? This check seems unnecessary.  We should be able to propagate
      into any kind of instruction, regardless of whether it's a single set.
@@ -820,7 +835,7 @@ forward_propagate_and_simplify (use_info *use, insn_info *def_insn,
   /* ??? Unconditionally propagating into PATTERN would work better
      for instructions that have match_dups.  */
   rtx *loc = need_single_set ? &use_set : &PATTERN (use_rtl);
-  return try_fwprop_subst (use, def_insn, loc, dest, src);
+  return try_fwprop_subst (use, def, loc, dest, src);
 }
 
 /* Given a use USE of an insn, if it has a single reaching
@@ -880,8 +895,8 @@ forward_propagate_into (use_info *use, bool reg_prop_only = false)
       && find_reg_note (use_rtl, REG_NON_LOCAL_GOTO, NULL_RTX))
     return false;
 
-  if (forward_propagate_and_simplify (use, def_insn, dest, src)
-      || forward_propagate_subreg (use, def_insn, dest, src))
+  if (forward_propagate_and_simplify (use, def, dest, src)
+      || forward_propagate_subreg (use, def, dest, src))
     return true;
 
   return false;
diff --git a/gcc/iterator-utils.h b/gcc/iterator-utils.h
index a02dfcc3c8f..01a7d5c947e 100644
--- a/gcc/iterator-utils.h
+++ b/gcc/iterator-utils.h
@@ -200,4 +200,14 @@ list_iterator<T, Next>::operator++ (int)
   return ret;
 }
 
+// Return true if CONTAINER has precisely one element.
+template <typename T>
+bool
+single_element_p (const T &container)
+{
+  auto it = container.begin ();
+  auto end = container.end ();
+  return it != end && ++it == end;
+}
+
 #endif
-- 
2.29.2

Re: [PATCH] fwprop: Fix single_use_p calculation

[Jeff Law]

On 3/2/21 3:37 PM, Ilya Leoshkevich via Gcc-patches wrote:
> Bootstrapped and regtested on x86_64-redhat-linux, ppc64le-redhat-linux
> and s390x-redhat-linux.  Ok for master?
>
>
>
> Commit efb6bc55a93a ("fwprop: Allow (subreg (mem)) simplifications")
> introduced a check that was supposed to look at the propagated def's
> number of uses.  It uses insn_info::num_uses (), which in reality
> returns the number of uses def's insn has.  The whole change therefore
> works only by accident.
>
> Fix by looking at def_info's uses instead of insn_info's uses.  This
> requires passing around def_info instead of insn_info.
>
> gcc/ChangeLog:
>
> 2021-03-02  Ilya Leoshkevich  <iii@linux.ibm.com>
>
> 	* fwprop.c (def_has_single_use_p): New function.
> 	(fwprop_propagation::fwprop_propagation): Look at
> 	def_info's uses.
> 	(try_fwprop_subst_note): Use def_info instead of insn_info.
> 	(try_fwprop_subst_pattern): Likewise.
> 	(try_fwprop_subst_notes): Likewise.
> 	(try_fwprop_subst): Likewise.
> 	(forward_propagate_subreg): Likewise.
> 	(forward_propagate_and_simplify): Likewise.
> 	(forward_propagate_into): Likewise.
> 	* iterator-utils.h (single_element_p): New function.
Given we're well into stage4, I'd recommend deferring to gcc-12 unless
this fixes a code correctness issue.

Jeff

Re: [PATCH] fwprop: Fix single_use_p calculation

[Ilya Leoshkevich]

On Wed, 2021-03-03 at 11:34 -0700, Jeff Law wrote:
> 
> 
> On 3/2/21 3:37 PM, Ilya Leoshkevich via Gcc-patches wrote:
> > Bootstrapped and regtested on x86_64-redhat-linux, ppc64le-redhat-
> > linux
> > and s390x-redhat-linux.  Ok for master?
> > 
> > 
> > 
> > Commit efb6bc55a93a ("fwprop: Allow (subreg (mem))
> > simplifications")
> > introduced a check that was supposed to look at the propagated
> > def's
> > number of uses.  It uses insn_info::num_uses (), which in reality
> > returns the number of uses def's insn has.  The whole change
> > therefore
> > works only by accident.
> > 
> > Fix by looking at def_info's uses instead of insn_info's uses. 
> > This
> > requires passing around def_info instead of insn_info.
> > 
> > gcc/ChangeLog:
> > 
> > 2021-03-02  Ilya Leoshkevich  <iii@linux.ibm.com>
> > 
> >         * fwprop.c (def_has_single_use_p): New function.
> >         (fwprop_propagation::fwprop_propagation): Look at
> >         def_info's uses.
> >         (try_fwprop_subst_note): Use def_info instead of insn_info.
> >         (try_fwprop_subst_pattern): Likewise.
> >         (try_fwprop_subst_notes): Likewise.
> >         (try_fwprop_subst): Likewise.
> >         (forward_propagate_subreg): Likewise.
> >         (forward_propagate_and_simplify): Likewise.
> >         (forward_propagate_into): Likewise.
> >         * iterator-utils.h (single_element_p): New function.
> Given we're well into stage4, I'd recommend deferring to gcc-12
> unless
> this fixes a code correctness issue.
> 
> Jeff
> 

Fortunately the issue here is not a miscompilation, but it's still
a regression: on s390 small functions that use long doubles get
a number of useless load/stores as well as a stack frame, where none
was required before.  Basically, the same issue efb6bc55a93a failed to
fully fix due to the num_uses() / nondebug_insn_uses() mixup.

Re: [PATCH] fwprop: Fix single_use_p calculation

[Stefan Liebler]

On 02/03/2021 23:37, Ilya Leoshkevich wrote:
> Bootstrapped and regtested on x86_64-redhat-linux, ppc64le-redhat-linux
> and s390x-redhat-linux.  Ok for master?
> 
> 
> 
> Commit efb6bc55a93a ("fwprop: Allow (subreg (mem)) simplifications")
> introduced a check that was supposed to look at the propagated def's
> number of uses.  It uses insn_info::num_uses (), which in reality
> returns the number of uses def's insn has.  The whole change therefore
> works only by accident.
> 
> Fix by looking at def_info's uses instead of insn_info's uses.  This
> requires passing around def_info instead of insn_info.
> 

If building glibc on s390x with -march=z14, there is currently an extra
"unneeded" stackframe in e.g. sqrtl, llroundl, lroundl, llrintl, llrint
and roundevenl.

0000000000000000 <__ieee754_sqrtl>:
   0:	b3 c1 00 4f       	ldgr	%f4,%r15
   4:	e3 f0 ff 50 ff 71 	lay	%r15,-176(%r15)
   a:	e3 00 f0 a8 00 20 	cg	%r0,168(%r15)
# The long double argument is loaded ...
  10:	e7 00 30 00 30 06 	vl	%v0,0(%r3),3
# ... stored to extra "unneeded" stackframe ...
  16:	e7 00 f0 a0 30 0e 	vst	%v0,160(%r15),3
# ... and loaded from there into a floating-point-pair
  1c:	68 00 f0 a0       	ld	%f0,160(%r15)
  20:	68 20 f0 a8       	ld	%f2,168(%r15)
  24:	b3 16 00 00       	sqxbr	%f0,%f0
  28:	60 00 20 00       	std	%f0,0(%r2)
  2c:	60 20 20 08       	std	%f2,8(%r2)
  30:	b3 cd 00 f4       	lgdr	%r15,%f4
  34:	07 fe             	br	%r14


With this patch, there is no stackframe in those functions anymore as it
was if build with gcc 10.

0000000000000000 <__ieee754_sqrtl>:
   0:	68 00 30 00       	ld	%f0,0(%r3)
   4:	68 20 30 08       	ld	%f2,8(%r3)
   8:	b3 16 00 00       	sqxbr	%f0,%f0
   c:	60 00 20 00       	std	%f0,0(%r2)
  10:	60 20 20 08       	std	%f2,8(%r2)
  14:	07 fe             	br	%r14


Thanks,
Stefan

Re: [PATCH] fwprop: Fix single_use_p calculation

[Richard Sandiford]

Ilya Leoshkevich <iii@linux.ibm.com> writes:
> Bootstrapped and regtested on x86_64-redhat-linux, ppc64le-redhat-linux
> and s390x-redhat-linux.  Ok for master?

Given what was said downthread, I agree we should fix this for GCC 11.
Sorry for missing this problem in the initial review.

> Commit efb6bc55a93a ("fwprop: Allow (subreg (mem)) simplifications")
> introduced a check that was supposed to look at the propagated def's
> number of uses.  It uses insn_info::num_uses (), which in reality
> returns the number of uses def's insn has.  The whole change therefore
> works only by accident.
>
> Fix by looking at def_info's uses instead of insn_info's uses.  This
> requires passing around def_info instead of insn_info.
>
> gcc/ChangeLog:
>
> 2021-03-02  Ilya Leoshkevich  <iii@linux.ibm.com>
>
> 	* fwprop.c (def_has_single_use_p): New function.
> 	(fwprop_propagation::fwprop_propagation): Look at
> 	def_info's uses.
> 	(try_fwprop_subst_note): Use def_info instead of insn_info.
> 	(try_fwprop_subst_pattern): Likewise.
> 	(try_fwprop_subst_notes): Likewise.
> 	(try_fwprop_subst): Likewise.
> 	(forward_propagate_subreg): Likewise.
> 	(forward_propagate_and_simplify): Likewise.
> 	(forward_propagate_into): Likewise.
> 	* iterator-utils.h (single_element_p): New function.
> ---
>  gcc/fwprop.c         | 89 ++++++++++++++++++++++++++------------------
>  gcc/iterator-utils.h | 10 +++++
>  2 files changed, 62 insertions(+), 37 deletions(-)
>
> diff --git a/gcc/fwprop.c b/gcc/fwprop.c
> index 4b8a554e823..478dcdd96cc 100644
> --- a/gcc/fwprop.c
> +++ b/gcc/fwprop.c
> @@ -175,7 +175,7 @@ namespace
>      static const uint16_t CONSTANT = FIRST_SPARE_RESULT << 1;
>      static const uint16_t PROFITABLE = FIRST_SPARE_RESULT << 2;
>  
> -    fwprop_propagation (insn_info *, insn_info *, rtx, rtx);
> +    fwprop_propagation (insn_info *, def_info *, rtx, rtx);

use->def () returns a set_info *, and since you want set_info stuff,
I think it would probably be better to pass around a set_info * instead.
(Let's keep the variable names the same though.  “def” is still accurate
and IMO the natural choice.)

> @@ -191,13 +191,27 @@ namespace
>    };
>  }
>  
> -/* Prepare to replace FROM with TO in INSN.  */
> +/* Return true if DEF has a single non-debug non-phi use.  */
> +
> +static bool
> +def_has_single_use_p (def_info *def)
> +{
> +  if (!is_a<set_info *> (def))
> +    return false;
> +
> +  set_info *set = as_a<set_info *> (def);
> +
> +  return single_element_p (set->nondebug_insn_uses ())
> +	 && !set->has_phi_uses ();

I think instead we should add:

  // If exactly one nondebug instruction uses the set's result, return
  // the use by that instruction, otherwise return null.
  use_info *single_nondebug_insn_use () const;

  // If there is exactly one nondebug use of the set's result,
  // return that use, otherwise return null.  The use might be in
  // instruction or a phi node.
  use_info *single_nondebug_use () const;

before the declaration of set_info::is_local_to_ebb.

> +}
> +
> +/* Prepare to replace FROM with TO in USE_INSN.  */
>  
>  fwprop_propagation::fwprop_propagation (insn_info *use_insn,
> -					insn_info *def_insn, rtx from, rtx to)
> +					def_info *def, rtx from, rtx to)
>    : insn_propagation (use_insn->rtl (), from, to),
> -    single_use_p (def_insn->num_uses () == 1),
> -    single_ebb_p (use_insn->ebb () == def_insn->ebb ())
> +    single_use_p (def_has_single_use_p (def)),
> +    single_ebb_p (use_insn->ebb () == def->insn ()->ebb ())

Just def->ebb ()

> @@ -538,7 +554,7 @@ try_fwprop_subst_pattern (obstack_watermark &attempt, insn_change &use_change,
>      {
>        if ((REG_NOTE_KIND (note) == REG_EQUAL
>  	   || REG_NOTE_KIND (note) == REG_EQUIV)
> -	  && try_fwprop_subst_note (use_insn, def_insn, note,
> +	  && try_fwprop_subst_note (use_insn, def, note,
>  				    dest, src, false) < 0)

Very minor, sorry, but this now fits on one line.

> @@ -584,10 +600,11 @@ try_fwprop_subst_notes (insn_info *use_insn, insn_info *def_insn,
>     Return true on success, otherwise leave USE_INSN unchanged.  */
>  
>  static bool
> -try_fwprop_subst (use_info *use, insn_info *def_insn,
> +try_fwprop_subst (use_info *use, def_info *def,
>  		  rtx *loc, rtx dest, rtx src)

Same here.

Thanks,
Richard

Re: [PATCH] fwprop: Fix single_use_p calculation

[Ilya Leoshkevich]

On Sun, 2021-03-21 at 13:19 +0000, Richard Sandiford wrote:
> Ilya Leoshkevich <iii@linux.ibm.com> writes:
> > Bootstrapped and regtested on x86_64-redhat-linux, ppc64le-redhat-
> > linux
> > and s390x-redhat-linux.  Ok for master?
> 
> Given what was said downthread, I agree we should fix this for GCC
> 11.
> Sorry for missing this problem in the initial review.
> 
> > Commit efb6bc55a93a ("fwprop: Allow (subreg (mem))
> > simplifications")
> > introduced a check that was supposed to look at the propagated
> > def's
> > number of uses.  It uses insn_info::num_uses (), which in reality
> > returns the number of uses def's insn has.  The whole change
> > therefore
> > works only by accident.
> > 
> > Fix by looking at def_info's uses instead of insn_info's uses. 
> > This
> > requires passing around def_info instead of insn_info.
> > 
> > gcc/ChangeLog:
> > 
> > 2021-03-02  Ilya Leoshkevich  <iii@linux.ibm.com>
> > 
> >         * fwprop.c (def_has_single_use_p): New function.
> >         (fwprop_propagation::fwprop_propagation): Look at
> >         def_info's uses.
> >         (try_fwprop_subst_note): Use def_info instead of insn_info.
> >         (try_fwprop_subst_pattern): Likewise.
> >         (try_fwprop_subst_notes): Likewise.
> >         (try_fwprop_subst): Likewise.
> >         (forward_propagate_subreg): Likewise.
> >         (forward_propagate_and_simplify): Likewise.
> >         (forward_propagate_into): Likewise.
> >         * iterator-utils.h (single_element_p): New function.
> > ---
> >  gcc/fwprop.c         | 89 ++++++++++++++++++++++++++--------------
> > ----
> >  gcc/iterator-utils.h | 10 +++++
> >  2 files changed, 62 insertions(+), 37 deletions(-)
> > 
> > diff --git a/gcc/fwprop.c b/gcc/fwprop.c
> > index 4b8a554e823..478dcdd96cc 100644
> > --- a/gcc/fwprop.c
> > +++ b/gcc/fwprop.c
> > @@ -175,7 +175,7 @@ namespace
> >      static const uint16_t CONSTANT = FIRST_SPARE_RESULT << 1;
> >      static const uint16_t PROFITABLE = FIRST_SPARE_RESULT << 2;
> >  
> > -    fwprop_propagation (insn_info *, insn_info *, rtx, rtx);
> > +    fwprop_propagation (insn_info *, def_info *, rtx, rtx);
> 
> use->def () returns a set_info *, and since you want set_info stuff,
> I think it would probably be better to pass around a set_info *
> instead.
> (Let's keep the variable names the same though.  “def” is still
> accurate
> and IMO the natural choice.)
> 
> > @@ -191,13 +191,27 @@ namespace
> >    };
> >  }
> >  
> > -/* Prepare to replace FROM with TO in INSN.  */
> > +/* Return true if DEF has a single non-debug non-phi use.  */
> > +
> > +static bool
> > +def_has_single_use_p (def_info *def)
> > +{
> > +  if (!is_a<set_info *> (def))
> > +    return false;
> > +
> > +  set_info *set = as_a<set_info *> (def);
> > +
> > +  return single_element_p (set->nondebug_insn_uses ())
> > +        && !set->has_phi_uses ();
> 
> I think instead we should add:
> 
>   // If exactly one nondebug instruction uses the set's result,
> return
>   // the use by that instruction, otherwise return null.
>   use_info *single_nondebug_insn_use () const;
> 
>   // If there is exactly one nondebug use of the set's result,
>   // return that use, otherwise return null.  The use might be in
>   // instruction or a phi node.
>   use_info *single_nondebug_use () const;
> 
> before the declaration of set_info::is_local_to_ebb.
> 
> > +}
> > +
> > +/* Prepare to replace FROM with TO in USE_INSN.  */
> >  
> >  fwprop_propagation::fwprop_propagation (insn_info *use_insn,
> > -                                       insn_info *def_insn, rtx
> > from, rtx to)
> > +                                       def_info *def, rtx from,
> > rtx to)
> >    : insn_propagation (use_insn->rtl (), from, to),
> > -    single_use_p (def_insn->num_uses () == 1),
> > -    single_ebb_p (use_insn->ebb () == def_insn->ebb ())
> > +    single_use_p (def_has_single_use_p (def)),
> > +    single_ebb_p (use_insn->ebb () == def->insn ()->ebb ())
> 
> Just def->ebb ()
> 
> > @@ -538,7 +554,7 @@ try_fwprop_subst_pattern (obstack_watermark
> > &attempt, insn_change &use_change,
> >      {
> >        if ((REG_NOTE_KIND (note) == REG_EQUAL
> >            || REG_NOTE_KIND (note) == REG_EQUIV)
> > -         && try_fwprop_subst_note (use_insn, def_insn, note,
> > +         && try_fwprop_subst_note (use_insn, def, note,
> >                                     dest, src, false) < 0)
> 
> Very minor, sorry, but this now fits on one line.
> 
> > @@ -584,10 +600,11 @@ try_fwprop_subst_notes (insn_info *use_insn,
> > insn_info *def_insn,
> >     Return true on success, otherwise leave USE_INSN unchanged.  */
> >  
> >  static bool
> > -try_fwprop_subst (use_info *use, insn_info *def_insn,
> > +try_fwprop_subst (use_info *use, def_info *def,
> >                   rtx *loc, rtx dest, rtx src)
> 
> Same here.
> 
> Thanks,
> Richard

Thanks for reviewing!  I'm currently regtesting a v2.

One thing though: I don't think we need single_nondebug_use() for this
fix, only single_nondebug_insn_use() - the fwprop check that I'm now
using is def->single_nondebug_insn_use () && !def->has_phi_uses ().

Do you still want me to add single_nondebug_use() for completeness in
this patch, or would it be better to add it later when it's actually
needed?

Re: [PATCH] fwprop: Fix single_use_p calculation

[Richard Sandiford]

Ilya Leoshkevich <iii@linux.ibm.com> writes:
> On Sun, 2021-03-21 at 13:19 +0000, Richard Sandiford wrote:
>> Ilya Leoshkevich <iii@linux.ibm.com> writes:
>> > Bootstrapped and regtested on x86_64-redhat-linux, ppc64le-redhat-
>> > linux
>> > and s390x-redhat-linux.  Ok for master?
>> 
>> Given what was said downthread, I agree we should fix this for GCC
>> 11.
>> Sorry for missing this problem in the initial review.
>> 
>> > Commit efb6bc55a93a ("fwprop: Allow (subreg (mem))
>> > simplifications")
>> > introduced a check that was supposed to look at the propagated
>> > def's
>> > number of uses.  It uses insn_info::num_uses (), which in reality
>> > returns the number of uses def's insn has.  The whole change
>> > therefore
>> > works only by accident.
>> > 
>> > Fix by looking at def_info's uses instead of insn_info's uses. 
>> > This
>> > requires passing around def_info instead of insn_info.
>> > 
>> > gcc/ChangeLog:
>> > 
>> > 2021-03-02  Ilya Leoshkevich  <iii@linux.ibm.com>
>> > 
>> >         * fwprop.c (def_has_single_use_p): New function.
>> >         (fwprop_propagation::fwprop_propagation): Look at
>> >         def_info's uses.
>> >         (try_fwprop_subst_note): Use def_info instead of insn_info.
>> >         (try_fwprop_subst_pattern): Likewise.
>> >         (try_fwprop_subst_notes): Likewise.
>> >         (try_fwprop_subst): Likewise.
>> >         (forward_propagate_subreg): Likewise.
>> >         (forward_propagate_and_simplify): Likewise.
>> >         (forward_propagate_into): Likewise.
>> >         * iterator-utils.h (single_element_p): New function.
>> > ---
>> >  gcc/fwprop.c         | 89 ++++++++++++++++++++++++++--------------
>> > ----
>> >  gcc/iterator-utils.h | 10 +++++
>> >  2 files changed, 62 insertions(+), 37 deletions(-)
>> > 
>> > diff --git a/gcc/fwprop.c b/gcc/fwprop.c
>> > index 4b8a554e823..478dcdd96cc 100644
>> > --- a/gcc/fwprop.c
>> > +++ b/gcc/fwprop.c
>> > @@ -175,7 +175,7 @@ namespace
>> >      static const uint16_t CONSTANT = FIRST_SPARE_RESULT << 1;
>> >      static const uint16_t PROFITABLE = FIRST_SPARE_RESULT << 2;
>> >  
>> > -    fwprop_propagation (insn_info *, insn_info *, rtx, rtx);
>> > +    fwprop_propagation (insn_info *, def_info *, rtx, rtx);
>> 
>> use->def () returns a set_info *, and since you want set_info stuff,
>> I think it would probably be better to pass around a set_info *
>> instead.
>> (Let's keep the variable names the same though.  “def” is still
>> accurate
>> and IMO the natural choice.)
>> 
>> > @@ -191,13 +191,27 @@ namespace
>> >    };
>> >  }
>> >  
>> > -/* Prepare to replace FROM with TO in INSN.  */
>> > +/* Return true if DEF has a single non-debug non-phi use.  */
>> > +
>> > +static bool
>> > +def_has_single_use_p (def_info *def)
>> > +{
>> > +  if (!is_a<set_info *> (def))
>> > +    return false;
>> > +
>> > +  set_info *set = as_a<set_info *> (def);
>> > +
>> > +  return single_element_p (set->nondebug_insn_uses ())
>> > +        && !set->has_phi_uses ();
>> 
>> I think instead we should add:
>> 
>>   // If exactly one nondebug instruction uses the set's result,
>> return
>>   // the use by that instruction, otherwise return null.
>>   use_info *single_nondebug_insn_use () const;
>> 
>>   // If there is exactly one nondebug use of the set's result,
>>   // return that use, otherwise return null.  The use might be in
>>   // instruction or a phi node.
>>   use_info *single_nondebug_use () const;
>> 
>> before the declaration of set_info::is_local_to_ebb.
>> 
>> > +}
>> > +
>> > +/* Prepare to replace FROM with TO in USE_INSN.  */
>> >  
>> >  fwprop_propagation::fwprop_propagation (insn_info *use_insn,
>> > -                                       insn_info *def_insn, rtx
>> > from, rtx to)
>> > +                                       def_info *def, rtx from,
>> > rtx to)
>> >    : insn_propagation (use_insn->rtl (), from, to),
>> > -    single_use_p (def_insn->num_uses () == 1),
>> > -    single_ebb_p (use_insn->ebb () == def_insn->ebb ())
>> > +    single_use_p (def_has_single_use_p (def)),
>> > +    single_ebb_p (use_insn->ebb () == def->insn ()->ebb ())
>> 
>> Just def->ebb ()
>> 
>> > @@ -538,7 +554,7 @@ try_fwprop_subst_pattern (obstack_watermark
>> > &attempt, insn_change &use_change,
>> >      {
>> >        if ((REG_NOTE_KIND (note) == REG_EQUAL
>> >            || REG_NOTE_KIND (note) == REG_EQUIV)
>> > -         && try_fwprop_subst_note (use_insn, def_insn, note,
>> > +         && try_fwprop_subst_note (use_insn, def, note,
>> >                                     dest, src, false) < 0)
>> 
>> Very minor, sorry, but this now fits on one line.
>> 
>> > @@ -584,10 +600,11 @@ try_fwprop_subst_notes (insn_info *use_insn,
>> > insn_info *def_insn,
>> >     Return true on success, otherwise leave USE_INSN unchanged.  */
>> >  
>> >  static bool
>> > -try_fwprop_subst (use_info *use, insn_info *def_insn,
>> > +try_fwprop_subst (use_info *use, def_info *def,
>> >                   rtx *loc, rtx dest, rtx src)
>> 
>> Same here.
>> 
>> Thanks,
>> Richard
>
> Thanks for reviewing!  I'm currently regtesting a v2.
>
> One thing though: I don't think we need single_nondebug_use() for this
> fix, only single_nondebug_insn_use() - the fwprop check that I'm now
> using is def->single_nondebug_insn_use () && !def->has_phi_uses ().
>
> Do you still want me to add single_nondebug_use() for completeness in
> this patch, or would it be better to add it later when it's actually
> needed?

I was thinking that the fwprop.c code would use
def->single_nondebug_use () instead of
def->single_nondebug_insn_use () && !def->has_phi_uses ().
What the fwprop.c code is asking is: ignoring debug instructions,
is this value used only in one place?  That seems like it might
be a common query and so I'd rather have a single function for it.
(It happens that, in this context, we already know that any single
user would be the use that we already looked at.  But that's OK. :-))

I'd suggested adding def->single_nondebug_insn_use () too because
I was imagining that def->single_nondebug_use () would use it
internally.

FWIW, it would also be possible to query directly whether a given
use_info is the only non-debug use, if that's easier/more natural.

Thanks,
Richard

Re: [PATCH] fwprop: Fix single_use_p calculation

[Ilya Leoshkevich]

On Mon, 2021-03-22 at 18:23 +0000, Richard Sandiford wrote:
> Ilya Leoshkevich <iii@linux.ibm.com> writes:

[...]

> > Do you still want me to add single_nondebug_use() for completeness
> > in
> > this patch, or would it be better to add it later when it's
> > actually
> > needed?
> 
> I was thinking that the fwprop.c code would use
> def->single_nondebug_use () instead of
> def->single_nondebug_insn_use () && !def->has_phi_uses ().

But these two are not equivalent, are they?  single_nondebug_use()
that you proposed explicitly allows phis:

  // If there is exactly one nondebug use of the set's result,
  // return that use, otherwise return null.  The use might be in
  // instruction or a phi node.
  use_info *single_nondebug_use () const;

but I don't think we want to propagate into phis here.
Or should the check be a bit bigger, like the following?

use_info *single = def->single_nondebug_use ();
single_use_p = single && !single->is_in_phi ();


[...]

Best regards,
Ilya

Re: [PATCH] fwprop: Fix single_use_p calculation

[Richard Sandiford]

Ilya Leoshkevich <iii@linux.ibm.com> writes:
> On Mon, 2021-03-22 at 18:23 +0000, Richard Sandiford wrote:
>> Ilya Leoshkevich <iii@linux.ibm.com> writes:
>
> [...]
>
>> > Do you still want me to add single_nondebug_use() for completeness
>> > in
>> > this patch, or would it be better to add it later when it's
>> > actually
>> > needed?
>> 
>> I was thinking that the fwprop.c code would use
>> def->single_nondebug_use () instead of
>> def->single_nondebug_insn_use () && !def->has_phi_uses ().
>
> But these two are not equivalent, are they?  single_nondebug_use()
> that you proposed explicitly allows phis:
>
>   // If there is exactly one nondebug use of the set's result,
>   // return that use, otherwise return null.  The use might be in
>   // instruction or a phi node.
>   use_info *single_nondebug_use () const;
>
> but I don't think we want to propagate into phis here.
> Or should the check be a bit bigger, like the following?

But we're in the process of substituting the definition into an
insn use.  So we know that an insn use exists.  I think the
question we're trying to answer is: is this insn use the only
nondebug use?  I'd rather test that with a single accessor rather
than break it down into individual data structure tests.

Thanks,
Richard

Re: [PATCH] fwprop: Fix single_use_p calculation

[Ilya Leoshkevich]

On Mon, 2021-03-22 at 22:55 +0000, Richard Sandiford wrote:
> Ilya Leoshkevich <iii@linux.ibm.com> writes:
> > On Mon, 2021-03-22 at 18:23 +0000, Richard Sandiford wrote:
> > > Ilya Leoshkevich <iii@linux.ibm.com> writes:
> > 
> > [...]
> > 
> > > > Do you still want me to add single_nondebug_use() for
> > > > completeness
> > > > in
> > > > this patch, or would it be better to add it later when it's
> > > > actually
> > > > needed?
> > > 
> > > I was thinking that the fwprop.c code would use
> > > def->single_nondebug_use () instead of
> > > def->single_nondebug_insn_use () && !def->has_phi_uses ().
> > 
> > But these two are not equivalent, are they?  single_nondebug_use()
> > that you proposed explicitly allows phis:
> > 
> >   // If there is exactly one nondebug use of the set's result,
> >   // return that use, otherwise return null.  The use might be in
> >   // instruction or a phi node.
> >   use_info *single_nondebug_use () const;
> > 
> > but I don't think we want to propagate into phis here.
> > Or should the check be a bit bigger, like the following?
> 
> But we're in the process of substituting the definition into an
> insn use.  So we know that an insn use exists.  I think the
> question we're trying to answer is: is this insn use the only
> nondebug use?  I'd rather test that with a single accessor rather
> than break it down into individual data structure tests.

Ah, you are absolutely right - now I get it.  Please ignore the v2
then, I will send a v3.

[PATCH] fwprop: Fix single_use_p calculation

[Ilya Leoshkevich]

Bootstrapped and regtested on x86_64-redhat-linux, ppc64le-redhat-linux
and s390x-redhat-linux.  Ok for master?

v1: https://gcc.gnu.org/pipermail/gcc-patches/2021-March/566127.html
v1 -> v2: Pass a set_info instead of a def_info around.
          Add single_nondebug_insn_use () - maybe this could be improved
          further? [1]
          Simplify def->insn ()->ebb ().
          Improve formatting.

[1] https://gcc.gnu.org/pipermail/gcc-patches/2021-March/567118.html

---

Commit efb6bc55a93a ("fwprop: Allow (subreg (mem)) simplifications")
introduced a check that was supposed to look at the propagated def's
number of uses.  It uses insn_info::num_uses (), which in reality
returns the number of uses def's insn has.  The whole change therefore
works only by accident.

Fix by looking at set_info's uses instead of insn_info's uses.  This
requires passing around set_info instead of insn_info.

gcc/ChangeLog:

2021-03-02  Ilya Leoshkevich  <iii@linux.ibm.com>

	* fwprop.c (fwprop_propagation::fwprop_propagation): Look at
	set_info's uses.
	(try_fwprop_subst_note): Use set_info instead of insn_info.
	(try_fwprop_subst_pattern): Likewise.
	(try_fwprop_subst_notes): Likewise.
	(try_fwprop_subst): Likewise.
	(forward_propagate_subreg): Likewise.
	(forward_propagate_and_simplify): Likewise.
	(forward_propagate_into): Likewise.
	* rtl-ssa/accesses.h (set_info::single_nondebug_insn_use): New
	method.
	* rtl-ssa/member-fns.inl (set_info::single_nondebug_insn_use):
	Likewise.

gcc/testsuite/ChangeLog:

	* gcc.target/s390/vector/long-double-asm-abi.c: New test.
---
 gcc/fwprop.c                                  | 79 +++++++++----------
 gcc/rtl-ssa/accesses.h                        |  4 +
 gcc/rtl-ssa/member-fns.inl                    |  9 +++
 .../s390/vector/long-double-asm-abi.c         | 26 ++++++
 4 files changed, 78 insertions(+), 40 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/s390/vector/long-double-asm-abi.c

diff --git a/gcc/fwprop.c b/gcc/fwprop.c
index 4b8a554e823..6173c9248eb 100644
--- a/gcc/fwprop.c
+++ b/gcc/fwprop.c
@@ -175,7 +175,7 @@ namespace
     static const uint16_t CONSTANT = FIRST_SPARE_RESULT << 1;
     static const uint16_t PROFITABLE = FIRST_SPARE_RESULT << 2;
 
-    fwprop_propagation (insn_info *, insn_info *, rtx, rtx);
+    fwprop_propagation (insn_info *, set_info *, rtx, rtx);
 
     bool changed_mem_p () const { return result_flags & CHANGED_MEM; }
     bool folded_to_constants_p () const;
@@ -191,13 +191,13 @@ namespace
   };
 }
 
-/* Prepare to replace FROM with TO in INSN.  */
+/* Prepare to replace FROM with TO in USE_INSN.  */
 
 fwprop_propagation::fwprop_propagation (insn_info *use_insn,
-					insn_info *def_insn, rtx from, rtx to)
+					set_info *def, rtx from, rtx to)
   : insn_propagation (use_insn->rtl (), from, to),
-    single_use_p (def_insn->num_uses () == 1),
-    single_ebb_p (use_insn->ebb () == def_insn->ebb ())
+    single_use_p (def->single_nondebug_insn_use () && !def->has_phi_uses ()),
+    single_ebb_p (use_insn->ebb () == def->ebb ())
 {
   should_check_mems = true;
   should_note_simplifications = true;
@@ -368,9 +368,9 @@ contains_paradoxical_subreg_p (rtx x)
   return false;
 }
 
-/* Try to substitute (set DEST SRC) from DEF_INSN into note NOTE of USE_INSN.
-   Return the number of substitutions on success, otherwise return -1 and
-   leave USE_INSN unchanged.
+/* Try to substitute (set DEST SRC), which defines DEF, into note NOTE of
+   USE_INSN.  Return the number of substitutions on success, otherwise return
+   -1 and leave USE_INSN unchanged.
 
    If REQUIRE_CONSTANT is true, require all substituted occurences of SRC
    to fold to a constant, so that the note does not use any more registers
@@ -379,13 +379,14 @@ contains_paradoxical_subreg_p (rtx x)
    instruction pattern.  */
 
 static int
-try_fwprop_subst_note (insn_info *use_insn, insn_info *def_insn,
+try_fwprop_subst_note (insn_info *use_insn, set_info *def,
 		       rtx note, rtx dest, rtx src, bool require_constant)
 {
   rtx_insn *use_rtl = use_insn->rtl ();
+  insn_info *def_insn = def->insn ();
 
   insn_change_watermark watermark;
-  fwprop_propagation prop (use_insn, def_insn, dest, src);
+  fwprop_propagation prop (use_insn, def, dest, src);
   if (!prop.apply_to_rvalue (&XEXP (note, 0)))
     {
       if (dump_file && (dump_flags & TDF_DETAILS))
@@ -436,19 +437,20 @@ try_fwprop_subst_note (insn_info *use_insn, insn_info *def_insn,
   return prop.num_replacements;
 }
 
-/* Try to substitute (set DEST SRC) from DEF_INSN into location LOC of
+/* Try to substitute (set DEST SRC), which defines DEF, into location LOC of
    USE_INSN's pattern.  Return true on success, otherwise leave USE_INSN
    unchanged.  */
 
 static bool
 try_fwprop_subst_pattern (obstack_watermark &attempt, insn_change &use_change,
-			  insn_info *def_insn, rtx *loc, rtx dest, rtx src)
+			  set_info *def, rtx *loc, rtx dest, rtx src)
 {
   insn_info *use_insn = use_change.insn ();
   rtx_insn *use_rtl = use_insn->rtl ();
+  insn_info *def_insn = def->insn ();
 
   insn_change_watermark watermark;
-  fwprop_propagation prop (use_insn, def_insn, dest, src);
+  fwprop_propagation prop (use_insn, def, dest, src);
   if (!prop.apply_to_pattern (loc))
     {
       if (dump_file && (dump_flags & TDF_DETAILS))
@@ -538,8 +540,7 @@ try_fwprop_subst_pattern (obstack_watermark &attempt, insn_change &use_change,
     {
       if ((REG_NOTE_KIND (note) == REG_EQUAL
 	   || REG_NOTE_KIND (note) == REG_EQUIV)
-	  && try_fwprop_subst_note (use_insn, def_insn, note,
-				    dest, src, false) < 0)
+	  && try_fwprop_subst_note (use_insn, def, note, dest, src, false) < 0)
 	{
 	  *note_ptr = XEXP (note, 1);
 	  free_EXPR_LIST_node (note);
@@ -554,20 +555,19 @@ try_fwprop_subst_pattern (obstack_watermark &attempt, insn_change &use_change,
   return true;
 }
 
-/* Try to substitute (set DEST SRC) from DEF_INSN into USE_INSN's notes,
+/* Try to substitute (set DEST SRC), which defines DEF, into USE_INSN's notes,
    given that it was not possible to do this for USE_INSN's main pattern.
    Return true on success, otherwise leave USE_INSN unchanged.  */
 
 static bool
-try_fwprop_subst_notes (insn_info *use_insn, insn_info *def_insn,
+try_fwprop_subst_notes (insn_info *use_insn, set_info *def,
 			rtx dest, rtx src)
 {
   rtx_insn *use_rtl = use_insn->rtl ();
   for (rtx note = REG_NOTES (use_rtl); note; note = XEXP (note, 1))
     if ((REG_NOTE_KIND (note) == REG_EQUAL
 	 || REG_NOTE_KIND (note) == REG_EQUIV)
-	&& try_fwprop_subst_note (use_insn, def_insn, note,
-				  dest, src, true) > 0)
+	&& try_fwprop_subst_note (use_insn, def, note, dest, src, true) > 0)
       {
 	confirm_change_group ();
 	return true;
@@ -576,7 +576,7 @@ try_fwprop_subst_notes (insn_info *use_insn, insn_info *def_insn,
   return false;
 }
 
-/* Check whether we could validly substitute (set DEST SRC) from DEF_INSN
+/* Check whether we could validly substitute (set DEST SRC), which defines DEF,
    into USE.  If so, first try performing the substitution in location LOC
    of USE->insn ()'s pattern.  If that fails, try instead to substitute
    into the notes.
@@ -584,10 +584,11 @@ try_fwprop_subst_notes (insn_info *use_insn, insn_info *def_insn,
    Return true on success, otherwise leave USE_INSN unchanged.  */
 
 static bool
-try_fwprop_subst (use_info *use, insn_info *def_insn,
+try_fwprop_subst (use_info *use, set_info *def,
 		  rtx *loc, rtx dest, rtx src)
 {
   insn_info *use_insn = use->insn ();
+  insn_info *def_insn = def->insn ();
 
   auto attempt = crtl->ssa->new_change_attempt ();
   use_array src_uses = remove_note_accesses (attempt, def_insn->uses ());
@@ -622,9 +623,8 @@ try_fwprop_subst (use_info *use, insn_info *def_insn,
   if (!restrict_movement (use_change))
     return false;
 
-  return (try_fwprop_subst_pattern (attempt, use_change, def_insn,
-				    loc, dest, src)
-	  || try_fwprop_subst_notes (use_insn, def_insn, dest, src));
+  return (try_fwprop_subst_pattern (attempt, use_change, def, loc, dest, src)
+	  || try_fwprop_subst_notes (use_insn, def, dest, src));
 }
 
 /* For the given single_set INSN, containing SRC known to be a
@@ -671,7 +671,7 @@ free_load_extend (rtx src, insn_info *insn)
    in REF.  The other parameters are the same.  */
 
 static bool
-forward_propagate_subreg (use_info *use, insn_info *def_insn,
+forward_propagate_subreg (use_info *use, set_info *def,
 			  rtx dest, rtx src, df_ref ref)
 {
   scalar_int_mode int_use_mode, src_mode;
@@ -697,8 +697,7 @@ forward_propagate_subreg (use_info *use, insn_info *def_insn,
 	  && REGNO (SUBREG_REG (src)) >= FIRST_PSEUDO_REGISTER
 	  && GET_MODE (SUBREG_REG (src)) == use_mode
 	  && subreg_lowpart_p (src))
-	return try_fwprop_subst (use, def_insn, loc,
-				 use_reg, SUBREG_REG (src));
+	return try_fwprop_subst (use, def, loc, use_reg, SUBREG_REG (src));
     }
 
   /* If this is a SUBREG of a ZERO_EXTEND or SIGN_EXTEND, and the SUBREG
@@ -725,22 +724,21 @@ forward_propagate_subreg (use_info *use, insn_info *def_insn,
 	  && REG_P (XEXP (src, 0))
 	  && REGNO (XEXP (src, 0)) >= FIRST_PSEUDO_REGISTER
 	  && GET_MODE (XEXP (src, 0)) == use_mode
-	  && !free_load_extend (src, def_insn)
+	  && !free_load_extend (src, def->insn ())
 	  && (targetm.mode_rep_extended (int_use_mode, src_mode)
 	      != (int) GET_CODE (src)))
-	return try_fwprop_subst (use, def_insn, loc, use_reg, XEXP (src, 0));
+	return try_fwprop_subst (use, def, loc, use_reg, XEXP (src, 0));
     }
 
   return false;
 }
 
-/* Try to substitute (set DEST SRC) from DEF_INSN into USE and simplify
+/* Try to substitute (set DEST SRC), which defines DEF, into USE and simplify
    the result, handling cases where DEST is used in a subreg and where
    applying that subreg to SRC results in a useful simplification.  */
 
 static bool
-forward_propagate_subreg (use_info *use, insn_info *def_insn,
-			  rtx dest, rtx src)
+forward_propagate_subreg (use_info *use, set_info *def, rtx dest, rtx src)
 {
   if (!use->includes_subregs () || !REG_P (dest))
     return false;
@@ -755,26 +753,27 @@ forward_propagate_subreg (use_info *use, insn_info *def_insn,
 
   FOR_EACH_INSN_USE (ref, use_rtl)
     if (DF_REF_REGNO (ref) == use->regno ()
-	&& forward_propagate_subreg (use, def_insn, dest, src, ref))
+	&& forward_propagate_subreg (use, def, dest, src, ref))
       return true;
 
   FOR_EACH_INSN_EQ_USE (ref, use_rtl)
     if (DF_REF_REGNO (ref) == use->regno ()
-	&& forward_propagate_subreg (use, def_insn, dest, src, ref))
+	&& forward_propagate_subreg (use, def, dest, src, ref))
       return true;
 
   return false;
 }
 
-/* Try to substitute (set DEST SRC) from DEF_INSN into USE and
+/* Try to substitute (set DEST SRC), which defines DEF, into USE and
    simplify the result.  */
 
 static bool
-forward_propagate_and_simplify (use_info *use, insn_info *def_insn,
+forward_propagate_and_simplify (use_info *use, set_info *def,
 				rtx dest, rtx src)
 {
   insn_info *use_insn = use->insn ();
   rtx_insn *use_rtl = use_insn->rtl ();
+  insn_info *def_insn = def->insn ();
 
   /* ??? This check seems unnecessary.  We should be able to propagate
      into any kind of instruction, regardless of whether it's a single set.
@@ -820,7 +819,7 @@ forward_propagate_and_simplify (use_info *use, insn_info *def_insn,
   /* ??? Unconditionally propagating into PATTERN would work better
      for instructions that have match_dups.  */
   rtx *loc = need_single_set ? &use_set : &PATTERN (use_rtl);
-  return try_fwprop_subst (use, def_insn, loc, dest, src);
+  return try_fwprop_subst (use, def, loc, dest, src);
 }
 
 /* Given a use USE of an insn, if it has a single reaching
@@ -836,7 +835,7 @@ forward_propagate_into (use_info *use, bool reg_prop_only = false)
     return false;
 
   /* Disregard uninitialized uses.  */
-  def_info *def = use->def ();
+  set_info *def = use->def ();
   if (!def)
     return false;
 
@@ -880,8 +879,8 @@ forward_propagate_into (use_info *use, bool reg_prop_only = false)
       && find_reg_note (use_rtl, REG_NON_LOCAL_GOTO, NULL_RTX))
     return false;
 
-  if (forward_propagate_and_simplify (use, def_insn, dest, src)
-      || forward_propagate_subreg (use, def_insn, dest, src))
+  if (forward_propagate_and_simplify (use, def, dest, src)
+      || forward_propagate_subreg (use, def, dest, src))
     return true;
 
   return false;
diff --git a/gcc/rtl-ssa/accesses.h b/gcc/rtl-ssa/accesses.h
index 09ae583f993..ae76162805d 100644
--- a/gcc/rtl-ssa/accesses.h
+++ b/gcc/rtl-ssa/accesses.h
@@ -705,6 +705,10 @@ public:
   // Return true if at least one phi node uses the set's result.
   bool has_phi_uses () const;
 
+  // If exactly one nondebug instruction uses the set's result, return
+  // the use by that instruction, otherwise return null.
+  use_info *single_nondebug_insn_use () const;
+
   // Return true if the set and its uses are contained within a single
   // extended basic block, with the set coming first.  This implies
   // that all uses are by instructions rather than phi nodes.
diff --git a/gcc/rtl-ssa/member-fns.inl b/gcc/rtl-ssa/member-fns.inl
index e1ab7d1ba84..37fffb050ff 100644
--- a/gcc/rtl-ssa/member-fns.inl
+++ b/gcc/rtl-ssa/member-fns.inl
@@ -250,6 +250,15 @@ set_info::has_phi_uses () const
   return m_first_use && m_first_use->last_use ()->is_in_phi ();
 }
 
+inline use_info *
+set_info::single_nondebug_insn_use () const
+{
+  use_info *first = first_nondebug_insn_use ();
+  if (first && !first->next_nondebug_insn_use ())
+    return first;
+  return nullptr;
+}
+
 inline bool
 set_info::is_local_to_ebb () const
 {
diff --git a/gcc/testsuite/gcc.target/s390/vector/long-double-asm-abi.c b/gcc/testsuite/gcc.target/s390/vector/long-double-asm-abi.c
new file mode 100644
index 00000000000..f9f2d1286e2
--- /dev/null
+++ b/gcc/testsuite/gcc.target/s390/vector/long-double-asm-abi.c
@@ -0,0 +1,26 @@
+/* { dg-do compile } */
+/* { dg-options "-O3 -march=z14 -mzarch --save-temps" } */
+/* { dg-do run { target { s390_z14_hw } } } */
+#include <assert.h>
+#include <stdint.h>
+
+__attribute__ ((noipa)) static long double
+xsqrt (long double x)
+{
+  long double res;
+  asm("sqxbr\t%0,%1" : "=f"(res) : "f"(x));
+  return res;
+}
+
+/* Check that the generated code is very small and straightforward.  In
+   particular, there must be no unnecessary copying and no stack frame.  */
+/* { dg-final { scan-assembler {\n\tld\t[^\n]*\n\tld\t[^\n]*\n(#[^\n]*\n)*\tsqxbr\t.*\n(#[^\n]*\n)*\tstd\t[^\n]*\n\tstd\t[^\n]*\n\tbr\t%r14\n} } } */
+
+int
+main (void)
+{
+  long double res, x = 0x1.0000000000001p+0L,
+		   exp = 1.00000000000000011102230246251564788e+0L;
+  res = xsqrt (x);
+  assert (res == exp);
+}
-- 
2.29.2