From patchwork Sat Feb 12 17:35:43 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: "H.J. Lu" X-Patchwork-Id: 82939 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 93722B7123 for ; Sun, 13 Feb 2011 04:35:56 +1100 (EST) Received: (qmail 2720 invoked by alias); 12 Feb 2011 17:35:54 -0000 Received: (qmail 2299 invoked by uid 22791); 12 Feb 2011 17:35:52 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM X-Spam-Check-By: sourceware.org Received: from mail-vx0-f175.google.com (HELO mail-vx0-f175.google.com) (209.85.220.175) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 12 Feb 2011 17:35:45 +0000 Received: by vxa37 with SMTP id 37so1930856vxa.20 for ; Sat, 12 Feb 2011 09:35:44 -0800 (PST) MIME-Version: 1.0 Received: by 10.220.177.65 with SMTP id bh1mr2430844vcb.207.1297532143707; Sat, 12 Feb 2011 09:35:43 -0800 (PST) Received: by 10.220.190.1 with HTTP; Sat, 12 Feb 2011 09:35:43 -0800 (PST) In-Reply-To: References: <20110208230802.GA21938@intel.com> Date: Sat, 12 Feb 2011 09:35:43 -0800 Message-ID: Subject: Re: PATCH: PR middle-end/47383: ivopts miscompiles Pmode != ptr_mode From: "H.J. Lu" To: Richard Guenther Cc: gcc-patches@gcc.gnu.org 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 On Sat, Feb 12, 2011 at 9:27 AM, H.J. Lu wrote: > On Wed, Feb 9, 2011 at 1:17 PM, Richard Guenther > wrote: >> On Wed, Feb 9, 2011 at 9:45 PM, H.J. Lu wrote: >>> On Wed, Feb 9, 2011 at 12:17 PM, Richard Guenther >>> wrote: >>>> On Wed, Feb 9, 2011 at 6:17 PM, H.J. Lu wrote: >>>>> On Wed, Feb 9, 2011 at 2:04 AM, Richard Guenther >>>>> wrote: >>>>>> On Wed, Feb 9, 2011 at 12:08 AM, H.J. Lu wrote: >>>>>>> Hi, >>>>>>> >>>>>>> tree-ssa-loop-ivopts.c has >>>>>>> >>>>>>> --- >>>>>>> /* Returns variant of TYPE that can be used as base for different uses. >>>>>>>   We return unsigned type with the same precision, which avoids problems >>>>>>>   with overflows.  */ >>>>>>> >>>>>>> static tree >>>>>>> generic_type_for (tree type) >>>>>>> { >>>>>>>  if (POINTER_TYPE_P (type)) >>>>>>>    return unsigned_type_for (type); >>>>>>> >>>>>>>  if (TYPE_UNSIGNED (type)) >>>>>>>    return type; >>>>>>> >>>>>>>  return unsigned_type_for (type); >>>>>>> } >>>>>>> --- >>>>>>> >>>>>>> It converts negative step to unsigned type with the same precision. >>>>>>> It doesn't work when Pmode != ptr_mode.  This patch disables it. >>>>>>> OK for trunk in stage 1? >>>>>> >>>>>> This does not look correct.  It rather sounds you are expanding >>>>>> TARGET_MEM_REFs the wrong way - the address computed by >>>>>> it is supposed to be calculated in ptr_mode and only the final >>>>>> result extended to Pmode. >>>>>> >>>>> >>>>> >>>>> TARGET_MEM_REF is OK. The problem is ivopts passes >>>>> the negative offset to TARGET_MEM_REF as unsigned and >>>>> expects offset will wrap. >>>> >>>> But they will wrap, as arithmetic is carried out in a type with >>>> the same precision (that of ptr_mode). >>>> >>>>>  It only works when Pmode == >>>>> ptr_mode.  I am checking in this patch into x32 branch to >>>>> avoid such cases. >>>> >>>> I think you are wrong. >>>> >>> >>> If you have a patch, I can give a try. >> >> I'd have to debug it and I don't have a target that shows the failure, but >> just looking around I assume that in >> >> rtx >> addr_for_mem_ref (struct mem_address *addr, addr_space_t as, >>                  bool really_expand) >> { >>  enum machine_mode address_mode = targetm.addr_space.address_mode (as); >> >> address_mode will be Pmode - that would be already wrong.  We need to >> use ptr_mode here and at the end of the function convert the >> generated address to Pmode appropriately.  Can you verify that theory? >> > > This patch works for me.  However, if I add > Here are 2 patches. I am checking them into x32 branch. diff --git a/gcc/ChangeLog.x32 b/gcc/ChangeLog.x32 index 41bfe18..01acfdd 100644 --- a/gcc/ChangeLog.x32 +++ b/gcc/ChangeLog.x32 @@ -1,3 +1,13 @@ +2011-02-11 H.J. Lu + + PR middle-end/47383 + * config/i386/i386.c (ix86_decompose_address): Support 32bit + address in x32 mode. + (ix86_legitimate_address_p): Likewise. + (ix86_fixup_binary_operands): Likewise. + + * config/i386/i386.md (*lea_1_x32): New. + 2011-01-29 H.J. Lu PR target/47537 diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index b090e7f..63add39 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -11614,6 +11614,13 @@ ix86_decompose_address (rtx addr, struct ix86_address *out) int retval = 1; enum ix86_address_seg seg = SEG_DEFAULT; + /* Support 32bit address in x32 mode. */ + if (TARGET_X32 + && GET_CODE (addr) == ZERO_EXTEND + && GET_MODE (addr) == Pmode + && GET_CODE (XEXP (addr, 0)) == PLUS) + addr = XEXP (addr, 0); + if (REG_P (addr) || GET_CODE (addr) == SUBREG) base = addr; else if (GET_CODE (addr) == PLUS) @@ -12136,6 +12143,7 @@ ix86_legitimate_address_p (enum machine_mode mode ATTRIBUTE_UNUSED, struct ix86_address parts; rtx base, index, disp; HOST_WIDE_INT scale; + enum machine_mode base_mode; if (ix86_decompose_address (addr, &parts) <= 0) /* Decomposition failed. */ @@ -12167,8 +12175,11 @@ ix86_legitimate_address_p (enum machine_mode mode ATTRIBUTE_UNUSED, /* Base is not a register. */ return false; - if (GET_MODE (base) != Pmode) - /* Base is not in Pmode. */ + base_mode = GET_MODE (base); + if (base_mode != Pmode + && !(TARGET_X32 + && base_mode == ptr_mode)) + /* Base is not in Pmode nor ptr_mode. */ return false; if ((strict && ! REG_OK_FOR_BASE_STRICT_P (reg)) @@ -12176,6 +12187,8 @@ ix86_legitimate_address_p (enum machine_mode mode ATTRIBUTE_UNUSED, /* Base is not valid. */ return false; } + else + base_mode = VOIDmode; /* Validate index register. @@ -12184,6 +12197,7 @@ ix86_legitimate_address_p (enum machine_mode mode ATTRIBUTE_UNUSED, if (index) { rtx reg; + enum machine_mode index_mode; if (REG_P (index)) reg = index; @@ -12196,8 +12210,13 @@ ix86_legitimate_address_p (enum machine_mode mode ATTRIBUTE_UNUSED, /* Index is not a register. */ return false; - if (GET_MODE (index) != Pmode) - /* Index is not in Pmode. */ + index_mode = GET_MODE (index); + if ((base_mode != VOIDmode + && base_mode != index_mode) + || (index_mode != Pmode + && !(TARGET_X32 + && index_mode == ptr_mode))) + /* Index is not in Pmode nor ptr_mode. */ return false; if ((strict && ! REG_OK_FOR_INDEX_STRICT_P (reg)) @@ -15954,6 +15973,16 @@ ix86_fixup_binary_operands (enum rtx_code code, enum machine_mode mode, else src2 = force_reg (mode, src2); } + else + { + /* Support 32bit address in x32 mode. */ + if (TARGET_X32 + && code == PLUS + && !MEM_P (dst) + && !MEM_P (src1) + && MEM_P (src2) ) + src2 = force_reg (mode, src2); + } /* If the destination is memory, and we do not have matching source operands, do things in registers. */ diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md index cb044df..f94bd38 100644 --- a/gcc/config/i386/i386.md +++ b/gcc/config/i386/i386.md @@ -5672,6 +5672,14 @@ [(set_attr "type" "lea") (set_attr "mode" "")]) +(define_insn "*lea_1_x32" + [(set (match_operand:SI 0 "register_operand" "=r") + (match_operand:SI 1 "no_seg_address_operand" "p"))] + "TARGET_X32" + "lea{l}\t{%a1, %0|%0, %a1}" + [(set_attr "type" "lea") + (set_attr "mode" "SI")]) + (define_insn "*lea_2" [(set (match_operand:SI 0 "register_operand" "=r") (subreg:SI (match_operand:DI 1 "no_seg_address_operand" "p") 0))]