diff mbox

PATCH: PR middle-end/47383: ivopts miscompiles Pmode != ptr_mode

Message ID AANLkTinjnV7CSno9q06x16aM9n-JW4e+0kkQQ+HdB+16@mail.gmail.com
State New
Headers show

Commit Message

H.J. Lu Feb. 12, 2011, 5:35 p.m. UTC 
On Sat, Feb 12, 2011 at 9:27 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Wed, Feb 9, 2011 at 1:17 PM, Richard Guenther
> <richard.guenther@gmail.com> wrote:
>> On Wed, Feb 9, 2011 at 9:45 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> On Wed, Feb 9, 2011 at 12:17 PM, Richard Guenther
>>> <richard.guenther@gmail.com> wrote:
>>>> On Wed, Feb 9, 2011 at 6:17 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>> On Wed, Feb 9, 2011 at 2:04 AM, Richard Guenther
>>>>> <richard.guenther@gmail.com> wrote:
>>>>>> On Wed, Feb 9, 2011 at 12:08 AM, H.J. Lu <hongjiu.lu@intel.com> 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 mbox

Patch

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  <hongjiu.lu@intel.com>
+
+	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  <hongjiu.lu@intel.com>
 
 	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" "<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))]