[1/2] sccopy: Handle ssa info for copies in sccopy

Message ID 20260603020635.2641527-1-andrew.pinski@oss.qualcomm.com
State Committed
Headers
Series [1/2] sccopy: Handle ssa info for copies in sccopy |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Build passed

Commit Message

Andrew Pinski June 3, 2026, 2:06 a.m. UTC
  Currently if the ssa info (range or aliasing) is different
between the src and dest of a copy, sccopy will reject it.
This fixes the FIXME and handles it in a similar way as copyprop
handles it.  This is needed to able to move sccopy earlier (before phiopt1)
and to act like a copyprop pass.

Bootstrapped and tested on x86_64-linux-gnu with no regressions.

gcc/ChangeLog:

	* gimple-ssa-sccopy.cc (stmt_may_generate_copy): Remove restriction
	on the ssa info being different for the "copy".
	(scc_copy_prop::replace_scc_by_value): Call maybe_duplicate_ssa_info_at_copy
	when copyproping a ssa name.

Signed-off-by: Andrew Pinski <andrew.pinski@oss.qualcomm.com>
---
 gcc/gimple-ssa-sccopy.cc | 18 ++----------------
 1 file changed, 2 insertions(+), 16 deletions(-)
  

Comments

Filip Kastl June 3, 2026, 7:30 a.m. UTC | #1
Hi.

Thanks for doing this!  This was on my TODO list for some time.  I didn't
realize it was as simple as this.


Just something I've noticed: When both lhs and rhs have alignment/range info,
maybe_duplicate_ssa_info_at_copy doesn't do anything.  So we lose any
additional info asociated with lhs.  I assume that this also happens in other
copy propagation passes.  I wonder if we could perhaps intersect the infos in
that case.

But then I guess a situation like this could happen:

unsigned x, y;
x = getchar();
if (x <= 4)
    error();
y = x;
if (y <= 4)
    <do something>
<never use y again>

GCC maybe computes range of y to be [0,4]?  Then if we copy propagate y = x
and intersected the range information, we would get that range of x is [0,4]
but that is not true and will lead to a miscompilation.

Filip

On Tue 2026-06-02 19:06:34, Andrew Pinski wrote:
> Currently if the ssa info (range or aliasing) is different
> between the src and dest of a copy, sccopy will reject it.
> This fixes the FIXME and handles it in a similar way as copyprop
> handles it.  This is needed to able to move sccopy earlier (before phiopt1)
> and to act like a copyprop pass.
> 
> Bootstrapped and tested on x86_64-linux-gnu with no regressions.
> 
> gcc/ChangeLog:
> 
> 	* gimple-ssa-sccopy.cc (stmt_may_generate_copy): Remove restriction
> 	on the ssa info being different for the "copy".
> 	(scc_copy_prop::replace_scc_by_value): Call maybe_duplicate_ssa_info_at_copy
> 	when copyproping a ssa name.
> 
> Signed-off-by: Andrew Pinski <andrew.pinski@oss.qualcomm.com>
> ---
>  gcc/gimple-ssa-sccopy.cc | 18 ++----------------
>  1 file changed, 2 insertions(+), 16 deletions(-)
> 
> diff --git a/gcc/gimple-ssa-sccopy.cc b/gcc/gimple-ssa-sccopy.cc
> index 0badfb11b93..4252e5295df 100644
> --- a/gcc/gimple-ssa-sccopy.cc
> +++ b/gcc/gimple-ssa-sccopy.cc
> @@ -405,22 +405,6 @@ stmt_may_generate_copy (gimple *stmt)
>    if (SSA_NAME_OCCURS_IN_ABNORMAL_PHI (rhs))
>      return false;
>  
> -  /* It is possible that lhs has more alignment or value range information.  By
> -     propagating we would lose this information.  So in the case that alignment
> -     or value range information differs, we are conservative and do not
> -     propagate.
> -
> -     FIXME: Propagate alignment and value range info the same way copy-prop
> -     does.  */
> -  if (POINTER_TYPE_P (TREE_TYPE (lhs))
> -      && POINTER_TYPE_P (TREE_TYPE (rhs))
> -      && SSA_NAME_PTR_INFO (lhs) != SSA_NAME_PTR_INFO (rhs))
> -    return false;
> -  if (!POINTER_TYPE_P (TREE_TYPE (lhs))
> -      && !POINTER_TYPE_P (TREE_TYPE (rhs))
> -      && SSA_NAME_RANGE_INFO (lhs) != SSA_NAME_RANGE_INFO (rhs))
> -    return false;
> -
>    return true;  /* A statement of type _2 = _1;.  */
>  }
>  
> @@ -497,6 +481,8 @@ scc_copy_prop::replace_scc_by_value (vec<gimple *> scc, tree val)
>  	  
>  	}
>        replace_uses_by (name, val);
> +      if (TREE_CODE (val) == SSA_NAME)
> +        maybe_duplicate_ssa_info_at_copy (name, val);
>        bitmap_set_bit (dead_stmts, SSA_NAME_VERSION (name));
>        didsomething = true;
>      }
> -- 
> 2.43.0
>
  
