From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11042 invoked by alias); 14 Jan 2015 06:16:20 -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 11032 invoked by uid 89); 14 Jan 2015 06:16:18 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.8 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD,UNSUBSCRIBE_BODY autolearn=no version=3.3.2 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; Wed, 14 Jan 2015 06:16:17 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t0E6GBQd018357 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Wed, 14 Jan 2015 01:16:12 -0500 Received: from [10.3.113.77] (ovpn-113-77.phx2.redhat.com [10.3.113.77]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t0E6G9jx005860; Wed, 14 Jan 2015 01:16:09 -0500 Message-ID: <54B609A9.9090800@redhat.com> Date: Wed, 14 Jan 2015 06:52:00 -0000 From: Jeff Law User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.3.0 MIME-Version: 1.0 To: Segher Boessenkool CC: Jakub Jelinek , Richard Biener , Eric Botcazou , gcc-patches@gcc.gnu.org Subject: Re: [PATCH] Reenable CSE of non-volatile inline asm (PR rtl-optimization/63637) References: <20150113161819.GD1405@tucnak.redhat.com> <20150113163840.GA4183@gate.crashing.org> <54B575D7.8030107@redhat.com> <20150113201322.GJ1405@tucnak.redhat.com> <54B59964.7070707@redhat.com> <20150114000315.GA32710@gate.crashing.org> In-Reply-To: <20150114000315.GA32710@gate.crashing.org> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2015-01/txt/msg00971.txt.bz2 On 01/13/15 17:03, Segher Boessenkool wrote: > On Tue, Jan 13, 2015 at 03:17:08PM -0700, Jeff Law wrote: >>> And finally there is the case of non-volatile asm with "memory" clobber >>> with >>> no memory stores in between the two - the posted (safer) patch will not >>> allow to CSE the two, while in theory we could CSE them into just one asm. >> I think we have to assume that CSEing them is wrong. The first may set >> something in memory that is read by the second. >> >> Thoughts? > > I agree with pretty much everything you say in the thread, except for this > idea that a memory clobber reads memory. No clobber reads anything. > > The commit that introduced the memory clobber concept, 426b38c9 (svn 1207), > by rms, has as only comment > > /* `memory', don't cache memory across asm */ RMS botched this and you can see it in that the scheduler was not updated at the same time. The scheduler absolutely must track if an ASM does a memory read of an arbitrary location. I'd have to dig deeper to see when this got fixed, but it was clearly botched. Many years later another pass which needs to precisely track such things came along, namely DSE. The code in DSE is actually easier to grok. First, if you look at the ASM handling in cfgexpand.c you'll find: if (j == -4) /* `memory', don't cache memory across asm */ { XVECEXP (body, 0, i++) = gen_rtx_CLOBBER (VOIDmode, gen_rtx_MEM (BLKmode, gen_rtx_SCRATCH (VOIDmode))); continue; } So we generate (CLOBBER (MEM:BLK (SCRATCH))) when we see "memory" in the "clobber" list of an ASM. If you then look at dse.c we have this in record_store: /* At this point we know mem is a mem. */ if (GET_MODE (mem) == BLKmode) { if (GET_CODE (XEXP (mem, 0)) == SCRATCH) { if (dump_file && (dump_flags & TDF_DETAILS)) fprintf (dump_file, " adding wild read for (clobber (mem:BLK (scratch))\n"); add_wild_read (bb_info); insn_info->cannot_delete = true; return 0; } Which says very precisely that we treat (CLOBBER (MEM:BLK (SCRATCH))) as potentially *reading* any location. If you trace through how the scheduler builds dependencies, paying particular attention to alias.c you'll see that (CLOBBER (MEM:BLK (SCRATCH))) is treated as both a read and a write of an arbitrary location. It's unfortunate that RMS put the "memory" tag in the "clobber" list. But he really wasn't a compiler junkie and didn't realize the right thing to do was to have a memory tag in both the inputs and [output|clobber] section to represent a read of an arbitrary location and a write to an arbitrary location independently. But it is what it is at this point and we have to treat "memory" appearing in the "clobber" list as an arbitrary memory read and an arbitrary memory write. Jeff