PATCH [5/n] X32: Supprot 32bit address

PATCH [5/n] X32: Supprot 32bit address

[H.J. Lu]

Hi,

TARGET_MEM_REF only works on ptr_mode.  That means base and index parts
of x86 address operand in x32 mode may be in ptr_mode.  This patch
supports 32bit base and index parts in x32 mode.  OK for trunk?

Thanks.


H.J.
---
2011-07-09  H.J. Lu  <hongjiu.lu@intel.com>

	* config/i386/i386.c (ix86_simplify_base_index_disp): New.
	(ix86_decompose_address): Support 32bit address in x32 mode.
	(ix86_legitimate_address_p): Likewise.
	(ix86_fixup_binary_operands): Likewise.

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 04cb07d..c852719 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -10984,6 +11010,190 @@ ix86_live_on_entry (bitmap regs)
     }
 }
 \f
+/* For TARGET_X32, IRA may generate
+
+   (set (reg:SI 40 r11)
+        (plus:SI (plus:SI (mult:SI (reg:SI 1 dx)
+				   (const_int 8))
+			  (subreg:SI (plus:DI (reg/f:DI 7 sp)
+					      (const_int CONST1)) 0))
+		 (const_int CONST2)))
+
+   We translate it into
+
+   (set (reg:SI 40 r11)
+        (plus:SI (plus:SI (mult:SI (reg:SI 1 dx)
+				   (const_int 8))
+			  (reg/f:SI 7 sp))
+		 (const_int [CONST1 + CONST2])))
+
+   We also translate
+
+   (plus:DI (zero_extend:DI (plus:SI (plus:SI (reg:SI 4 si [70])
+					      (reg:SI 2 cx [86]))
+				     (const_int CONST1)))
+	    (const_int CONST2))
+
+   into
+
+   (plus:DI (zero_extend:DI (plus:SI (reg:SI 4 si [70])
+				     (reg:SI 2 cx [86]))
+	    (const_int [CONST1 + CONST2])))
+
+   We also translate
+
+   (plus:SI (plus:SI (plus:SI (reg:SI 4 si [70])
+			      (reg:SI 2 cx [86]))
+		     (symbol_ref:SI ("A.193.2210")))
+	    (const_int CONST))
+
+   into
+
+   (plus:SI (plus:SI (reg:SI 4 si [70])
+		     (reg:SI 2 cx [86]))
+	    (const (plus:SI (symbol_ref:SI ("A.193.2210"))
+			    (const_int CONST))))
+
+   We also translate
+
+   (plus:SI (reg:SI 0 ax [orig:74 D.4067 ] [74])
+	    (subreg:SI (plus:DI (reg/f:DI 7 sp)
+				(const_int 64 [0x40])) 0))
+
+   into
+
+   (plus:SI (reg:SI 0 ax [orig:74 D.4067 ] [74])
+	    (plus:SI (reg/f:SI 7 sp) (const_int 64 [0x40])))
+
+   If PLUS is true, we also translate
+
+   (set (reg:SI 40 r11)
+        (plus:SI (plus:SI (reg:SI 1 dx)
+			  (subreg:SI (plus:DI (reg/f:DI 7 sp)
+					      (const_int CONST1)) 0))
+		 (const_int CONST2)))
+
+   into
+
+   (set (reg:SI 40 r11)
+        (plus:SI (plus:SI (reg:SI 1 dx)
+			  (reg/f:SI 7 sp))
+		 (const_int [CONST1 + CONST2])))
+
+ */
+
+static void
+ix86_simplify_base_index_disp (rtx *base_p, rtx *index_p, rtx *disp_p,
+			       bool plus)
+{
+  rtx base = *base_p;
+  rtx disp, index, op0, op1;
+
+  if (!base || GET_MODE (base) != ptr_mode)
+    return;
+
+  disp = *disp_p;
+  if (disp != NULL_RTX
+      && disp != const0_rtx
+      && !CONST_INT_P (disp))
+    return;
+
+  if (GET_CODE (base) == SUBREG)
+    base = SUBREG_REG (base);
+
+  if (GET_CODE (base) == PLUS)
+    {
+      rtx addend;
+
+      op0 = XEXP (base, 0);
+      op1 = XEXP (base, 1);
+
+      if ((REG_P (op0)
+	   || (!plus
+	       && GET_CODE (op0) == PLUS
+	       && GET_MODE (op0) == ptr_mode
+	       && REG_P (XEXP (op0, 0))
+	       && REG_P (XEXP (op0, 1))))
+	  && (CONST_INT_P (op1)
+	      || GET_CODE (op1) == SYMBOL_REF
+	      || GET_CODE (op1) == LABEL_REF))
+	{
+	  base = op0;
+	  addend = op1;
+	}
+      else if (REG_P (op1)
+	       && (CONST_INT_P (op0)
+		   || GET_CODE (op0) == SYMBOL_REF
+		   || GET_CODE (op0) == LABEL_REF))
+	{
+	  base = op1;
+	  addend = op0;
+	}
+      else if (plus
+	       && GET_CODE (op1) == SUBREG
+	       && GET_MODE (op1) == ptr_mode)
+	{
+	  op1 = SUBREG_REG (op1);
+	  if (GET_CODE (op1) == PLUS)
+	    {
+	      addend = XEXP (op1, 1);
+	      op1 = XEXP (op1, 0);
+	      if (REG_P (op1) && CONST_INT_P (addend))
+		{
+		  op1 = gen_rtx_REG (ptr_mode, REGNO (op1));
+		  *base_p = gen_rtx_PLUS (ptr_mode, op0, op1);
+		}
+	      else
+		return;
+	    }
+	  else
+	    return;
+	}
+      else
+	return;
+
+      if (disp == NULL_RTX || disp == const0_rtx)
+	*disp_p = addend;
+      else
+	{
+	  if (CONST_INT_P (addend))
+	    *disp_p = GEN_INT (INTVAL (disp) + INTVAL (addend));
+	  else
+	    {
+	      disp = gen_rtx_PLUS (ptr_mode, addend, disp);
+	      *disp_p = gen_rtx_CONST (ptr_mode, disp);
+	    }
+	}
+
+      if (!plus)
+	{
+	  if (REG_P (base))
+	    *base_p = gen_rtx_REG (ptr_mode, REGNO (base));
+	  else
+	    *base_p = base;
+	}
+    }
+  else if (!plus
+	   && (disp == NULL_RTX || disp == const0_rtx)
+	   && index_p
+	   && (index = *index_p) != NULL_RTX
+	   && GET_CODE (index) == SUBREG
+	   && GET_MODE (index) == ptr_mode)
+    {
+      index = SUBREG_REG (index);
+      if (GET_CODE (index) == PLUS && GET_MODE (index) == Pmode)
+	{
+	  op0 = XEXP (index, 0);
+	  op1 = XEXP (index, 1);
+	  if (REG_P (op0) && CONST_INT_P (op1))
+	    {
+	      *index_p = gen_rtx_REG (ptr_mode, REGNO (op0));
+	      *disp_p = op1;
+	    }
+	}
+    }
+}
+
 /* Extract the parts of an RTL expression that is a valid memory address
    for an instruction.  Return 0 if the structure of the address is
    grossly off.  Return -1 if the address contains ASHIFT, so it is not
@@ -11000,6 +11210,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)
@@ -11014,6 +11231,24 @@ ix86_decompose_address (rtx addr, struct ix86_address *out)
 	    return 0;
 	  addends[n++] = XEXP (op, 1);
 	  op = XEXP (op, 0);
+	  /* Support 32bit address in x32 mode.  */
+	  if (TARGET_X32 && reload_completed)
+	    {
+	      if (GET_CODE (op) == ZERO_EXTEND
+		  && GET_MODE (op) == Pmode
+		  && GET_CODE (XEXP (op, 0)) == PLUS)
+		{
+		  op = XEXP (op, 0);
+		  if (n == 1)
+		    ix86_simplify_base_index_disp (&op, NULL,
+						   &addends[0], false);
+		}
+	      else if (n == 1
+		       && GET_CODE (op) == PLUS
+		       && GET_MODE (op) == ptr_mode)
+		ix86_simplify_base_index_disp (&op, NULL, &addends[0],
+					       true);
+	    }
 	}
       while (GET_CODE (op) == PLUS);
       if (n >= 4)