Andrew Pinski June 3, 2026, 7:43 a.m. UTC | #2
On Wed, Jun 3, 2026 at 12:31 AM Filip Kastl <fkastl@suse.cz> wrote:
>
> Hi.
>
> Thanks for doing this!  This was on my TODO list for some time.  I didn't
> realize it was as simple as this.
>
>
> Just something I've noticed: When both lhs and rhs have alignment/range info,
> maybe_duplicate_ssa_info_at_copy doesn't do anything.  So we lose any
> additional info asociated with lhs.  I assume that this also happens in other
> copy propagation passes.  I wonder if we could perhaps intersect the infos in
> that case.

Yes, that is correct. In fact I looked into copyprop to see what was
done and maybe_duplicate_ssa_info_at_copy was the only thing used.
forwprop does that only too (see fwprop_set_lattice_val).

>
> But then I guess a situation like this could happen:
>
> unsigned x, y;
> x = getchar();
> if (x <= 4)
>     error();
> y = x;
> if (y <= 4)
>     <do something>
> <never use y again>
>
> GCC maybe computes range of y to be [0,4]?  Then if we copy propagate y = x
> and intersected the range information, we would get that range of x is [0,4]
> but that is not true and will lead to a miscompilation.

Right copying that would be bad news.
Anyways with more and more passes using the ranger, global range
information is going to be less useful.

Thanks,
Andrea

>
> Filip
>
> On Tue 2026-06-02 19:06:34, Andrew Pinski wrote:
> > Currently if the ssa info (range or aliasing) is different
> > between the src and dest of a copy, sccopy will reject it.
> > This fixes the FIXME and handles it in a similar way as copyprop
> > handles it.  This is needed to able to move sccopy earlier (before phiopt1)
> > and to act like a copyprop pass.
> >
> > Bootstrapped and tested on x86_64-linux-gnu with no regressions.
> >
> > gcc/ChangeLog:
> >
> >       * gimple-ssa-sccopy.cc (stmt_may_generate_copy): Remove restriction
> >       on the ssa info being different for the "copy".
> >       (scc_copy_prop::replace_scc_by_value): Call maybe_duplicate_ssa_info_at_copy
> >       when copyproping a ssa name.
> >
> > Signed-off-by: Andrew Pinski <andrew.pinski@oss.qualcomm.com>
> > ---
> >  gcc/gimple-ssa-sccopy.cc | 18 ++----------------
> >  1 file changed, 2 insertions(+), 16 deletions(-)
> >
> > diff --git a/gcc/gimple-ssa-sccopy.cc b/gcc/gimple-ssa-sccopy.cc
> > index 0badfb11b93..4252e5295df 100644
> > --- a/gcc/gimple-ssa-sccopy.cc
> > +++ b/gcc/gimple-ssa-sccopy.cc
> > @@ -405,22 +405,6 @@ stmt_may_generate_copy (gimple *stmt)
> >    if (SSA_NAME_OCCURS_IN_ABNORMAL_PHI (rhs))
> >      return false;
> >
> > -  /* It is possible that lhs has more alignment or value range information.  By
> > -     propagating we would lose this information.  So in the case that alignment
> > -     or value range information differs, we are conservative and do not
> > -     propagate.
> > -
> > -     FIXME: Propagate alignment and value range info the same way copy-prop
> > -     does.  */
> > -  if (POINTER_TYPE_P (TREE_TYPE (lhs))
> > -      && POINTER_TYPE_P (TREE_TYPE (rhs))
> > -      && SSA_NAME_PTR_INFO (lhs) != SSA_NAME_PTR_INFO (rhs))
> > -    return false;
> > -  if (!POINTER_TYPE_P (TREE_TYPE (lhs))
> > -      && !POINTER_TYPE_P (TREE_TYPE (rhs))
> > -      && SSA_NAME_RANGE_INFO (lhs) != SSA_NAME_RANGE_INFO (rhs))
> > -    return false;
> > -
> >    return true;  /* A statement of type _2 = _1;.  */
> >  }
> >
> > @@ -497,6 +481,8 @@ scc_copy_prop::replace_scc_by_value (vec<gimple *> scc, tree val)
> >
> >       }
> >        replace_uses_by (name, val);
> > +      if (TREE_CODE (val) == SSA_NAME)
> > +        maybe_duplicate_ssa_info_at_copy (name, val);
> >        bitmap_set_bit (dead_stmts, SSA_NAME_VERSION (name));
> >        didsomething = true;
> >      }
> > --
> > 2.43.0
> >
  
