[TCWG CI] Regression caused by gcc: phiopt: Optimize partial_ordering spaceship 
>= 0 -ffinite-math-only [PR94589]:
commit 65061ea287a80cfb214e402cfd2373a14bfec95a
Author: Jakub Jelinek <ja...@redhat.com>

    phiopt: Optimize partial_ordering spaceship >= 0 -ffinite-math-only 
[PR94589]

Results regressed to
# reset_artifacts:
-10
# build_abe binutils:
-9
# build_abe stage1:
-5
# build_abe qemu:
-2
# linux_n_obj:
33
# First few build errors in logs:
# 00:01:16 cc1plus: fatal error: ./include/generated/utsrelease.h: No such file 
or directory
# 00:01:16 cc1plus: fatal error: ./include/generated/utsrelease.h: No such file 
or directory
# 00:01:16 cc1plus: fatal error: ./include/generated/utsrelease.h: No such file 
or directory
# 00:01:16 make[2]: *** [scripts/gcc-plugins/stackleak_plugin.so] Error 1
# 00:01:16 make[2]: *** [scripts/gcc-plugins/latent_entropy_plugin.so] Error 1
# 00:01:16 make[2]: *** [scripts/gcc-plugins/randomize_layout_plugin.so] Error 1
# 00:01:16 make[1]: *** [scripts/gcc-plugins] Error 2
# 00:01:17 make: *** [scripts] Error 2

from
# reset_artifacts:
-10
# build_abe binutils:
-9
# build_abe stage1:
-5
# build_abe qemu:
-2
# linux_n_obj:
21070
# linux build successful:
all

THIS IS THE END OF INTERESTING STUFF.  BELOW ARE LINKS TO BUILDS, REPRODUCTION 
INSTRUCTIONS, AND THE RAW COMMIT.

This commit has regressed these CI configurations:
 - tcwg_kernel/gnu-release-aarch64-next-allyesconfig

First_bad build: 
https://ci.linaro.org/job/tcwg_kernel-gnu-bisect-gnu-release-aarch64-next-allyesconfig/138/artifact/artifacts/build-65061ea287a80cfb214e402cfd2373a14bfec95a/
Last_good build: 
https://ci.linaro.org/job/tcwg_kernel-gnu-bisect-gnu-release-aarch64-next-allyesconfig/138/artifact/artifacts/build-b2a09773155892e426b90ce8367e2ed6996b44ef/
Baseline build: 
https://ci.linaro.org/job/tcwg_kernel-gnu-bisect-gnu-release-aarch64-next-allyesconfig/138/artifact/artifacts/build-baseline/
Even more details: 
https://ci.linaro.org/job/tcwg_kernel-gnu-bisect-gnu-release-aarch64-next-allyesconfig/138/artifact/artifacts/

Reproduce builds:
<cut>
mkdir investigate-gcc-65061ea287a80cfb214e402cfd2373a14bfec95a
cd investigate-gcc-65061ea287a80cfb214e402cfd2373a14bfec95a

# Fetch scripts
git clone https://git.linaro.org/toolchain/jenkins-scripts

# Fetch manifests and test.sh script
mkdir -p artifacts/manifests
curl -o artifacts/manifests/build-baseline.sh 
https://ci.linaro.org/job/tcwg_kernel-gnu-bisect-gnu-release-aarch64-next-allyesconfig/138/artifact/artifacts/manifests/build-baseline.sh
 --fail
curl -o artifacts/manifests/build-parameters.sh 
https://ci.linaro.org/job/tcwg_kernel-gnu-bisect-gnu-release-aarch64-next-allyesconfig/138/artifact/artifacts/manifests/build-parameters.sh
 --fail
curl -o artifacts/test.sh 
https://ci.linaro.org/job/tcwg_kernel-gnu-bisect-gnu-release-aarch64-next-allyesconfig/138/artifact/artifacts/test.sh
 --fail
chmod +x artifacts/test.sh

# Reproduce the baseline build (build all pre-requisites)
./jenkins-scripts/tcwg_kernel-build.sh @@ artifacts/manifests/build-baseline.sh

# Save baseline build state (which is then restored in artifacts/test.sh)
mkdir -p ./bisect
rsync -a --del --delete-excluded --exclude /bisect/ --exclude /artifacts/ 
--exclude /gcc/ ./ ./bisect/baseline/

cd gcc

# Reproduce first_bad build
git checkout --detach 65061ea287a80cfb214e402cfd2373a14bfec95a
../artifacts/test.sh

# Reproduce last_good build
git checkout --detach b2a09773155892e426b90ce8367e2ed6996b44ef
../artifacts/test.sh

cd ..
</cut>

Full commit (up to 1000 lines):
<cut>
commit 65061ea287a80cfb214e402cfd2373a14bfec95a
Author: Jakub Jelinek <ja...@redhat.com>
Date:   Tue May 18 10:08:51 2021 +0200

    phiopt: Optimize partial_ordering spaceship >= 0 -ffinite-math-only 
