From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 2017) id 17D563858D1E; Tue, 8 Feb 2022 19:49:12 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 17D563858D1E MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="utf-8" From: Robin Dapp To: gcc-cvs@gcc.gnu.org Subject: [gcc r12-7114] ifcvt: Fix PR104153 and PR104198. X-Act-Checkin: gcc X-Git-Author: Robin Dapp X-Git-Refname: refs/heads/master X-Git-Oldrev: 71e6353e1b03021bc8bbcf4bd67a5f14d09b5fb1 X-Git-Newrev: d0d4601ccde3c4849f6e7244035f1a899d608cb7 Message-Id: <20220208194912.17D563858D1E@sourceware.org> Date: Tue, 8 Feb 2022 19:49:12 +0000 (GMT) X-BeenThere: gcc-cvs@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-cvs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 08 Feb 2022 19:49:12 -0000 https://gcc.gnu.org/g:d0d4601ccde3c4849f6e7244035f1a899d608cb7 commit r12-7114-gd0d4601ccde3c4849f6e7244035f1a899d608cb7 Author: Robin Dapp Date: Tue Feb 8 16:11:20 2022 +0100 ifcvt: Fix PR104153 and PR104198. This is a bugfix for r12-6747-gaa8cfe785953a0 which caused an ICE on or1k (PR104153) and broke SPARC bootstrap (PR104198). cond_exec_get_condition () returns the jump condition directly and we now pass it to the backend. The or1k backend modified the condition in-place (other backends do that as well) but this modification is not reverted when the sequence in question is discarded. Therefore we copy the RTX instead of using it directly. The SPARC problem is due to the SPARC backend recreating the initial condition when being passed a CC comparison. This causes the sequence to read from an already overwritten condition operand. Generally, this could also happen on other targets. The workaround is to always first emit to a temporary. In a second run of noce_convert_multiple_sets_1 we know which sequences actually require the comparison and will use no temporaries if all sequences after the current one do not require it. PR rtl-optimization/104198 PR rtl-optimization/104153 gcc/ChangeLog: * ifcvt.cc (noce_convert_multiple_sets_1): Copy rtx instead of using it directly. Rework comparison handling and always perform a second pass. gcc/testsuite/ChangeLog: * gcc.dg/pr104198.c: New test. Diff: --- gcc/ifcvt.cc | 46 ++++++++++++++++++++++++++++++++++++++++- gcc/testsuite/gcc.dg/pr104198.c | 36 ++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 1 deletion(-) diff --git a/gcc/ifcvt.cc b/gcc/ifcvt.cc index fe250d508e1..6305621e460 100644 --- a/gcc/ifcvt.cc +++ b/gcc/ifcvt.cc @@ -3391,7 +3391,11 @@ noce_convert_multiple_sets_1 (struct noce_if_info *if_info, rtx cond = noce_get_condition (jump, &cond_earliest, false); rtx cc_cmp = cond_exec_get_condition (jump); + if (cc_cmp) + cc_cmp = copy_rtx (cc_cmp); rtx rev_cc_cmp = cond_exec_get_condition (jump, /* get_reversed */ true); + if (rev_cc_cmp) + rev_cc_cmp = copy_rtx (rev_cc_cmp); rtx_insn *insn; int count = 0; @@ -3515,6 +3519,7 @@ noce_convert_multiple_sets_1 (struct noce_if_info *if_info, unsigned cost1 = 0, cost2 = 0; rtx_insn *seq, *seq1, *seq2; rtx temp_dest = NULL_RTX, temp_dest1 = NULL_RTX, temp_dest2 = NULL_RTX; + bool read_comparison = false; seq1 = try_emit_cmove_seq (if_info, temp, cond, new_val, old_val, need_cmov, @@ -3524,10 +3529,41 @@ noce_convert_multiple_sets_1 (struct noce_if_info *if_info, as well. This allows the backend to emit a cmov directly without creating an additional compare for each. If successful, costing is easier and this sequence is usually preferred. */ - seq2 = try_emit_cmove_seq (if_info, target, cond, + seq2 = try_emit_cmove_seq (if_info, temp, cond, new_val, old_val, need_cmov, &cost2, &temp_dest2, cc_cmp, rev_cc_cmp); + /* The backend might have created a sequence that uses the + condition. Check this. */ + rtx_insn *walk = seq2; + while (walk) + { + rtx set = single_set (walk); + + if (!set || !SET_SRC (set)) + { + walk = NEXT_INSN (walk); + continue; + } + + rtx src = SET_SRC (set); + + if (XEXP (set, 1) && GET_CODE (XEXP (set, 1)) == IF_THEN_ELSE) + ; /* We assume that this is the cmove created by the backend that + naturally uses the condition. Therefore we ignore it. */ + else + { + if (reg_mentioned_p (XEXP (cond, 0), src) + || reg_mentioned_p (XEXP (cond, 1), src)) + { + read_comparison = true; + break; + } + } + + walk = NEXT_INSN (walk); + } + /* Check which version is less expensive. */ if (seq1 != NULL_RTX && (cost1 <= cost2 || seq2 == NULL_RTX)) { @@ -3540,6 +3576,8 @@ noce_convert_multiple_sets_1 (struct noce_if_info *if_info, { seq = seq2; temp_dest = temp_dest2; + if (!second_try && read_comparison) + *last_needs_comparison = count; } else { @@ -3558,6 +3596,12 @@ noce_convert_multiple_sets_1 (struct noce_if_info *if_info, unmodified_insns->safe_push (insn); } + /* Even if we did not actually need the comparison, we want to make sure + to try a second time in order to get rid of the temporaries. */ + if (*last_needs_comparison == -1) + *last_needs_comparison = 0; + + return true; } diff --git a/gcc/testsuite/gcc.dg/pr104198.c b/gcc/testsuite/gcc.dg/pr104198.c new file mode 100644 index 00000000000..bfc7a777184 --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr104198.c @@ -0,0 +1,36 @@ +/* Make sure if conversion for two instructions does not break + anything (if it runs). */ + +/* { dg-do run } */ +/* { dg-options "-O2 -std=c99" } */ + +#include +#include + +__attribute__ ((noinline)) +int foo (int *a, int n) +{ + int min = 999999; + int bla = 0; + for (int i = 0; i < n; i++) + { + if (a[i] < min) + { + min = a[i]; + bla = 1; + } + } + + if (bla) + min += 1; + return min; +} + +int main() +{ + int a[] = {2, 1, -13, INT_MAX, INT_MIN, 0}; + + int res = foo (a, sizeof (a) / sizeof (a[0])); + + assert (res == (INT_MIN + 1)); +}