Richard Biener June 3, 2026, 10:38 a.m. UTC | #3
On Wed, Jun 3, 2026 at 4:16 AM Andrew Pinski
<andrew.pinski@oss.qualcomm.com> wrote:
>
> Currently if the ssa info (range or aliasing) is different
> between the src and dest of a copy, sccopy will reject it.
> This fixes the FIXME and handles it in a similar way as copyprop
> handles it.  This is needed to able to move sccopy earlier (before phiopt1)
> and to act like a copyprop pass.
>
> Bootstrapped and tested on x86_64-linux-gnu with no regressions.

OK.

> gcc/ChangeLog:
>
>         * gimple-ssa-sccopy.cc (stmt_may_generate_copy): Remove restriction
>         on the ssa info being different for the "copy".
>         (scc_copy_prop::replace_scc_by_value): Call maybe_duplicate_ssa_info_at_copy
>         when copyproping a ssa name.
>
> Signed-off-by: Andrew Pinski <andrew.pinski@oss.qualcomm.com>
> ---
>  gcc/gimple-ssa-sccopy.cc | 18 ++----------------
>  1 file changed, 2 insertions(+), 16 deletions(-)
>
> diff --git a/gcc/gimple-ssa-sccopy.cc b/gcc/gimple-ssa-sccopy.cc
> index 0badfb11b93..4252e5295df 100644
> --- a/gcc/gimple-ssa-sccopy.cc
> +++ b/gcc/gimple-ssa-sccopy.cc
> @@ -405,22 +405,6 @@ stmt_may_generate_copy (gimple *stmt)
>    if (SSA_NAME_OCCURS_IN_ABNORMAL_PHI (rhs))
>      return false;
>
> -  /* It is possible that lhs has more alignment or value range information.  By
> -     propagating we would lose this information.  So in the case that alignment
> -     or value range information differs, we are conservative and do not
> -     propagate.
> -
> -     FIXME: Propagate alignment and value range info the same way copy-prop
> -     does.  */
> -  if (POINTER_TYPE_P (TREE_TYPE (lhs))
> -      && POINTER_TYPE_P (TREE_TYPE (rhs))
> -      && SSA_NAME_PTR_INFO (lhs) != SSA_NAME_PTR_INFO (rhs))
> -    return false;
> -  if (!POINTER_TYPE_P (TREE_TYPE (lhs))
> -      && !POINTER_TYPE_P (TREE_TYPE (rhs))
> -      && SSA_NAME_RANGE_INFO (lhs) != SSA_NAME_RANGE_INFO (rhs))
> -    return false;
> -
>    return true;  /* A statement of type _2 = _1;.  */
>  }
>
> @@ -497,6 +481,8 @@ scc_copy_prop::replace_scc_by_value (vec<gimple *> scc, tree val)
>
>         }
>        replace_uses_by (name, val);
> +      if (TREE_CODE (val) == SSA_NAME)
> +        maybe_duplicate_ssa_info_at_copy (name, val);
>        bitmap_set_bit (dead_stmts, SSA_NAME_VERSION (name));
>        didsomething = true;
>      }
> --
> 2.43.0
>
  

Patch

diff --git a/gcc/gimple-ssa-sccopy.cc b/gcc/gimple-ssa-sccopy.cc
index 0badfb11b93..4252e5295df 100644
--- a/gcc/gimple-ssa-sccopy.cc
+++ b/gcc/gimple-ssa-sccopy.cc
@@ -405,22 +405,6 @@  stmt_may_generate_copy (gimple *stmt)
   if (SSA_NAME_OCCURS_IN_ABNORMAL_PHI (rhs))
     return false;
 
-  /* It is possible that lhs has more alignment or value range information.  By
-     propagating we would lose this information.  So in the case that alignment
-     or value range information differs, we are conservative and do not
-     propagate.
-
-     FIXME: Propagate alignment and value range info the same way copy-prop
-     does.  */
-  if (POINTER_TYPE_P (TREE_TYPE (lhs))
-      && POINTER_TYPE_P (TREE_TYPE (rhs))
-      && SSA_NAME_PTR_INFO (lhs) != SSA_NAME_PTR_INFO (rhs))
-    return false;
-  if (!POINTER_TYPE_P (TREE_TYPE (lhs))
-      && !POINTER_TYPE_P (TREE_TYPE (rhs))
-      && SSA_NAME_RANGE_INFO (lhs) != SSA_NAME_RANGE_INFO (rhs))
-    return false;
-
   return true;  /* A statement of type _2 = _1;.  */
 }
 
@@ -497,6 +481,8 @@  scc_copy_prop::replace_scc_by_value (vec<gimple *> scc, tree val)
 	  
 	}
       replace_uses_by (name, val);
+      if (TREE_CODE (val) == SSA_NAME)
+        maybe_duplicate_ssa_info_at_copy (name, val);
       bitmap_set_bit (dead_stmts, SSA_NAME_VERSION (name));
       didsomething = true;
     }