@@ -11107,13 +11342,17 @@ ix86_decompose_address (rtx addr, struct ix86_address *out)
       scale = INTVAL (scale_rtx);
     }
 
-  base_reg = base && GET_CODE (base) == SUBREG ? SUBREG_REG (base) : base;
-  index_reg = index && GET_CODE (index) == SUBREG ? SUBREG_REG (index) : index;
+  if (TARGET_X32 && reload_completed)
+    ix86_simplify_base_index_disp (&base, &index, &disp, false);
 
   /* Avoid useless 0 displacement.  */
   if (disp == const0_rtx && (base || index))
     disp = NULL_RTX;
 
+  index_reg = index && GET_CODE (index) == SUBREG ? SUBREG_REG (index) : index;
+
+  base_reg = base && GET_CODE (base) == SUBREG ? SUBREG_REG (base) : base;
+
   /* Allow arg pointer and stack pointer as index if there is not scaling.  */
   if (base_reg && index_reg && scale == 1
       && (index_reg == arg_pointer_rtx
@@ -11522,6 +11761,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.  */
@@ -11553,8 +11793,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))
@@ -11562,6 +11805,8 @@ ix86_legitimate_address_p (enum machine_mode mode ATTRIBUTE_UNUSED,
 	/* Base is not valid.  */
 	return false;
     }
+  else
+    base_mode = VOIDmode;
 
   /* Validate index register.
 
@@ -11570,6 +11815,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;
@@ -11582,8 +11828,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))
@@ -15461,6 +15757,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.  */

Re: PATCH [5/n] X32: Supprot 32bit address

[Uros Bizjak]

On Sun, Jul 10, 2011 at 12:20 AM, H.J. Lu <hongjiu.lu@intel.com> wrote:

> TARGET_MEM_REF only works on ptr_mode.  That means base and index parts
> of x86 address operand in x32 mode may be in ptr_mode.  This patch
> supports 32bit base and index parts in x32 mode.  OK for trunk?
>
> Thanks.
>
>
> H.J.
> ---
> 2011-07-09  H.J. Lu  <hongjiu.lu@intel.com>
>
>        * config/i386/i386.c (ix86_simplify_base_index_disp): New.
>        (ix86_decompose_address): Support 32bit address in x32 mode.
>        (ix86_legitimate_address_p): Likewise.
>        (ix86_fixup_binary_operands): Likewise.

