From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 59418 invoked by alias); 17 Mar 2016 16:35:22 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 59396 invoked by uid 89); 17 Mar 2016 16:35:21 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=capture, analyses, remembering, Failing X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Thu, 17 Mar 2016 16:35:11 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id 755F8C00F222; Thu, 17 Mar 2016 16:35:10 +0000 (UTC) Received: from slagheap.utah.redhat.com (ovpn-113-164.phx2.redhat.com [10.3.113.164]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u2HGZ9Jt008130; Thu, 17 Mar 2016 12:35:09 -0400 Subject: Re: [05/05] Fix PR 69102 To: Andrey Belevantsev , GCC Patches References: <0ad96ef3-4296-d9db-7576-4366c83d0912@ispras.ru> <7fe9ed9d-d431-cfe6-4708-c07bd23f893e@ispras.ru> Cc: Alexander Monakov , Bernd Schmidt From: Jeff Law Message-ID: <56EADCBD.7080206@redhat.com> Date: Thu, 17 Mar 2016 16:39:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <7fe9ed9d-d431-cfe6-4708-c07bd23f893e@ispras.ru> Content-Type: text/plain; charset=koi8-r; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2016-03/txt/msg01026.txt.bz2 On 03/15/2016 09:55 AM, Andrey Belevantsev wrote: > Hello, > > On 14.03.2016 12:52, Andrey Belevantsev wrote: >> Hello, >> >> The problem here is readonly dependence contexts in selective scheduler. >> We're trying to cache the effect of initializing a dependence context >> with >> remembering that context and setting a readonly bit on it. When first >> moving the insn 43 with REG_ARGS_SIZE note through the insn 3 (a >> simple eax >> set) we also set the last_args_size field of the context. Later, when we >> make a copy of insn 43 and try to move it again through insn 3, we >> take the >> cached dependency context and notice the (fake) dep with last_args_size >> insn, which is the old insn 43. Then the assert saying that we should be >> able to lift the bookkeeping copy up the same way as we did with the >> original insn breaks. >> >> Fixed by the attached patch that makes us notice only deps with the >> current >> producer insn. >> >> Ok for trunk? > > We've discussed the patch with Alexander a bit and decided to take the > different approach. The core issue here is not handling the new > last_args_size field in the readonly contexts. In general, the readonly > context approach, when analyzing an insn with a readonly context, would > create the necessary dependencies with all of the last_ fields but > refrain from modifying those fields. The reason is we need to capture > the effect of only the single producer in the readonly context. Failing > to do so may update the last_ fields with the effect of subsequent > analyses and having the fake dependencies with the insns that got into > those fields instead of having the dependencies with the currently used > producer. > > So the right fix here is to guard setting of the last_args_size field > with !deps->readonly test as it is done elsewhere in the sched-deps.c. > In stage 1 we will also want to set the asserts in the sel-sched > dependency hooks (where I have placed early returns in the previous > version of the patch) actually checking that the dependency is always > created with the current producer, and such cases will be caught sooner. > > The new patch bootstrapped and tested on x86-64 with selective scheduler > forced enabled with no regressions. It needs the maintainer outside of > sel-sched as we touch sched-deps.c file. Ok for trunk? The test is the > same as in previous patch. > > Andrey > > 2016-03-15 Andrey Belevantsev > > PR rtl-optimization/69102 > * sched-deps.c (sched_analyze_insn): Do not set last_args_size > field > when we have a readonly dependency context. > >> >> gcc/ >> >> 2016-03-14 Andrey Belevantsev >> >> PR rtl-optimization/69102 >> * sel-sched.c (has_dependence_note_dep): Only take into >> account dependencies produced by the current producer insn. >> (has_dependence_note_mem_dep): Likewise. >> >> testsuite/ >> >> 2016-03-14 Andrey Belevantsev >> >> PR rtl-optimization/69102 >> * gcc.c-torture/compile/pr69102.c: New test. OK. jeff