From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id 3A36E3858284 for ; Tue, 30 Aug 2022 12:27:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 3A36E3858284 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 005201063; Tue, 30 Aug 2022 05:27:21 -0700 (PDT) Received: from localhost (e121540-lin.manchester.arm.com [10.32.98.62]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 0A8873F71A; Tue, 30 Aug 2022 05:27:13 -0700 (PDT) From: Richard Sandiford To: Surya Kumari Jangala via Gcc-patches Mail-Followup-To: Surya Kumari Jangala via Gcc-patches ,bergner@linux.ibm.com, Segher Boessenkool , pthaugen@us.ibm.com, Surya Kumari Jangala , richard.sandiford@arm.com Cc: bergner@linux.ibm.com, Segher Boessenkool , pthaugen@us.ibm.com, Surya Kumari Jangala Subject: Re: [PATCH] sched1: Fix -fcompare-debug issue in schedule_region [PR105586] References: Date: Tue, 30 Aug 2022 13:27:12 +0100 In-Reply-To: (Surya Kumari Jangala via Gcc-patches's message of "Tue, 23 Aug 2022 17:19:39 +0530") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-49.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,KAM_DMARC_NONE,KAM_DMARC_STATUS,KAM_LAZY_DOMAIN_SECURITY,KAM_SHORT,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Surya Kumari Jangala via Gcc-patches writes: > sched1: Fix -fcompare-debug issue in schedule_region [PR105586] > > In schedule_region(), a basic block that does not contain any real insns > is not scheduled and the dfa state at the entry of the bb is not copied > to the fallthru basic block. However a DEBUG insn is treated as a real > insn, and if a bb contains non-real insns and a DEBUG insn, it's dfa > state is copied to the fallthru bb. This was resulting in > -fcompare-debug failure as the incoming dfa state of the fallthru block > is different with -g. We should always copy the dfa state of a bb to > it's fallthru bb even if the bb does not contain real insns. > > 2022-08-22 Surya Kumari Jangala > > gcc/ > PR rtl-optimization/105586 > * sched-rgn.cc (schedule_region): Always copy dfa state to > fallthru block. > > gcc/testsuite/ > PR rtl-optimization/105586 > * gcc.target/powerpc/pr105586.c: New test. > > > diff --git a/gcc/sched-rgn.cc b/gcc/sched-rgn.cc > index 0dc2a8f2851..6c9202c0b2b 100644 > --- a/gcc/sched-rgn.cc > +++ b/gcc/sched-rgn.cc > @@ -3082,6 +3082,24 @@ free_bb_state_array (void) > bb_state = NULL; > } > > +static void > +save_state_for_fallthru_edge (basic_block last_bb, state_t state) The convention (not always followed) is to have a comment above each function explaining what it does. How about something like: /* If LAST_BB falls through to another block B, record that B should start with DFA start STATE. */ OK for trunk with that change (and with the new changelog). Thanks, Richard > +{ > + edge f = find_fallthru_edge (last_bb->succs); > + if (f > + && (!f->probability.initialized_p () > + || (f->probability.to_reg_br_prob_base () * 100 > + / REG_BR_PROB_BASE > + >= param_sched_state_edge_prob_cutoff))) > + { > + memcpy (bb_state[f->dest->index], state, > + dfa_state_size); > + if (sched_verbose >= 5) > + fprintf (sched_dump, "saving state for edge %d->%d\n", > + f->src->index, f->dest->index); > + } > +} > + > /* Schedule a region. A region is either an inner loop, a loop-free > subroutine, or a single basic block. Each bb in the region is > scheduled after its flow predecessors. */ > @@ -3155,6 +3173,7 @@ schedule_region (int rgn) > if (no_real_insns_p (head, tail)) > { > gcc_assert (first_bb == last_bb); > + save_state_for_fallthru_edge (last_bb, bb_state[first_bb->index]); > continue; > } > > @@ -3173,26 +3192,13 @@ schedule_region (int rgn) > curr_bb = first_bb; > if (dbg_cnt (sched_block)) > { > - edge f; > int saved_last_basic_block = last_basic_block_for_fn (cfun); > > schedule_block (&curr_bb, bb_state[first_bb->index]); > gcc_assert (EBB_FIRST_BB (bb) == first_bb); > sched_rgn_n_insns += sched_n_insns; > realloc_bb_state_array (saved_last_basic_block); > - f = find_fallthru_edge (last_bb->succs); > - if (f > - && (!f->probability.initialized_p () > - || (f->probability.to_reg_br_prob_base () * 100 > - / REG_BR_PROB_BASE > - >= param_sched_state_edge_prob_cutoff))) > - { > - memcpy (bb_state[f->dest->index], curr_state, > - dfa_state_size); > - if (sched_verbose >= 5) > - fprintf (sched_dump, "saving state for edge %d->%d\n", > - f->src->index, f->dest->index); > - } > + save_state_for_fallthru_edge (last_bb, curr_state); > } > else > { > diff --git a/gcc/testsuite/gcc.target/powerpc/pr105586.c b/gcc/testsuite/gcc.target/powerpc/pr105586.c > new file mode 100644 > index 00000000000..bd397f58bc0 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/powerpc/pr105586.c > @@ -0,0 +1,19 @@ > +/* { dg-options "-mdejagnu-tune=power4 -O2 -fcompare-debug -fno-if-conversion -fno-guess-branch-probability" } */ > + > +extern int bar(int i); > + > +typedef unsigned long u64; > +int g; > + > +__int128 h; > + > +void > +foo(int a, int b) { > + int i; > + char u8_1 = g, u8_3 = a; > + u64 u64_1 = bar(0), u64_3 = u8_3 * u64_1; > + __int128 u128_1 = h ^ __builtin_expect(i, 0); > + u64 u64_4 = u64_1 << ((__builtin_sub_overflow_p(0, u8_1, (u64)0) ^ u128_1) & 8); > + u64 u64_r = b + u64_3 + u64_4; > + bar((short)u64_r + u8_3); > +}