Why don't you handle translations in TARGET_LEGITIMIZE_ADDRESS (or
maybe also LEGITIMIZE_RELOAD_ADDRESS) ?

Uros.

Re: PATCH [5/n] X32: Supprot 32bit address

[H.J. Lu]

On Fri, Jul 15, 2011 at 5:49 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Sun, Jul 10, 2011 at 12:20 AM, H.J. Lu <hongjiu.lu@intel.com> wrote:
>
>> TARGET_MEM_REF only works on ptr_mode.  That means base and index parts
>> of x86 address operand in x32 mode may be in ptr_mode.  This patch
>> supports 32bit base and index parts in x32 mode.  OK for trunk?
>>
>> Thanks.
>>
>>
>> H.J.
>> ---
>> 2011-07-09  H.J. Lu  <hongjiu.lu@intel.com>
>>
>>        * config/i386/i386.c (ix86_simplify_base_index_disp): New.
>>        (ix86_decompose_address): Support 32bit address in x32 mode.
>>        (ix86_legitimate_address_p): Likewise.
>>        (ix86_fixup_binary_operands): Likewise.
>
> Why don't you handle translations in TARGET_LEGITIMIZE_ADDRESS (or
> maybe also LEGITIMIZE_RELOAD_ADDRESS) ?
>

It is because ix86_decompose_address is also called from:

predicates.md:  ok = ix86_decompose_address (op, &parts);
predicates.md:  ok = ix86_decompose_address (op, &parts);
predicates.md:  ok = ix86_decompose_address (XEXP (op, 0), &parts);
predicates.md:  ok = ix86_decompose_address (XEXP (op, 0), &parts);
predicates.md:  ok = ix86_decompose_address (XEXP (op, 0), &parts);


-- 
H.J.

Re: PATCH [5/n] X32: Supprot 32bit address

[Uros Bizjak]

On Fri, Jul 15, 2011 at 3:03 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Fri, Jul 15, 2011 at 5:49 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
>> On Sun, Jul 10, 2011 at 12:20 AM, H.J. Lu <hongjiu.lu@intel.com> wrote:
>>
>>> TARGET_MEM_REF only works on ptr_mode.  That means base and index parts
>>> of x86 address operand in x32 mode may be in ptr_mode.  This patch
>>> supports 32bit base and index parts in x32 mode.  OK for trunk?
>>>
>>> Thanks.
>>>
>>>
>>> H.J.
>>> ---
>>> 2011-07-09  H.J. Lu  <hongjiu.lu@intel.com>
>>>
>>>        * config/i386/i386.c (ix86_simplify_base_index_disp): New.
>>>        (ix86_decompose_address): Support 32bit address in x32 mode.
>>>        (ix86_legitimate_address_p): Likewise.
>>>        (ix86_fixup_binary_operands): Likewise.
>>
>> Why don't you handle translations in TARGET_LEGITIMIZE_ADDRESS (or
>> maybe also LEGITIMIZE_RELOAD_ADDRESS) ?
>>
>
> It is because ix86_decompose_address is also called from:
>
> predicates.md:  ok = ix86_decompose_address (op, &parts);
> predicates.md:  ok = ix86_decompose_address (op, &parts);
> predicates.md:  ok = ix86_decompose_address (XEXP (op, 0), &parts);
> predicates.md:  ok = ix86_decompose_address (XEXP (op, 0), &parts);
> predicates.md:  ok = ix86_decompose_address (XEXP (op, 0), &parts);

Yes, but you should legitimize the address created by reload before it
enters into predicates.

So, the questions are:

+   (set (reg:SI 40 r11)
+        (plus:SI (plus:SI (mult:SI (reg:SI 1 dx)
+                                  (const_int 8))
+                         (subreg:SI (plus:DI (reg/f:DI 7 sp)
+                                             (const_int CONST1)) 0))
+                (const_int CONST2)))
+
+   We translate it into
+
+   (set (reg:SI 40 r11)
+        (plus:SI (plus:SI (mult:SI (reg:SI 1 dx)
+                                  (const_int 8))
+                         (reg/f:SI 7 sp))
+                (const_int [CONST1 + CONST2])))

If the first form of the address is not OK (it does not represent the
hardware operation), then it should not enter into the insn stream.
This means, that it should be fixed ("legitimized") to second form by
appropriate function (it looks that LEGITIMIZE_RELOAD_ADDRESS should
fix it, since the incorrect address is generated by IRA/reload). After
this operation, various predicates, based on ix86_decompose_address
will start to work, since they will decompose valid memory addresses.

Uros.

Re: PATCH [5/n] X32: Supprot 32bit address

[H.J. Lu]

