[TCWG CI] Regression caused by gcc: phiopt: Optimize partial_ordering spaceship >= 0 -ffinite-math-only [PR94589]: commit 65061ea287a80cfb214e402cfd2373a14bfec95a Author: Jakub Jelinek jakub@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-al... Last_good build: https://ci.linaro.org/job/tcwg_kernel-gnu-bisect-gnu-release-aarch64-next-al... Baseline build: https://ci.linaro.org/job/tcwg_kernel-gnu-bisect-gnu-release-aarch64-next-al... Even more details: https://ci.linaro.org/job/tcwg_kernel-gnu-bisect-gnu-release-aarch64-next-al...
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-al... --fail curl -o artifacts/manifests/build-parameters.sh https://ci.linaro.org/job/tcwg_kernel-gnu-bisect-gnu-release-aarch64-next-al... --fail curl -o artifacts/test.sh https://ci.linaro.org/job/tcwg_kernel-gnu-bisect-gnu-release-aarch64-next-al... --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 jakub@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 jakub@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>