From patchwork Fri Nov 16 14:10:12 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tejas Belagod X-Patchwork-Id: 199630 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 014E62C0091 for ; Sat, 17 Nov 2012 01:10:29 +1100 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1353679830; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Received: Message-ID:Date:From:User-Agent:MIME-Version:To:Subject: Content-Type:Mailing-List:Precedence:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:Sender:Delivered-To; bh=nQfbLIs HCjPENMJEVo0GHkfMtVk=; b=JL10koxeZTJymH1G+uucSM6CFZ5fTjhdk0tlARY a/1/aDX9iu3+4rJGmCgcriUJtsO0Fh+kUTlwSlYJnZPi8GZiR4bzup2D04oiacDd qg4Uahqo7TPOpa0fCvphHRauN3ATw91YKve1oS421cWLzdQ6BIioGMTvj2kgBwci 9NiM= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:Received:Message-ID:Date:From:User-Agent:MIME-Version:To:Subject:X-MC-Unique:Content-Type:X-IsSubscribed:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=xkLItohngcbqc/egOF6qhKWAzbT9rbFLmL9+AgsihmD+9DPI/HBcv/2nd4lmO3 Lapky/HuvwseMXg6qnCC/Sk+UYz9fN/IVAHMTyG1j2MNkxALs+Hg3WymvDpuggFR JKmmmkVPZA+DW3eqVTzk+xNXtZl7rdqUVQ5NlPkGeBVYo=; Received: (qmail 31089 invoked by alias); 16 Nov 2012 14:10:24 -0000 Received: (qmail 31069 invoked by uid 22791); 16 Nov 2012 14:10:22 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL, BAYES_00, KHOP_RCVD_UNTRUST, RCVD_IN_DNSWL_LOW X-Spam-Check-By: sourceware.org Received: from service87.mimecast.com (HELO service87.mimecast.com) (91.220.42.44) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 16 Nov 2012 14:10:16 +0000 Received: from cam-owa1.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.21]) by service87.mimecast.com; Fri, 16 Nov 2012 14:10:14 +0000 Received: from [10.1.79.66] ([10.1.255.212]) by cam-owa1.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.0); Fri, 16 Nov 2012 14:10:13 +0000 Message-ID: <50A64944.5070804@arm.com> Date: Fri, 16 Nov 2012 14:10:12 +0000 From: Tejas Belagod User-Agent: Thunderbird 2.0.0.18 (X11/20081120) MIME-Version: 1.0 To: "gcc-patches@gcc.gnu.org" Subject: [PATCH][RFC] Bug handling SUBREG (MEM) - MEM having side-effects? X-MC-Unique: 112111614101414401 X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Hi, I seem to have uncovered what seems to be a bug with handling SUBREG (MEM) with the MEM having side-effects while testing aarch64-4.7. This bug seems to be latent on trunk. Here is a test case reduced from gcc.c-torture/execute/scal-to-vec1.c. #define vector(elcount, type) \ __attribute__((vector_size((elcount)*sizeof(type)))) type #define vidx(type, vec, idx) (*((type *) &(vec) + idx)) #define operl(a, b, op) (a op b) #define operr(a, b, op) (b op a) #define check(type, count, vec0, vec1, num, op, lr) \ do {\ int __i; \ for (__i = 0; __i < count; __i++) {\ if (vidx (type, vec1, __i) != oper##lr (num, vidx (type, vec0, __i), op)) \ __builtin_abort (); \ }\ } while (0) #define veccompare(type, count, v0, v1) \ do {\ int __i; \ for (__i = 0; __i < count; __i++) { \ if (vidx (type, v0, __i) != vidx (type, v1, __i)) \ __builtin_abort (); \ } \ } while (0) volatile int one = 1; int main (int argc, char *argv[]) { vector(8, short) v0 = {one, 1, 2, 3, 4, 5, 6, 7}; vector(8, short) v1; v1 = 2 << v0; check (short, 8, v0, v1, 2, <<, l); return 0; } When compiled with -O1, the combiner tries to combine these two instructions: (insn 87 86 89 4 (set (reg:SI 154 [ MEM[base: D.2294_40, offset: 0B] ]) (sign_extend:SI (mem:HI (pre_inc:DI (reg:DI 113 [ ivtmp.23 ])) [0 MEM[base: D.2294_40, offset: 0B]+0 S2 A16]))) scal-to-vec1.c:35 54 {*extendhisi2_aarch64} (expr_list:REG_INC (reg:DI 113 [ ivtmp.23 ]) (nil))) (insn 89 87 90 4 (set (reg:SI 155) (ashift:SI (reg:SI 158) (subreg:QI (reg:SI 154 [ MEM[base: D.2294_40, offset: 0B] ]) 0))) scal-to-vec1.c:35 331 {*ashlsi3_insn} (expr_list:REG_DEAD (reg:SI 154 [ MEM[base: D.2294_40, offset: 0B] ]) (nil))) It tries to forward-propagate reg 154 to insn 89 and the intermediate RTL looks like this: (insn 89 87 90 4 (set (reg:SI 155) (ashift:SI (reg:SI 158) (subreg:QI ((sign_extend:SI (mem:HI (pre_inc:DI (reg:DI 113 [ivtmp.23 ])) [0 MEM[base: D.2294_40, offset: 0B]+0 S2 A16]))) The combiner then recursively tries to simplify this RTL. During simplifying the subreg part of the RTL i.e. (subreg:QI ((sign_extend:SI (mem:HI (pre_inc:DI (reg:DI 113 ivtmp.23 ])) [0 MEM[base: D.2294_40, offset: 0B]+0 S2 A16]))) combine_simplify_rtx () calls simplify_subreg () and this returns (subreg:QI (mem:HI (pre_inc:DI (reg:DI 113 ivtmp.23 ])) [0 MEM[base: D.2294_40, offset: 0B]+0 S2 A16]))) The code that does this is in combine.c:combine_simplify_rtx () ... rtx temp; temp = simplify_subreg (mode, SUBREG_REG (x), op0_mode, SUBREG_BYTE (x)); if (temp) return temp; } ... So, the above tree is returned as is and insn 87 gets combined into insn 89 to become (insn 89 87 90 4 (set (reg:SI 155) (ashift:SI (reg:SI 158) (subreg:QI (mem:HI (pre_inc:DI (reg:DI 113 ivtmp.23 ])) [0 MEM[base: D.2294_40, offset: 0B]+0 S2 A16]))) This ends up in reload and in cleanup_subreg_operands () the subreg:QI (mem:HI) gets narrowed to mem:QI, but the interesting bit is that the pre_inc stays the same, so becomes a pre_inc for a QI mode address rather than the original HI mode address and therefore instead of addressing a byte and incrementing the address by 2 to get the next half word, the address gets incremented by 1! Also, I feel this is a latent bug on the trunk. This is because while the combiner takes the same path on the trunk, the address expression is not a pre_inc there - the trunk wants to keep the addressing to a mem (plus (reg) (const)) and therefore combine_simplify_rtx () simplifies the subreg expression right down to the narrowed memref i.e. mem:QI (plus:DI (reg:DI) (const)). This gets combined to insn 89 to become (insn 89 87 90 4 (set (reg:SI 155) (ashift:SI (reg:SI 158) (mem:QI (plus:DI (reg:DI) (const)))) This then, gets later checked with recog () and since the predicate for operand 2 does not allow memory operands for shifts in aarch64.md, does not get combined and all is well. After the discussions Richard Earnshaw had on IRC with Andrew Pinski, it was felt that it was best to fix the standard predicate 'general_operand' to not allow SUBREG (MEM) with side-effects as it has known issues associated with it - particularly reload not being able to deal with such cases. 'general_operand' currently outlaws cases like paradoxical SUBREG (MEM) on targets with insn scheduling and SUBREG (MEM) with non-zero SUBREG_BYTE. This is another case it should not allow. Here is a patch that tightens general_operands to not allow SUBREG (MEM) with MEM having side-effects. I would like to hear some thoughts on this. Thanks, Tejas Belagod ARM. diff --git a/gcc/recog.c b/gcc/recog.c index 39a9dda..fa323df 100644 --- a/gcc/recog.c +++ b/gcc/recog.c @@ -984,6 +984,11 @@ general_operand (rtx op, enum machine_mode mode) && MEM_P (sub)) return 0; + /* Similarly, avoid SUBREG (MEM) with side effects (e.g. volatile + mem, PRE_INC address etc). */ + if (!reload_completed && MEM_P (sub) && side_effects_p (sub)) + return 0; + /* FLOAT_MODE subregs can't be paradoxical. Combine will occasionally create such rtl, and we must reject it. */ if (SCALAR_FLOAT_MODE_P (GET_MODE (op))