On Fri, Jul 15, 2011 at 8:25 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Fri, Jul 15, 2011 at 3:03 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Fri, Jul 15, 2011 at 5:49 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
>>> On Sun, Jul 10, 2011 at 12:20 AM, H.J. Lu <hongjiu.lu@intel.com> wrote:
>>>
>>>> TARGET_MEM_REF only works on ptr_mode.  That means base and index parts
>>>> of x86 address operand in x32 mode may be in ptr_mode.  This patch
>>>> supports 32bit base and index parts in x32 mode.  OK for trunk?
>>>>
>>>> Thanks.
>>>>
>>>>
>>>> H.J.
>>>> ---
>>>> 2011-07-09  H.J. Lu  <hongjiu.lu@intel.com>
>>>>
>>>>        * config/i386/i386.c (ix86_simplify_base_index_disp): New.
>>>>        (ix86_decompose_address): Support 32bit address in x32 mode.
>>>>        (ix86_legitimate_address_p): Likewise.
>>>>        (ix86_fixup_binary_operands): Likewise.
>>>
>>> Why don't you handle translations in TARGET_LEGITIMIZE_ADDRESS (or
>>> maybe also LEGITIMIZE_RELOAD_ADDRESS) ?
>>>
>>
>> It is because ix86_decompose_address is also called from:
>>
>> predicates.md:  ok = ix86_decompose_address (op, &parts);
>> predicates.md:  ok = ix86_decompose_address (op, &parts);
>> predicates.md:  ok = ix86_decompose_address (XEXP (op, 0), &parts);
>> predicates.md:  ok = ix86_decompose_address (XEXP (op, 0), &parts);
>> predicates.md:  ok = ix86_decompose_address (XEXP (op, 0), &parts);
>
> Yes, but you should legitimize the address created by reload before it
> enters into predicates.
>
> So, the questions are:
>
> +   (set (reg:SI 40 r11)
> +        (plus:SI (plus:SI (mult:SI (reg:SI 1 dx)
> +                                  (const_int 8))
> +                         (subreg:SI (plus:DI (reg/f:DI 7 sp)
> +                                             (const_int CONST1)) 0))
> +                (const_int CONST2)))
> +
> +   We translate it into
> +
> +   (set (reg:SI 40 r11)
> +        (plus:SI (plus:SI (mult:SI (reg:SI 1 dx)
> +                                  (const_int 8))
> +                         (reg/f:SI 7 sp))
> +                (const_int [CONST1 + CONST2])))
>
> If the first form of the address is not OK (it does not represent the
> hardware operation), then it should not enter into the insn stream.
> This means, that it should be fixed ("legitimized") to second form by
> appropriate function (it looks that LEGITIMIZE_RELOAD_ADDRESS should
> fix it, since the incorrect address is generated by IRA/reload). After
> this operation, various predicates, based on ix86_decompose_address
> will start to work, since they will decompose valid memory addresses.
>

IRA/.RELOAD isn't prepared to deal with it and it just ICEs.  I opened
a few GCC bugs on this.

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47744

is one of them.  That is why I went this route.




-- 
H.J.

Re: PATCH [5/n] X32: Supprot 32bit address

[H.J. Lu]

On Fri, Jul 15, 2011 at 9:01 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Fri, Jul 15, 2011 at 5:44 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>
>>>>>> TARGET_MEM_REF only works on ptr_mode.  That means base and index parts
>>>>>> of x86 address operand in x32 mode may be in ptr_mode.  This patch
>>>>>> supports 32bit base and index parts in x32 mode.  OK for trunk?
>>>>>>
>>>>>> Thanks.
>>>>>>
>>>>>>
>>>>>> H.J.
>>>>>> ---
>>>>>> 2011-07-09  H.J. Lu  <hongjiu.lu@intel.com>
>>>>>>
>>>>>>        * config/i386/i386.c (ix86_simplify_base_index_disp): New.
>>>>>>        (ix86_decompose_address): Support 32bit address in x32 mode.
>>>>>>        (ix86_legitimate_address_p): Likewise.
>>>>>>        (ix86_fixup_binary_operands): Likewise.
>>>>>
>>>>> Why don't you handle translations in TARGET_LEGITIMIZE_ADDRESS (or
>>>>> maybe also LEGITIMIZE_RELOAD_ADDRESS) ?
>>>>>
>>>>
>>>> It is because ix86_decompose_address is also called from:
>>>>
>>>> predicates.md:  ok = ix86_decompose_address (op, &parts);
>>>> predicates.md:  ok = ix86_decompose_address (op, &parts);
>>>> predicates.md:  ok = ix86_decompose_address (XEXP (op, 0), &parts);
>>>> predicates.md:  ok = ix86_decompose_address (XEXP (op, 0), &parts);
>>>> predicates.md:  ok = ix86_decompose_address (XEXP (op, 0), &parts);
>>>
>>> Yes, but you should legitimize the address created by reload before it
>>> enters into predicates.
>>>
>>> So, the questions are:
>>>
>>> +   (set (reg:SI 40 r11)
>>> +        (plus:SI (plus:SI (mult:SI (reg:SI 1 dx)
>>> +                                  (const_int 8))
>>> +                         (subreg:SI (plus:DI (reg/f:DI 7 sp)
>>> +                                             (const_int CONST1)) 0))
>>> +                (const_int CONST2)))
>>> +
>>> +   We translate it into
>>> +
>>> +   (set (reg:SI 40 r11)
>>> +        (plus:SI (plus:SI (mult:SI (reg:SI 1 dx)
>>> +                                  (const_int 8))
>>> +                         (reg/f:SI 7 sp))
>>> +                (const_int [CONST1 + CONST2])))
>>>
>>> If the first form of the address is not OK (it does not represent the
>>> hardware operation), then it should not enter into the insn stream.
>>> This means, that it should be fixed ("legitimized") to second form by
>>> appropriate function (it looks that LEGITIMIZE_RELOAD_ADDRESS should
>>> fix it, since the incorrect address is generated by IRA/reload). After
>>> this operation, various predicates, based on ix86_decompose_address
>>> will start to work, since they will decompose valid memory addresses.
>>>
>>
>> IRA/.RELOAD isn't prepared to deal with it and it just ICEs.  I opened
>> a few GCC bugs on this.
>>
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47744
>>
>> is one of them.  That is why I went this route.
>
> Hm, but it crashed in postreload pass since the address was not in the
> legitimate form.  This is exactly what LEGITIMIZE_RELOAD_ADDRESS
> fixes. Did you try to go this route?
>

