From patchwork Thu Jun 5 12:19:30 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Senthil Kumar Selvaraj X-Patchwork-Id: 356334 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]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3D4FE1400D5 for ; Thu, 5 Jun 2014 22:19:36 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; q=dns; s=default; b=IAuv/OHIUWbHJcL0P 2g98+FUwmHCe8J3CnyokdCYa4NSYdGzf+YhEvDv9s3TPPIbQ0MDptrtZw1/yBM3z myMqXKz9Q720d6dLyHRxKSru0s4UC4F1z4fr1uYG01kWRJIcrv+Ns2sHzVC+l9Eb 3x2KeuyQ18ybv+nM2zncataywU= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; s=default; bh=Q5e+3Bblk0EPSmn3aQ13xNZ Ulks=; b=UaPlx+I7HGOdOL1FICkZAp+Co49GHd3ReqGL1eAc8tQkReVCBeOlG+/ SYU85VH+0mbeeuE7I6iPjqns2CM22HhNtR50TOq06Paxpfxl+1qaSO4G77HfNWoO jh9/8rMR+qi1aFnVQEYfPpbJdmW/sg5NOkYylTdMFJlqQqKAgsfo= Received: (qmail 28860 invoked by alias); 5 Jun 2014 12:19:29 -0000 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 Received: (qmail 28848 invoked by uid 89); 5 Jun 2014 12:19:29 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 X-HELO: eusmtp01.atmel.com Received: from eusmtp01.atmel.com (HELO eusmtp01.atmel.com) (212.144.249.243) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Thu, 05 Jun 2014 12:19:28 +0000 Received: from apsmtp01.atmel.com (10.168.254.31) by eusmtp01.atmel.com (10.161.101.31) with Microsoft SMTP Server id 14.2.347.0; Thu, 5 Jun 2014 14:19:18 +0200 Received: from PENCHT01.corp.atmel.com (10.168.5.161) by apsmtp01.atmel.com (10.168.254.31) with Microsoft SMTP Server (TLS) id 14.2.347.0; Thu, 5 Jun 2014 20:21:00 +0800 Received: from atmel.com (10.168.5.13) by cas-ap.atmel.com (10.168.5.161) with Microsoft SMTP Server (TLS) id 14.2.342.3; Thu, 5 Jun 2014 20:19:20 +0800 Date: Thu, 5 Jun 2014 17:49:30 +0530 From: Senthil Kumar Selvaraj To: Richard Biener CC: GCC Patches , Richard Biener , Jakub Jelinek , Georg-Johann Lay Subject: Re: Fix address space computation in expand_debug_expr Message-ID: <20140605121929.GA21817@atmel.com> References: <20140604200641.GA10914@atmel.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) X-IsSubscribed: yes On Thu, Jun 05, 2014 at 09:46:25AM +0200, Richard Biener wrote: > On Wed, Jun 4, 2014 at 10:06 PM, Senthil Kumar Selvaraj > wrote: > > For the AVR target, assertions in convert_debug_memory_address cause a > > couple of ICEs (see PR 52472). Jakub suggested returning a NULL rtx, > > which works, but on debugging further, I found that expand_debug_expr > > appears to incorrectly compute the address space for ADDR_EXPR and > > MEM_REFs. > > > > For ADDR_EXPR, TREE_TYPE(exp) is a POINTER_TYPE (always?), but in > > the generic address space, even if the object whose address is taken > > is in a different address space. expand_debug_expr takes > > TYPE_ADDR_SPACE(TREE_TYPE(exp)) and therefore computes the address > > space as generic. convert_debug_memory_address then asserts that the > > mode is a valid pointer mode in the address space and fails. > > > > Similarly, for MEM_REFs, TREE_TYPE(exp) is the type of the > > dereferenced value, and therefore checking if it is a POINTER_TYPE > > doesn't help for a single pointer dereference. The address space > > gets computed as generic even if the pointer points to a different > > address space. The address mode for the generic address space is > > passed to convert_debug_memory_address, and the assertion that that mode > > must match the mode of the rtx then fails. > > > > The below patch attempts to fix this by picking the right TREE_TYPE to > > pass to TYPE_ADDR_SPACE for MEM_REF (use type of arg 0) and > > ADDR_EXPR (check for pointer type and look at nested addr space). > > > > Does this look reasonable or did I get it all wrong? > > > > Regards > > Senthil > > > > diff --git gcc/cfgexpand.c gcc/cfgexpand.c > > index 8b0e466..ca78953 100644 > > --- gcc/cfgexpand.c > > +++ gcc/cfgexpand.c > > @@ -3941,8 +3941,8 @@ expand_debug_expr (tree exp) > > op0 = plus_constant (inner_mode, op0, INTVAL (op1)); > > } > > > > - if (POINTER_TYPE_P (TREE_TYPE (exp))) > > - as = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (exp))); > > + if (POINTER_TYPE_P (TREE_TYPE (TREE_OPERAND (exp, 0)))) > > + as = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (TREE_OPERAND (exp, 0)))); > > else > > as = ADDR_SPACE_GENERIC; > > TREE_OPERAND (exp, 0) always has pointer type so I'd change this to > > as = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (TREE_OPERAND (exp, 0)))); > > > @@ -4467,7 +4467,11 @@ expand_debug_expr (tree exp) > > return NULL; > > } > > > > - as = TYPE_ADDR_SPACE (TREE_TYPE (exp)); > > + if (POINTER_TYPE_P (TREE_TYPE (exp))) > > + as = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (exp))); > > + else > > + as = TYPE_ADDR_SPACE (TREE_TYPE (exp)); > > + > > Likewise - TREE_TYPE (exp) is always a pointer type. Otherwise the > patch looks ok to me. > > Richard. > > > op0 = convert_debug_memory_address (mode, XEXP (op0, 0), as); > > > > return op0; Modified patch attached. This fixes PR 52472 (the ADDR_EXPR case) as well as a couple of ICEs in gcc.target/avr/torture/addr-space-2-x.c (MEM_REF case). I've also added a new testcase for the PR. I don't have commit access - could someone apply this for me please? It'd be great if this was backported to 4.9 and 4.8 branches as well. Regards Senthil gcc/ChangeLog 2014-06-05 Senthil Kumar Selvaraj PR target/52472 * cfgexpand.c (expand_debug_expr): Use address space of nested TREE_TYPE for ADDR_EXPR and MEM_REF. gcc/testsuite/ChangeLog 2014-06-05 Senthil Kumar Selvaraj PR target/52472 * gcc.target/avr/pr52472.c: New test. diff --git gcc/cfgexpand.c gcc/cfgexpand.c index 8b0e466..e161cb7 100644 --- gcc/cfgexpand.c +++ gcc/cfgexpand.c @@ -3941,10 +3941,7 @@ expand_debug_expr (tree exp) op0 = plus_constant (inner_mode, op0, INTVAL (op1)); } - if (POINTER_TYPE_P (TREE_TYPE (exp))) - as = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (exp))); - else - as = ADDR_SPACE_GENERIC; + as = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (TREE_OPERAND (exp, 0)))); op0 = convert_debug_memory_address (targetm.addr_space.address_mode (as), op0, as); @@ -4467,7 +4464,7 @@ expand_debug_expr (tree exp) return NULL; } - as = TYPE_ADDR_SPACE (TREE_TYPE (exp)); + as = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (exp))); op0 = convert_debug_memory_address (mode, XEXP (op0, 0), as); return op0; diff --git gcc/testsuite/gcc.target/avr/pr52472.c gcc/testsuite/gcc.target/avr/pr52472.c new file mode 100644 index 0000000..701cfb4 --- /dev/null +++ gcc/testsuite/gcc.target/avr/pr52472.c @@ -0,0 +1,16 @@ +/* { dg-do compile } */ +/* { dg-options "-Os -g -Wno-pointer-to-int-cast" } */ + +/* This testcase exposes PR52472. expand_debug_expr mistakenly + considers the address space of data to be generic, and + asserts that PSImode pointers aren't valid in the generic + address space. */ + +extern const __memx unsigned data[][10]; + +unsigned long ice (void) +{ + unsigned long addr32; + + return addr32 = ((unsigned long) data); +}