From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 92842 invoked by alias); 17 Dec 2015 10:20:51 -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 92831 invoked by uid 89); 17 Dec 2015 10:20:51 -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 autolearn=ham version=3.3.2 spammy=reliance, check-in, intrinsically, mems X-HELO: mailrelay7.public.one.com Received: from mailrelay7.public.one.com (HELO mailrelay7.public.one.com) (91.198.169.215) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Thu, 17 Dec 2015 10:20:50 +0000 X-HalOne-Cookie: 5d3f0bd8c54001cacd8c1df1d88e331ef4ef5b4d X-HalOne-ID: d585e42d-a4a7-11e5-b59b-b82a72cffc46 Received: from localhost.localdomain (unknown [85.178.7.141]) by smtpfilter4.public.one.com (Halon Mail Gateway) with ESMTPSA; Thu, 17 Dec 2015 10:20:44 +0000 (GMT) Subject: Re: [PATCH 2/4] gcc/arc: Remove load_update_operand predicate To: Andrew Burgess , gcc-patches@gcc.gnu.org References: <3d33e39746ca16dcd9b533ad50f1a19e0efe853d.1450224136.git.andrew.burgess@embecosm.com> Cc: Claudiu.Zissulescu@synopsys.com From: Joern Wolfgang Rennecke Message-ID: <56728C7C.2060807@amylaar.uk> Date: Thu, 17 Dec 2015 10:20:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 MIME-Version: 1.0 In-Reply-To: <3d33e39746ca16dcd9b533ad50f1a19e0efe853d.1450224136.git.andrew.burgess@embecosm.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2015-12/txt/msg01729.txt.bz2 On 16/12/15 00:15, Andrew Burgess wrote: > > > * config/arc/arc.md (*loadqi_update): Use 'memory_operand' and fix > RTL pattern to include the plus. > (*load_zeroextendqisi_update): Likewise. > (*load_signextendqisi_update): Likewise. > (*loadhi_update): Likewise. > (*load_zeroextendhisi_update): Likewise. > (*load_signextendhisi_update): Likewise. > (*loadsi_update): Likewise. > (*loadsf_update): Likewise. > * config/arc/predicates.md (load_update_operand): Delete. Store_update_operand has the very same problem, so it would make sense to fix that in the same check-in. FWIW, while using "memory_operand" makes for simple source code, it introduces duplicated checks (and they are appropriate only because the the update and some of the non-update addressing modes on the ARC are different modes of the same encoding). It checks that the inside of the MEM is a valid memory address, which is redundant with the pattern-provided checks that there's a plus with appropriate base and index/update operand inside. Also, by using memory_operand, you are adding a check to reject volatile memory operands during most optimization passes. Note that ARC's move_src_operand and move_dest_operand are fine with volatile MEMs irrespective of the setting of volatile_ok. Problems with volatiles can rally only be expected if there are multiple MEMs in a single pattern, that might alias, arithmetic on MEM that might result from simplifications using a different set of MEMs, or if the machine instructions that a pattern corresponds to are intrinsically unsuitable for volatile. Needlessly rejecting volatile MEMs will reduce optimization potential; this tends to be more visible on embedded platforms than with embedded code than with typical workstation code. Less reliance on this addressing modes orthogonality, no change of volatile behaviour, and maybe a slightly faster compiler, would be to just have an "update_operand" or "any_mem_operand" predicate checking that the inside is a MEM. and leave the address processing entirely to the instruction pattern and its operand predicates.