It ran into various ICEs like:

/export/build/gnu/gcc-x32/build-x86_64-linux/gcc/xgcc
-B/export/build/gnu/gcc-x32/build-x86_64-linux/gcc/ -S -o m.s -mx32 -std=gnu99
-O2 -fPIC    m.i
m.i: In function \u2018__kernel_rem_pio2\u2019:
m.i:18:1: error: insn does not satisfy its constraints:
(insn 108 106 186 3 (set (reg:SI 40 r11 [207])
        (plus:SI (plus:SI (mult:SI (reg:SI 1 dx [205])
                    (const_int 8 [0x8]))
                (subreg:SI (plus:DI (reg/f:DI 7 sp)
                        (const_int 208 [0xd0])) 0))
            (const_int -160 [0xffffffffffffff60]))) m.i:3 251 {*lea_1_x32}
     (nil))
m.i:18:1: internal compiler error: in reload_cse_simplify_operands, at
postreload.c:403
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://gcc.gnu.org/bugs.html> for instructions.
make: *** [m.s] Error 1


H.J.

Re: PATCH [5/n] X32: Supprot 32bit address

[Uros Bizjak]

On Fri, Jul 15, 2011 at 5:44 PM, H.J. Lu <hjl.tools@gmail.com> wrote:

>>>>> TARGET_MEM_REF only works on ptr_mode.  That means base and index parts
>>>>> of x86 address operand in x32 mode may be in ptr_mode.  This patch
>>>>> supports 32bit base and index parts in x32 mode.  OK for trunk?
>>>>>
>>>>> Thanks.
>>>>>
>>>>>
>>>>> H.J.
>>>>> ---
>>>>> 2011-07-09  H.J. Lu  <hongjiu.lu@intel.com>
>>>>>
>>>>>        * config/i386/i386.c (ix86_simplify_base_index_disp): New.
>>>>>        (ix86_decompose_address): Support 32bit address in x32 mode.
>>>>>        (ix86_legitimate_address_p): Likewise.
>>>>>        (ix86_fixup_binary_operands): Likewise.
>>>>
>>>> Why don't you handle translations in TARGET_LEGITIMIZE_ADDRESS (or
>>>> maybe also LEGITIMIZE_RELOAD_ADDRESS) ?
>>>>
>>>
>>> It is because ix86_decompose_address is also called from:
>>>
>>> predicates.md:  ok = ix86_decompose_address (op, &parts);
>>> predicates.md:  ok = ix86_decompose_address (op, &parts);
>>> predicates.md:  ok = ix86_decompose_address (XEXP (op, 0), &parts);
>>> predicates.md:  ok = ix86_decompose_address (XEXP (op, 0), &parts);
>>> predicates.md:  ok = ix86_decompose_address (XEXP (op, 0), &parts);
>>
>> Yes, but you should legitimize the address created by reload before it
>> enters into predicates.
>>
>> So, the questions are:
>>
>> +   (set (reg:SI 40 r11)
>> +        (plus:SI (plus:SI (mult:SI (reg:SI 1 dx)
>> +                                  (const_int 8))
>> +                         (subreg:SI (plus:DI (reg/f:DI 7 sp)
>> +                                             (const_int CONST1)) 0))
>> +                (const_int CONST2)))
>> +
>> +   We translate it into
>> +
>> +   (set (reg:SI 40 r11)
>> +        (plus:SI (plus:SI (mult:SI (reg:SI 1 dx)
>> +                                  (const_int 8))
>> +                         (reg/f:SI 7 sp))
>> +                (const_int [CONST1 + CONST2])))
>>
>> If the first form of the address is not OK (it does not represent the
>> hardware operation), then it should not enter into the insn stream.
>> This means, that it should be fixed ("legitimized") to second form by
>> appropriate function (it looks that LEGITIMIZE_RELOAD_ADDRESS should
>> fix it, since the incorrect address is generated by IRA/reload). After
>> this operation, various predicates, based on ix86_decompose_address
>> will start to work, since they will decompose valid memory addresses.
>>
>
> IRA/.RELOAD isn't prepared to deal with it and it just ICEs.  I opened
> a few GCC bugs on this.
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47744
>
> is one of them.  That is why I went this route.

Hm, but it crashed in postreload pass since the address was not in the
legitimate form.  This is exactly what LEGITIMIZE_RELOAD_ADDRESS
fixes. Did you try to go this route?

Uros.

Re: PATCH [5/n] X32: Supprot 32bit address

[Uros Bizjak]

