From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31201 invoked by alias); 8 Aug 2012 13:43:58 -0000 Received: (qmail 31185 invoked by uid 22791); 8 Aug 2012 13:43:57 -0000 X-SWARE-Spam-Status: No, hits=-5.0 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,KHOP_RCVD_TRUST,KHOP_THREADED,RCVD_IN_DNSWL_LOW,RCVD_IN_HOSTKARMA_YE X-Spam-Check-By: sourceware.org Received: from mail-pb0-f47.google.com (HELO mail-pb0-f47.google.com) (209.85.160.47) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 08 Aug 2012 13:43:44 +0000 Received: by pbcwy7 with SMTP id wy7so1449947pbc.20 for ; Wed, 08 Aug 2012 06:43:43 -0700 (PDT) MIME-Version: 1.0 Received: by 10.68.201.198 with SMTP id kc6mr36353697pbc.122.1344433423648; Wed, 08 Aug 2012 06:43:43 -0700 (PDT) Received: by 10.66.11.130 with HTTP; Wed, 8 Aug 2012 06:43:43 -0700 (PDT) In-Reply-To: References: <20120801184138.GA3787@intel.com> <87a9ye1lsh.fsf@talisman.home> <87r4rlkcf1.fsf@talisman.home> Date: Wed, 08 Aug 2012 13:43:00 -0000 Message-ID: Subject: Re: PATCH: PR rtl-optimization/54157: [x32] -maddress-mode=long failures From: Uros Bizjak To: "H.J. Lu" Cc: gcc-patches@gcc.gnu.org, rdsandiford@googlemail.com Content-Type: text/plain; charset=ISO-8859-1 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 X-SW-Source: 2012-08/txt/msg00432.txt.bz2 On Wed, Aug 8, 2012 at 3:40 PM, H.J. Lu wrote: > wrote: >> "H.J. Lu" writes: >>> diff --git a/gcc/combine.c b/gcc/combine.c >>> index a07c046..b9a3589 100644 >>> --- a/gcc/combine.c >>> +++ b/gcc/combine.c >>> @@ -10784,12 +10784,30 @@ gen_lowpart_for_combine (enum machine_mode omode, rtx >>> x) >>> if (omode == imode) >>> return x; >>> >>> - /* Return identity if this is a CONST or symbolic reference. */ >>> - if (omode == Pmode >>> - && (GET_CODE (x) == CONST >>> - || GET_CODE (x) == SYMBOL_REF >>> - || GET_CODE (x) == LABEL_REF)) >>> - return x; >>> + if (omode == Pmode) >>> + { >>> + /* Return identity if this is a symbolic reference. */ >>> + if (GET_CODE (x) == SYMBOL_REF || GET_CODE (x) == LABEL_REF) >>> + return x; >>> + >>> + /* Return identity for CONST unless this is a PLUS of 2 constant >>> + operands. */ >>> + if (GET_CODE (x) == CONST) >>> + { >>> + rtx y = XEXP (x, 0); >>> + if (GET_CODE (y) == PLUS >>> + && ((CONST_INT_P (XEXP (y, 1)) >>> + && (GET_CODE (XEXP (y, 0)) == CONST >>> + || GET_CODE (XEXP (y, 0)) == SYMBOL_REF >>> + || GET_CODE (XEXP (y, 0)) == LABEL_REF)) >>> + || (CONST_INT_P (XEXP (y, 1)) >>> + && (GET_CODE (XEXP (y, 0)) == CONST >>> + || GET_CODE (XEXP (y, 0)) == SYMBOL_REF >>> + || GET_CODE (XEXP (y, 0)) == LABEL_REF)))) >>> + goto fail; >>> + return x; >>> + } >>> + } >>> >>> /* We can only support MODE being wider than a word if X is a >>> constant integer or has a mode the same size. */ >>> >>> works for the testcase. >> >> My point was that the "return x" is always wrong. Whenever we return x >> here, we know we're returning something in a different mode from the one >> that the caller wanted. Returning a Pmode LABEL_REF might not trigger >> that plus_constant assert, but it's still wrong. >> >> It looks like this came from the mips-rewrite branch: >> >> 2003-03-13 Richard Sandiford >> >> * combine.c (gen_lowpart_for_combine): Treat the lowpart Pmode of >> a CONST as identity. Check the return value of gen_lowpart_common. >> >> so I can categorically confirm that the person who wrote it didn't >> know what they were doing. It also means that this case was motivated >> by an experiment to make Pmode == DImode for n32, which we ended up >> discarding because it produced worse code. >> >> If this case really is important, we might consider using >> convert_memory_address (Pmode, x) instead. I'm not sure whether >> that would be right for targets with address spaces though, because >> we don't know which address space is associated with the address. >> Hopefully someone who knows address spaces can comment. >> >> If it is correct, then it should probably go in gen_lowpart_common >> rather than gen_lowpart_for_combine. >> >> But given how few people have hit this, it doesn't look like a >> particularly important attempted optimisation. I'll pre-approve >> a patch that undoes my mistake and simply removes: >> >> /* Return identity if this is a CONST or symbolic reference. */ >> if (omode == Pmode >> && (GET_CODE (x) == CONST >> || GET_CODE (x) == SYMBOL_REF >> || GET_CODE (x) == LABEL_REF)) >> return x; >> >> Richard > > This is the patch I checked in. Probably we need to backport this patch to 4.7, where x32 is -maddress-mode=long by default. Uros.