[PR94589]
    
    As mentioned earlier, spaceship_replacement didn't optimize partial_ordering
    >= 0 comparisons, because the possible values are -1, 0, 1, 2 and the
    >= comparison is implemented as (res & 1) == res to choose the 0 and 1
    cases from that.  As we optimize that only with -ffinite-math-only, the
    2 case is assumed not to happen and my earlier match.pd change optimizes
    (res & 1) == res into (res & ~1) == 0, so this patch pattern matches
    that case and handles it like res >= 0.
    
    2021-05-18  Jakub Jelinek  <ja...@redhat.com>
    
            PR tree-optimization/94589
            * tree-ssa-phiopt.c (spaceship_replacement): Pattern match
            phi result used in (res & ~1) == 0 comparison as res >= 0 as
            res == 2 would be UB with -ffinite-math-only.
    
            * g++.dg/opt/pr94589-2.C: Adjust scan-tree-dump count from 14 to 12.
---
 gcc/testsuite/g++.dg/opt/pr94589-2.C |  4 +-
 gcc/tree-ssa-phiopt.c                | 72 +++++++++++++++++++++++++++++++-----
 2 files changed, 65 insertions(+), 11 deletions(-)

diff --git a/gcc/testsuite/g++.dg/opt/pr94589-2.C 
b/gcc/testsuite/g++.dg/opt/pr94589-2.C
index dda947e22b1..e9ef84b1912 100644
--- a/gcc/testsuite/g++.dg/opt/pr94589-2.C
+++ b/gcc/testsuite/g++.dg/opt/pr94589-2.C
@@ -1,8 +1,8 @@
 // PR tree-optimization/94589
 // { dg-do compile { target c++20 } }
 // { dg-options "-O2 -g0 -ffast-math -fdump-tree-optimized" }
-// { dg-final { scan-tree-dump-times "\[ij]_\[0-9]+\\(D\\) (?:<|<=|==|!=|>|>=) 
\[ij]_\[0-9]+\\(D\\)" 14 "optimized" } }
-// { dg-final { scan-tree-dump-times "i_\[0-9]+\\(D\\) (?:<|<=|==|!=|>|>=) 
5\\.0" 14 "optimized" } }
+// { dg-final { scan-tree-dump-times "\[ij]_\[0-9]+\\(D\\) (?:<|<=|==|!=|>|>=) 
\[ij]_\[0-9]+\\(D\\)" 12 "optimized" } }
+// { dg-final { scan-tree-dump-times "i_\[0-9]+\\(D\\) (?:<|<=|==|!=|>|>=) 
5\\.0" 12 "optimized" } }
 
 #include <compare>
 
diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c
index e0a41d41fba..8e8a08bc679 100644
--- a/gcc/tree-ssa-phiopt.c
+++ b/gcc/tree-ssa-phiopt.c
@@ -1887,8 +1887,9 @@ spaceship_replacement (basic_block cond_bb, basic_block 
middle_bb,
                       edge e0, edge e1, gphi *phi,
                       tree arg0, tree arg1)
 {
-  if (!INTEGRAL_TYPE_P (TREE_TYPE (PHI_RESULT (phi)))
-      || TYPE_UNSIGNED (TREE_TYPE (PHI_RESULT (phi)))
+  tree phires = PHI_RESULT (phi);
+  if (!INTEGRAL_TYPE_P (TREE_TYPE (phires))
+      || TYPE_UNSIGNED (TREE_TYPE (phires))
       || !tree_fits_shwi_p (arg0)
       || !tree_fits_shwi_p (arg1)
       || !IN_RANGE (tree_to_shwi (arg0), -1, 2)
@@ -1902,12 +1903,32 @@ spaceship_replacement (basic_block cond_bb, basic_block 
middle_bb,
 
   use_operand_p use_p;
   gimple *use_stmt;
-  if (SSA_NAME_OCCURS_IN_ABNORMAL_PHI (PHI_RESULT (phi)))
+  if (SSA_NAME_OCCURS_IN_ABNORMAL_PHI (phires))
     return false;
-  if (!single_imm_use (PHI_RESULT (phi), &use_p, &use_stmt))
+  if (!single_imm_use (phires, &use_p, &use_stmt))
     return false;
   enum tree_code cmp;
   tree lhs, rhs;
+  gimple *orig_use_stmt = use_stmt;
+  tree orig_use_lhs = NULL_TREE;
+  int prec = TYPE_PRECISION (TREE_TYPE (phires));
+  if (is_gimple_assign (use_stmt)
+      && gimple_assign_rhs_code (use_stmt) == BIT_AND_EXPR
+      && TREE_CODE (gimple_assign_rhs2 (use_stmt)) == INTEGER_CST
+      && (wi::to_wide (gimple_assign_rhs2 (use_stmt))
+         == wi::shifted_mask (1, prec - 1, false, prec)))
+    {
+      /* For partial_ordering result operator>= with unspec as second
+        argument is (res & 1) == res, folded by match.pd into
+        (res & ~1) == 0.  */
+      orig_use_lhs = gimple_assign_lhs (use_stmt);
+      if (SSA_NAME_OCCURS_IN_ABNORMAL_PHI (orig_use_lhs))
+       return false;
+      if (EDGE_COUNT (phi_bb->preds) != 4)
+       return false;
+      if (!single_imm_use (orig_use_lhs, &use_p, &use_stmt))
+       return false;
+    }
   if (gimple_code (use_stmt) == GIMPLE_COND)
     {
       cmp = gimple_cond_code (use_stmt);
@@ -1948,10 +1969,19 @@ spaceship_replacement (basic_block cond_bb, basic_block 
middle_bb,
     default:
       return false;
     }
-  if (lhs != PHI_RESULT (phi)
+  if (lhs != (orig_use_lhs ? orig_use_lhs : phires)
       || !tree_fits_shwi_p (rhs)
       || !IN_RANGE (tree_to_shwi (rhs), -1, 1))
     return false;
+  if (orig_use_lhs)
+    {
+      if ((cmp != EQ_EXPR && cmp != NE_EXPR) || !integer_zerop (rhs))
+       return false;
+      /* As for -ffast-math we assume the 2 return to be
+        impossible, canonicalize (res & ~1) == 0 into
+        res >= 0 and (res & ~1) != 0 as res < 0.  */
+      cmp = cmp == EQ_EXPR ? GE_EXPR : LT_EXPR;
+    }
 
   if (!empty_block_p (middle_bb))
     return false;
@@ -2092,7 +2122,7 @@ spaceship_replacement (basic_block cond_bb, basic_block 
middle_bb,
                                ? EDGE_TRUE_VALUE : EDGE_FALSE_VALUE)) == 0)
     return false;
 
-  /* lhs1 one_cmp rhs1 results in PHI_RESULT (phi) of 1.  */
+  /* lhs1 one_cmp rhs1 results in phires of 1.  */
   enum tree_code one_cmp;
   if ((cmp1 == LT_EXPR)
       ^ (!integer_onep ((e1->flags & EDGE_TRUE_VALUE) ? arg1 : arg0)))
@@ -2185,13 +2215,29 @@ spaceship_replacement (basic_block cond_bb, basic_block 
middle_bb,
       use_operand_p use_p;
       imm_use_iterator iter;
       bool has_debug_uses = false;
-      FOR_EACH_IMM_USE_FAST (use_p, iter, PHI_RESULT (phi))
+      FOR_EACH_IMM_USE_FAST (use_p, iter, phires)
        {
          gimple *use_stmt = USE_STMT (use_p);
+         if (orig_use_lhs && use_stmt == orig_use_stmt)
+           continue;
          gcc_assert (is_gimple_debug (use_stmt));
          has_debug_uses = true;
          break;
        }
+      if (orig_use_lhs)
+       {
+         if (!has_debug_uses)
+           FOR_EACH_IMM_USE_FAST (use_p, iter, orig_use_lhs)
+             {
+               gimple *use_stmt = USE_STMT (use_p);
+               gcc_assert (is_gimple_debug (use_stmt));
+               has_debug_uses = true;
+             }
+         gimple_stmt_iterator gsi = gsi_for_stmt (orig_use_stmt);
+         tree zero = build_zero_cst (TREE_TYPE (orig_use_lhs));
+         gimple_assign_set_rhs_with_ops (&gsi, INTEGER_CST, zero);
+         update_stmt (orig_use_stmt);
+       }
 
       if (has_debug_uses)
        {
@@ -2203,7 +2249,7 @@ spaceship_replacement (basic_block cond_bb, basic_block 
middle_bb,
             Ignore the value of 2, because if NaNs aren't expected,
             all floating point numbers should be comparable.  */
          gimple_stmt_iterator gsi = gsi_after_labels (gimple_bb (phi));
-         tree type = TREE_TYPE (PHI_RESULT (phi));
+         tree type = TREE_TYPE (phires);
          tree temp1 = make_node (DEBUG_EXPR_DECL);
          DECL_ARTIFICIAL (temp1) = 1;
          TREE_TYPE (temp1) = type;
@@ -2221,10 +2267,18 @@ spaceship_replacement (basic_block cond_bb, basic_block 
middle_bb,
          t = build3 (COND_EXPR, type, t, build_zero_cst (type), temp1);
          g = gimple_build_debug_bind (temp2, t, phi);
          gsi_insert_before (&gsi, g, GSI_SAME_STMT);
-         replace_uses_by (PHI_RESULT (phi), temp2);
+         replace_uses_by (phires, temp2);
+         if (orig_use_lhs)
+           replace_uses_by (orig_use_lhs, temp2);
        }
     }
 
+  if (orig_use_lhs)
+    {
+      gimple_stmt_iterator gsi = gsi_for_stmt (orig_use_stmt);
+      gsi_remove (&gsi, true);
+    }
+
   gimple_stmt_iterator psi = gsi_for_stmt (phi);
   remove_phi_node (&psi, true);
 
</cut>
_______________________________________________
linaro-toolchain mailing list -- linaro-toolchain@lists.linaro.org
To unsubscribe send an email to linaro-toolchain-le...@lists.linaro.org

Reply via email to