On Fri, Jul 15, 2011 at 6:07 PM, H.J. Lu <hjl.tools@gmail.com> wrote:

>>>> If the first form of the address is not OK (it does not represent the
>>>> hardware operation), then it should not enter into the insn stream.
>>>> This means, that it should be fixed ("legitimized") to second form by
>>>> appropriate function (it looks that LEGITIMIZE_RELOAD_ADDRESS should
>>>> fix it, since the incorrect address is generated by IRA/reload). After
>>>> this operation, various predicates, based on ix86_decompose_address
>>>> will start to work, since they will decompose valid memory addresses.
>>>>
>>>
>>> IRA/.RELOAD isn't prepared to deal with it and it just ICEs.  I opened
>>> a few GCC bugs on this.
>>>
>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47744
>>>
>>> is one of them.  That is why I went this route.
>>
>> Hm, but it crashed in postreload pass since the address was not in the
>> legitimate form.  This is exactly what LEGITIMIZE_RELOAD_ADDRESS
>> fixes. Did you try to go this route?
>>
>
> It ran into various ICEs like:
>
> /export/build/gnu/gcc-x32/build-x86_64-linux/gcc/xgcc
> -B/export/build/gnu/gcc-x32/build-x86_64-linux/gcc/ -S -o m.s -mx32 -std=gnu99
> -O2 -fPIC    m.i
> m.i: In function \u2018__kernel_rem_pio2\u2019:
> m.i:18:1: error: insn does not satisfy its constraints:
> (insn 108 106 186 3 (set (reg:SI 40 r11 [207])
>        (plus:SI (plus:SI (mult:SI (reg:SI 1 dx [205])
>                    (const_int 8 [0x8]))
>                (subreg:SI (plus:DI (reg/f:DI 7 sp)
>                        (const_int 208 [0xd0])) 0))
>            (const_int -160 [0xffffffffffffff60]))) m.i:3 251 {*lea_1_x32}
>     (nil))
> m.i:18:1: internal compiler error: in reload_cse_simplify_operands, at
> postreload.c:403
> Please submit a full bug report,
> with preprocessed source if appropriate.
> See <http://gcc.gnu.org/bugs.html> for instructions.
> make: *** [m.s] Error 1

Yes, this is an example from PR I am referring to. Did you try to
define LEGITIMIZE_RELOAD_ADDRESS? It is supposed to fix this.

Uros.

Re: PATCH [5/n] X32: Supprot 32bit address

[H.J. Lu]

On Fri, Jul 15, 2011 at 9:09 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Fri, Jul 15, 2011 at 6:07 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>
>>>>> If the first form of the address is not OK (it does not represent the
>>>>> hardware operation), then it should not enter into the insn stream.
>>>>> This means, that it should be fixed ("legitimized") to second form by
>>>>> appropriate function (it looks that LEGITIMIZE_RELOAD_ADDRESS should
>>>>> fix it, since the incorrect address is generated by IRA/reload). After
>>>>> this operation, various predicates, based on ix86_decompose_address
>>>>> will start to work, since they will decompose valid memory addresses.
>>>>>
>>>>
>>>> IRA/.RELOAD isn't prepared to deal with it and it just ICEs.  I opened
>>>> a few GCC bugs on this.
>>>>
>>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47744
>>>>
>>>> is one of them.  That is why I went this route.
>>>
>>> Hm, but it crashed in postreload pass since the address was not in the
>>> legitimate form.  This is exactly what LEGITIMIZE_RELOAD_ADDRESS
>>> fixes. Did you try to go this route?
>>>
>>
>> It ran into various ICEs like:
>>
>> /export/build/gnu/gcc-x32/build-x86_64-linux/gcc/xgcc
>> -B/export/build/gnu/gcc-x32/build-x86_64-linux/gcc/ -S -o m.s -mx32 -std=gnu99
>> -O2 -fPIC    m.i
>> m.i: In function \u2018__kernel_rem_pio2\u2019:
>> m.i:18:1: error: insn does not satisfy its constraints:
>> (insn 108 106 186 3 (set (reg:SI 40 r11 [207])
>>        (plus:SI (plus:SI (mult:SI (reg:SI 1 dx [205])
>>                    (const_int 8 [0x8]))
>>                (subreg:SI (plus:DI (reg/f:DI 7 sp)
>>                        (const_int 208 [0xd0])) 0))
>>            (const_int -160 [0xffffffffffffff60]))) m.i:3 251 {*lea_1_x32}
>>     (nil))
>> m.i:18:1: internal compiler error: in reload_cse_simplify_operands, at
>> postreload.c:403
>> Please submit a full bug report,
>> with preprocessed source if appropriate.
>> See <http://gcc.gnu.org/bugs.html> for instructions.
>> make: *** [m.s] Error 1
>
> Yes, this is an example from PR I am referring to. Did you try to
> define LEGITIMIZE_RELOAD_ADDRESS? It is supposed to fix this.
>
>

I will take a look.

Thanks.



-- 
H.J.

Re: PATCH [5/n] X32: Supprot 32bit address

[H.J. Lu]

