On Tue, Jul 6, 2021 at 3:14 AM apinski--- via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> From: Andrew Pinski <apin...@marvell.com>
>
> So the problem here is that replace_phi_edge_with_variable
> will copy range information to a already (not newly) defined
> ssa name. This causes wrong code later on.
> This fixes the problem by require the new ssa name to
> be defined in the same bb as the conditional that is
> about to be deleted.
>
> OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.
OK.
Richard.
> Changes from v1:
> * this is a simplification of what was trying to be done before.
>
> gcc/ChangeLog:
>
> PR tree-optimization/101256
> * dbgcnt.def (phiopt_edge_range): New counter.
> * tree-ssa-phiopt.c (replace_phi_edge_with_variable):
>
> gcc/testsuite/ChangeLog:
>
> PR tree-optimization/101256
> * g++.dg/torture/pr101256.C: New test.
> ---
> gcc/dbgcnt.def | 1 +
> gcc/testsuite/g++.dg/torture/pr101256.C | 28 +++++++++++++++++++++++++
> gcc/tree-ssa-phiopt.c | 16 ++++++++------
> 3 files changed, 39 insertions(+), 6 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/torture/pr101256.C
>
> diff --git a/gcc/dbgcnt.def b/gcc/dbgcnt.def
> index 93e7b4fd30e..2345899ba68 100644
> --- a/gcc/dbgcnt.def
> +++ b/gcc/dbgcnt.def
> @@ -183,6 +183,7 @@ DEBUG_COUNTER (lim)
> DEBUG_COUNTER (local_alloc_for_sched)
> DEBUG_COUNTER (match)
> DEBUG_COUNTER (merged_ipa_icf)
> +DEBUG_COUNTER (phiopt_edge_range)
> DEBUG_COUNTER (postreload_cse)
> DEBUG_COUNTER (pre)
> DEBUG_COUNTER (pre_insn)
> diff --git a/gcc/testsuite/g++.dg/torture/pr101256.C
> b/gcc/testsuite/g++.dg/torture/pr101256.C
> new file mode 100644
> index 00000000000..973a8b4caf3
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/torture/pr101256.C
> @@ -0,0 +1,28 @@
> +// { dg-do run }
> +
> +template<class T>
> +const T& max(const T& a, const T& b)
> +{
> + return (a < b) ? b : a;
> +}
> +
> +signed char var_5 = -128;
> +unsigned int var_11 = 2144479212U;
> +unsigned long long int arr [22];
> +
> +void
> +__attribute__((noipa))
> +test(signed char var_5, unsigned var_11) {
> + for (short i_61 = 0; i_61 < var_5 + 149; i_61 += 10000)
> + arr[i_61] = max((signed char)0, var_5) ? max((signed char)1, var_5) :
> var_11;
> +}
> +
> +int main() {
> + for (int i_0 = 0; i_0 < 22; ++i_0)
> + arr [i_0] = 11834725929543695741ULL;
> +
> + test(var_5, var_11);
> + if (arr [0] != 2144479212ULL && arr [0] != 11834725929543695741ULL)
> + __builtin_abort ();
> + return 0;
> +}
> diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c
> index ab63bf699e3..8b60ee81082 100644
> --- a/gcc/tree-ssa-phiopt.c
> +++ b/gcc/tree-ssa-phiopt.c
> @@ -51,6 +51,7 @@ along with GCC; see the file COPYING3. If not see
> #include "internal-fn.h"
> #include "gimple-range.h"
> #include "gimple-match.h"
> +#include "dbgcnt.h"
>
> static unsigned int tree_ssa_phiopt_worker (bool, bool, bool);
> static bool two_value_replacement (basic_block, basic_block, edge, gphi *,
> @@ -390,7 +391,7 @@ replace_phi_edge_with_variable (basic_block cond_block,
> gimple_stmt_iterator gsi;
> tree phi_result = PHI_RESULT (phi);
>
> - /* Duplicate range info if we're the only things setting the target PHI.
> + /* Duplicate range info if they are the only things setting the target PHI.
> This is needed as later on, the new_tree will be replacing
> The assignement of the PHI.
> For an example:
> @@ -398,19 +399,22 @@ replace_phi_edge_with_variable (basic_block cond_block,
> _4 = min<a_1, 255>
> goto bb2
>
> - range<-INF,255>
> + # RANGE [-INF, 255]
> a_3 = PHI<_4(1)>
> bb3:
>
> use(a_3)
> - And _4 gets prograted into the use of a_3 and losing the range info.
> - This can't be done for more than 2 incoming edges as the progration
> - won't happen. */
> + And _4 gets propagated into the use of a_3 and losing the range info.
> + This can't be done for more than 2 incoming edges as the propagation
> + won't happen.
> + The new_tree needs to be defined in the same basic block as the
> conditional. */
> if (TREE_CODE (new_tree) == SSA_NAME
> && EDGE_COUNT (gimple_bb (phi)->preds) == 2
> && INTEGRAL_TYPE_P (TREE_TYPE (phi_result))
> && !SSA_NAME_RANGE_INFO (new_tree)
> - && SSA_NAME_RANGE_INFO (phi_result))
> + && SSA_NAME_RANGE_INFO (phi_result)
> + && gimple_bb (SSA_NAME_DEF_STMT (new_tree)) == cond_block
> + && dbg_cnt (phiopt_edge_range))
> duplicate_ssa_name_range_info (new_tree,
> SSA_NAME_RANGE_TYPE (phi_result),
> SSA_NAME_RANGE_INFO (phi_result));
> --
> 2.27.0
>