On Fri, Jul 15, 2011 at 9:09 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Fri, Jul 15, 2011 at 6:07 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>
>>>>> If the first form of the address is not OK (it does not represent the
>>>>> hardware operation), then it should not enter into the insn stream.
>>>>> This means, that it should be fixed ("legitimized") to second form by
>>>>> appropriate function (it looks that LEGITIMIZE_RELOAD_ADDRESS should
>>>>> fix it, since the incorrect address is generated by IRA/reload). After
>>>>> this operation, various predicates, based on ix86_decompose_address
>>>>> will start to work, since they will decompose valid memory addresses.
>>>>>
>>>>
>>>> IRA/.RELOAD isn't prepared to deal with it and it just ICEs.  I opened
>>>> a few GCC bugs on this.
>>>>
>>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47744
>>>>
>>>> is one of them.  That is why I went this route.
>>>
>>> Hm, but it crashed in postreload pass since the address was not in the
>>> legitimate form.  This is exactly what LEGITIMIZE_RELOAD_ADDRESS
>>> fixes. Did you try to go this route?
>>>
>>
>> It ran into various ICEs like:
>>
>> /export/build/gnu/gcc-x32/build-x86_64-linux/gcc/xgcc
>> -B/export/build/gnu/gcc-x32/build-x86_64-linux/gcc/ -S -o m.s -mx32 -std=gnu99
>> -O2 -fPIC    m.i
>> m.i: In function \u2018__kernel_rem_pio2\u2019:
>> m.i:18:1: error: insn does not satisfy its constraints:
>> (insn 108 106 186 3 (set (reg:SI 40 r11 [207])
>>        (plus:SI (plus:SI (mult:SI (reg:SI 1 dx [205])
>>                    (const_int 8 [0x8]))
>>                (subreg:SI (plus:DI (reg/f:DI 7 sp)
>>                        (const_int 208 [0xd0])) 0))
>>            (const_int -160 [0xffffffffffffff60]))) m.i:3 251 {*lea_1_x32}
>>     (nil))
>> m.i:18:1: internal compiler error: in reload_cse_simplify_operands, at
>> postreload.c:403
>> Please submit a full bug report,
>> with preprocessed source if appropriate.
>> See <http://gcc.gnu.org/bugs.html> for instructions.
>> make: *** [m.s] Error 1
>
> Yes, this is an example from PR I am referring to. Did you try to
> define LEGITIMIZE_RELOAD_ADDRESS? It is supposed to fix this.
>

They make things even more complex. ix86_simplify_base_index_disp
is called after reload is done since we can do this translation safely
only on hard registers, not on pseudo registers.


-- 
H.J.

Re: PATCH [5/n] X32: Supprot 32bit address

[H.J. Lu]

On Fri, Jul 15, 2011 at 10:18 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Fri, Jul 15, 2011 at 9:09 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
>> On Fri, Jul 15, 2011 at 6:07 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>
>>>>>> If the first form of the address is not OK (it does not represent the
>>>>>> hardware operation), then it should not enter into the insn stream.
>>>>>> This means, that it should be fixed ("legitimized") to second form by
>>>>>> appropriate function (it looks that LEGITIMIZE_RELOAD_ADDRESS should
>>>>>> fix it, since the incorrect address is generated by IRA/reload). After
>>>>>> this operation, various predicates, based on ix86_decompose_address
>>>>>> will start to work, since they will decompose valid memory addresses.
>>>>>>
>>>>>
>>>>> IRA/.RELOAD isn't prepared to deal with it and it just ICEs.  I opened
>>>>> a few GCC bugs on this.
>>>>>
>>>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47744
>>>>>
>>>>> is one of them.  That is why I went this route.
>>>>
>>>> Hm, but it crashed in postreload pass since the address was not in the
>>>> legitimate form.  This is exactly what LEGITIMIZE_RELOAD_ADDRESS
>>>> fixes. Did you try to go this route?
>>>>
>>>
>>> It ran into various ICEs like:
>>>
>>> /export/build/gnu/gcc-x32/build-x86_64-linux/gcc/xgcc
>>> -B/export/build/gnu/gcc-x32/build-x86_64-linux/gcc/ -S -o m.s -mx32 -std=gnu99
>>> -O2 -fPIC    m.i
>>> m.i: In function \u2018__kernel_rem_pio2\u2019:
>>> m.i:18:1: error: insn does not satisfy its constraints:
>>> (insn 108 106 186 3 (set (reg:SI 40 r11 [207])
>>>        (plus:SI (plus:SI (mult:SI (reg:SI 1 dx [205])
>>>                    (const_int 8 [0x8]))
>>>                (subreg:SI (plus:DI (reg/f:DI 7 sp)
>>>                        (const_int 208 [0xd0])) 0))
>>>            (const_int -160 [0xffffffffffffff60]))) m.i:3 251 {*lea_1_x32}
>>>     (nil))
>>> m.i:18:1: internal compiler error: in reload_cse_simplify_operands, at
>>> postreload.c:403
>>> Please submit a full bug report,
>>> with preprocessed source if appropriate.
>>> See <http://gcc.gnu.org/bugs.html> for instructions.
>>> make: *** [m.s] Error 1
>>
>> Yes, this is an example from PR I am referring to. Did you try to
>> define LEGITIMIZE_RELOAD_ADDRESS? It is supposed to fix this.
>>
>
> They make things even more complex. ix86_simplify_base_index_disp
> is called after reload is done since we can do this translation safely
> only on hard registers, not on pseudo registers.
>

Hi Uros,

The current implementation  has been tested extensively. I'd like to keep
it ASIS so that we can have a working x32 support.  We will revisit it later:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49765

after we have a working x32 GCC.

Thanks.

-- 
H.J.

Re: PATCH [5/n] X32: Supprot 32bit address

[Uros Bizjak]

On Sat, Jul 16, 2011 at 6:47 PM, H.J. Lu <hjl.tools@gmail.com> wrote:


>>> Yes, this is an example from PR I am referring to. Did you try to
>>> define LEGITIMIZE_RELOAD_ADDRESS? It is supposed to fix this.
>>>
>>
>> They make things even more complex. ix86_simplify_base_index_disp
>> is called after reload is done since we can do this translation safely
>> only on hard registers, not on pseudo registers.
>>
>
> Hi Uros,
>
> The current implementation  has been tested extensively. I'd like to keep
> it ASIS so that we can have a working x32 support.  We will revisit it later:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49765
>
> after we have a working x32 GCC.

This can not be only my decision, I have CCd other x86 maintainers and
RMs for their opinion on this question.

Uros.

Re: PATCH [5/n] X32: Supprot 32bit address

[Richard Sandiford]

Uros Bizjak <ubizjak@gmail.com> writes:
> On Sat, Jul 16, 2011 at 6:47 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>> Yes, this is an example from PR I am referring to. Did you try to
>>>> define LEGITIMIZE_RELOAD_ADDRESS? It is supposed to fix this.
>>>>
>>>
>>> They make things even more complex. ix86_simplify_base_index_disp
>>> is called after reload is done since we can do this translation safely
>>> only on hard registers, not on pseudo registers.
>>>
>>
>> Hi Uros,
>>
>> The current implementation  has been tested extensively. I'd like to keep
>> it ASIS so that we can have a working x32 support.  We will revisit it later:
>>
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49765
>>
>> after we have a working x32 GCC.
>
> This can not be only my decision, I have CCd other x86 maintainers and
> RMs for their opinion on this question.

FWIW, I agree with you that things like:

   (set (reg:SI 40 r11)
        (plus:SI (plus:SI (mult:SI (reg:SI 1 dx)
				   (const_int 8))
			  (subreg:SI (plus:DI (reg/f:DI 7 sp)
					      (const_int CONST1)) 0))
		 (const_int CONST2)))

do not look like things that should ever enter the insn stream.
They're liable to confuse other code besides the x86 predicates.
The target of the conversion:

   (set (reg:SI 40 r11)
        (plus:SI (plus:SI (mult:SI (reg:SI 1 dx)
				   (const_int 8))
			  (reg/f:SI 7 sp))
		 (const_int [CONST1 + CONST2])))

looks like the generally preferred form.  It isn't an x32-ism.

LEGITIMIZE_RELOAD_ADDRESS is supposed to be for optimisation only,
not correctness.  Why doesn't reload have enough information to
generate the correct form itself?

Richard

Re: PATCH [5/n] X32: Supprot 32bit address

[Uros Bizjak]

On Tue, Jul 19, 2011 at 1:25 PM, Richard Sandiford
<richard.sandiford@linaro.org> wrote:

>> On Sat, Jul 16, 2011 at 6:47 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>> Yes, this is an example from PR I am referring to. Did you try to
>>>>> define LEGITIMIZE_RELOAD_ADDRESS? It is supposed to fix this.
>>>>>
>>>>
>>>> They make things even more complex. ix86_simplify_base_index_disp
>>>> is called after reload is done since we can do this translation safely
>>>> only on hard registers, not on pseudo registers.
>>>>
>>>
>>> Hi Uros,
>>>
>>> The current implementation  has been tested extensively. I'd like to keep
>>> it ASIS so that we can have a working x32 support.  We will revisit it later:
>>>
>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49765
>>>
>>> after we have a working x32 GCC.
>>
>> This can not be only my decision, I have CCd other x86 maintainers and
>> RMs for their opinion on this question.
>
> FWIW, I agree with you that things like:
>
>   (set (reg:SI 40 r11)
>        (plus:SI (plus:SI (mult:SI (reg:SI 1 dx)
>                                   (const_int 8))
>                          (subreg:SI (plus:DI (reg/f:DI 7 sp)
>                                              (const_int CONST1)) 0))
>                 (const_int CONST2)))
>
> do not look like things that should ever enter the insn stream.
> They're liable to confuse other code besides the x86 predicates.
> The target of the conversion:
>
>   (set (reg:SI 40 r11)
>        (plus:SI (plus:SI (mult:SI (reg:SI 1 dx)
>                                   (const_int 8))
>                          (reg/f:SI 7 sp))
>                 (const_int [CONST1 + CONST2])))
>
> looks like the generally preferred form.  It isn't an x32-ism.
>
> LEGITIMIZE_RELOAD_ADDRESS is supposed to be for optimisation only,
> not correctness.  Why doesn't reload have enough information to
> generate the correct form itself?

Please see the solution at [1]. The problem was that x86 target
allowed SImode subregs of DImode operations (i.e. PLUS).  When these
are rejected, everything works as expected.

IMO, LEGITIMIZE_RELOAD_ADDRESS can not optimize resulting RTX, as shown in [1].

[1] http://gcc.gnu.org/ml/gcc-patches/2011-07/msg01427.html

Uros.