public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* PING: PATCH [4/n]: Prepare x32: Permute the conversion and addition if one operand is a constant
@ 2011-07-05 14:30 H.J. Lu
  2011-07-09 21:22 ` H.J. Lu
  2011-07-09 21:23 ` Paolo Bonzini
  0 siblings, 2 replies; 37+ messages in thread
From: H.J. Lu @ 2011-07-05 14:30 UTC (permalink / raw)
  To: GCC Patches

Ping.

On Sat, Jun 11, 2011 at 8:58 AM, H.J. Lu <hongjiu.lu@intel.com> wrote:
> Hi,
>
> convert_memory_address_addr_space has a special PLUS/MULT case for
> POINTERS_EXTEND_UNSIGNED < 0.  It turns out that it is also needed
> for all Pmode != ptr_mode cases.  OK for trunk?
>
> Thanks.
>
>
> H.J.
> ---
> 2011-06-11  H.J. Lu  <hongjiu.lu@intel.com>
>
>        PR middle-end/47727
>        * explow.c (convert_memory_address_addr_space): Permute the
>        conversion and addition if one operand is a constant.
>
> diff --git a/gcc/explow.c b/gcc/explow.c
> index 7387dad..b343bf8 100644
> --- a/gcc/explow.c
> +++ b/gcc/explow.c
> @@ -383,18 +383,13 @@ convert_memory_address_addr_space (enum machine_mode to_mode ATTRIBUTE_UNUSED,
>
>     case PLUS:
>     case MULT:
> -      /* For addition we can safely permute the conversion and addition
> -        operation if one operand is a constant and converting the constant
> -        does not change it or if one operand is a constant and we are
> -        using a ptr_extend instruction  (POINTERS_EXTEND_UNSIGNED < 0).
> -        We can always safely permute them if we are making the address
> -        narrower.  */
> +      /* For addition we safely permute the conversion and addition
> +        operation if one operand is a constant since we can't generate
> +        new instructions.  We can always safely permute them if we are
> +        making the address narrower.  */
>       if (GET_MODE_SIZE (to_mode) < GET_MODE_SIZE (from_mode)
>          || (GET_CODE (x) == PLUS
> -             && CONST_INT_P (XEXP (x, 1))
> -             && (XEXP (x, 1) == convert_memory_address_addr_space
> -                                  (to_mode, XEXP (x, 1), as)
> -                 || POINTERS_EXTEND_UNSIGNED < 0)))
> +             && CONST_INT_P (XEXP (x, 1))))
>        return gen_rtx_fmt_ee (GET_CODE (x), to_mode,
>                               convert_memory_address_addr_space
>                                 (to_mode, XEXP (x, 0), as),
>



-- 
H.J.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: PING: PATCH [4/n]: Prepare x32: Permute the conversion and addition if one operand is a constant
  2011-07-05 14:30 PING: PATCH [4/n]: Prepare x32: Permute the conversion and addition if one operand is a constant H.J. Lu
@ 2011-07-09 21:22 ` H.J. Lu
  2011-07-09 21:23 ` Paolo Bonzini
  1 sibling, 0 replies; 37+ messages in thread
From: H.J. Lu @ 2011-07-09 21:22 UTC (permalink / raw)
  To: GCC Patches, Jeffrey Law

Hi Jeff,

Can you take a look at this?

Thanks.

H.J.
On Tue, Jul 5, 2011 at 7:27 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> Ping.
>
> On Sat, Jun 11, 2011 at 8:58 AM, H.J. Lu <hongjiu.lu@intel.com> wrote:
>> Hi,
>>
>> convert_memory_address_addr_space has a special PLUS/MULT case for
>> POINTERS_EXTEND_UNSIGNED < 0.  It turns out that it is also needed
>> for all Pmode != ptr_mode cases.  OK for trunk?
>>
>> Thanks.
>>
>>
>> H.J.
>> ---
>> 2011-06-11  H.J. Lu  <hongjiu.lu@intel.com>
>>
>>        PR middle-end/47727
>>        * explow.c (convert_memory_address_addr_space): Permute the
>>        conversion and addition if one operand is a constant.
>>
>> diff --git a/gcc/explow.c b/gcc/explow.c
>> index 7387dad..b343bf8 100644
>> --- a/gcc/explow.c
>> +++ b/gcc/explow.c
>> @@ -383,18 +383,13 @@ convert_memory_address_addr_space (enum machine_mode to_mode ATTRIBUTE_UNUSED,
>>
>>     case PLUS:
>>     case MULT:
>> -      /* For addition we can safely permute the conversion and addition
>> -        operation if one operand is a constant and converting the constant
>> -        does not change it or if one operand is a constant and we are
>> -        using a ptr_extend instruction  (POINTERS_EXTEND_UNSIGNED < 0).
>> -        We can always safely permute them if we are making the address
>> -        narrower.  */
>> +      /* For addition we safely permute the conversion and addition
>> +        operation if one operand is a constant since we can't generate
>> +        new instructions.  We can always safely permute them if we are
>> +        making the address narrower.  */
>>       if (GET_MODE_SIZE (to_mode) < GET_MODE_SIZE (from_mode)
>>          || (GET_CODE (x) == PLUS
>> -             && CONST_INT_P (XEXP (x, 1))
>> -             && (XEXP (x, 1) == convert_memory_address_addr_space
>> -                                  (to_mode, XEXP (x, 1), as)
>> -                 || POINTERS_EXTEND_UNSIGNED < 0)))
>> +             && CONST_INT_P (XEXP (x, 1))))
>>        return gen_rtx_fmt_ee (GET_CODE (x), to_mode,
>>                               convert_memory_address_addr_space
>>                                 (to_mode, XEXP (x, 0), as),
>>
>
>
>
> --
> H.J.
>



-- 
H.J.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: PING: PATCH [4/n]: Prepare x32: Permute the conversion and addition if one operand is a constant
  2011-07-05 14:30 PING: PATCH [4/n]: Prepare x32: Permute the conversion and addition if one operand is a constant H.J. Lu
  2011-07-09 21:22 ` H.J. Lu
@ 2011-07-09 21:23 ` Paolo Bonzini
  2011-07-09 21:41   ` H.J. Lu
  1 sibling, 1 reply; 37+ messages in thread
From: Paolo Bonzini @ 2011-07-09 21:23 UTC (permalink / raw)
  To: H.J. Lu; +Cc: GCC Patches

On 07/05/2011 04:27 PM, H.J. Lu wrote:
>>  diff --git a/gcc/explow.c b/gcc/explow.c
>>  index 7387dad..b343bf8 100644
>>  --- a/gcc/explow.c
>>  +++ b/gcc/explow.c
>>  @@ -383,18 +383,13 @@ convert_memory_address_addr_space (enum machine_mode to_mode ATTRIBUTE_UNUSED,
>>
>>       case PLUS:
>>       case MULT:
>>  -      /* For addition we can safely permute the conversion and addition
>>  -        operation if one operand is a constant and converting the constant
>>  -        does not change it or if one operand is a constant and we are
>>  -        using a ptr_extend instruction  (POINTERS_EXTEND_UNSIGNED<  0).
>>  -        We can always safely permute them if we are making the address
>>  -        narrower.  */
>>  +      /* For addition we safely permute the conversion and addition
>>  +        operation if one operand is a constant since we can't generate
>>  +        new instructions.  We can always safely permute them if we are
>>  +        making the address narrower.  */
>>         if (GET_MODE_SIZE (to_mode)<  GET_MODE_SIZE (from_mode)
>>            || (GET_CODE (x) == PLUS
>>  -&&  CONST_INT_P (XEXP (x, 1))
>>  -&&  (XEXP (x, 1) == convert_memory_address_addr_space
>>  -                                  (to_mode, XEXP (x, 1), as)
>>  -                 || POINTERS_EXTEND_UNSIGNED<  0)))
>>  +&&  CONST_INT_P (XEXP (x, 1))))
>>          return gen_rtx_fmt_ee (GET_CODE (x), to_mode,
>>                                 convert_memory_address_addr_space
>>                                   (to_mode, XEXP (x, 0), as),

This does not seem safe to me.

Paolo

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: PING: PATCH [4/n]: Prepare x32: Permute the conversion and addition if one operand is a constant
  2011-07-09 21:23 ` Paolo Bonzini
@ 2011-07-09 21:41   ` H.J. Lu
  2011-07-10 17:04     ` Paolo Bonzini
  0 siblings, 1 reply; 37+ messages in thread
From: H.J. Lu @ 2011-07-09 21:41 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: GCC Patches

On Sat, Jul 9, 2011 at 2:18 PM, Paolo Bonzini <bonzini@gnu.org> wrote:
> On 07/05/2011 04:27 PM, H.J. Lu wrote:
>>>
>>>  diff --git a/gcc/explow.c b/gcc/explow.c
>>>  index 7387dad..b343bf8 100644
>>>  --- a/gcc/explow.c
>>>  +++ b/gcc/explow.c
>>>  @@ -383,18 +383,13 @@ convert_memory_address_addr_space (enum
>>> machine_mode to_mode ATTRIBUTE_UNUSED,
>>>
>>>      case PLUS:
>>>      case MULT:
>>>  -      /* For addition we can safely permute the conversion and addition
>>>  -        operation if one operand is a constant and converting the
>>> constant
>>>  -        does not change it or if one operand is a constant and we are
>>>  -        using a ptr_extend instruction  (POINTERS_EXTEND_UNSIGNED<  0).
>>>  -        We can always safely permute them if we are making the address
>>>  -        narrower.  */
>>>  +      /* For addition we safely permute the conversion and addition
>>>  +        operation if one operand is a constant since we can't generate
>>>  +        new instructions.  We can always safely permute them if we are
>>>  +        making the address narrower.  */
>>>        if (GET_MODE_SIZE (to_mode)<  GET_MODE_SIZE (from_mode)
>>>           || (GET_CODE (x) == PLUS
>>>  -&&  CONST_INT_P (XEXP (x, 1))
>>>  -&&  (XEXP (x, 1) == convert_memory_address_addr_space
>>>  -                                  (to_mode, XEXP (x, 1), as)
>>>  -                 || POINTERS_EXTEND_UNSIGNED<  0)))
>>>  +&&  CONST_INT_P (XEXP (x, 1))))
>>>         return gen_rtx_fmt_ee (GET_CODE (x), to_mode,
>>>                                convert_memory_address_addr_space
>>>                                  (to_mode, XEXP (x, 0), as),
>
> This does not seem safe to me.
>
> Paolo
>

The current code is broken for x32.  See:

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

We can't generate any new instructions.  Do you have any suggestions.

Thanks.

-- 
H.J.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: PING: PATCH [4/n]: Prepare x32: Permute the conversion and addition if one operand is a constant
  2011-07-09 21:41   ` H.J. Lu
@ 2011-07-10 17:04     ` Paolo Bonzini
  2011-07-10 21:16       ` H.J. Lu
  0 siblings, 1 reply; 37+ messages in thread
From: Paolo Bonzini @ 2011-07-10 17:04 UTC (permalink / raw)
  To: H.J. Lu; +Cc: GCC Patches

On Sat, Jul 9, 2011 at 23:31, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Sat, Jul 9, 2011 at 2:18 PM, Paolo Bonzini <bonzini@gnu.org> wrote:
>> On 07/05/2011 04:27 PM, H.J. Lu wrote:
>>>>
>>>>  diff --git a/gcc/explow.c b/gcc/explow.c
>>>>  index 7387dad..b343bf8 100644
>>>>  --- a/gcc/explow.c
>>>>  +++ b/gcc/explow.c
>>>>  @@ -383,18 +383,13 @@ convert_memory_address_addr_space (enum
>>>> machine_mode to_mode ATTRIBUTE_UNUSED,
>>>>
>>>>      case PLUS:
>>>>      case MULT:
>>>>  -      /* For addition we can safely permute the conversion and addition
>>>>  -        operation if one operand is a constant and converting the
>>>> constant
>>>>  -        does not change it or if one operand is a constant and we are
>>>>  -        using a ptr_extend instruction  (POINTERS_EXTEND_UNSIGNED<  0).
>>>>  -        We can always safely permute them if we are making the address
>>>>  -        narrower.  */
>>>>  +      /* For addition we safely permute the conversion and addition
>>>>  +        operation if one operand is a constant since we can't generate
>>>>  +        new instructions.  We can always safely permute them if we are
>>>>  +        making the address narrower.  */
>>>>        if (GET_MODE_SIZE (to_mode)<  GET_MODE_SIZE (from_mode)
>>>>           || (GET_CODE (x) == PLUS
>>>>  -&&  CONST_INT_P (XEXP (x, 1))
>>>>  -&&  (XEXP (x, 1) == convert_memory_address_addr_space
>>>>  -                                  (to_mode, XEXP (x, 1), as)
>>>>  -                 || POINTERS_EXTEND_UNSIGNED<  0)))
>>>>  +&&  CONST_INT_P (XEXP (x, 1))))
>>>>         return gen_rtx_fmt_ee (GET_CODE (x), to_mode,
>>>>                                convert_memory_address_addr_space
>>>>                                  (to_mode, XEXP (x, 0), as),
>>
>> This does not seem safe to me.
>
> The current code is broken for x32.  See:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47727
>
> We can't generate any new instructions.  Do you have any suggestions.

By "safe" I mean that the new condition might be too wide and generate
wrong code.  Richard is definitely right in comment 6, generating new
code in simplify-rtx is a no-no (see its usage of
gen_lowpart_no_emit).

Paolo

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: PING: PATCH [4/n]: Prepare x32: Permute the conversion and addition if one operand is a constant
  2011-07-10 17:04     ` Paolo Bonzini
@ 2011-07-10 21:16       ` H.J. Lu
  2011-07-11  0:48         ` H.J. Lu
  0 siblings, 1 reply; 37+ messages in thread
From: H.J. Lu @ 2011-07-10 21:16 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: GCC Patches

[-- Attachment #1: Type: text/plain, Size: 3049 bytes --]

On Sun, Jul 10, 2011 at 9:36 AM, Paolo Bonzini <bonzini@gnu.org> wrote:
> On Sat, Jul 9, 2011 at 23:31, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Sat, Jul 9, 2011 at 2:18 PM, Paolo Bonzini <bonzini@gnu.org> wrote:
>>> On 07/05/2011 04:27 PM, H.J. Lu wrote:
>>>>>
>>>>>  diff --git a/gcc/explow.c b/gcc/explow.c
>>>>>  index 7387dad..b343bf8 100644
>>>>>  --- a/gcc/explow.c
>>>>>  +++ b/gcc/explow.c
>>>>>  @@ -383,18 +383,13 @@ convert_memory_address_addr_space (enum
>>>>> machine_mode to_mode ATTRIBUTE_UNUSED,
>>>>>
>>>>>      case PLUS:
>>>>>      case MULT:
>>>>>  -      /* For addition we can safely permute the conversion and addition
>>>>>  -        operation if one operand is a constant and converting the
>>>>> constant
>>>>>  -        does not change it or if one operand is a constant and we are
>>>>>  -        using a ptr_extend instruction  (POINTERS_EXTEND_UNSIGNED<  0).
>>>>>  -        We can always safely permute them if we are making the address
>>>>>  -        narrower.  */
>>>>>  +      /* For addition we safely permute the conversion and addition
>>>>>  +        operation if one operand is a constant since we can't generate
>>>>>  +        new instructions.  We can always safely permute them if we are
>>>>>  +        making the address narrower.  */
>>>>>        if (GET_MODE_SIZE (to_mode)<  GET_MODE_SIZE (from_mode)
>>>>>           || (GET_CODE (x) == PLUS
>>>>>  -&&  CONST_INT_P (XEXP (x, 1))
>>>>>  -&&  (XEXP (x, 1) == convert_memory_address_addr_space
>>>>>  -                                  (to_mode, XEXP (x, 1), as)
>>>>>  -                 || POINTERS_EXTEND_UNSIGNED<  0)))
>>>>>  +&&  CONST_INT_P (XEXP (x, 1))))
>>>>>         return gen_rtx_fmt_ee (GET_CODE (x), to_mode,
>>>>>                                convert_memory_address_addr_space
>>>>>                                  (to_mode, XEXP (x, 0), as),
>>>
>>> This does not seem safe to me.
>>
>> The current code is broken for x32.  See:
>>
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47727
>>
>> We can't generate any new instructions.  Do you have any suggestions.
>
> By "safe" I mean that the new condition might be too wide and generate
> wrong code.  Richard is definitely right in comment 6, generating new
> code in simplify-rtx is a no-no (see its usage of
> gen_lowpart_no_emit).

Here is a different approach.  I added convert_memory_address_addr_space_1
and convert_modes_1 so that simplify-rtx won't generate new insns.  OK
for trunk if there are no regressions on Linux/x86?

Thanks.

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

	* explow.c (convert_memory_address_addr_space_1): New.
	(convert_memory_address_addr_space): Use it.

	* expr.c (convert_modes_1): New.
	(convert_modes): Use it.

	* expr.h (convert_modes_1): New.

	* rtl.h (convert_memory_address_addr_space_1): New.
	(convert_memory_address_1): Likewise.

	* simplify-rtx.c (simplify_unary_operation_1): Call
	convert_memory_address_1 instead of convert_memory_address.

[-- Attachment #2: gcc-x32-rtl-1.patch --]
[-- Type: text/plain, Size: 6118 bytes --]

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

	* explow.c (convert_memory_address_addr_space_1): New.
	(convert_memory_address_addr_space): Use it.

	* expr.c (convert_modes_1): New.
	(convert_modes): Use it.

	* expr.h (convert_modes_1): New.

	* rtl.h (convert_memory_address_addr_space_1): New.
	(convert_memory_address_1): Likewise.

	* simplify-rtx.c (simplify_unary_operation_1): Call
	convert_memory_address_1 instead of convert_memory_address.

diff --git a/gcc/explow.c b/gcc/explow.c
index 3c692f4..ee52e92 100644
--- a/gcc/explow.c
+++ b/gcc/explow.c
@@ -320,8 +320,9 @@ break_out_memory_refs (rtx x)
    arithmetic insns can be used.  */
 
 rtx
-convert_memory_address_addr_space (enum machine_mode to_mode ATTRIBUTE_UNUSED,
-				   rtx x, addr_space_t as ATTRIBUTE_UNUSED)
+convert_memory_address_addr_space_1 (enum machine_mode to_mode ATTRIBUTE_UNUSED,
+				     rtx x, addr_space_t as ATTRIBUTE_UNUSED,
+				     bool no_emit ATTRIBUTE_UNUSED)
 {
 #ifndef POINTERS_EXTEND_UNSIGNED
   gcc_assert (GET_MODE (x) == to_mode || GET_MODE (x) == VOIDmode);
@@ -406,10 +407,17 @@ convert_memory_address_addr_space (enum machine_mode to_mode ATTRIBUTE_UNUSED,
       break;
     }
 
-  return convert_modes (to_mode, from_mode,
-			x, POINTERS_EXTEND_UNSIGNED);
+  return convert_modes_1 (to_mode, from_mode, x,
+			  POINTERS_EXTEND_UNSIGNED, no_emit);
 #endif /* defined(POINTERS_EXTEND_UNSIGNED) */
 }
+
+rtx
+convert_memory_address_addr_space (enum machine_mode to_mode,
+				   rtx x, addr_space_t as)
+{
+  return convert_memory_address_addr_space_1 (to_mode, x, as, false);
+}
 \f
 /* Return something equivalent to X but valid as a memory address for something
    of mode MODE in the named address space AS.  When X is not itself valid,
diff --git a/gcc/expr.c b/gcc/expr.c
index fb4379f..de7f150 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -693,13 +693,16 @@ convert_to_mode (enum machine_mode mode, rtx x, int unsignedp)
    Both modes may be floating, or both integer.
    UNSIGNEDP is nonzero if X is an unsigned value.
 
+   If NO_EMIT is true, don't emit any instructions.
+
    This can be done by referring to a part of X in place
    or by copying to a new temporary with conversion.
 
    You can give VOIDmode for OLDMODE, if you are sure X has a nonvoid mode.  */
 
 rtx
-convert_modes (enum machine_mode mode, enum machine_mode oldmode, rtx x, int unsignedp)
+convert_modes_1 (enum machine_mode mode, enum machine_mode oldmode,
+		 rtx x, int unsignedp, bool no_emit)
 {
   rtx temp;
 
@@ -709,7 +712,12 @@ convert_modes (enum machine_mode mode, enum machine_mode oldmode, rtx x, int uns
   if (GET_CODE (x) == SUBREG && SUBREG_PROMOTED_VAR_P (x)
       && GET_MODE_SIZE (GET_MODE (SUBREG_REG (x))) >= GET_MODE_SIZE (mode)
       && SUBREG_PROMOTED_UNSIGNED_P (x) == unsignedp)
-    x = gen_lowpart (mode, x);
+    {
+      if (no_emit)
+	x = rtl_hooks.gen_lowpart_no_emit (mode, x);
+      else
+	x = gen_lowpart (mode, x);
+    }
 
   if (GET_MODE (x) != VOIDmode)
     oldmode = GET_MODE (x);
@@ -773,7 +781,10 @@ convert_modes (enum machine_mode mode, enum machine_mode oldmode, rtx x, int uns
 	  return gen_int_mode (val, mode);
 	}
 
-      return gen_lowpart (mode, x);
+      if (no_emit)
+	return rtl_hooks.gen_lowpart_no_emit (mode, x);
+      else
+	return gen_lowpart (mode, x);
     }
 
   /* Converting from integer constant into mode is always equivalent to an
@@ -784,10 +795,18 @@ convert_modes (enum machine_mode mode, enum machine_mode oldmode, rtx x, int uns
       return simplify_gen_subreg (mode, x, oldmode, 0);
     }
 
+  gcc_assert (!no_emit);
   temp = gen_reg_rtx (mode);
   convert_move (temp, x, unsignedp);
   return temp;
 }
+
+rtx
+convert_modes (enum machine_mode mode, enum machine_mode oldmode,
+	       rtx x, int unsignedp)
+{
+  return convert_modes_1 (mode, oldmode, x, unsignedp, false);
+}
 \f
 /* Return the largest alignment we can use for doing a move (or store)
    of MAX_PIECES.  ALIGN is the largest alignment we could use.  */
diff --git a/gcc/expr.h b/gcc/expr.h
index cb4050d..2ac9788 100644
--- a/gcc/expr.h
+++ b/gcc/expr.h
@@ -267,6 +267,8 @@ extern rtx convert_to_mode (enum machine_mode, rtx, int);
 
 /* Convert an rtx to MODE from OLDMODE and return the result.  */
 extern rtx convert_modes (enum machine_mode, enum machine_mode, rtx, int);
+extern rtx convert_modes_1 (enum machine_mode, enum machine_mode, rtx,
+			    int, bool);
 
 /* Emit code to move a block Y to a block X.  */
 
diff --git a/gcc/rtl.h b/gcc/rtl.h
index e3ceecd..b01eef8 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -1638,8 +1638,13 @@ extern int byte_lowpart_offset (enum machine_mode, enum machine_mode);
 extern rtx make_safe_from (rtx, rtx);
 extern rtx convert_memory_address_addr_space (enum machine_mode, rtx,
 					      addr_space_t);
+extern rtx convert_memory_address_addr_space_1 (enum machine_mode, rtx,
+						addr_space_t, bool);
 #define convert_memory_address(to_mode,x) \
 	convert_memory_address_addr_space ((to_mode), (x), ADDR_SPACE_GENERIC)
+#define convert_memory_address_1(to_mode,x,no_emit) \
+	convert_memory_address_addr_space_1 ((to_mode), (x), \
+					     ADDR_SPACE_GENERIC, (no_emit))
 extern const char *get_insn_name (int);
 extern rtx get_last_insn_anywhere (void);
 extern rtx get_first_nonnote_insn (void);
diff --git a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c
index 82b818b..189c201 100644
--- a/gcc/simplify-rtx.c
+++ b/gcc/simplify-rtx.c
@@ -1150,7 +1150,7 @@ simplify_unary_operation_1 (enum rtx_code code, enum machine_mode mode, rtx op)
 		  && REG_P (SUBREG_REG (op))
 		  && REG_POINTER (SUBREG_REG (op))
 		  && GET_MODE (SUBREG_REG (op)) == Pmode)))
-	return convert_memory_address (Pmode, op);
+	return convert_memory_address_1 (Pmode, op, true);
 #endif
       break;
 
@@ -1243,7 +1243,7 @@ simplify_unary_operation_1 (enum rtx_code code, enum machine_mode mode, rtx op)
 		  && REG_P (SUBREG_REG (op))
 		  && REG_POINTER (SUBREG_REG (op))
 		  && GET_MODE (SUBREG_REG (op)) == Pmode)))
-	return convert_memory_address (Pmode, op);
+	return convert_memory_address_1 (Pmode, op, true);
 #endif
       break;
 

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: PING: PATCH [4/n]: Prepare x32: Permute the conversion and addition if one operand is a constant
  2011-07-10 21:16       ` H.J. Lu
@ 2011-07-11  0:48         ` H.J. Lu
  2011-07-11  1:14           ` H.J. Lu
  0 siblings, 1 reply; 37+ messages in thread
From: H.J. Lu @ 2011-07-11  0:48 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: GCC Patches

On Sun, Jul 10, 2011 at 2:04 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Sun, Jul 10, 2011 at 9:36 AM, Paolo Bonzini <bonzini@gnu.org> wrote:
>> On Sat, Jul 9, 2011 at 23:31, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> On Sat, Jul 9, 2011 at 2:18 PM, Paolo Bonzini <bonzini@gnu.org> wrote:
>>>> On 07/05/2011 04:27 PM, H.J. Lu wrote:
>>>>>>
>>>>>>  diff --git a/gcc/explow.c b/gcc/explow.c
>>>>>>  index 7387dad..b343bf8 100644
>>>>>>  --- a/gcc/explow.c
>>>>>>  +++ b/gcc/explow.c
>>>>>>  @@ -383,18 +383,13 @@ convert_memory_address_addr_space (enum
>>>>>> machine_mode to_mode ATTRIBUTE_UNUSED,
>>>>>>
>>>>>>      case PLUS:
>>>>>>      case MULT:
>>>>>>  -      /* For addition we can safely permute the conversion and addition
>>>>>>  -        operation if one operand is a constant and converting the
>>>>>> constant
>>>>>>  -        does not change it or if one operand is a constant and we are
>>>>>>  -        using a ptr_extend instruction  (POINTERS_EXTEND_UNSIGNED<  0).
>>>>>>  -        We can always safely permute them if we are making the address
>>>>>>  -        narrower.  */
>>>>>>  +      /* For addition we safely permute the conversion and addition
>>>>>>  +        operation if one operand is a constant since we can't generate
>>>>>>  +        new instructions.  We can always safely permute them if we are
>>>>>>  +        making the address narrower.  */
>>>>>>        if (GET_MODE_SIZE (to_mode)<  GET_MODE_SIZE (from_mode)
>>>>>>           || (GET_CODE (x) == PLUS
>>>>>>  -&&  CONST_INT_P (XEXP (x, 1))
>>>>>>  -&&  (XEXP (x, 1) == convert_memory_address_addr_space
>>>>>>  -                                  (to_mode, XEXP (x, 1), as)
>>>>>>  -                 || POINTERS_EXTEND_UNSIGNED<  0)))
>>>>>>  +&&  CONST_INT_P (XEXP (x, 1))))
>>>>>>         return gen_rtx_fmt_ee (GET_CODE (x), to_mode,
>>>>>>                                convert_memory_address_addr_space
>>>>>>                                  (to_mode, XEXP (x, 0), as),
>>>>
>>>> This does not seem safe to me.
>>>
>>> The current code is broken for x32.  See:
>>>
>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47727
>>>
>>> We can't generate any new instructions.  Do you have any suggestions.
>>
>> By "safe" I mean that the new condition might be too wide and generate
>> wrong code.  Richard is definitely right in comment 6, generating new
>> code in simplify-rtx is a no-no (see its usage of
>> gen_lowpart_no_emit).
>
> Here is a different approach.  I added convert_memory_address_addr_space_1
> and convert_modes_1 so that simplify-rtx won't generate new insns.  OK
> for trunk if there are no regressions on Linux/x86?
>
> Thanks.
>
> --
> H.J.
> ---
> 2011-07-10  H.J. Lu  <hongjiu.lu@intel.com>
>
>        * explow.c (convert_memory_address_addr_space_1): New.
>        (convert_memory_address_addr_space): Use it.
>
>        * expr.c (convert_modes_1): New.
>        (convert_modes): Use it.
>
>        * expr.h (convert_modes_1): New.
>
>        * rtl.h (convert_memory_address_addr_space_1): New.
>        (convert_memory_address_1): Likewise.
>
>        * simplify-rtx.c (simplify_unary_operation_1): Call
>        convert_memory_address_1 instead of convert_memory_address.
>

It doesn't work.  I got

(gdb) f 2
#2  0x000000000078735a in convert_memory_address_addr_space_1 (to_mode=DImode,
    x=0x7ffff05ac4e0, as=0 '\000', no_emit=1 '\001')
    at /export/gnu/import/git/gcc-x32/gcc/explow.c:410
410	  return convert_modes_1 (to_mode, from_mode, x,
(gdb) call debug_rtx (x)
(plus:SI (symbol_ref:SI ("iplane.1577") [flags 0x2] <var_decl
0x7ffff0857960 iplane>)
    (const_int -4 [0xfffffffffffffffc]))
(gdb) bt
#0  fancy_abort (file=0x13531a8 "/export/gnu/import/git/gcc-x32/gcc/expr.c",
    line=798, function=0x1354a00 "convert_modes_1")
    at /export/gnu/import/git/gcc-x32/gcc/diagnostic.c:893
#1  0x000000000079c60c in convert_modes_1 (mode=DImode, oldmode=SImode,
    x=0x7ffff05ac4e0, unsignedp=1, no_emit=1 '\001')
    at /export/gnu/import/git/gcc-x32/gcc/expr.c:798
#2  0x000000000078735a in convert_memory_address_addr_space_1 (to_mode=DImode,
    x=0x7ffff05ac4e0, as=0 '\000', no_emit=1 '\001')
    at /export/gnu/import/git/gcc-x32/gcc/explow.c:410
#3  0x0000000000787281 in convert_memory_address_addr_space_1 (to_mode=DImode,
    x=0x7ffff05616d0, as=0 '\000', no_emit=1 '\001')
    at /export/gnu/import/git/gcc-x32/gcc/explow.c:381
#4  0x0000000000b0faf4 in simplify_unary_operation_1 (code=ZERO_EXTEND,
    mode=DImode, op=0x7ffff05616d0)
    at /export/gnu/import/git/gcc-x32/gcc/simplify-rtx.c:1246
#5  0x0000000000b0d851 in simplify_unary_operation (code=ZERO_EXTEND,
    mode=DImode, op=0x7ffff05616d0, op_mode=SImode)
    at /export/gnu/import/git/gcc-x32/gcc/simplify-rtx.c:582
#6  0x0000000000b0d035 in simplify_gen_unary (code=ZERO_EXTEND, mode=DImode,
    op=0x7ffff05616d0, op_mode=SImode)
    at /export/gnu/import/git/gcc-x32/gcc/simplify-rtx.c:370
#7  0x000000000117078a in if_then_else_cond (x=0x7ffff02ebb00,
    ptrue=0x7fffffffd720, pfalse=0x7fffffffd718)
---Type <return> to continue, or q <return> to quit---
    at /export/gnu/import/git/gcc-x32/gcc/combine.c:8649
#8  0x00000000011675b5 in combine_simplify_rtx (x=0x7ffff02ebb00,
    op0_mode=SImode, in_dest=0, in_cond=0)
    at /export/gnu/import/git/gcc-x32/gcc/combine.c:5316
#9  0x0000000001167315 in subst (x=0x7ffff02ebb00, from=0x7ffff02f8120,
    to=0x7ffff05ac4f8, in_dest=0, in_cond=0, unique_copy=0)
    at /export/gnu/import/git/gcc-x32/gcc/combine.c:5253
#10 0x0000000001167104 in subst (x=0x7ffff02f5558, from=0x7ffff02f8120,
    to=0x7ffff05ac4f8, in_dest=0, in_cond=0, unique_copy=0)
    at /export/gnu/import/git/gcc-x32/gcc/combine.c:5189
#11 0x00000000011611ae in try_combine (i3=0x7ffff02f4c60, i2=0x7ffff02f4c18,
    i1=0x0, i0=0x0, new_direct_jump_p=0x7fffffffde14,
    last_combined_insn=0x7ffff02f4c60)
    at /export/gnu/import/git/gcc-x32/gcc/combine.c:3178
#12 0x000000000115c487 in combine_instructions (f=0x7ffff07e7700, nregs=3344)
    at /export/gnu/import/git/gcc-x32/gcc/combine.c:1223
#13 0x000000000117c64e in rest_of_handle_combine ()
    at /export/gnu/import/git/gcc-x32/gcc/combine.c:13879
#14 0x0000000000a500e7 in execute_one_pass (pass=0x190d320)
    at /export/gnu/import/git/gcc-x32/gcc/passes.c:2062
#15 0x0000000000a502cd in execute_pass_list (pass=0x190d320)
    at /export/gnu/import/git/gcc-x32/gcc/passes.c:2117
#16 0x0000000000a502ee in execute_pass_list (pass=0x1908180)
---Type <return> to continue, or q <return> to quit---
    at /export/gnu/import/git/gcc-x32/gcc/passes.c:2118
#17 0x0000000000be93e0 in tree_rest_of_compilation (fndecl=0x7ffff074c200)
    at /export/gnu/import/git/gcc-x32/gcc/tree-optimize.c:416
#18 0x00000000006d3ff7 in cgraph_expand_function (node=0x7ffff0792000)
    at /export/gnu/import/git/gcc-x32/gcc/cgraphunit.c:1804
#19 0x00000000006d41b6 in cgraph_expand_all_functions ()
    at /export/gnu/import/git/gcc-x32/gcc/cgraphunit.c:1863
#20 0x00000000006d48b2 in cgraph_optimize ()
    at /export/gnu/import/git/gcc-x32/gcc/cgraphunit.c:2133
#21 0x00000000006d1b2a in cgraph_finalize_compilation_unit ()
    at /export/gnu/import/git/gcc-x32/gcc/cgraphunit.c:1304
#22 0x00000000009cb7b0 in write_global_declarations ()
    at /export/gnu/import/git/gcc-x32/gcc/langhooks.c:303
#23 0x0000000000559cac in gfc_write_global_declarations ()
    at /export/gnu/import/git/gcc-x32/gcc/fortran/f95-lang.c:322
#24 0x0000000000b4649c in compile_file ()
    at /export/gnu/import/git/gcc-x32/gcc/toplev.c:564
#25 0x0000000000b48686 in do_compile ()
    at /export/gnu/import/git/gcc-x32/gcc/toplev.c:1886
#26 0x0000000000b487ec in toplev_main (argc=19, argv=0x7fffffffe2a8)
    at /export/gnu/import/git/gcc-x32/gcc/toplev.c:1958
#27 0x000000000060be84 in main (argc=19, argv=0x7fffffffe2a8)
    at /export/gnu/import/git/gcc-x32/gcc/main.c:36
(gdb)


H.J.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: PING: PATCH [4/n]: Prepare x32: Permute the conversion and addition if one operand is a constant
  2011-07-11  0:48         ` H.J. Lu
@ 2011-07-11  1:14           ` H.J. Lu
  2011-07-11  6:49             ` H.J. Lu
  2011-07-11 11:09             ` Paolo Bonzini
  0 siblings, 2 replies; 37+ messages in thread
From: H.J. Lu @ 2011-07-11  1:14 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: GCC Patches

On Sun, Jul 10, 2011 at 4:51 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Sun, Jul 10, 2011 at 2:04 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Sun, Jul 10, 2011 at 9:36 AM, Paolo Bonzini <bonzini@gnu.org> wrote:
>>> On Sat, Jul 9, 2011 at 23:31, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>> On Sat, Jul 9, 2011 at 2:18 PM, Paolo Bonzini <bonzini@gnu.org> wrote:
>>>>> On 07/05/2011 04:27 PM, H.J. Lu wrote:
>>>>>>>
>>>>>>>  diff --git a/gcc/explow.c b/gcc/explow.c
>>>>>>>  index 7387dad..b343bf8 100644
>>>>>>>  --- a/gcc/explow.c
>>>>>>>  +++ b/gcc/explow.c
>>>>>>>  @@ -383,18 +383,13 @@ convert_memory_address_addr_space (enum
>>>>>>> machine_mode to_mode ATTRIBUTE_UNUSED,
>>>>>>>
>>>>>>>      case PLUS:
>>>>>>>      case MULT:
>>>>>>>  -      /* For addition we can safely permute the conversion and addition
>>>>>>>  -        operation if one operand is a constant and converting the
>>>>>>> constant
>>>>>>>  -        does not change it or if one operand is a constant and we are
>>>>>>>  -        using a ptr_extend instruction  (POINTERS_EXTEND_UNSIGNED<  0).
>>>>>>>  -        We can always safely permute them if we are making the address
>>>>>>>  -        narrower.  */
>>>>>>>  +      /* For addition we safely permute the conversion and addition
>>>>>>>  +        operation if one operand is a constant since we can't generate
>>>>>>>  +        new instructions.  We can always safely permute them if we are
>>>>>>>  +        making the address narrower.  */
>>>>>>>        if (GET_MODE_SIZE (to_mode)<  GET_MODE_SIZE (from_mode)
>>>>>>>           || (GET_CODE (x) == PLUS
>>>>>>>  -&&  CONST_INT_P (XEXP (x, 1))
>>>>>>>  -&&  (XEXP (x, 1) == convert_memory_address_addr_space
>>>>>>>  -                                  (to_mode, XEXP (x, 1), as)
>>>>>>>  -                 || POINTERS_EXTEND_UNSIGNED<  0)))
>>>>>>>  +&&  CONST_INT_P (XEXP (x, 1))))
>>>>>>>         return gen_rtx_fmt_ee (GET_CODE (x), to_mode,
>>>>>>>                                convert_memory_address_addr_space
>>>>>>>                                  (to_mode, XEXP (x, 0), as),
>>>>>
>>>>> This does not seem safe to me.
>>>>
>>>> The current code is broken for x32.  See:
>>>>
>>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47727
>>>>
>>>> We can't generate any new instructions.  Do you have any suggestions.
>>>
>>> By "safe" I mean that the new condition might be too wide and generate
>>> wrong code.  Richard is definitely right in comment 6, generating new
>>> code in simplify-rtx is a no-no (see its usage of
>>> gen_lowpart_no_emit).
>>
>> Here is a different approach.  I added convert_memory_address_addr_space_1
>> and convert_modes_1 so that simplify-rtx won't generate new insns.  OK
>> for trunk if there are no regressions on Linux/x86?
>>
>> Thanks.
>>
>> --
>> H.J.
>> ---
>> 2011-07-10  H.J. Lu  <hongjiu.lu@intel.com>
>>
>>        * explow.c (convert_memory_address_addr_space_1): New.
>>        (convert_memory_address_addr_space): Use it.
>>
>>        * expr.c (convert_modes_1): New.
>>        (convert_modes): Use it.
>>
>>        * expr.h (convert_modes_1): New.
>>
>>        * rtl.h (convert_memory_address_addr_space_1): New.
>>        (convert_memory_address_1): Likewise.
>>
>>        * simplify-rtx.c (simplify_unary_operation_1): Call
>>        convert_memory_address_1 instead of convert_memory_address.
>>
>
> It doesn't work.  I got
>
> (gdb) f 2
> #2  0x000000000078735a in convert_memory_address_addr_space_1 (to_mode=DImode,
>    x=0x7ffff05ac4e0, as=0 '\000', no_emit=1 '\001')
>    at /export/gnu/import/git/gcc-x32/gcc/explow.c:410
> 410       return convert_modes_1 (to_mode, from_mode, x,
> (gdb) call debug_rtx (x)
> (plus:SI (symbol_ref:SI ("iplane.1577") [flags 0x2] <var_decl
> 0x7ffff0857960 iplane>)
>    (const_int -4 [0xfffffffffffffffc]))
> (gdb) bt
> #0  fancy_abort (file=0x13531a8 "/export/gnu/import/git/gcc-x32/gcc/expr.c",
>    line=798, function=0x1354a00 "convert_modes_1")
>    at /export/gnu/import/git/gcc-x32/gcc/diagnostic.c:893
> #1  0x000000000079c60c in convert_modes_1 (mode=DImode, oldmode=SImode,
>    x=0x7ffff05ac4e0, unsignedp=1, no_emit=1 '\001')
>    at /export/gnu/import/git/gcc-x32/gcc/expr.c:798
> #2  0x000000000078735a in convert_memory_address_addr_space_1 (to_mode=DImode,
>    x=0x7ffff05ac4e0, as=0 '\000', no_emit=1 '\001')
>    at /export/gnu/import/git/gcc-x32/gcc/explow.c:410
> #3  0x0000000000787281 in convert_memory_address_addr_space_1 (to_mode=DImode,
>    x=0x7ffff05616d0, as=0 '\000', no_emit=1 '\001')
>    at /export/gnu/import/git/gcc-x32/gcc/explow.c:381
> #4  0x0000000000b0faf4 in simplify_unary_operation_1 (code=ZERO_EXTEND,
>    mode=DImode, op=0x7ffff05616d0)
>    at /export/gnu/import/git/gcc-x32/gcc/simplify-rtx.c:1246
> #5  0x0000000000b0d851 in simplify_unary_operation (code=ZERO_EXTEND,
>    mode=DImode, op=0x7ffff05616d0, op_mode=SImode)
>    at /export/gnu/import/git/gcc-x32/gcc/simplify-rtx.c:582
> #6  0x0000000000b0d035 in simplify_gen_unary (code=ZERO_EXTEND, mode=DImode,
>    op=0x7ffff05616d0, op_mode=SImode)
>    at /export/gnu/import/git/gcc-x32/gcc/simplify-rtx.c:370
> #7  0x000000000117078a in if_then_else_cond (x=0x7ffff02ebb00,
>    ptrue=0x7fffffffd720, pfalse=0x7fffffffd718)
> ---Type <return> to continue, or q <return> to quit---
>    at /export/gnu/import/git/gcc-x32/gcc/combine.c:8649
> #8  0x00000000011675b5 in combine_simplify_rtx (x=0x7ffff02ebb00,
>    op0_mode=SImode, in_dest=0, in_cond=0)
>    at /export/gnu/import/git/gcc-x32/gcc/combine.c:5316
> #9  0x0000000001167315 in subst (x=0x7ffff02ebb00, from=0x7ffff02f8120,
>    to=0x7ffff05ac4f8, in_dest=0, in_cond=0, unique_copy=0)
>    at /export/gnu/import/git/gcc-x32/gcc/combine.c:5253
> #10 0x0000000001167104 in subst (x=0x7ffff02f5558, from=0x7ffff02f8120,
>    to=0x7ffff05ac4f8, in_dest=0, in_cond=0, unique_copy=0)
>    at /export/gnu/import/git/gcc-x32/gcc/combine.c:5189
> #11 0x00000000011611ae in try_combine (i3=0x7ffff02f4c60, i2=0x7ffff02f4c18,
>    i1=0x0, i0=0x0, new_direct_jump_p=0x7fffffffde14,
>    last_combined_insn=0x7ffff02f4c60)
>    at /export/gnu/import/git/gcc-x32/gcc/combine.c:3178
> #12 0x000000000115c487 in combine_instructions (f=0x7ffff07e7700, nregs=3344)
>    at /export/gnu/import/git/gcc-x32/gcc/combine.c:1223
> #13 0x000000000117c64e in rest_of_handle_combine ()
>    at /export/gnu/import/git/gcc-x32/gcc/combine.c:13879
> #14 0x0000000000a500e7 in execute_one_pass (pass=0x190d320)
>    at /export/gnu/import/git/gcc-x32/gcc/passes.c:2062
> #15 0x0000000000a502cd in execute_pass_list (pass=0x190d320)
>    at /export/gnu/import/git/gcc-x32/gcc/passes.c:2117
> #16 0x0000000000a502ee in execute_pass_list (pass=0x1908180)
> ---Type <return> to continue, or q <return> to quit---
>    at /export/gnu/import/git/gcc-x32/gcc/passes.c:2118
> #17 0x0000000000be93e0 in tree_rest_of_compilation (fndecl=0x7ffff074c200)
>    at /export/gnu/import/git/gcc-x32/gcc/tree-optimize.c:416
> #18 0x00000000006d3ff7 in cgraph_expand_function (node=0x7ffff0792000)
>    at /export/gnu/import/git/gcc-x32/gcc/cgraphunit.c:1804
> #19 0x00000000006d41b6 in cgraph_expand_all_functions ()
>    at /export/gnu/import/git/gcc-x32/gcc/cgraphunit.c:1863
> #20 0x00000000006d48b2 in cgraph_optimize ()
>    at /export/gnu/import/git/gcc-x32/gcc/cgraphunit.c:2133
> #21 0x00000000006d1b2a in cgraph_finalize_compilation_unit ()
>    at /export/gnu/import/git/gcc-x32/gcc/cgraphunit.c:1304
> #22 0x00000000009cb7b0 in write_global_declarations ()
>    at /export/gnu/import/git/gcc-x32/gcc/langhooks.c:303
> #23 0x0000000000559cac in gfc_write_global_declarations ()
>    at /export/gnu/import/git/gcc-x32/gcc/fortran/f95-lang.c:322
> #24 0x0000000000b4649c in compile_file ()
>    at /export/gnu/import/git/gcc-x32/gcc/toplev.c:564
> #25 0x0000000000b48686 in do_compile ()
>    at /export/gnu/import/git/gcc-x32/gcc/toplev.c:1886
> #26 0x0000000000b487ec in toplev_main (argc=19, argv=0x7fffffffe2a8)
>    at /export/gnu/import/git/gcc-x32/gcc/toplev.c:1958
> #27 0x000000000060be84 in main (argc=19, argv=0x7fffffffe2a8)
>    at /export/gnu/import/git/gcc-x32/gcc/main.c:36
> (gdb)
>
>
> H.J.
>

With my original change,  I got

(const:DI (plus:DI (symbol_ref:DI ("iplane.1577") [flags 0x2]
<var_decl 0x7ffff0857960 iplane>)
        (const_int -4 [0xfffffffffffffffc])))

I think it is safe to permute the conversion and addition operation
if one operand is a constant and we are zero-extending.  This is
how zero-extending works.

-- 
H.J.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: PING: PATCH [4/n]: Prepare x32: Permute the conversion and addition if one operand is a constant
  2011-07-11  1:14           ` H.J. Lu
@ 2011-07-11  6:49             ` H.J. Lu
  2011-07-11 11:09             ` Paolo Bonzini
  1 sibling, 0 replies; 37+ messages in thread
From: H.J. Lu @ 2011-07-11  6:49 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: GCC Patches

[-- Attachment #1: Type: text/plain, Size: 9463 bytes --]

On Sun, Jul 10, 2011 at 5:04 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Sun, Jul 10, 2011 at 4:51 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Sun, Jul 10, 2011 at 2:04 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> On Sun, Jul 10, 2011 at 9:36 AM, Paolo Bonzini <bonzini@gnu.org> wrote:
>>>> On Sat, Jul 9, 2011 at 23:31, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>> On Sat, Jul 9, 2011 at 2:18 PM, Paolo Bonzini <bonzini@gnu.org> wrote:
>>>>>> On 07/05/2011 04:27 PM, H.J. Lu wrote:
>>>>>>>>
>>>>>>>>  diff --git a/gcc/explow.c b/gcc/explow.c
>>>>>>>>  index 7387dad..b343bf8 100644
>>>>>>>>  --- a/gcc/explow.c
>>>>>>>>  +++ b/gcc/explow.c
>>>>>>>>  @@ -383,18 +383,13 @@ convert_memory_address_addr_space (enum
>>>>>>>> machine_mode to_mode ATTRIBUTE_UNUSED,
>>>>>>>>
>>>>>>>>      case PLUS:
>>>>>>>>      case MULT:
>>>>>>>>  -      /* For addition we can safely permute the conversion and addition
>>>>>>>>  -        operation if one operand is a constant and converting the
>>>>>>>> constant
>>>>>>>>  -        does not change it or if one operand is a constant and we are
>>>>>>>>  -        using a ptr_extend instruction  (POINTERS_EXTEND_UNSIGNED<  0).
>>>>>>>>  -        We can always safely permute them if we are making the address
>>>>>>>>  -        narrower.  */
>>>>>>>>  +      /* For addition we safely permute the conversion and addition
>>>>>>>>  +        operation if one operand is a constant since we can't generate
>>>>>>>>  +        new instructions.  We can always safely permute them if we are
>>>>>>>>  +        making the address narrower.  */
>>>>>>>>        if (GET_MODE_SIZE (to_mode)<  GET_MODE_SIZE (from_mode)
>>>>>>>>           || (GET_CODE (x) == PLUS
>>>>>>>>  -&&  CONST_INT_P (XEXP (x, 1))
>>>>>>>>  -&&  (XEXP (x, 1) == convert_memory_address_addr_space
>>>>>>>>  -                                  (to_mode, XEXP (x, 1), as)
>>>>>>>>  -                 || POINTERS_EXTEND_UNSIGNED<  0)))
>>>>>>>>  +&&  CONST_INT_P (XEXP (x, 1))))
>>>>>>>>         return gen_rtx_fmt_ee (GET_CODE (x), to_mode,
>>>>>>>>                                convert_memory_address_addr_space
>>>>>>>>                                  (to_mode, XEXP (x, 0), as),
>>>>>>
>>>>>> This does not seem safe to me.
>>>>>
>>>>> The current code is broken for x32.  See:
>>>>>
>>>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47727
>>>>>
>>>>> We can't generate any new instructions.  Do you have any suggestions.
>>>>
>>>> By "safe" I mean that the new condition might be too wide and generate
>>>> wrong code.  Richard is definitely right in comment 6, generating new
>>>> code in simplify-rtx is a no-no (see its usage of
>>>> gen_lowpart_no_emit).
>>>
>>> Here is a different approach.  I added convert_memory_address_addr_space_1
>>> and convert_modes_1 so that simplify-rtx won't generate new insns.  OK
>>> for trunk if there are no regressions on Linux/x86?
>>>
>>> Thanks.
>>>
>>> --
>>> H.J.
>>> ---
>>> 2011-07-10  H.J. Lu  <hongjiu.lu@intel.com>
>>>
>>>        * explow.c (convert_memory_address_addr_space_1): New.
>>>        (convert_memory_address_addr_space): Use it.
>>>
>>>        * expr.c (convert_modes_1): New.
>>>        (convert_modes): Use it.
>>>
>>>        * expr.h (convert_modes_1): New.
>>>
>>>        * rtl.h (convert_memory_address_addr_space_1): New.
>>>        (convert_memory_address_1): Likewise.
>>>
>>>        * simplify-rtx.c (simplify_unary_operation_1): Call
>>>        convert_memory_address_1 instead of convert_memory_address.
>>>
>>
>> It doesn't work.  I got
>>
>> (gdb) f 2
>> #2  0x000000000078735a in convert_memory_address_addr_space_1 (to_mode=DImode,
>>    x=0x7ffff05ac4e0, as=0 '\000', no_emit=1 '\001')
>>    at /export/gnu/import/git/gcc-x32/gcc/explow.c:410
>> 410       return convert_modes_1 (to_mode, from_mode, x,
>> (gdb) call debug_rtx (x)
>> (plus:SI (symbol_ref:SI ("iplane.1577") [flags 0x2] <var_decl
>> 0x7ffff0857960 iplane>)
>>    (const_int -4 [0xfffffffffffffffc]))
>> (gdb) bt
>> #0  fancy_abort (file=0x13531a8 "/export/gnu/import/git/gcc-x32/gcc/expr.c",
>>    line=798, function=0x1354a00 "convert_modes_1")
>>    at /export/gnu/import/git/gcc-x32/gcc/diagnostic.c:893
>> #1  0x000000000079c60c in convert_modes_1 (mode=DImode, oldmode=SImode,
>>    x=0x7ffff05ac4e0, unsignedp=1, no_emit=1 '\001')
>>    at /export/gnu/import/git/gcc-x32/gcc/expr.c:798
>> #2  0x000000000078735a in convert_memory_address_addr_space_1 (to_mode=DImode,
>>    x=0x7ffff05ac4e0, as=0 '\000', no_emit=1 '\001')
>>    at /export/gnu/import/git/gcc-x32/gcc/explow.c:410
>> #3  0x0000000000787281 in convert_memory_address_addr_space_1 (to_mode=DImode,
>>    x=0x7ffff05616d0, as=0 '\000', no_emit=1 '\001')
>>    at /export/gnu/import/git/gcc-x32/gcc/explow.c:381
>> #4  0x0000000000b0faf4 in simplify_unary_operation_1 (code=ZERO_EXTEND,
>>    mode=DImode, op=0x7ffff05616d0)
>>    at /export/gnu/import/git/gcc-x32/gcc/simplify-rtx.c:1246
>> #5  0x0000000000b0d851 in simplify_unary_operation (code=ZERO_EXTEND,
>>    mode=DImode, op=0x7ffff05616d0, op_mode=SImode)
>>    at /export/gnu/import/git/gcc-x32/gcc/simplify-rtx.c:582
>> #6  0x0000000000b0d035 in simplify_gen_unary (code=ZERO_EXTEND, mode=DImode,
>>    op=0x7ffff05616d0, op_mode=SImode)
>>    at /export/gnu/import/git/gcc-x32/gcc/simplify-rtx.c:370
>> #7  0x000000000117078a in if_then_else_cond (x=0x7ffff02ebb00,
>>    ptrue=0x7fffffffd720, pfalse=0x7fffffffd718)
>> ---Type <return> to continue, or q <return> to quit---
>>    at /export/gnu/import/git/gcc-x32/gcc/combine.c:8649
>> #8  0x00000000011675b5 in combine_simplify_rtx (x=0x7ffff02ebb00,
>>    op0_mode=SImode, in_dest=0, in_cond=0)
>>    at /export/gnu/import/git/gcc-x32/gcc/combine.c:5316
>> #9  0x0000000001167315 in subst (x=0x7ffff02ebb00, from=0x7ffff02f8120,
>>    to=0x7ffff05ac4f8, in_dest=0, in_cond=0, unique_copy=0)
>>    at /export/gnu/import/git/gcc-x32/gcc/combine.c:5253
>> #10 0x0000000001167104 in subst (x=0x7ffff02f5558, from=0x7ffff02f8120,
>>    to=0x7ffff05ac4f8, in_dest=0, in_cond=0, unique_copy=0)
>>    at /export/gnu/import/git/gcc-x32/gcc/combine.c:5189
>> #11 0x00000000011611ae in try_combine (i3=0x7ffff02f4c60, i2=0x7ffff02f4c18,
>>    i1=0x0, i0=0x0, new_direct_jump_p=0x7fffffffde14,
>>    last_combined_insn=0x7ffff02f4c60)
>>    at /export/gnu/import/git/gcc-x32/gcc/combine.c:3178
>> #12 0x000000000115c487 in combine_instructions (f=0x7ffff07e7700, nregs=3344)
>>    at /export/gnu/import/git/gcc-x32/gcc/combine.c:1223
>> #13 0x000000000117c64e in rest_of_handle_combine ()
>>    at /export/gnu/import/git/gcc-x32/gcc/combine.c:13879
>> #14 0x0000000000a500e7 in execute_one_pass (pass=0x190d320)
>>    at /export/gnu/import/git/gcc-x32/gcc/passes.c:2062
>> #15 0x0000000000a502cd in execute_pass_list (pass=0x190d320)
>>    at /export/gnu/import/git/gcc-x32/gcc/passes.c:2117
>> #16 0x0000000000a502ee in execute_pass_list (pass=0x1908180)
>> ---Type <return> to continue, or q <return> to quit---
>>    at /export/gnu/import/git/gcc-x32/gcc/passes.c:2118
>> #17 0x0000000000be93e0 in tree_rest_of_compilation (fndecl=0x7ffff074c200)
>>    at /export/gnu/import/git/gcc-x32/gcc/tree-optimize.c:416
>> #18 0x00000000006d3ff7 in cgraph_expand_function (node=0x7ffff0792000)
>>    at /export/gnu/import/git/gcc-x32/gcc/cgraphunit.c:1804
>> #19 0x00000000006d41b6 in cgraph_expand_all_functions ()
>>    at /export/gnu/import/git/gcc-x32/gcc/cgraphunit.c:1863
>> #20 0x00000000006d48b2 in cgraph_optimize ()
>>    at /export/gnu/import/git/gcc-x32/gcc/cgraphunit.c:2133
>> #21 0x00000000006d1b2a in cgraph_finalize_compilation_unit ()
>>    at /export/gnu/import/git/gcc-x32/gcc/cgraphunit.c:1304
>> #22 0x00000000009cb7b0 in write_global_declarations ()
>>    at /export/gnu/import/git/gcc-x32/gcc/langhooks.c:303
>> #23 0x0000000000559cac in gfc_write_global_declarations ()
>>    at /export/gnu/import/git/gcc-x32/gcc/fortran/f95-lang.c:322
>> #24 0x0000000000b4649c in compile_file ()
>>    at /export/gnu/import/git/gcc-x32/gcc/toplev.c:564
>> #25 0x0000000000b48686 in do_compile ()
>>    at /export/gnu/import/git/gcc-x32/gcc/toplev.c:1886
>> #26 0x0000000000b487ec in toplev_main (argc=19, argv=0x7fffffffe2a8)
>>    at /export/gnu/import/git/gcc-x32/gcc/toplev.c:1958
>> #27 0x000000000060be84 in main (argc=19, argv=0x7fffffffe2a8)
>>    at /export/gnu/import/git/gcc-x32/gcc/main.c:36
>> (gdb)
>>
>>
>> H.J.
>>
>
> With my original change,  I got
>
> (const:DI (plus:DI (symbol_ref:DI ("iplane.1577") [flags 0x2]
> <var_decl 0x7ffff0857960 iplane>)
>        (const_int -4 [0xfffffffffffffffc])))
>
> I think it is safe to permute the conversion and addition operation
> if one operand is a constant and we are zero-extending.  This is
> how zero-extending works.

I am testing this updated patch.  OK for trunks if it works?

Thanks.

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

	PR middle-end/47727
	* explow.c (convert_memory_address_addr_space_1): New.
	(convert_memory_address_addr_space): Use it.

	* expr.c (convert_modes_1): New.
	(convert_modes): Use it.

	* expr.h (convert_modes_1): New.

	* rtl.h (convert_memory_address_addr_space_1): New.
	(convert_memory_address_1): Likewise.

	* simplify-rtx.c (simplify_unary_operation_1): Call
	convert_memory_address_1 instead of convert_memory_address.

[-- Attachment #2: gcc-x32-pr47727-2.patch --]
[-- Type: text/plain, Size: 7867 bytes --]

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

	PR middle-end/47727
	* explow.c (convert_memory_address_addr_space_1): New.
	(convert_memory_address_addr_space): Use it.

	* expr.c (convert_modes_1): New.
	(convert_modes): Use it.

	* expr.h (convert_modes_1): New.

	* rtl.h (convert_memory_address_addr_space_1): New.
	(convert_memory_address_1): Likewise.

	* simplify-rtx.c (simplify_unary_operation_1): Call
	convert_memory_address_1 instead of convert_memory_address.

diff --git a/gcc/explow.c b/gcc/explow.c
index 3c692f4..389b500 100644
--- a/gcc/explow.c
+++ b/gcc/explow.c
@@ -320,8 +320,9 @@ break_out_memory_refs (rtx x)
    arithmetic insns can be used.  */
 
 rtx
-convert_memory_address_addr_space (enum machine_mode to_mode ATTRIBUTE_UNUSED,
-				   rtx x, addr_space_t as ATTRIBUTE_UNUSED)
+convert_memory_address_addr_space_1 (enum machine_mode to_mode ATTRIBUTE_UNUSED,
+				     rtx x, addr_space_t as ATTRIBUTE_UNUSED,
+				     bool no_emit ATTRIBUTE_UNUSED)
 {
 #ifndef POINTERS_EXTEND_UNSIGNED
   gcc_assert (GET_MODE (x) == to_mode || GET_MODE (x) == VOIDmode);
@@ -378,27 +379,27 @@ convert_memory_address_addr_space (enum machine_mode to_mode ATTRIBUTE_UNUSED,
 
     case CONST:
       return gen_rtx_CONST (to_mode,
-			    convert_memory_address_addr_space
-			      (to_mode, XEXP (x, 0), as));
+			    convert_memory_address_addr_space_1
+			      (to_mode, XEXP (x, 0), as, no_emit));
       break;
 
     case PLUS:
     case MULT:
       /* For addition we can safely permute the conversion and addition
-	 operation if one operand is a constant and converting the constant
-	 does not change it or if one operand is a constant and we are
-	 using a ptr_extend instruction  (POINTERS_EXTEND_UNSIGNED < 0).
-	 We can always safely permute them if we are making the address
-	 narrower.  */
+	 operation if one operand is a constant and converting the
+	 constant does not change it or if one operand is a constant
+	 and we are using a ptr_extend instruction or zero-extending
+	 (POINTERS_EXTEND_UNSIGNED != 0).  We can always safely permute
+	 them if we are making the address narrower.  */
       if (GET_MODE_SIZE (to_mode) < GET_MODE_SIZE (from_mode)
 	  || (GET_CODE (x) == PLUS
 	      && CONST_INT_P (XEXP (x, 1))
-	      && (XEXP (x, 1) == convert_memory_address_addr_space
-				   (to_mode, XEXP (x, 1), as)
-                 || POINTERS_EXTEND_UNSIGNED < 0)))
+	      && (POINTERS_EXTEND_UNSIGNED != 0
+		  || XEXP (x, 1) == convert_memory_address_addr_space_1
+				      (to_mode, XEXP (x, 1), as, no_emit))))
 	return gen_rtx_fmt_ee (GET_CODE (x), to_mode,
-			       convert_memory_address_addr_space
-				 (to_mode, XEXP (x, 0), as),
+			       convert_memory_address_addr_space_1
+				 (to_mode, XEXP (x, 0), as, no_emit),
 			       XEXP (x, 1));
       break;
 
@@ -406,10 +407,17 @@ convert_memory_address_addr_space (enum machine_mode to_mode ATTRIBUTE_UNUSED,
       break;
     }
 
-  return convert_modes (to_mode, from_mode,
-			x, POINTERS_EXTEND_UNSIGNED);
+  return convert_modes_1 (to_mode, from_mode, x,
+			  POINTERS_EXTEND_UNSIGNED, no_emit);
 #endif /* defined(POINTERS_EXTEND_UNSIGNED) */
 }
+
+rtx
+convert_memory_address_addr_space (enum machine_mode to_mode,
+				   rtx x, addr_space_t as)
+{
+  return convert_memory_address_addr_space_1 (to_mode, x, as, false);
+}
 \f
 /* Return something equivalent to X but valid as a memory address for something
    of mode MODE in the named address space AS.  When X is not itself valid,
diff --git a/gcc/expr.c b/gcc/expr.c
index fb4379f..de7f150 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -693,13 +693,16 @@ convert_to_mode (enum machine_mode mode, rtx x, int unsignedp)
    Both modes may be floating, or both integer.
    UNSIGNEDP is nonzero if X is an unsigned value.
 
+   If NO_EMIT is true, don't emit any instructions.
+
    This can be done by referring to a part of X in place
    or by copying to a new temporary with conversion.
 
    You can give VOIDmode for OLDMODE, if you are sure X has a nonvoid mode.  */
 
 rtx
-convert_modes (enum machine_mode mode, enum machine_mode oldmode, rtx x, int unsignedp)
+convert_modes_1 (enum machine_mode mode, enum machine_mode oldmode,
+		 rtx x, int unsignedp, bool no_emit)
 {
   rtx temp;
 
@@ -709,7 +712,12 @@ convert_modes (enum machine_mode mode, enum machine_mode oldmode, rtx x, int uns
   if (GET_CODE (x) == SUBREG && SUBREG_PROMOTED_VAR_P (x)
       && GET_MODE_SIZE (GET_MODE (SUBREG_REG (x))) >= GET_MODE_SIZE (mode)
       && SUBREG_PROMOTED_UNSIGNED_P (x) == unsignedp)
-    x = gen_lowpart (mode, x);
+    {
+      if (no_emit)
+	x = rtl_hooks.gen_lowpart_no_emit (mode, x);
+      else
+	x = gen_lowpart (mode, x);
+    }
 
   if (GET_MODE (x) != VOIDmode)
     oldmode = GET_MODE (x);
@@ -773,7 +781,10 @@ convert_modes (enum machine_mode mode, enum machine_mode oldmode, rtx x, int uns
 	  return gen_int_mode (val, mode);
 	}
 
-      return gen_lowpart (mode, x);
+      if (no_emit)
+	return rtl_hooks.gen_lowpart_no_emit (mode, x);
+      else
+	return gen_lowpart (mode, x);
     }
 
   /* Converting from integer constant into mode is always equivalent to an
@@ -784,10 +795,18 @@ convert_modes (enum machine_mode mode, enum machine_mode oldmode, rtx x, int uns
       return simplify_gen_subreg (mode, x, oldmode, 0);
     }
 
+  gcc_assert (!no_emit);
   temp = gen_reg_rtx (mode);
   convert_move (temp, x, unsignedp);
   return temp;
 }
+
+rtx
+convert_modes (enum machine_mode mode, enum machine_mode oldmode,
+	       rtx x, int unsignedp)
+{
+  return convert_modes_1 (mode, oldmode, x, unsignedp, false);
+}
 \f
 /* Return the largest alignment we can use for doing a move (or store)
    of MAX_PIECES.  ALIGN is the largest alignment we could use.  */
diff --git a/gcc/expr.h b/gcc/expr.h
index cb4050d..2ac9788 100644
--- a/gcc/expr.h
+++ b/gcc/expr.h
@@ -267,6 +267,8 @@ extern rtx convert_to_mode (enum machine_mode, rtx, int);
 
 /* Convert an rtx to MODE from OLDMODE and return the result.  */
 extern rtx convert_modes (enum machine_mode, enum machine_mode, rtx, int);
+extern rtx convert_modes_1 (enum machine_mode, enum machine_mode, rtx,
+			    int, bool);
 
 /* Emit code to move a block Y to a block X.  */
 
diff --git a/gcc/rtl.h b/gcc/rtl.h
index e3ceecd..b01eef8 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -1638,8 +1638,13 @@ extern int byte_lowpart_offset (enum machine_mode, enum machine_mode);
 extern rtx make_safe_from (rtx, rtx);
 extern rtx convert_memory_address_addr_space (enum machine_mode, rtx,
 					      addr_space_t);
+extern rtx convert_memory_address_addr_space_1 (enum machine_mode, rtx,
+						addr_space_t, bool);
 #define convert_memory_address(to_mode,x) \
 	convert_memory_address_addr_space ((to_mode), (x), ADDR_SPACE_GENERIC)
+#define convert_memory_address_1(to_mode,x,no_emit) \
+	convert_memory_address_addr_space_1 ((to_mode), (x), \
+					     ADDR_SPACE_GENERIC, (no_emit))
 extern const char *get_insn_name (int);
 extern rtx get_last_insn_anywhere (void);
 extern rtx get_first_nonnote_insn (void);
diff --git a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c
index 82b818b..189c201 100644
--- a/gcc/simplify-rtx.c
+++ b/gcc/simplify-rtx.c
@@ -1150,7 +1150,7 @@ simplify_unary_operation_1 (enum rtx_code code, enum machine_mode mode, rtx op)
 		  && REG_P (SUBREG_REG (op))
 		  && REG_POINTER (SUBREG_REG (op))
 		  && GET_MODE (SUBREG_REG (op)) == Pmode)))
-	return convert_memory_address (Pmode, op);
+	return convert_memory_address_1 (Pmode, op, true);
 #endif
       break;
 
@@ -1243,7 +1243,7 @@ simplify_unary_operation_1 (enum rtx_code code, enum machine_mode mode, rtx op)
 		  && REG_P (SUBREG_REG (op))
 		  && REG_POINTER (SUBREG_REG (op))
 		  && GET_MODE (SUBREG_REG (op)) == Pmode)))
-	return convert_memory_address (Pmode, op);
+	return convert_memory_address_1 (Pmode, op, true);
 #endif
       break;
 

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: PING: PATCH [4/n]: Prepare x32: Permute the conversion and addition if one operand is a constant
  2011-07-11  1:14           ` H.J. Lu
  2011-07-11  6:49             ` H.J. Lu
@ 2011-07-11 11:09             ` Paolo Bonzini
  2011-07-11 15:58               ` H.J. Lu
  1 sibling, 1 reply; 37+ messages in thread
From: Paolo Bonzini @ 2011-07-11 11:09 UTC (permalink / raw)
  To: H.J. Lu; +Cc: GCC Patches

On 07/11/2011 02:04 AM, H.J. Lu wrote:
> With my original change,  I got
>
> (const:DI (plus:DI (symbol_ref:DI ("iplane.1577") [flags 0x2]
> <var_decl 0x7ffff0857960 iplane>)
>          (const_int -4 [0xfffffffffffffffc])))
>
> I think it is safe to permute the conversion and addition operation
> if one operand is a constant and we are zero-extending.  This is
> how zero-extending works.

Ok, I think I understand what you mean.  The key is the

    XEXP (x, 1) == convert_memory_address_addr_space
                   (to_mode, XEXP (x, 1), as)

test.  It ensures basically that the constant has 31-bit precision, 
because otherwise the constant would change from e.g. (const_int 
-0x7ffffffc) to (const_int 0x80000004) when zero-extending it from 
SImode to DImode.

But I'm not sure it's safe.  You have,

   (zero_extend:DI (plus:SI FOO:SI) (const_int Y))

and you want to convert it to

   (plus:DI FOO:DI (zero_extend:DI (const_int Y)))

(where the zero_extend is folded).  Ignore that FOO is a SYMBOL_REF 
(this piece of code does not assume anything about its shape); if FOO == 
0xfffffffc and Y = 8, the result will be respectively 0x4 (valid) and 
0x100000004 (invalid).

If pointers extend as signed you also have a similar case.   If FOO == 
0x7ffffffc and Y = 8, the result of

   (sign_extend:DI (plus:SI FOO:SI) (const_int Y))

and

   (plus:DI FOO:DI (sign_extend:DI (const_int Y)))

will be respectively 0xffffffff80000004 (valid) and 0x80000004 (invalid).


What happens if you just return NULL instead of the assertion (good idea 
adding it!)?

Of course then you need to:

1) check the return values of convert_memory_address_addr_space_1, and 
propagate NULL up to simplify_unary_operation;

2) check in simplify-rtx.c whether the return value of 
convert_memory_address_1 is NULL, and only return if the return value is 
not NULL.  This is not yet necessary (convert_memory_address is the last 
transformation for both SIGN_EXTEND and ZERO_EXTEND) but it is better to 
keep code clean.

Thanks,

Paolo

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: PING: PATCH [4/n]: Prepare x32: Permute the conversion and addition if one operand is a constant
  2011-07-11 11:09             ` Paolo Bonzini
@ 2011-07-11 15:58               ` H.J. Lu
  2011-07-11 16:57                 ` H.J. Lu
  2011-07-13 16:24                 ` Paolo Bonzini
  0 siblings, 2 replies; 37+ messages in thread
From: H.J. Lu @ 2011-07-11 15:58 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: GCC Patches

On Mon, Jul 11, 2011 at 4:03 AM, Paolo Bonzini <bonzini@gnu.org> wrote:
> On 07/11/2011 02:04 AM, H.J. Lu wrote:
>>
>> With my original change,  I got
>>
>> (const:DI (plus:DI (symbol_ref:DI ("iplane.1577") [flags 0x2]
>> <var_decl 0x7ffff0857960 iplane>)
>>         (const_int -4 [0xfffffffffffffffc])))
>>
>> I think it is safe to permute the conversion and addition operation
>> if one operand is a constant and we are zero-extending.  This is
>> how zero-extending works.
>
> Ok, I think I understand what you mean.  The key is the
>
>   XEXP (x, 1) == convert_memory_address_addr_space
>                  (to_mode, XEXP (x, 1), as)
>
> test.  It ensures basically that the constant has 31-bit precision, because
> otherwise the constant would change from e.g. (const_int -0x7ffffffc) to
> (const_int 0x80000004) when zero-extending it from SImode to DImode.
>
> But I'm not sure it's safe.  You have,
>
>  (zero_extend:DI (plus:SI FOO:SI) (const_int Y))
>
> and you want to convert it to
>
>  (plus:DI FOO:DI (zero_extend:DI (const_int Y)))
>
> (where the zero_extend is folded).  Ignore that FOO is a SYMBOL_REF (this
> piece of code does not assume anything about its shape); if FOO ==
> 0xfffffffc and Y = 8, the result will be respectively 0x4 (valid) and
> 0x100000004 (invalid).

This example contradicts what you said above "It ensures basically that the
constant has 31-bit precision".  For zero-extend, the issue is address-wrap.
As I understand, to support address-wrap, you need to use ptr_mode.

> If pointers extend as signed you also have a similar case.   If FOO ==
> 0x7ffffffc and Y = 8, the result of
>
>  (sign_extend:DI (plus:SI FOO:SI) (const_int Y))
>
> and
>
>  (plus:DI FOO:DI (sign_extend:DI (const_int Y)))
>
> will be respectively 0xffffffff80000004 (valid) and 0x80000004 (invalid).
>
>
> What happens if you just return NULL instead of the assertion (good idea
> adding it!)?
>
> Of course then you need to:
>
> 1) check the return values of convert_memory_address_addr_space_1, and
> propagate NULL up to simplify_unary_operation;
>
> 2) check in simplify-rtx.c whether the return value of
> convert_memory_address_1 is NULL, and only return if the return value is not
> NULL.  This is not yet necessary (convert_memory_address is the last
> transformation for both SIGN_EXTEND and ZERO_EXTEND) but it is better to
> keep code clean.

I will give it a try.

Thanks.


-- 
H.J.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: PING: PATCH [4/n]: Prepare x32: Permute the conversion and addition if one operand is a constant
  2011-07-11 15:58               ` H.J. Lu
@ 2011-07-11 16:57                 ` H.J. Lu
  2011-07-11 17:26                   ` H.J. Lu
  2011-07-13 16:24                 ` Paolo Bonzini
  1 sibling, 1 reply; 37+ messages in thread
From: H.J. Lu @ 2011-07-11 16:57 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: GCC Patches

On Mon, Jul 11, 2011 at 8:54 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Mon, Jul 11, 2011 at 4:03 AM, Paolo Bonzini <bonzini@gnu.org> wrote:
>> On 07/11/2011 02:04 AM, H.J. Lu wrote:
>>>
>>> With my original change,  I got
>>>
>>> (const:DI (plus:DI (symbol_ref:DI ("iplane.1577") [flags 0x2]
>>> <var_decl 0x7ffff0857960 iplane>)
>>>         (const_int -4 [0xfffffffffffffffc])))
>>>
>>> I think it is safe to permute the conversion and addition operation
>>> if one operand is a constant and we are zero-extending.  This is
>>> how zero-extending works.
>>
>> Ok, I think I understand what you mean.  The key is the
>>
>>   XEXP (x, 1) == convert_memory_address_addr_space
>>                  (to_mode, XEXP (x, 1), as)
>>
>> test.  It ensures basically that the constant has 31-bit precision, because
>> otherwise the constant would change from e.g. (const_int -0x7ffffffc) to
>> (const_int 0x80000004) when zero-extending it from SImode to DImode.
>>
>> But I'm not sure it's safe.  You have,
>>
>>  (zero_extend:DI (plus:SI FOO:SI) (const_int Y))
>>
>> and you want to convert it to
>>
>>  (plus:DI FOO:DI (zero_extend:DI (const_int Y)))
>>
>> (where the zero_extend is folded).  Ignore that FOO is a SYMBOL_REF (this
>> piece of code does not assume anything about its shape); if FOO ==
>> 0xfffffffc and Y = 8, the result will be respectively 0x4 (valid) and
>> 0x100000004 (invalid).
>
> This example contradicts what you said above "It ensures basically that the
> constant has 31-bit precision".  For zero-extend, the issue is address-wrap.
> As I understand, to support address-wrap, you need to use ptr_mode.
>

I am totally confused what the current code

     /* For addition we can safely permute the conversion and addition
         operation if one operand is a constant and converting the constant
         does not change it or if one operand is a constant and we are
         using a ptr_extend instruction  (POINTERS_EXTEND_UNSIGNED < 0).
         We can always safely permute them if we are making the address
         narrower.  */
      if (GET_MODE_SIZE (to_mode) < GET_MODE_SIZE (from_mode)
          || (GET_CODE (x) == PLUS
              && CONST_INT_P (XEXP (x, 1))
              && (XEXP (x, 1) == convert_memory_address_addr_space
                                   (to_mode, XEXP (x, 1), as)
                 || POINTERS_EXTEND_UNSIGNED < 0)))
        return gen_rtx_fmt_ee (GET_CODE (x), to_mode,
                               convert_memory_address_addr_space
                                 (to_mode, XEXP (x, 0), as),
                               XEXP (x, 1));

is trying to do.  It doesn't support address-wrap at all, regardless if
converting the constant changes the constant.  I think it should be
OK to permute if no instructions are allowed, like:

     if (GET_MODE_SIZE (to_mode) < GET_MODE_SIZE (from_mode)
          || (GET_CODE (x) == PLUS
              && CONST_INT_P (XEXP (x, 1))
              && POINTERS_EXTEND_UNSIGNED != 0
              && no_emit))
        return gen_rtx_fmt_ee (GET_CODE (x), to_mode,
                               convert_memory_address_addr_space_1
                                 (to_mode, XEXP (x, 0), as, no_emit),
                               XEXP (x, 1));


-- 
H.J.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: PING: PATCH [4/n]: Prepare x32: Permute the conversion and addition if one operand is a constant
  2011-07-11 16:57                 ` H.J. Lu
@ 2011-07-11 17:26                   ` H.J. Lu
  0 siblings, 0 replies; 37+ messages in thread
From: H.J. Lu @ 2011-07-11 17:26 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: GCC Patches

[-- Attachment #1: Type: text/plain, Size: 4008 bytes --]

On Mon, Jul 11, 2011 at 9:55 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Mon, Jul 11, 2011 at 8:54 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Mon, Jul 11, 2011 at 4:03 AM, Paolo Bonzini <bonzini@gnu.org> wrote:
>>> On 07/11/2011 02:04 AM, H.J. Lu wrote:
>>>>
>>>> With my original change,  I got
>>>>
>>>> (const:DI (plus:DI (symbol_ref:DI ("iplane.1577") [flags 0x2]
>>>> <var_decl 0x7ffff0857960 iplane>)
>>>>         (const_int -4 [0xfffffffffffffffc])))
>>>>
>>>> I think it is safe to permute the conversion and addition operation
>>>> if one operand is a constant and we are zero-extending.  This is
>>>> how zero-extending works.
>>>
>>> Ok, I think I understand what you mean.  The key is the
>>>
>>>   XEXP (x, 1) == convert_memory_address_addr_space
>>>                  (to_mode, XEXP (x, 1), as)
>>>
>>> test.  It ensures basically that the constant has 31-bit precision, because
>>> otherwise the constant would change from e.g. (const_int -0x7ffffffc) to
>>> (const_int 0x80000004) when zero-extending it from SImode to DImode.
>>>
>>> But I'm not sure it's safe.  You have,
>>>
>>>  (zero_extend:DI (plus:SI FOO:SI) (const_int Y))
>>>
>>> and you want to convert it to
>>>
>>>  (plus:DI FOO:DI (zero_extend:DI (const_int Y)))
>>>
>>> (where the zero_extend is folded).  Ignore that FOO is a SYMBOL_REF (this
>>> piece of code does not assume anything about its shape); if FOO ==
>>> 0xfffffffc and Y = 8, the result will be respectively 0x4 (valid) and
>>> 0x100000004 (invalid).
>>
>> This example contradicts what you said above "It ensures basically that the
>> constant has 31-bit precision".  For zero-extend, the issue is address-wrap.
>> As I understand, to support address-wrap, you need to use ptr_mode.
>>
>
> I am totally confused what the current code
>
>     /* For addition we can safely permute the conversion and addition
>         operation if one operand is a constant and converting the constant
>         does not change it or if one operand is a constant and we are
>         using a ptr_extend instruction  (POINTERS_EXTEND_UNSIGNED < 0).
>         We can always safely permute them if we are making the address
>         narrower.  */
>      if (GET_MODE_SIZE (to_mode) < GET_MODE_SIZE (from_mode)
>          || (GET_CODE (x) == PLUS
>              && CONST_INT_P (XEXP (x, 1))
>              && (XEXP (x, 1) == convert_memory_address_addr_space
>                                   (to_mode, XEXP (x, 1), as)
>                 || POINTERS_EXTEND_UNSIGNED < 0)))
>        return gen_rtx_fmt_ee (GET_CODE (x), to_mode,
>                               convert_memory_address_addr_space
>                                 (to_mode, XEXP (x, 0), as),
>                               XEXP (x, 1));
>
> is trying to do.  It doesn't support address-wrap at all, regardless if
> converting the constant changes the constant.  I think it should be
> OK to permute if no instructions are allowed, like:
>
>     if (GET_MODE_SIZE (to_mode) < GET_MODE_SIZE (from_mode)
>          || (GET_CODE (x) == PLUS
>              && CONST_INT_P (XEXP (x, 1))
>              && POINTERS_EXTEND_UNSIGNED != 0
>              && no_emit))
>        return gen_rtx_fmt_ee (GET_CODE (x), to_mode,
>                               convert_memory_address_addr_space_1
>                                 (to_mode, XEXP (x, 0), as, no_emit),
>                               XEXP (x, 1));
>
>

This patch implements it.

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

	PR middle-end/47727
	* explow.c (convert_memory_address_addr_space_1): New.
	(convert_memory_address_addr_space): Use it.

	* expr.c (convert_modes_1): New.
	(convert_modes): Use it.

	* expr.h (convert_modes_1): New.

	* rtl.h (convert_memory_address_addr_space_1): New.
	(convert_memory_address_1): Likewise.

	* simplify-rtx.c (simplify_unary_operation_1): Call
	convert_memory_address_1 instead of convert_memory_address.

[-- Attachment #2: gcc-x32-pr47727-3.patch --]
[-- Type: text/plain, Size: 7792 bytes --]

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

	PR middle-end/47727
	* explow.c (convert_memory_address_addr_space_1): New.
	(convert_memory_address_addr_space): Use it.

	* expr.c (convert_modes_1): New.
	(convert_modes): Use it.

	* expr.h (convert_modes_1): New.

	* rtl.h (convert_memory_address_addr_space_1): New.
	(convert_memory_address_1): Likewise.

	* simplify-rtx.c (simplify_unary_operation_1): Call
	convert_memory_address_1 instead of convert_memory_address.

diff --git a/gcc/explow.c b/gcc/explow.c
index 3c692f4..d2c54ff 100644
--- a/gcc/explow.c
+++ b/gcc/explow.c
@@ -320,8 +320,9 @@ break_out_memory_refs (rtx x)
    arithmetic insns can be used.  */
 
 rtx
-convert_memory_address_addr_space (enum machine_mode to_mode ATTRIBUTE_UNUSED,
-				   rtx x, addr_space_t as ATTRIBUTE_UNUSED)
+convert_memory_address_addr_space_1 (enum machine_mode to_mode ATTRIBUTE_UNUSED,
+				     rtx x, addr_space_t as ATTRIBUTE_UNUSED,
+				     bool no_emit ATTRIBUTE_UNUSED)
 {
 #ifndef POINTERS_EXTEND_UNSIGNED
   gcc_assert (GET_MODE (x) == to_mode || GET_MODE (x) == VOIDmode);
@@ -378,27 +379,25 @@ convert_memory_address_addr_space (enum machine_mode to_mode ATTRIBUTE_UNUSED,
 
     case CONST:
       return gen_rtx_CONST (to_mode,
-			    convert_memory_address_addr_space
-			      (to_mode, XEXP (x, 0), as));
+			    convert_memory_address_addr_space_1
+			      (to_mode, XEXP (x, 0), as, no_emit));
       break;
 
     case PLUS:
     case MULT:
-      /* For addition we can safely permute the conversion and addition
-	 operation if one operand is a constant and converting the constant
-	 does not change it or if one operand is a constant and we are
-	 using a ptr_extend instruction  (POINTERS_EXTEND_UNSIGNED < 0).
-	 We can always safely permute them if we are making the address
-	 narrower.  */
+      /* For addition, we permute the conversion and addition operation
+	 if one operand is a constant, no new instructions are allowed
+	 and we are using a ptr_extend instruction or zero-extending
+	 (POINTERS_EXTEND_UNSIGNED != 0).  We can always safely permute
+	 them if we are making the address narrower.  */
       if (GET_MODE_SIZE (to_mode) < GET_MODE_SIZE (from_mode)
 	  || (GET_CODE (x) == PLUS
 	      && CONST_INT_P (XEXP (x, 1))
-	      && (XEXP (x, 1) == convert_memory_address_addr_space
-				   (to_mode, XEXP (x, 1), as)
-                 || POINTERS_EXTEND_UNSIGNED < 0)))
+	      && POINTERS_EXTEND_UNSIGNED != 0
+	      && no_emit))
 	return gen_rtx_fmt_ee (GET_CODE (x), to_mode,
-			       convert_memory_address_addr_space
-				 (to_mode, XEXP (x, 0), as),
+			       convert_memory_address_addr_space_1
+				 (to_mode, XEXP (x, 0), as, no_emit),
 			       XEXP (x, 1));
       break;
 
@@ -406,10 +405,17 @@ convert_memory_address_addr_space (enum machine_mode to_mode ATTRIBUTE_UNUSED,
       break;
     }
 
-  return convert_modes (to_mode, from_mode,
-			x, POINTERS_EXTEND_UNSIGNED);
+  return convert_modes_1 (to_mode, from_mode, x,
+			  POINTERS_EXTEND_UNSIGNED, no_emit);
 #endif /* defined(POINTERS_EXTEND_UNSIGNED) */
 }
+
+rtx
+convert_memory_address_addr_space (enum machine_mode to_mode,
+				   rtx x, addr_space_t as)
+{
+  return convert_memory_address_addr_space_1 (to_mode, x, as, false);
+}
 \f
 /* Return something equivalent to X but valid as a memory address for something
    of mode MODE in the named address space AS.  When X is not itself valid,
diff --git a/gcc/expr.c b/gcc/expr.c
index fb4379f..de7f150 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -693,13 +693,16 @@ convert_to_mode (enum machine_mode mode, rtx x, int unsignedp)
    Both modes may be floating, or both integer.
    UNSIGNEDP is nonzero if X is an unsigned value.
 
+   If NO_EMIT is true, don't emit any instructions.
+
    This can be done by referring to a part of X in place
    or by copying to a new temporary with conversion.
 
    You can give VOIDmode for OLDMODE, if you are sure X has a nonvoid mode.  */
 
 rtx
-convert_modes (enum machine_mode mode, enum machine_mode oldmode, rtx x, int unsignedp)
+convert_modes_1 (enum machine_mode mode, enum machine_mode oldmode,
+		 rtx x, int unsignedp, bool no_emit)
 {
   rtx temp;
 
@@ -709,7 +712,12 @@ convert_modes (enum machine_mode mode, enum machine_mode oldmode, rtx x, int uns
   if (GET_CODE (x) == SUBREG && SUBREG_PROMOTED_VAR_P (x)
       && GET_MODE_SIZE (GET_MODE (SUBREG_REG (x))) >= GET_MODE_SIZE (mode)
       && SUBREG_PROMOTED_UNSIGNED_P (x) == unsignedp)
-    x = gen_lowpart (mode, x);
+    {
+      if (no_emit)
+	x = rtl_hooks.gen_lowpart_no_emit (mode, x);
+      else
+	x = gen_lowpart (mode, x);
+    }
 
   if (GET_MODE (x) != VOIDmode)
     oldmode = GET_MODE (x);
@@ -773,7 +781,10 @@ convert_modes (enum machine_mode mode, enum machine_mode oldmode, rtx x, int uns
 	  return gen_int_mode (val, mode);
 	}
 
-      return gen_lowpart (mode, x);
+      if (no_emit)
+	return rtl_hooks.gen_lowpart_no_emit (mode, x);
+      else
+	return gen_lowpart (mode, x);
     }
 
   /* Converting from integer constant into mode is always equivalent to an
@@ -784,10 +795,18 @@ convert_modes (enum machine_mode mode, enum machine_mode oldmode, rtx x, int uns
       return simplify_gen_subreg (mode, x, oldmode, 0);
     }
 
+  gcc_assert (!no_emit);
   temp = gen_reg_rtx (mode);
   convert_move (temp, x, unsignedp);
   return temp;
 }
+
+rtx
+convert_modes (enum machine_mode mode, enum machine_mode oldmode,
+	       rtx x, int unsignedp)
+{
+  return convert_modes_1 (mode, oldmode, x, unsignedp, false);
+}
 \f
 /* Return the largest alignment we can use for doing a move (or store)
    of MAX_PIECES.  ALIGN is the largest alignment we could use.  */
diff --git a/gcc/expr.h b/gcc/expr.h
index cb4050d..2ac9788 100644
--- a/gcc/expr.h
+++ b/gcc/expr.h
@@ -267,6 +267,8 @@ extern rtx convert_to_mode (enum machine_mode, rtx, int);
 
 /* Convert an rtx to MODE from OLDMODE and return the result.  */
 extern rtx convert_modes (enum machine_mode, enum machine_mode, rtx, int);
+extern rtx convert_modes_1 (enum machine_mode, enum machine_mode, rtx,
+			    int, bool);
 
 /* Emit code to move a block Y to a block X.  */
 
diff --git a/gcc/rtl.h b/gcc/rtl.h
index e3ceecd..b01eef8 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -1638,8 +1638,13 @@ extern int byte_lowpart_offset (enum machine_mode, enum machine_mode);
 extern rtx make_safe_from (rtx, rtx);
 extern rtx convert_memory_address_addr_space (enum machine_mode, rtx,
 					      addr_space_t);
+extern rtx convert_memory_address_addr_space_1 (enum machine_mode, rtx,
+						addr_space_t, bool);
 #define convert_memory_address(to_mode,x) \
 	convert_memory_address_addr_space ((to_mode), (x), ADDR_SPACE_GENERIC)
+#define convert_memory_address_1(to_mode,x,no_emit) \
+	convert_memory_address_addr_space_1 ((to_mode), (x), \
+					     ADDR_SPACE_GENERIC, (no_emit))
 extern const char *get_insn_name (int);
 extern rtx get_last_insn_anywhere (void);
 extern rtx get_first_nonnote_insn (void);
diff --git a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c
index 82b818b..189c201 100644
--- a/gcc/simplify-rtx.c
+++ b/gcc/simplify-rtx.c
@@ -1150,7 +1150,7 @@ simplify_unary_operation_1 (enum rtx_code code, enum machine_mode mode, rtx op)
 		  && REG_P (SUBREG_REG (op))
 		  && REG_POINTER (SUBREG_REG (op))
 		  && GET_MODE (SUBREG_REG (op)) == Pmode)))
-	return convert_memory_address (Pmode, op);
+	return convert_memory_address_1 (Pmode, op, true);
 #endif
       break;
 
@@ -1243,7 +1243,7 @@ simplify_unary_operation_1 (enum rtx_code code, enum machine_mode mode, rtx op)
 		  && REG_P (SUBREG_REG (op))
 		  && REG_POINTER (SUBREG_REG (op))
 		  && GET_MODE (SUBREG_REG (op)) == Pmode)))
-	return convert_memory_address (Pmode, op);
+	return convert_memory_address_1 (Pmode, op, true);
 #endif
       break;
 

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: PING: PATCH [4/n]: Prepare x32: Permute the conversion and addition if one operand is a constant
  2011-07-11 15:58               ` H.J. Lu
  2011-07-11 16:57                 ` H.J. Lu
@ 2011-07-13 16:24                 ` Paolo Bonzini
  2011-07-13 16:52                   ` H.J. Lu
  1 sibling, 1 reply; 37+ messages in thread
From: Paolo Bonzini @ 2011-07-13 16:24 UTC (permalink / raw)
  To: H.J. Lu; +Cc: GCC Patches

On 07/11/2011 05:54 PM, H.J. Lu wrote:
> The key is the
> >
> >     XEXP (x, 1) == convert_memory_address_addr_space
> >                    (to_mode, XEXP (x, 1), as)
> >
> >  test.  It ensures basically that the constant has 31-bit precision, because
> >  otherwise the constant would change from e.g. (const_int -0x7ffffffc) to
> >  (const_int 0x80000004) when zero-extending it from SImode to DImode.
> >
> >  But I'm not sure it's safe.  You have,
> >
> >    (zero_extend:DI (plus:SI FOO:SI) (const_int Y))
> >
> >  and you want to convert it to
> >
> >    (plus:DI FOO:DI (zero_extend:DI (const_int Y)))
> >
> >  (where the zero_extend is folded).  Ignore that FOO is a SYMBOL_REF (this
> >  piece of code does not assume anything about its shape); if FOO ==
> >  0xfffffffc and Y = 8, the result will be respectively 0x4 (valid) and
> >  0x100000004 (invalid).
>
> This example contradicts what you said above "It ensures basically that the
> constant has 31-bit precision".

Why?  Certainly Y = 8 has 31-bit (or less) precision.  So it has the 
same representation in SImode and DImode, and the test above on XEXP (x, 
1) succeeds.

> >  What happens if you just return NULL instead of the assertion (good idea
> >  adding it!)?
> >
> >  Of course then you need to:
> >
> >  1) check the return values of convert_memory_address_addr_space_1, and
> >  propagate NULL up to simplify_unary_operation;
> >
> >  2) check in simplify-rtx.c whether the return value of
> >  convert_memory_address_1 is NULL, and only return if the return value is not
> >  NULL.  This is not yet necessary (convert_memory_address is the last
> >  transformation for both SIGN_EXTEND and ZERO_EXTEND) but it is better to
> >  keep code clean.
>
> I will give it a try.

Thanks, did you get any result?  There's no "I think" in this code.  So 
even if I cannot approve it, I'd really like to see a version that I 
understand and that is clearly conservative, if it works.

Paolo

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: PING: PATCH [4/n]: Prepare x32: Permute the conversion and addition if one operand is a constant
  2011-07-13 16:24                 ` Paolo Bonzini
@ 2011-07-13 16:52                   ` H.J. Lu
  2011-07-13 16:55                     ` Paolo Bonzini
  2014-05-29  4:52                     ` Andrew Pinski
  0 siblings, 2 replies; 37+ messages in thread
From: H.J. Lu @ 2011-07-13 16:52 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: GCC Patches

On Wed, Jul 13, 2011 at 9:13 AM, Paolo Bonzini <bonzini@gnu.org> wrote:
> On 07/11/2011 05:54 PM, H.J. Lu wrote:
>>
>> The key is the
>> >
>> >     XEXP (x, 1) == convert_memory_address_addr_space
>> >                    (to_mode, XEXP (x, 1), as)
>> >
>> >  test.  It ensures basically that the constant has 31-bit precision,
>> > because
>> >  otherwise the constant would change from e.g. (const_int -0x7ffffffc)
>> > to
>> >  (const_int 0x80000004) when zero-extending it from SImode to DImode.
>> >
>> >  But I'm not sure it's safe.  You have,
>> >
>> >    (zero_extend:DI (plus:SI FOO:SI) (const_int Y))
>> >
>> >  and you want to convert it to
>> >
>> >    (plus:DI FOO:DI (zero_extend:DI (const_int Y)))
>> >
>> >  (where the zero_extend is folded).  Ignore that FOO is a SYMBOL_REF
>> > (this
>> >  piece of code does not assume anything about its shape); if FOO ==
>> >  0xfffffffc and Y = 8, the result will be respectively 0x4 (valid) and
>> >  0x100000004 (invalid).
>>
>> This example contradicts what you said above "It ensures basically that
>> the
>> constant has 31-bit precision".
>
> Why?  Certainly Y = 8 has 31-bit (or less) precision.  So it has the same
> representation in SImode and DImode, and the test above on XEXP (x, 1)
> succeeds.

And then we permute conversion and addition, which leads to the issue you
raised above.  In another word, the current code permutes conversion
and addition.
It leads to different values in case of symbol (0xfffffffc) + 8.
Basically the current
test for 31-bit (or less) precision is bogus.  The real question is
for a address
computation, A + B, if address wrap-around is supported in
convert_memory_address_addr_space.

>> >  What happens if you just return NULL instead of the assertion (good
>> > idea
>> >  adding it!)?
>> >
>> >  Of course then you need to:
>> >
>> >  1) check the return values of convert_memory_address_addr_space_1, and
>> >  propagate NULL up to simplify_unary_operation;
>> >
>> >  2) check in simplify-rtx.c whether the return value of
>> >  convert_memory_address_1 is NULL, and only return if the return value
>> > is not
>> >  NULL.  This is not yet necessary (convert_memory_address is the last
>> >  transformation for both SIGN_EXTEND and ZERO_EXTEND) but it is better
>> > to
>> >  keep code clean.
>>
>> I will give it a try.
>
> Thanks, did you get any result?  There's no "I think" in this code.  So even
> if I cannot approve it, I'd really like to see a version that I understand
> and that is clearly conservative, if it works.
>

I opened a new bug:

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

My current code looks like:

   case CONST:
      temp = convert_memory_address_addr_space_1 (to_mode, XEXP (x, 0),
                                                  as, no_emit,
                                                  ignore_address_wrap_around);
      return temp ? gen_rtx_CONST (to_mode, temp) : temp;
      break;

    case PLUS:
    case MULT:
      /* For addition we can safely permute the conversion and addition
         operation if one operand is a constant, address wrap-around
         is ignored and we are using a ptr_extend instruction or
         zero-extending (POINTERS_EXTEND_UNSIGNED != 0).  We can always
         safely permute them if we are making the address narrower.  */
      if (GET_MODE_SIZE (to_mode) < GET_MODE_SIZE (from_mode)
          || (GET_CODE (x) == PLUS
              && CONST_INT_P (XEXP (x, 1))
              && (POINTERS_EXTEND_UNSIGNED != 0
                  && ignore_address_wrap_around)))
        return gen_rtx_fmt_ee (GET_CODE (x), to_mode,
                               convert_memory_address_addr_space_1
                                 (to_mode, XEXP (x, 0), as, no_emit,
                                  ignore_address_wrap_around),
                               XEXP (x, 1));
      break;

-- 
H.J.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: PING: PATCH [4/n]: Prepare x32: Permute the conversion and addition if one operand is a constant
  2011-07-13 16:52                   ` H.J. Lu
@ 2011-07-13 16:55                     ` Paolo Bonzini
  2011-07-13 16:58                       ` Paolo Bonzini
  2014-05-29  4:52                     ` Andrew Pinski
  1 sibling, 1 reply; 37+ messages in thread
From: Paolo Bonzini @ 2011-07-13 16:55 UTC (permalink / raw)
  To: H.J. Lu; +Cc: GCC Patches

On Wed, Jul 13, 2011 at 18:39, H.J. Lu <hjl.tools@gmail.com> wrote:

>> Why?  Certainly Y = 8 has 31-bit (or less) precision.  So it has the same
>> representation in SImode and DImode, and the test above on XEXP (x, 1)
>> succeeds.
>
> And then we permute conversion and addition, which leads to the issue you
> raised above.  In another word, the current code permutes conversion
> and addition.

No, only if we have ptr_extend.  It may be buggy as well, but let's
make sure first that x32 is done right, then perhaps whoever cares can
fix ptr_extend if it has to be fixed.  I don't know the semantics of
ia64 addp4 so I cannot tell.

> I opened a new bug:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49721

Good, thanks.

> My current code looks like:
>
>   case CONST:
>      temp = convert_memory_address_addr_space_1 (to_mode, XEXP (x, 0),
>                                                  as, no_emit,
>                                                  ignore_address_wrap_around);

Here I stopped reading.  It's not what I asked for, so at least you
should say clearly _why_.

Paolo

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: PING: PATCH [4/n]: Prepare x32: Permute the conversion and addition if one operand is a constant
  2011-07-13 16:55                     ` Paolo Bonzini
@ 2011-07-13 16:58                       ` Paolo Bonzini
  2011-07-13 18:42                         ` H.J. Lu
  0 siblings, 1 reply; 37+ messages in thread
From: Paolo Bonzini @ 2011-07-13 16:58 UTC (permalink / raw)
  To: H.J. Lu; +Cc: GCC Patches

>>> Why?  Certainly Y = 8 has 31-bit (or less) precision.  So it has the same
>>> representation in SImode and DImode, and the test above on XEXP (x, 1)
>>> succeeds.
>>
>> And then we permute conversion and addition, which leads to the issue you
>> raised above.  In another word, the current code permutes conversion
>> and addition.
>
> No, only if we have ptr_extend.

Oops, hit send too early, I understand now what you mean.  But even
more so, let's make sure x32 is done right so that perhaps we can
remove the bogus test on XEXP (x, 1) for other Pmode != ptr_mode
targets, non-ptr_extend.  Then we can worry perhaps of
POINTERS_EXTEND_UNSIGNED < 0.

Paolo

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: PING: PATCH [4/n]: Prepare x32: Permute the conversion and addition if one operand is a constant
  2011-07-13 16:58                       ` Paolo Bonzini
@ 2011-07-13 18:42                         ` H.J. Lu
  2011-07-25 10:34                           ` Paolo Bonzini
  0 siblings, 1 reply; 37+ messages in thread
From: H.J. Lu @ 2011-07-13 18:42 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: GCC Patches

[-- Attachment #1: Type: text/plain, Size: 1325 bytes --]

On Wed, Jul 13, 2011 at 9:54 AM, Paolo Bonzini <bonzini@gnu.org> wrote:
>>>> Why?  Certainly Y = 8 has 31-bit (or less) precision.  So it has the same
>>>> representation in SImode and DImode, and the test above on XEXP (x, 1)
>>>> succeeds.
>>>
>>> And then we permute conversion and addition, which leads to the issue you
>>> raised above.  In another word, the current code permutes conversion
>>> and addition.
>>
>> No, only if we have ptr_extend.
>
> Oops, hit send too early, I understand now what you mean.  But even
> more so, let's make sure x32 is done right so that perhaps we can
> remove the bogus test on XEXP (x, 1) for other Pmode != ptr_mode
> targets, non-ptr_extend.  Then we can worry perhaps of
> POINTERS_EXTEND_UNSIGNED < 0.
>

Here is the patch.  OK for trunk?

Thanks.


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

	PR middle-end/49721
	* explow.c (convert_memory_address_addr_space_1): New.
	(convert_memory_address_addr_space): Use it.

	* expr.c (convert_modes_1): New.
	(convert_modes): Use it.

	* expr.h (convert_modes_1): New.

	* rtl.h (convert_memory_address_addr_space_1): New.
	(convert_memory_address_1): Likewise.

	* simplify-rtx.c (simplify_unary_operation_1): Call
	convert_memory_address_1 instead of convert_memory_address.

[-- Attachment #2: gcc-x32-pr49721-2.patch --]
[-- Type: text/plain, Size: 8122 bytes --]

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

	PR middle-end/49721
	* explow.c (convert_memory_address_addr_space_1): New.
	(convert_memory_address_addr_space): Use it.

	* expr.c (convert_modes_1): New.
	(convert_modes): Use it.

	* expr.h (convert_modes_1): New.

	* rtl.h (convert_memory_address_addr_space_1): New.
	(convert_memory_address_1): Likewise.

	* simplify-rtx.c (simplify_unary_operation_1): Call
	convert_memory_address_1 instead of convert_memory_address.

diff --git a/gcc/explow.c b/gcc/explow.c
index 3c692f4..8551fe8 100644
--- a/gcc/explow.c
+++ b/gcc/explow.c
@@ -320,8 +320,10 @@ break_out_memory_refs (rtx x)
    arithmetic insns can be used.  */
 
 rtx
-convert_memory_address_addr_space (enum machine_mode to_mode ATTRIBUTE_UNUSED,
-				   rtx x, addr_space_t as ATTRIBUTE_UNUSED)
+convert_memory_address_addr_space_1 (enum machine_mode to_mode ATTRIBUTE_UNUSED,
+				     rtx x, addr_space_t as ATTRIBUTE_UNUSED,
+				     bool no_emit ATTRIBUTE_UNUSED,
+				     bool ignore_address_wrap_around ATTRIBUTE_UNUSED)
 {
 #ifndef POINTERS_EXTEND_UNSIGNED
   gcc_assert (GET_MODE (x) == to_mode || GET_MODE (x) == VOIDmode);
@@ -377,28 +379,28 @@ convert_memory_address_addr_space (enum machine_mode to_mode ATTRIBUTE_UNUSED,
       break;
 
     case CONST:
-      return gen_rtx_CONST (to_mode,
-			    convert_memory_address_addr_space
-			      (to_mode, XEXP (x, 0), as));
+      temp = convert_memory_address_addr_space_1 (to_mode, XEXP (x, 0),
+						  as, no_emit,
+						  ignore_address_wrap_around);
+      return temp ? gen_rtx_CONST (to_mode, temp) : temp;
       break;
 
     case PLUS:
     case MULT:
-      /* For addition we can safely permute the conversion and addition
-	 operation if one operand is a constant and converting the constant
-	 does not change it or if one operand is a constant and we are
-	 using a ptr_extend instruction  (POINTERS_EXTEND_UNSIGNED < 0).
-	 We can always safely permute them if we are making the address
-	 narrower.  */
+      /* For addition, we can safely permute the conversion and addition
+	 operation if one operand is a constant and we are using a
+	 ptr_extend instruction (POINTERS_EXTEND_UNSIGNED < 0) or address
+	 wrap-around is ignored.  We can always safely permute them if
+	 we are making the address narrower.  */
       if (GET_MODE_SIZE (to_mode) < GET_MODE_SIZE (from_mode)
 	  || (GET_CODE (x) == PLUS
 	      && CONST_INT_P (XEXP (x, 1))
-	      && (XEXP (x, 1) == convert_memory_address_addr_space
-				   (to_mode, XEXP (x, 1), as)
-                 || POINTERS_EXTEND_UNSIGNED < 0)))
+	      && (POINTERS_EXTEND_UNSIGNED < 0
+		  || ignore_address_wrap_around)))
 	return gen_rtx_fmt_ee (GET_CODE (x), to_mode,
-			       convert_memory_address_addr_space
-				 (to_mode, XEXP (x, 0), as),
+			       convert_memory_address_addr_space_1
+				 (to_mode, XEXP (x, 0), as, no_emit,
+				  ignore_address_wrap_around),
 			       XEXP (x, 1));
       break;
 
@@ -406,10 +408,17 @@ convert_memory_address_addr_space (enum machine_mode to_mode ATTRIBUTE_UNUSED,
       break;
     }
 
-  return convert_modes (to_mode, from_mode,
-			x, POINTERS_EXTEND_UNSIGNED);
+  return convert_modes_1 (to_mode, from_mode, x,
+			  POINTERS_EXTEND_UNSIGNED, no_emit);
 #endif /* defined(POINTERS_EXTEND_UNSIGNED) */
 }
+
+rtx
+convert_memory_address_addr_space (enum machine_mode to_mode,
+				   rtx x, addr_space_t as)
+{
+  return convert_memory_address_addr_space_1 (to_mode, x, as, false, true);
+}
 \f
 /* Return something equivalent to X but valid as a memory address for something
    of mode MODE in the named address space AS.  When X is not itself valid,
diff --git a/gcc/expr.c b/gcc/expr.c
index 77039e8..e42ab04 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -693,13 +693,16 @@ convert_to_mode (enum machine_mode mode, rtx x, int unsignedp)
    Both modes may be floating, or both integer.
    UNSIGNEDP is nonzero if X is an unsigned value.
 
+   If NO_EMIT is true, don't emit any instructions.
+
    This can be done by referring to a part of X in place
    or by copying to a new temporary with conversion.
 
    You can give VOIDmode for OLDMODE, if you are sure X has a nonvoid mode.  */
 
 rtx
-convert_modes (enum machine_mode mode, enum machine_mode oldmode, rtx x, int unsignedp)
+convert_modes_1 (enum machine_mode mode, enum machine_mode oldmode,
+		 rtx x, int unsignedp, bool no_emit)
 {
   rtx temp;
 
@@ -709,7 +712,12 @@ convert_modes (enum machine_mode mode, enum machine_mode oldmode, rtx x, int uns
   if (GET_CODE (x) == SUBREG && SUBREG_PROMOTED_VAR_P (x)
       && GET_MODE_SIZE (GET_MODE (SUBREG_REG (x))) >= GET_MODE_SIZE (mode)
       && SUBREG_PROMOTED_UNSIGNED_P (x) == unsignedp)
-    x = gen_lowpart (mode, x);
+    {
+      if (no_emit)
+	x = rtl_hooks.gen_lowpart_no_emit (mode, x);
+      else
+	x = gen_lowpart (mode, x);
+    }
 
   if (GET_MODE (x) != VOIDmode)
     oldmode = GET_MODE (x);
@@ -773,7 +781,10 @@ convert_modes (enum machine_mode mode, enum machine_mode oldmode, rtx x, int uns
 	  return gen_int_mode (val, mode);
 	}
 
-      return gen_lowpart (mode, x);
+      if (no_emit)
+	return rtl_hooks.gen_lowpart_no_emit (mode, x);
+      else
+	return gen_lowpart (mode, x);
     }
 
   /* Converting from integer constant into mode is always equivalent to an
@@ -784,10 +795,20 @@ convert_modes (enum machine_mode mode, enum machine_mode oldmode, rtx x, int uns
       return simplify_gen_subreg (mode, x, oldmode, 0);
     }
 
+  if (no_emit)
+    return NULL_RTX;
+
   temp = gen_reg_rtx (mode);
   convert_move (temp, x, unsignedp);
   return temp;
 }
+
+rtx
+convert_modes (enum machine_mode mode, enum machine_mode oldmode,
+	       rtx x, int unsignedp)
+{
+  return convert_modes_1 (mode, oldmode, x, unsignedp, false);
+}
 \f
 /* Return the largest alignment we can use for doing a move (or store)
    of MAX_PIECES.  ALIGN is the largest alignment we could use.  */
diff --git a/gcc/expr.h b/gcc/expr.h
index cb4050d..2ac9788 100644
--- a/gcc/expr.h
+++ b/gcc/expr.h
@@ -267,6 +267,8 @@ extern rtx convert_to_mode (enum machine_mode, rtx, int);
 
 /* Convert an rtx to MODE from OLDMODE and return the result.  */
 extern rtx convert_modes (enum machine_mode, enum machine_mode, rtx, int);
+extern rtx convert_modes_1 (enum machine_mode, enum machine_mode, rtx,
+			    int, bool);
 
 /* Emit code to move a block Y to a block X.  */
 
diff --git a/gcc/rtl.h b/gcc/rtl.h
index e3ceecd..1497d50 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -1638,8 +1638,14 @@ extern int byte_lowpart_offset (enum machine_mode, enum machine_mode);
 extern rtx make_safe_from (rtx, rtx);
 extern rtx convert_memory_address_addr_space (enum machine_mode, rtx,
 					      addr_space_t);
+extern rtx convert_memory_address_addr_space_1 (enum machine_mode, rtx,
+						addr_space_t, bool, bool);
 #define convert_memory_address(to_mode,x) \
 	convert_memory_address_addr_space ((to_mode), (x), ADDR_SPACE_GENERIC)
+#define convert_memory_address_1(to_mode,x,no_emit,ignore_address_wrap_around) \
+	convert_memory_address_addr_space_1 ((to_mode), (x), \
+					     ADDR_SPACE_GENERIC, (no_emit), \
+					     (ignore_address_wrap_around))
 extern const char *get_insn_name (int);
 extern rtx get_last_insn_anywhere (void);
 extern rtx get_first_nonnote_insn (void);
diff --git a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c
index 82b818b..c62aab1 100644
--- a/gcc/simplify-rtx.c
+++ b/gcc/simplify-rtx.c
@@ -1150,7 +1150,7 @@ simplify_unary_operation_1 (enum rtx_code code, enum machine_mode mode, rtx op)
 		  && REG_P (SUBREG_REG (op))
 		  && REG_POINTER (SUBREG_REG (op))
 		  && GET_MODE (SUBREG_REG (op)) == Pmode)))
-	return convert_memory_address (Pmode, op);
+	return convert_memory_address_1 (Pmode, op, true, false);
 #endif
       break;
 
@@ -1243,7 +1243,7 @@ simplify_unary_operation_1 (enum rtx_code code, enum machine_mode mode, rtx op)
 		  && REG_P (SUBREG_REG (op))
 		  && REG_POINTER (SUBREG_REG (op))
 		  && GET_MODE (SUBREG_REG (op)) == Pmode)))
-	return convert_memory_address (Pmode, op);
+	return convert_memory_address_1 (Pmode, op, true, false);
 #endif
       break;
 

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: PING: PATCH [4/n]: Prepare x32: Permute the conversion and addition if one operand is a constant
  2011-07-13 18:42                         ` H.J. Lu
@ 2011-07-25 10:34                           ` Paolo Bonzini
  2011-07-27 18:18                             ` H.J. Lu
  0 siblings, 1 reply; 37+ messages in thread
From: Paolo Bonzini @ 2011-07-25 10:34 UTC (permalink / raw)
  To: H.J. Lu; +Cc: GCC Patches

On 07/13/2011 07:48 PM, H.J. Lu wrote:
> Here is the patch.  OK for trunk?

Again, at least you should explain clearly _why_ you need 
ignore_address_wrap_around.  You said elsewhere x32 should be first 
clean, then fast.

>    if (GET_CODE (x) == SUBREG && SUBREG_PROMOTED_VAR_P (x)
>        && GET_MODE_SIZE (GET_MODE (SUBREG_REG (x))) >= GET_MODE_SIZE (mode)
>        && SUBREG_PROMOTED_UNSIGNED_P (x) == unsignedp)
> +    {
> +      if (no_emit)
> +	x = rtl_hooks.gen_lowpart_no_emit (mode, x);
> +      else
> +	x = gen_lowpart (mode, x);
> +    }
> @@ -773,7 +781,10 @@ convert_modes (enum machine_mode mode, enum machine_mode oldmode, rtx x, int uns
>  	  return gen_int_mode (val, mode);
>  	}
>
> -      return gen_lowpart (mode, x);
> +      if (no_emit)
> +	return rtl_hooks.gen_lowpart_no_emit (mode, x);
> +      else
> +	return gen_lowpart (mode, x);
>      }

These should be

   rtx tem = rtl_hooks.gen_lowpart_no_emit (mode, x);
   if (tem)
     x = tem;

   rtx tem = rtl_hooks.gen_lowpart_no_emit (mode, x);
   if (tem)
     return x;

since the "emitting" case can just reuse the code below.  However, see 
the patch I'm sending now.

Paolo

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: PING: PATCH [4/n]: Prepare x32: Permute the conversion and addition if one operand is a constant
  2011-07-25 10:34                           ` Paolo Bonzini
@ 2011-07-27 18:18                             ` H.J. Lu
  2011-07-27 22:41                               ` Paolo Bonzini
  0 siblings, 1 reply; 37+ messages in thread
From: H.J. Lu @ 2011-07-27 18:18 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: GCC Patches

[-- Attachment #1: Type: text/plain, Size: 1921 bytes --]

On Mon, Jul 25, 2011 at 2:58 AM, Paolo Bonzini <bonzini@gnu.org> wrote:
> On 07/13/2011 07:48 PM, H.J. Lu wrote:
>>
>> Here is the patch.  OK for trunk?
>
> Again, at least you should explain clearly _why_ you need
> ignore_address_wrap_around.  You said elsewhere x32 should be first clean,
> then fast.
>
>>   if (GET_CODE (x) == SUBREG && SUBREG_PROMOTED_VAR_P (x)
>>       && GET_MODE_SIZE (GET_MODE (SUBREG_REG (x))) >= GET_MODE_SIZE (mode)
>>       && SUBREG_PROMOTED_UNSIGNED_P (x) == unsignedp)
>> +    {
>> +      if (no_emit)
>> +       x = rtl_hooks.gen_lowpart_no_emit (mode, x);
>> +      else
>> +       x = gen_lowpart (mode, x);
>> +    }
>> @@ -773,7 +781,10 @@ convert_modes (enum machine_mode mode, enum
>> machine_mode oldmode, rtx x, int uns
>>          return gen_int_mode (val, mode);
>>        }
>>
>> -      return gen_lowpart (mode, x);
>> +      if (no_emit)
>> +       return rtl_hooks.gen_lowpart_no_emit (mode, x);
>> +      else
>> +       return gen_lowpart (mode, x);
>>     }
>
> These should be
>
>  rtx tem = rtl_hooks.gen_lowpart_no_emit (mode, x);
>  if (tem)
>    x = tem;
>
>  rtx tem = rtl_hooks.gen_lowpart_no_emit (mode, x);
>  if (tem)
>    return x;
>
> since the "emitting" case can just reuse the code below.  However, see the
> patch I'm sending now.
>
> Paolo
>

Here is the updated patch.  OK for trunk?

Thanks.


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

	PR middle-end/49721
	* explow.c (convert_memory_address_addr_space_1): New.
	(convert_memory_address_addr_space): Use it.

	* expr.c (convert_modes_1): New.
	(convert_modes): Use it.

	* expr.h (convert_modes_1): New.

	* rtl.h (convert_memory_address_addr_space_1): New.
	(convert_memory_address_1): Likewise.

	* simplify-rtx.c (simplify_unary_operation_1): Call
	convert_memory_address_1 instead of convert_memory_address.

[-- Attachment #2: gcc-x32-pr49721-3.patch --]
[-- Type: text/x-diff, Size: 8640 bytes --]

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

	PR middle-end/49721
	* explow.c (convert_memory_address_addr_space_1): New.
	(convert_memory_address_addr_space): Use it.

	* expr.c (convert_modes_1): New.
	(convert_modes): Use it.

	* expr.h (convert_modes_1): New.

	* rtl.h (convert_memory_address_addr_space_1): New.
	(convert_memory_address_1): Likewise.

	* simplify-rtx.c (simplify_unary_operation_1): Call
	convert_memory_address_1 instead of convert_memory_address.

diff --git a/gcc/explow.c b/gcc/explow.c
index 3c692f4..1b2b4e9 100644
--- a/gcc/explow.c
+++ b/gcc/explow.c
@@ -317,11 +317,16 @@ break_out_memory_refs (rtx x)
    an address in the address space's address mode, or vice versa (TO_MODE says
    which way).  We take advantage of the fact that pointers are not allowed to
    overflow by commuting arithmetic operations over conversions so that address
-   arithmetic insns can be used.  */
+   arithmetic insns can be used.  If IRNORE_ADDRESS_WRAP_AROUND is TRUE, we
+   also permute the conversion and addition of a constant.  It is used to
+   optimize cases where overflow of base + constant offset won't happen or
+   its behavior is implementation-defined for a given target.  */
 
 rtx
-convert_memory_address_addr_space (enum machine_mode to_mode ATTRIBUTE_UNUSED,
-				   rtx x, addr_space_t as ATTRIBUTE_UNUSED)
+convert_memory_address_addr_space_1 (enum machine_mode to_mode ATTRIBUTE_UNUSED,
+				     rtx x, addr_space_t as ATTRIBUTE_UNUSED,
+				     bool no_emit ATTRIBUTE_UNUSED,
+				     bool ignore_address_wrap_around ATTRIBUTE_UNUSED)
 {
 #ifndef POINTERS_EXTEND_UNSIGNED
   gcc_assert (GET_MODE (x) == to_mode || GET_MODE (x) == VOIDmode);
@@ -377,28 +382,28 @@ convert_memory_address_addr_space (enum machine_mode to_mode ATTRIBUTE_UNUSED,
       break;
 
     case CONST:
-      return gen_rtx_CONST (to_mode,
-			    convert_memory_address_addr_space
-			      (to_mode, XEXP (x, 0), as));
+      temp = convert_memory_address_addr_space_1 (to_mode, XEXP (x, 0),
+						  as, no_emit,
+						  ignore_address_wrap_around);
+      return temp ? gen_rtx_CONST (to_mode, temp) : temp;
       break;
 
     case PLUS:
     case MULT:
-      /* For addition we can safely permute the conversion and addition
-	 operation if one operand is a constant and converting the constant
-	 does not change it or if one operand is a constant and we are
-	 using a ptr_extend instruction  (POINTERS_EXTEND_UNSIGNED < 0).
-	 We can always safely permute them if we are making the address
-	 narrower.  */
+      /* For addition, we can safely permute the conversion and addition
+	 operation if one operand is a constant and we are using a
+	 ptr_extend instruction (POINTERS_EXTEND_UNSIGNED < 0) or address
+	 wrap-around is ignored.  We can always safely permute them if
+	 we are making the address narrower.  */
       if (GET_MODE_SIZE (to_mode) < GET_MODE_SIZE (from_mode)
 	  || (GET_CODE (x) == PLUS
 	      && CONST_INT_P (XEXP (x, 1))
-	      && (XEXP (x, 1) == convert_memory_address_addr_space
-				   (to_mode, XEXP (x, 1), as)
-                 || POINTERS_EXTEND_UNSIGNED < 0)))
+	      && (POINTERS_EXTEND_UNSIGNED < 0
+		  || ignore_address_wrap_around)))
 	return gen_rtx_fmt_ee (GET_CODE (x), to_mode,
-			       convert_memory_address_addr_space
-				 (to_mode, XEXP (x, 0), as),
+			       convert_memory_address_addr_space_1
+				 (to_mode, XEXP (x, 0), as, no_emit,
+				  ignore_address_wrap_around),
 			       XEXP (x, 1));
       break;
 
@@ -406,10 +411,17 @@ convert_memory_address_addr_space (enum machine_mode to_mode ATTRIBUTE_UNUSED,
       break;
     }
 
-  return convert_modes (to_mode, from_mode,
-			x, POINTERS_EXTEND_UNSIGNED);
+  return convert_modes_1 (to_mode, from_mode, x,
+			  POINTERS_EXTEND_UNSIGNED, no_emit);
 #endif /* defined(POINTERS_EXTEND_UNSIGNED) */
 }
+
+rtx
+convert_memory_address_addr_space (enum machine_mode to_mode,
+				   rtx x, addr_space_t as)
+{
+  return convert_memory_address_addr_space_1 (to_mode, x, as, false, true);
+}
 \f
 /* Return something equivalent to X but valid as a memory address for something
    of mode MODE in the named address space AS.  When X is not itself valid,
diff --git a/gcc/expr.c b/gcc/expr.c
index 27bca17..d57651b 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -693,13 +693,16 @@ convert_to_mode (enum machine_mode mode, rtx x, int unsignedp)
    Both modes may be floating, or both integer.
    UNSIGNEDP is nonzero if X is an unsigned value.
 
+   If NO_EMIT is true, don't emit any instructions.
+
    This can be done by referring to a part of X in place
    or by copying to a new temporary with conversion.
 
    You can give VOIDmode for OLDMODE, if you are sure X has a nonvoid mode.  */
 
 rtx
-convert_modes (enum machine_mode mode, enum machine_mode oldmode, rtx x, int unsignedp)
+convert_modes_1 (enum machine_mode mode, enum machine_mode oldmode,
+		 rtx x, int unsignedp, bool no_emit)
 {
   rtx temp;
 
@@ -709,7 +712,16 @@ convert_modes (enum machine_mode mode, enum machine_mode oldmode, rtx x, int uns
   if (GET_CODE (x) == SUBREG && SUBREG_PROMOTED_VAR_P (x)
       && GET_MODE_SIZE (GET_MODE (SUBREG_REG (x))) >= GET_MODE_SIZE (mode)
       && SUBREG_PROMOTED_UNSIGNED_P (x) == unsignedp)
-    x = gen_lowpart (mode, x);
+    {
+      temp = rtl_hooks.gen_lowpart_no_emit (mode, x);
+      if (temp)
+	x = temp;
+      else
+	{
+	  gcc_assert (!no_emit);
+	  x = gen_lowpart (mode, x);
+	}
+    }
 
   if (GET_MODE (x) != VOIDmode)
     oldmode = GET_MODE (x);
@@ -773,6 +785,10 @@ convert_modes (enum machine_mode mode, enum machine_mode oldmode, rtx x, int uns
 	  return gen_int_mode (val, mode);
 	}
 
+      temp = rtl_hooks.gen_lowpart_no_emit (mode, x);
+      if (temp)
+	return temp;
+      gcc_assert (!no_emit);
       return gen_lowpart (mode, x);
     }
 
@@ -784,10 +800,20 @@ convert_modes (enum machine_mode mode, enum machine_mode oldmode, rtx x, int uns
       return simplify_gen_subreg (mode, x, oldmode, 0);
     }
 
+  if (no_emit)
+    return NULL_RTX;
+
   temp = gen_reg_rtx (mode);
   convert_move (temp, x, unsignedp);
   return temp;
 }
+
+rtx
+convert_modes (enum machine_mode mode, enum machine_mode oldmode,
+	       rtx x, int unsignedp)
+{
+  return convert_modes_1 (mode, oldmode, x, unsignedp, false);
+}
 \f
 /* Return the largest alignment we can use for doing a move (or store)
    of MAX_PIECES.  ALIGN is the largest alignment we could use.  */
diff --git a/gcc/expr.h b/gcc/expr.h
index cb4050d..2ac9788 100644
--- a/gcc/expr.h
+++ b/gcc/expr.h
@@ -267,6 +267,8 @@ extern rtx convert_to_mode (enum machine_mode, rtx, int);
 
 /* Convert an rtx to MODE from OLDMODE and return the result.  */
 extern rtx convert_modes (enum machine_mode, enum machine_mode, rtx, int);
+extern rtx convert_modes_1 (enum machine_mode, enum machine_mode, rtx,
+			    int, bool);
 
 /* Emit code to move a block Y to a block X.  */
 
diff --git a/gcc/rtl.h b/gcc/rtl.h
index 1490bfe..3818db4 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -1658,8 +1658,14 @@ extern int byte_lowpart_offset (enum machine_mode, enum machine_mode);
 extern rtx make_safe_from (rtx, rtx);
 extern rtx convert_memory_address_addr_space (enum machine_mode, rtx,
 					      addr_space_t);
+extern rtx convert_memory_address_addr_space_1 (enum machine_mode, rtx,
+						addr_space_t, bool, bool);
 #define convert_memory_address(to_mode,x) \
 	convert_memory_address_addr_space ((to_mode), (x), ADDR_SPACE_GENERIC)
+#define convert_memory_address_1(to_mode,x,no_emit,ignore_address_wrap_around) \
+	convert_memory_address_addr_space_1 ((to_mode), (x), \
+					     ADDR_SPACE_GENERIC, (no_emit), \
+					     (ignore_address_wrap_around))
 extern const char *get_insn_name (int);
 extern rtx get_last_insn_anywhere (void);
 extern rtx get_first_nonnote_insn (void);
diff --git a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c
index 2d7d8dd..3a93ce0 100644
--- a/gcc/simplify-rtx.c
+++ b/gcc/simplify-rtx.c
@@ -1150,7 +1150,7 @@ simplify_unary_operation_1 (enum rtx_code code, enum machine_mode mode, rtx op)
 		  && REG_P (SUBREG_REG (op))
 		  && REG_POINTER (SUBREG_REG (op))
 		  && GET_MODE (SUBREG_REG (op)) == Pmode)))
-	return convert_memory_address (Pmode, op);
+	return convert_memory_address_1 (Pmode, op, true, false);
 #endif
       break;
 
@@ -1243,7 +1243,7 @@ simplify_unary_operation_1 (enum rtx_code code, enum machine_mode mode, rtx op)
 		  && REG_P (SUBREG_REG (op))
 		  && REG_POINTER (SUBREG_REG (op))
 		  && GET_MODE (SUBREG_REG (op)) == Pmode)))
-	return convert_memory_address (Pmode, op);
+	return convert_memory_address_1 (Pmode, op, true, false);
 #endif
       break;
 

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: PING: PATCH [4/n]: Prepare x32: Permute the conversion and addition if one operand is a constant
  2011-07-27 18:18                             ` H.J. Lu
@ 2011-07-27 22:41                               ` Paolo Bonzini
  2011-07-28  3:11                                 ` H.J. Lu
  0 siblings, 1 reply; 37+ messages in thread
From: Paolo Bonzini @ 2011-07-27 22:41 UTC (permalink / raw)
  To: H.J. Lu; +Cc: GCC Patches

On 07/27/2011 07:29 PM, H.J. Lu wrote:
> If IRNORE_ADDRESS_WRAP_AROUND is TRUE, we
> +   also permute the conversion and addition of a constant.  It is used to
> +   optimize cases where overflow of base + constant offset won't happen or
> +   its behavior is implementation-defined for a given target.  */

Regarding correctness: you're converting a SImode operation to DImode by 
"pushing in" the zero_extend operation.  What makes you think that base 
+ constant offset won't overflow in any case?

And also: what are you gaining by allowing the wrap around?  I don't 
need to know what ignore_address_wrap_around does, I need to know _why_ 
it is necessary.

DO NOT post another patch.  Answer questions in English, here, please.

Paolo

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: PING: PATCH [4/n]: Prepare x32: Permute the conversion and addition if one operand is a constant
  2011-07-27 22:41                               ` Paolo Bonzini
@ 2011-07-28  3:11                                 ` H.J. Lu
  2011-07-28  7:59                                   ` Paolo Bonzini
  0 siblings, 1 reply; 37+ messages in thread
From: H.J. Lu @ 2011-07-28  3:11 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: GCC Patches

On Wed, Jul 27, 2011 at 1:25 PM, Paolo Bonzini <bonzini@gnu.org> wrote:
> On 07/27/2011 07:29 PM, H.J. Lu wrote:
>>
>> If IRNORE_ADDRESS_WRAP_AROUND is TRUE, we
>> +   also permute the conversion and addition of a constant.  It is used to
>> +   optimize cases where overflow of base + constant offset won't happen
>> or
>> +   its behavior is implementation-defined for a given target.  */
>
> Regarding correctness: you're converting a SImode operation to DImode by
> "pushing in" the zero_extend operation.  What makes you think that base +
> constant offset won't overflow in any case?
>
> And also: what are you gaining by allowing the wrap around?  I don't need to
> know what ignore_address_wrap_around does, I need to know _why_ it is
> necessary.
>

We have

(zero_extend:DI (plus:SI (FOO:SI) (const_int Y)))

I want to convert it to

(plus:DI (zero_extend:DI (FOO:SI)) (const_int Y))

There is no zero-extend on (const_int Y).  if FOO == 0xfffffffc and Y = 8,

(zero_extend:DI (plus:SI (FOO:SI) (const_int Y)))

gives 0x4 and

(plus:DI (zero_extend:DI (FOO:SI)) (const_int Y))

gives 0x100000004.   If (plus:SI (FOO:SI) (const_int Y)) won't overflow
or its behavior is implementation-defined, the conversion is safe. If
it isn't the case, we should just drop it.


-- 
H.J.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: PING: PATCH [4/n]: Prepare x32: Permute the conversion and addition if one operand is a constant
  2011-07-28  3:11                                 ` H.J. Lu
@ 2011-07-28  7:59                                   ` Paolo Bonzini
  0 siblings, 0 replies; 37+ messages in thread
From: Paolo Bonzini @ 2011-07-28  7:59 UTC (permalink / raw)
  To: H.J. Lu; +Cc: GCC Patches

On 07/28/2011 12:59 AM, H.J. Lu wrote:
> >  Regarding correctness: you're converting a SImode operation to DImode by
> >  "pushing in" the zero_extend operation.  What makes you think that base +
> >  constant offset won't overflow in any case?

You have not answered this.

> >  And also: what are you gaining by allowing the wrap around?  I don't need to
> >  know what ignore_address_wrap_around does, I need to know _why_ it is
> >  necessary.
>
> We have
>
> (zero_extend:DI (plus:SI (FOO:SI) (const_int Y)))
>
> I want to convert it to
>
> (plus:DI (zero_extend:DI (FOO:SI)) (const_int Y))
>
> There is no zero-extend on (const_int Y).  if FOO == 0xfffffffc and Y = 8,
>
> (zero_extend:DI (plus:SI (FOO:SI) (const_int Y)))
>
> gives 0x4 and
>
> (plus:DI (zero_extend:DI (FOO:SI)) (const_int Y))
>
> gives 0x100000004.

This was already clear upthread.

I'm asking what it buys you in real code.

> If (plus:SI (FOO:SI) (const_int Y)) won't overflow
> or its behavior is implementation-defined,

Behavior of plus:SI is never implementation defined, it is the extension 
that is done with an UNSPEC.  (In fact I'm not even sure the 
optimization is ok when done with POINTER_EXTEND_UNSIGNED < 0, but I'm 
not touching that for now).

Paolo

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: PING: PATCH [4/n]: Prepare x32: Permute the conversion and addition if one operand is a constant
  2011-07-13 16:52                   ` H.J. Lu
  2011-07-13 16:55                     ` Paolo Bonzini
@ 2014-05-29  4:52                     ` Andrew Pinski
  2014-05-29 16:13                       ` H.J. Lu
  1 sibling, 1 reply; 37+ messages in thread
From: Andrew Pinski @ 2014-05-29  4:52 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Paolo Bonzini, GCC Patches

On Wed, Jul 13, 2011 at 9:39 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Wed, Jul 13, 2011 at 9:13 AM, Paolo Bonzini <bonzini@gnu.org> wrote:
>> On 07/11/2011 05:54 PM, H.J. Lu wrote:
>>>
>>> The key is the
>>> >
>>> >     XEXP (x, 1) == convert_memory_address_addr_space
>>> >                    (to_mode, XEXP (x, 1), as)
>>> >
>>> >  test.  It ensures basically that the constant has 31-bit precision,
>>> > because
>>> >  otherwise the constant would change from e.g. (const_int -0x7ffffffc)
>>> > to
>>> >  (const_int 0x80000004) when zero-extending it from SImode to DImode.
>>> >
>>> >  But I'm not sure it's safe.  You have,
>>> >
>>> >    (zero_extend:DI (plus:SI FOO:SI) (const_int Y))
>>> >
>>> >  and you want to convert it to
>>> >
>>> >    (plus:DI FOO:DI (zero_extend:DI (const_int Y)))
>>> >
>>> >  (where the zero_extend is folded).  Ignore that FOO is a SYMBOL_REF
>>> > (this
>>> >  piece of code does not assume anything about its shape); if FOO ==
>>> >  0xfffffffc and Y = 8, the result will be respectively 0x4 (valid) and
>>> >  0x100000004 (invalid).
>>>
>>> This example contradicts what you said above "It ensures basically that
>>> the
>>> constant has 31-bit precision".
>>
>> Why?  Certainly Y = 8 has 31-bit (or less) precision.  So it has the same
>> representation in SImode and DImode, and the test above on XEXP (x, 1)
>> succeeds.
>
> And then we permute conversion and addition, which leads to the issue you
> raised above.  In another word, the current code permutes conversion
> and addition.
> It leads to different values in case of symbol (0xfffffffc) + 8.
> Basically the current
> test for 31-bit (or less) precision is bogus.  The real question is
> for a address
> computation, A + B, if address wrap-around is supported in
> convert_memory_address_addr_space.

Unless the code has already reassociated the additions already.
Like in the AARCH64 ILP32 case:

(plus:SI (plus:SI (mult:SI (reg/v:SI 80 [ b ])
            (const_int -4 [0xfffffffffffffffc]))
        (subreg/s/u:SI (reg/v/f:DI 79 [ a ]) 0))
    (const_int -1073742592 [0xffffffffbffffd00]))

The Tree level is correct in that it did not reassociate the addition
but the RTL level ignores that.

So this patch is invalid and incorrect unless you know the non
constant part of the addition is a pointer (which is not the case
here).

Thanks,
Andrew Pinski



>
>>> >  What happens if you just return NULL instead of the assertion (good
>>> > idea
>>> >  adding it!)?
>>> >
>>> >  Of course then you need to:
>>> >
>>> >  1) check the return values of convert_memory_address_addr_space_1, and
>>> >  propagate NULL up to simplify_unary_operation;
>>> >
>>> >  2) check in simplify-rtx.c whether the return value of
>>> >  convert_memory_address_1 is NULL, and only return if the return value
>>> > is not
>>> >  NULL.  This is not yet necessary (convert_memory_address is the last
>>> >  transformation for both SIGN_EXTEND and ZERO_EXTEND) but it is better
>>> > to
>>> >  keep code clean.
>>>
>>> I will give it a try.
>>
>> Thanks, did you get any result?  There's no "I think" in this code.  So even
>> if I cannot approve it, I'd really like to see a version that I understand
>> and that is clearly conservative, if it works.
>>
>
> I opened a new bug:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49721
>
> My current code looks like:
>
>    case CONST:
>       temp = convert_memory_address_addr_space_1 (to_mode, XEXP (x, 0),
>                                                   as, no_emit,
>                                                   ignore_address_wrap_around);
>       return temp ? gen_rtx_CONST (to_mode, temp) : temp;
>       break;
>
>     case PLUS:
>     case MULT:
>       /* For addition we can safely permute the conversion and addition
>          operation if one operand is a constant, address wrap-around
>          is ignored and we are using a ptr_extend instruction or
>          zero-extending (POINTERS_EXTEND_UNSIGNED != 0).  We can always
>          safely permute them if we are making the address narrower.  */
>       if (GET_MODE_SIZE (to_mode) < GET_MODE_SIZE (from_mode)
>           || (GET_CODE (x) == PLUS
>               && CONST_INT_P (XEXP (x, 1))
>               && (POINTERS_EXTEND_UNSIGNED != 0
>                   && ignore_address_wrap_around)))
>         return gen_rtx_fmt_ee (GET_CODE (x), to_mode,
>                                convert_memory_address_addr_space_1
>                                  (to_mode, XEXP (x, 0), as, no_emit,
>                                   ignore_address_wrap_around),
>                                XEXP (x, 1));
>       break;
>
> --
> H.J.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: PING: PATCH [4/n]: Prepare x32: Permute the conversion and addition if one operand is a constant
  2014-05-29  4:52                     ` Andrew Pinski
@ 2014-05-29 16:13                       ` H.J. Lu
  2014-05-29 16:23                         ` pinskia
  0 siblings, 1 reply; 37+ messages in thread
From: H.J. Lu @ 2014-05-29 16:13 UTC (permalink / raw)
  To: Andrew Pinski; +Cc: Paolo Bonzini, GCC Patches

On Wed, May 28, 2014 at 9:52 PM, Andrew Pinski <pinskia@gmail.com> wrote:
> On Wed, Jul 13, 2011 at 9:39 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Wed, Jul 13, 2011 at 9:13 AM, Paolo Bonzini <bonzini@gnu.org> wrote:
>>> On 07/11/2011 05:54 PM, H.J. Lu wrote:
>>>>
>>>> The key is the
>>>> >
>>>> >     XEXP (x, 1) == convert_memory_address_addr_space
>>>> >                    (to_mode, XEXP (x, 1), as)
>>>> >
>>>> >  test.  It ensures basically that the constant has 31-bit precision,
>>>> > because
>>>> >  otherwise the constant would change from e.g. (const_int -0x7ffffffc)
>>>> > to
>>>> >  (const_int 0x80000004) when zero-extending it from SImode to DImode.
>>>> >
>>>> >  But I'm not sure it's safe.  You have,
>>>> >
>>>> >    (zero_extend:DI (plus:SI FOO:SI) (const_int Y))
>>>> >
>>>> >  and you want to convert it to
>>>> >
>>>> >    (plus:DI FOO:DI (zero_extend:DI (const_int Y)))
>>>> >
>>>> >  (where the zero_extend is folded).  Ignore that FOO is a SYMBOL_REF
>>>> > (this
>>>> >  piece of code does not assume anything about its shape); if FOO ==
>>>> >  0xfffffffc and Y = 8, the result will be respectively 0x4 (valid) and
>>>> >  0x100000004 (invalid).
>>>>
>>>> This example contradicts what you said above "It ensures basically that
>>>> the
>>>> constant has 31-bit precision".
>>>
>>> Why?  Certainly Y = 8 has 31-bit (or less) precision.  So it has the same
>>> representation in SImode and DImode, and the test above on XEXP (x, 1)
>>> succeeds.
>>
>> And then we permute conversion and addition, which leads to the issue you
>> raised above.  In another word, the current code permutes conversion
>> and addition.
>> It leads to different values in case of symbol (0xfffffffc) + 8.
>> Basically the current
>> test for 31-bit (or less) precision is bogus.  The real question is
>> for a address
>> computation, A + B, if address wrap-around is supported in
>> convert_memory_address_addr_space.
>
> Unless the code has already reassociated the additions already.
> Like in the AARCH64 ILP32 case:
>
> (plus:SI (plus:SI (mult:SI (reg/v:SI 80 [ b ])
>             (const_int -4 [0xfffffffffffffffc]))
>         (subreg/s/u:SI (reg/v/f:DI 79 [ a ]) 0))
>     (const_int -1073742592 [0xffffffffbffffd00]))
>
> The Tree level is correct in that it did not reassociate the addition
> but the RTL level ignores that.
>
> So this patch is invalid and incorrect unless you know the non
> constant part of the addition is a pointer (which is not the case
> here).
>

There is an address overflow.  Is the address overflow behavior
defined here?


-- 
H.J.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: PING: PATCH [4/n]: Prepare x32: Permute the conversion and addition if one operand is a constant
  2014-05-29 16:13                       ` H.J. Lu
@ 2014-05-29 16:23                         ` pinskia
  2014-05-29 17:09                           ` H.J. Lu
  0 siblings, 1 reply; 37+ messages in thread
From: pinskia @ 2014-05-29 16:23 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Paolo Bonzini, GCC Patches



> On May 29, 2014, at 9:13 AM, "H.J. Lu" <hjl.tools@gmail.com> wrote:
> 
>> On Wed, May 28, 2014 at 9:52 PM, Andrew Pinski <pinskia@gmail.com> wrote:
>>> On Wed, Jul 13, 2011 at 9:39 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>> On Wed, Jul 13, 2011 at 9:13 AM, Paolo Bonzini <bonzini@gnu.org> wrote:
>>>>> On 07/11/2011 05:54 PM, H.J. Lu wrote:
>>>>> 
>>>>> The key is the
>>>>>> 
>>>>>>    XEXP (x, 1) == convert_memory_address_addr_space
>>>>>>                   (to_mode, XEXP (x, 1), as)
>>>>>> 
>>>>>> test.  It ensures basically that the constant has 31-bit precision,
>>>>>> because
>>>>>> otherwise the constant would change from e.g. (const_int -0x7ffffffc)
>>>>>> to
>>>>>> (const_int 0x80000004) when zero-extending it from SImode to DImode.
>>>>>> 
>>>>>> But I'm not sure it's safe.  You have,
>>>>>> 
>>>>>>   (zero_extend:DI (plus:SI FOO:SI) (const_int Y))
>>>>>> 
>>>>>> and you want to convert it to
>>>>>> 
>>>>>>   (plus:DI FOO:DI (zero_extend:DI (const_int Y)))
>>>>>> 
>>>>>> (where the zero_extend is folded).  Ignore that FOO is a SYMBOL_REF
>>>>>> (this
>>>>>> piece of code does not assume anything about its shape); if FOO ==
>>>>>> 0xfffffffc and Y = 8, the result will be respectively 0x4 (valid) and
>>>>>> 0x100000004 (invalid).
>>>>> 
>>>>> This example contradicts what you said above "It ensures basically that
>>>>> the
>>>>> constant has 31-bit precision".
>>>> 
>>>> Why?  Certainly Y = 8 has 31-bit (or less) precision.  So it has the same
>>>> representation in SImode and DImode, and the test above on XEXP (x, 1)
>>>> succeeds.
>>> 
>>> And then we permute conversion and addition, which leads to the issue you
>>> raised above.  In another word, the current code permutes conversion
>>> and addition.
>>> It leads to different values in case of symbol (0xfffffffc) + 8.
>>> Basically the current
>>> test for 31-bit (or less) precision is bogus.  The real question is
>>> for a address
>>> computation, A + B, if address wrap-around is supported in
>>> convert_memory_address_addr_space.
>> 
>> Unless the code has already reassociated the additions already.
>> Like in the AARCH64 ILP32 case:
>> 
>> (plus:SI (plus:SI (mult:SI (reg/v:SI 80 [ b ])
>>            (const_int -4 [0xfffffffffffffffc]))
>>        (subreg/s/u:SI (reg/v/f:DI 79 [ a ]) 0))
>>    (const_int -1073742592 [0xffffffffbffffd00]))
>> 
>> The Tree level is correct in that it did not reassociate the addition
>> but the RTL level ignores that.
>> 
>> So this patch is invalid and incorrect unless you know the non
>> constant part of the addition is a pointer (which is not the case
>> here).
> 
> There is an address overflow.  Is the address overflow behavior
> defined here?

There was no address overflow in the original code and there was no address overflow in the tree level. The rtl level does introduce an address overflow but the semantics of plus is defined to be wrapping so there is no overflow.   This is blocking me from testing ilp32 under gnu/Linux as ld.so gets miscompiled and stack addresses have the "sign" bit set. 

Thanks,
Andrew


> 
> 
> -- 
> H.J.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: PING: PATCH [4/n]: Prepare x32: Permute the conversion and addition if one operand is a constant
  2014-05-29 16:23                         ` pinskia
@ 2014-05-29 17:09                           ` H.J. Lu
  2014-05-29 17:20                             ` pinskia
  0 siblings, 1 reply; 37+ messages in thread
From: H.J. Lu @ 2014-05-29 17:09 UTC (permalink / raw)
  To: Andrew Pinski; +Cc: Paolo Bonzini, GCC Patches

On Thu, May 29, 2014 at 9:23 AM,  <pinskia@gmail.com> wrote:
>
>
>> On May 29, 2014, at 9:13 AM, "H.J. Lu" <hjl.tools@gmail.com> wrote:
>>
>>> On Wed, May 28, 2014 at 9:52 PM, Andrew Pinski <pinskia@gmail.com> wrote:
>>>> On Wed, Jul 13, 2011 at 9:39 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>> On Wed, Jul 13, 2011 at 9:13 AM, Paolo Bonzini <bonzini@gnu.org> wrote:
>>>>>> On 07/11/2011 05:54 PM, H.J. Lu wrote:
>>>>>>
>>>>>> The key is the
>>>>>>>
>>>>>>>    XEXP (x, 1) == convert_memory_address_addr_space
>>>>>>>                   (to_mode, XEXP (x, 1), as)
>>>>>>>
>>>>>>> test.  It ensures basically that the constant has 31-bit precision,
>>>>>>> because
>>>>>>> otherwise the constant would change from e.g. (const_int -0x7ffffffc)
>>>>>>> to
>>>>>>> (const_int 0x80000004) when zero-extending it from SImode to DImode.
>>>>>>>
>>>>>>> But I'm not sure it's safe.  You have,
>>>>>>>
>>>>>>>   (zero_extend:DI (plus:SI FOO:SI) (const_int Y))
>>>>>>>
>>>>>>> and you want to convert it to
>>>>>>>
>>>>>>>   (plus:DI FOO:DI (zero_extend:DI (const_int Y)))
>>>>>>>
>>>>>>> (where the zero_extend is folded).  Ignore that FOO is a SYMBOL_REF
>>>>>>> (this
>>>>>>> piece of code does not assume anything about its shape); if FOO ==
>>>>>>> 0xfffffffc and Y = 8, the result will be respectively 0x4 (valid) and
>>>>>>> 0x100000004 (invalid).
>>>>>>
>>>>>> This example contradicts what you said above "It ensures basically that
>>>>>> the
>>>>>> constant has 31-bit precision".
>>>>>
>>>>> Why?  Certainly Y = 8 has 31-bit (or less) precision.  So it has the same
>>>>> representation in SImode and DImode, and the test above on XEXP (x, 1)
>>>>> succeeds.
>>>>
>>>> And then we permute conversion and addition, which leads to the issue you
>>>> raised above.  In another word, the current code permutes conversion
>>>> and addition.
>>>> It leads to different values in case of symbol (0xfffffffc) + 8.
>>>> Basically the current
>>>> test for 31-bit (or less) precision is bogus.  The real question is
>>>> for a address
>>>> computation, A + B, if address wrap-around is supported in
>>>> convert_memory_address_addr_space.
>>>
>>> Unless the code has already reassociated the additions already.
>>> Like in the AARCH64 ILP32 case:
>>>
>>> (plus:SI (plus:SI (mult:SI (reg/v:SI 80 [ b ])
>>>            (const_int -4 [0xfffffffffffffffc]))
>>>        (subreg/s/u:SI (reg/v/f:DI 79 [ a ]) 0))
>>>    (const_int -1073742592 [0xffffffffbffffd00]))
>>>
>>> The Tree level is correct in that it did not reassociate the addition
>>> but the RTL level ignores that.
>>>
>>> So this patch is invalid and incorrect unless you know the non
>>> constant part of the addition is a pointer (which is not the case
>>> here).
>>
>> There is an address overflow.  Is the address overflow behavior
>> defined here?
>
> There was no address overflow in the original code and there was no address overflow in the tree level. The rtl level does introduce an address overflow but the semantics of plus is defined to be wrapping so there is no overflow.   This is blocking me from testing ilp32 under gnu/Linux as ld.so gets miscompiled and stack addresses have the "sign" bit set.
>

What is your Pmode?


-- 
H.J.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: PING: PATCH [4/n]: Prepare x32: Permute the conversion and addition if one operand is a constant
  2014-05-29 17:09                           ` H.J. Lu
@ 2014-05-29 17:20                             ` pinskia
  2014-05-30  7:18                               ` Paolo Bonzini
  0 siblings, 1 reply; 37+ messages in thread
From: pinskia @ 2014-05-29 17:20 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Paolo Bonzini, GCC Patches



> On May 29, 2014, at 10:09 AM, "H.J. Lu" <hjl.tools@gmail.com> wrote:
> 
>> On Thu, May 29, 2014 at 9:23 AM,  <pinskia@gmail.com> wrote:
>> 
>> 
>>>> On May 29, 2014, at 9:13 AM, "H.J. Lu" <hjl.tools@gmail.com> wrote:
>>>> 
>>>>> On Wed, May 28, 2014 at 9:52 PM, Andrew Pinski <pinskia@gmail.com> wrote:
>>>>>> On Wed, Jul 13, 2011 at 9:39 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>>>> On Wed, Jul 13, 2011 at 9:13 AM, Paolo Bonzini <bonzini@gnu.org> wrote:
>>>>>>> On 07/11/2011 05:54 PM, H.J. Lu wrote:
>>>>>>> 
>>>>>>> The key is the
>>>>>>>> 
>>>>>>>>   XEXP (x, 1) == convert_memory_address_addr_space
>>>>>>>>                  (to_mode, XEXP (x, 1), as)
>>>>>>>> 
>>>>>>>> test.  It ensures basically that the constant has 31-bit precision,
>>>>>>>> because
>>>>>>>> otherwise the constant would change from e.g. (const_int -0x7ffffffc)
>>>>>>>> to
>>>>>>>> (const_int 0x80000004) when zero-extending it from SImode to DImode.
>>>>>>>> 
>>>>>>>> But I'm not sure it's safe.  You have,
>>>>>>>> 
>>>>>>>>  (zero_extend:DI (plus:SI FOO:SI) (const_int Y))
>>>>>>>> 
>>>>>>>> and you want to convert it to
>>>>>>>> 
>>>>>>>>  (plus:DI FOO:DI (zero_extend:DI (const_int Y)))
>>>>>>>> 
>>>>>>>> (where the zero_extend is folded).  Ignore that FOO is a SYMBOL_REF
>>>>>>>> (this
>>>>>>>> piece of code does not assume anything about its shape); if FOO ==
>>>>>>>> 0xfffffffc and Y = 8, the result will be respectively 0x4 (valid) and
>>>>>>>> 0x100000004 (invalid).
>>>>>>> 
>>>>>>> This example contradicts what you said above "It ensures basically that
>>>>>>> the
>>>>>>> constant has 31-bit precision".
>>>>>> 
>>>>>> Why?  Certainly Y = 8 has 31-bit (or less) precision.  So it has the same
>>>>>> representation in SImode and DImode, and the test above on XEXP (x, 1)
>>>>>> succeeds.
>>>>> 
>>>>> And then we permute conversion and addition, which leads to the issue you
>>>>> raised above.  In another word, the current code permutes conversion
>>>>> and addition.
>>>>> It leads to different values in case of symbol (0xfffffffc) + 8.
>>>>> Basically the current
>>>>> test for 31-bit (or less) precision is bogus.  The real question is
>>>>> for a address
>>>>> computation, A + B, if address wrap-around is supported in
>>>>> convert_memory_address_addr_space.
>>>> 
>>>> Unless the code has already reassociated the additions already.
>>>> Like in the AARCH64 ILP32 case:
>>>> 
>>>> (plus:SI (plus:SI (mult:SI (reg/v:SI 80 [ b ])
>>>>           (const_int -4 [0xfffffffffffffffc]))
>>>>       (subreg/s/u:SI (reg/v/f:DI 79 [ a ]) 0))
>>>>   (const_int -1073742592 [0xffffffffbffffd00]))
>>>> 
>>>> The Tree level is correct in that it did not reassociate the addition
>>>> but the RTL level ignores that.
>>>> 
>>>> So this patch is invalid and incorrect unless you know the non
>>>> constant part of the addition is a pointer (which is not the case
>>>> here).
>>> 
>>> There is an address overflow.  Is the address overflow behavior
>>> defined here?
>> 
>> There was no address overflow in the original code and there was no address overflow in the tree level. The rtl level does introduce an address overflow but the semantics of plus is defined to be wrapping so there is no overflow.   This is blocking me from testing ilp32 under gnu/Linux as ld.so gets miscompiled and stack addresses have the "sign" bit set.
>> 
> 
> What is your Pmode?

Pmode is dimode while ptr_mode is simode.  Pointers are zero extended when converting between si and di modes. 

Thanks,
Andrew


> 
> 
> -- 
> H.J.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: PING: PATCH [4/n]: Prepare x32: Permute the conversion and addition if one operand is a constant
  2014-05-29 17:20                             ` pinskia
@ 2014-05-30  7:18                               ` Paolo Bonzini
  0 siblings, 0 replies; 37+ messages in thread
From: Paolo Bonzini @ 2014-05-30  7:18 UTC (permalink / raw)
  To: pinskia, H.J. Lu; +Cc: GCC Patches

Il 29/05/2014 19:20, pinskia@gmail.com ha scritto:
>> What is your Pmode?
>
> Pmode is dimode while ptr_mode is simode.  Pointers are zero extended when converting between si and di modes.

As you noted, the fundamental difference between x32 and aarch64 is that 
aarch64 will still use 64-bit accesses instead of 32-bit.  Did you 
define the VALID_POINTER_MODE hook to rule out Pmode as a valid pointer 
mode?  Perhaps you can use it to make this transformation conditional on 
VALID_POINTER_MODE(Pmode).

Paolo

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: PING: PATCH [4/n]: Prepare x32: Permute the conversion and addition if one operand is a constant
  2011-07-30  0:47       ` H.J. Lu
@ 2011-07-30 16:35         ` Paolo Bonzini
  0 siblings, 0 replies; 37+ messages in thread
From: Paolo Bonzini @ 2011-07-30 16:35 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Uros Bizjak, gcc-patches

On Sat, Jul 30, 2011 at 00:32, H.J. Lu <hjl.tools@gmail.com> wrote:
> The whole approach doesn't work. The testcase at
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49721#c1
>
> shows GCC depends on transforming:
>
> (zero_extend:DI (plus:SI (FOO:SI) (const_int Y)))
>
> to
>
> (plus:DI (zero_extend:DI (FOO:SI)) (const_int Y))
>
> Otherwise we either get compiler crash or wrong codes.

Please explain how/why here or in the BZ.  Compiler crashes can be
fixed, wrong code is often a symptom of latent bugs.

Paolo

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: PING: PATCH [4/n]: Prepare x32: Permute the conversion and addition if one operand is a constant
  2011-07-29  9:32     ` Paolo Bonzini
  2011-07-29  9:37       ` Paolo Bonzini
@ 2011-07-30  0:47       ` H.J. Lu
  2011-07-30 16:35         ` Paolo Bonzini
  1 sibling, 1 reply; 37+ messages in thread
From: H.J. Lu @ 2011-07-30  0:47 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Uros Bizjak, gcc-patches

On Thu, Jul 28, 2011 at 11:34 PM, Paolo Bonzini <bonzini@gnu.org> wrote:
> Ok, you removed ignore_address_wrap_around, so we're almost there.
>
> On 07/28/2011 07:59 PM, H.J. Lu wrote:
>>
>> @@ -712,7 +715,16 @@ convert_modes (enum machine_mode mode, enum
>> machine_mode oldmode, rtx x, int uns
>>    if (GET_CODE (x) == SUBREG&&  SUBREG_PROMOTED_VAR_P (x)
>>        &&  GET_MODE_SIZE (GET_MODE (SUBREG_REG (x)))>= GET_MODE_SIZE
>> (mode)
>>        &&  SUBREG_PROMOTED_UNSIGNED_P (x) == unsignedp)
>> -    x = gen_lowpart (mode, x);
>> +    {
>> +      temp = rtl_hooks.gen_lowpart_no_emit (mode, x);
>> +      if (temp)
>> +       x = temp;
>> +      else
>> +       {
>> +         gcc_assert (!no_emit);
>> +         x = gen_lowpart (mode, x);
>> +       }
>> +    }
>
> +    {
> +       /* gen_lowpart_no_emit should always succeed here.  */
> +       x = rtl_hooks.gen_lowpart_no_emit (mode, x);
> +    }
>
>>
>>    if (GET_MODE (x) != VOIDmode)
>>      oldmode = GET_MODE (x);
>> @@ -776,6 +788,10 @@ convert_modes (enum machine_mode mode, enum
>> machine_mode oldmode, rtx x, int uns
>>          return gen_int_mode (val, mode);
>>        }
>>
>> +      temp = rtl_hooks.gen_lowpart_no_emit (mode, x);
>> +      if (temp)
>> +       return temp;
>> +      gcc_assert (!no_emit);
>>        return gen_lowpart (mode, x);
>
> Right now, gen_lowpart_no_emit will never return NULL, so these tests in
> convert_modes are dead.  Instead, please include in your patch mine at
> http://permalink.gmane.org/gmane.comp.gcc.patches/242085 and adjust as
> follows.
>
> +      temp = rtl_hooks.gen_lowpart_no_emit (mode, x);
> +      if (no_emit)
> +        return rtl_hooks.gen_lowpart_no_emit (mode, x);
> +      else
> +        return gen_lowpart (mode, x);
>
>>      }
>
> If it does not work, PLEASE say why instead of posting another "updated
> patch".

The whole approach doesn't work. The testcase at

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49721#c1

shows GCC depends on transforming:

(zero_extend:DI (plus:SI (FOO:SI) (const_int Y)))

to

(plus:DI (zero_extend:DI (FOO:SI)) (const_int Y))

Otherwise we either get compiler crash or wrong codes.


-- 
H.J.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: PING: PATCH [4/n]: Prepare x32: Permute the conversion and addition if one operand is a constant
  2011-07-28 10:27 ` Paolo Bonzini
@ 2011-07-29 13:29   ` H.J. Lu
  2011-07-28 18:23     ` Uros Bizjak
  2011-07-29  9:32     ` Paolo Bonzini
  0 siblings, 2 replies; 37+ messages in thread
From: H.J. Lu @ 2011-07-29 13:29 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Uros Bizjak, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1358 bytes --]

On Thu, Jul 28, 2011 at 2:31 AM, Paolo Bonzini <bonzini@gnu.org> wrote:
> On 07/28/2011 11:30 AM, Uros Bizjak wrote:
>>
>> >  convert_memory_address_addr_space has a special PLUS/MULT case for
>> >  POINTERS_EXTEND_UNSIGNED<  0. ?It turns out that it is also needed
>> >  for all Pmode != ptr_mode cases. ?OK for trunk?
>> >  2011-06-11 ?H.J. Lu ?<hongjiu.lu@intel.com>
>> >
>> >  ? ? ? ?PR middle-end/47727
>> >  ? ? ? ?* explow.c (convert_memory_address_addr_space): Permute the
>> >  ? ? ? ?conversion and addition if one operand is a constant.
>>
>> Do we still need this patch? With recent target changes the testcase
>> from PR can be compiled without problems with a gcc from an unpatched
>> trunk.
>
> Given the communication difficulties, I hope not...
>
> Paolo
>

Here is the updated patch.  OK for trunk?

Thanks.

-- 
H.J.
---

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

	PR middle-end/49721
	* explow.c (convert_memory_address_addr_space_1): New.
	(convert_memory_address_addr_space): Use it.

	* expr.c (convert_modes_1): New.
	(convert_modes): Use it.

	* expr.h (convert_modes_1): New.

	* rtl.h (convert_memory_address_addr_space_1): New.
	(convert_memory_address_1): Likewise.

	* simplify-rtx.c (simplify_unary_operation_1): Call
	convert_memory_address_1 instead of convert_memory_address.

[-- Attachment #2: gcc-x32-pr49721-4.patch --]
[-- Type: text/x-diff, Size: 7794 bytes --]

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

	PR middle-end/49721
	* explow.c (convert_memory_address_addr_space_1): New.
	(convert_memory_address_addr_space): Use it.

	* expr.c (convert_modes_1): New.
	(convert_modes): Use it.

	* expr.h (convert_modes_1): New.

	* rtl.h (convert_memory_address_addr_space_1): New.
	(convert_memory_address_1): Likewise.

	* simplify-rtx.c (simplify_unary_operation_1): Call
	convert_memory_address_1 instead of convert_memory_address.

diff --git a/gcc/explow.c b/gcc/explow.c
index 3c692f4..069a68a 100644
--- a/gcc/explow.c
+++ b/gcc/explow.c
@@ -320,8 +320,9 @@ break_out_memory_refs (rtx x)
    arithmetic insns can be used.  */
 
 rtx
-convert_memory_address_addr_space (enum machine_mode to_mode ATTRIBUTE_UNUSED,
-				   rtx x, addr_space_t as ATTRIBUTE_UNUSED)
+convert_memory_address_addr_space_1 (enum machine_mode to_mode ATTRIBUTE_UNUSED,
+				     rtx x, addr_space_t as ATTRIBUTE_UNUSED,
+				     bool no_emit ATTRIBUTE_UNUSED)
 {
 #ifndef POINTERS_EXTEND_UNSIGNED
   gcc_assert (GET_MODE (x) == to_mode || GET_MODE (x) == VOIDmode);
@@ -377,28 +378,27 @@ convert_memory_address_addr_space (enum machine_mode to_mode ATTRIBUTE_UNUSED,
       break;
 
     case CONST:
-      return gen_rtx_CONST (to_mode,
-			    convert_memory_address_addr_space
-			      (to_mode, XEXP (x, 0), as));
+      temp = convert_memory_address_addr_space_1 (to_mode, XEXP (x, 0),
+						  as, no_emit);
+      return temp ? gen_rtx_CONST (to_mode, temp) : temp;
       break;
 
     case PLUS:
     case MULT:
-      /* For addition we can safely permute the conversion and addition
-	 operation if one operand is a constant and converting the constant
-	 does not change it or if one operand is a constant and we are
-	 using a ptr_extend instruction  (POINTERS_EXTEND_UNSIGNED < 0).
+      /* FIXME: Is this really safe for POINTERS_EXTEND_UNSIGNED < 0?
+         For addition, we can safely permute the conversion and addition
+	 operation if one operand is a constant and we are using a
+	 ptr_extend instruction (POINTERS_EXTEND_UNSIGNED < 0).
+	 
 	 We can always safely permute them if we are making the address
 	 narrower.  */
       if (GET_MODE_SIZE (to_mode) < GET_MODE_SIZE (from_mode)
 	  || (GET_CODE (x) == PLUS
 	      && CONST_INT_P (XEXP (x, 1))
-	      && (XEXP (x, 1) == convert_memory_address_addr_space
-				   (to_mode, XEXP (x, 1), as)
-                 || POINTERS_EXTEND_UNSIGNED < 0)))
+	      && POINTERS_EXTEND_UNSIGNED < 0))
 	return gen_rtx_fmt_ee (GET_CODE (x), to_mode,
-			       convert_memory_address_addr_space
-				 (to_mode, XEXP (x, 0), as),
+			       convert_memory_address_addr_space_1
+				 (to_mode, XEXP (x, 0), as, no_emit),
 			       XEXP (x, 1));
       break;
 
@@ -406,10 +406,17 @@ convert_memory_address_addr_space (enum machine_mode to_mode ATTRIBUTE_UNUSED,
       break;
     }
 
-  return convert_modes (to_mode, from_mode,
-			x, POINTERS_EXTEND_UNSIGNED);
+  return convert_modes_1 (to_mode, from_mode, x,
+			  POINTERS_EXTEND_UNSIGNED, no_emit);
 #endif /* defined(POINTERS_EXTEND_UNSIGNED) */
 }
+
+rtx
+convert_memory_address_addr_space (enum machine_mode to_mode,
+				   rtx x, addr_space_t as)
+{
+  return convert_memory_address_addr_space_1 (to_mode, x, as, false);
+}
 \f
 /* Return something equivalent to X but valid as a memory address for something
    of mode MODE in the named address space AS.  When X is not itself valid,
diff --git a/gcc/expr.c b/gcc/expr.c
index 0988c51..8aec0a5 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -696,13 +696,16 @@ convert_to_mode (enum machine_mode mode, rtx x, int unsignedp)
    Both modes may be floating, or both integer.
    UNSIGNEDP is nonzero if X is an unsigned value.
 
+   If NO_EMIT is true, don't emit any instructions.
+
    This can be done by referring to a part of X in place
    or by copying to a new temporary with conversion.
 
    You can give VOIDmode for OLDMODE, if you are sure X has a nonvoid mode.  */
 
 rtx
-convert_modes (enum machine_mode mode, enum machine_mode oldmode, rtx x, int unsignedp)
+convert_modes_1 (enum machine_mode mode, enum machine_mode oldmode,
+		 rtx x, int unsignedp, bool no_emit)
 {
   rtx temp;
 
@@ -712,7 +715,16 @@ convert_modes (enum machine_mode mode, enum machine_mode oldmode, rtx x, int uns
   if (GET_CODE (x) == SUBREG && SUBREG_PROMOTED_VAR_P (x)
       && GET_MODE_SIZE (GET_MODE (SUBREG_REG (x))) >= GET_MODE_SIZE (mode)
       && SUBREG_PROMOTED_UNSIGNED_P (x) == unsignedp)
-    x = gen_lowpart (mode, x);
+    {
+      temp = rtl_hooks.gen_lowpart_no_emit (mode, x);
+      if (temp)
+	x = temp;
+      else
+	{
+	  gcc_assert (!no_emit);
+	  x = gen_lowpart (mode, x);
+	}
+    }
 
   if (GET_MODE (x) != VOIDmode)
     oldmode = GET_MODE (x);
@@ -776,6 +788,10 @@ convert_modes (enum machine_mode mode, enum machine_mode oldmode, rtx x, int uns
 	  return gen_int_mode (val, mode);
 	}
 
+      temp = rtl_hooks.gen_lowpart_no_emit (mode, x);
+      if (temp)
+	return temp;
+      gcc_assert (!no_emit);
       return gen_lowpart (mode, x);
     }
 
@@ -787,10 +803,20 @@ convert_modes (enum machine_mode mode, enum machine_mode oldmode, rtx x, int uns
       return simplify_gen_subreg (mode, x, oldmode, 0);
     }
 
+  if (no_emit)
+    return NULL_RTX;
+
   temp = gen_reg_rtx (mode);
   convert_move (temp, x, unsignedp);
   return temp;
 }
+
+rtx
+convert_modes (enum machine_mode mode, enum machine_mode oldmode,
+	       rtx x, int unsignedp)
+{
+  return convert_modes_1 (mode, oldmode, x, unsignedp, false);
+}
 \f
 /* Return the largest alignment we can use for doing a move (or store)
    of MAX_PIECES.  ALIGN is the largest alignment we could use.  */
diff --git a/gcc/expr.h b/gcc/expr.h
index 74c608a..dc40196 100644
--- a/gcc/expr.h
+++ b/gcc/expr.h
@@ -267,6 +267,8 @@ extern rtx convert_to_mode (enum machine_mode, rtx, int);
 
 /* Convert an rtx to MODE from OLDMODE and return the result.  */
 extern rtx convert_modes (enum machine_mode, enum machine_mode, rtx, int);
+extern rtx convert_modes_1 (enum machine_mode, enum machine_mode, rtx,
+			    int, bool);
 
 /* Emit code to move a block Y to a block X.  */
 
diff --git a/gcc/rtl.h b/gcc/rtl.h
index 1490bfe..d471efa 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -1658,8 +1658,13 @@ extern int byte_lowpart_offset (enum machine_mode, enum machine_mode);
 extern rtx make_safe_from (rtx, rtx);
 extern rtx convert_memory_address_addr_space (enum machine_mode, rtx,
 					      addr_space_t);
+extern rtx convert_memory_address_addr_space_1 (enum machine_mode, rtx,
+						addr_space_t, bool);
 #define convert_memory_address(to_mode,x) \
 	convert_memory_address_addr_space ((to_mode), (x), ADDR_SPACE_GENERIC)
+#define convert_memory_address_1(to_mode,x,no_emit) \
+	convert_memory_address_addr_space_1 ((to_mode), (x), \
+					     ADDR_SPACE_GENERIC, (no_emit))
 extern const char *get_insn_name (int);
 extern rtx get_last_insn_anywhere (void);
 extern rtx get_first_nonnote_insn (void);
diff --git a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c
index 2d7d8dd..6090268 100644
--- a/gcc/simplify-rtx.c
+++ b/gcc/simplify-rtx.c
@@ -1150,7 +1150,7 @@ simplify_unary_operation_1 (enum rtx_code code, enum machine_mode mode, rtx op)
 		  && REG_P (SUBREG_REG (op))
 		  && REG_POINTER (SUBREG_REG (op))
 		  && GET_MODE (SUBREG_REG (op)) == Pmode)))
-	return convert_memory_address (Pmode, op);
+	return convert_memory_address_1 (Pmode, op, true);
 #endif
       break;
 
@@ -1243,7 +1243,7 @@ simplify_unary_operation_1 (enum rtx_code code, enum machine_mode mode, rtx op)
 		  && REG_P (SUBREG_REG (op))
 		  && REG_POINTER (SUBREG_REG (op))
 		  && GET_MODE (SUBREG_REG (op)) == Pmode)))
-	return convert_memory_address (Pmode, op);
+	return convert_memory_address_1 (Pmode, op, true);
 #endif
       break;
 

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: PING: PATCH [4/n]: Prepare x32: Permute the conversion and addition if one operand is a constant
  2011-07-29  9:32     ` Paolo Bonzini
@ 2011-07-29  9:37       ` Paolo Bonzini
  2011-07-30  0:47       ` H.J. Lu
  1 sibling, 0 replies; 37+ messages in thread
From: Paolo Bonzini @ 2011-07-29  9:37 UTC (permalink / raw)
  Cc: H.J. Lu, Uros Bizjak, gcc-patches

On 07/29/2011 08:34 AM, Paolo Bonzini wrote:
> +      temp = rtl_hooks.gen_lowpart_no_emit (mode, x);

This line is obviously spurious, sorry.

> +      if (no_emit)
> +     return rtl_hooks.gen_lowpart_no_emit (mode, x);
> +      else
> +        return gen_lowpart (mode, x);

Paolo

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: PING: PATCH [4/n]: Prepare x32: Permute the conversion and addition if one operand is a constant
  2011-07-29 13:29   ` H.J. Lu
  2011-07-28 18:23     ` Uros Bizjak
@ 2011-07-29  9:32     ` Paolo Bonzini
  2011-07-29  9:37       ` Paolo Bonzini
  2011-07-30  0:47       ` H.J. Lu
  1 sibling, 2 replies; 37+ messages in thread
From: Paolo Bonzini @ 2011-07-29  9:32 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Uros Bizjak, gcc-patches

Ok, you removed ignore_address_wrap_around, so we're almost there.

On 07/28/2011 07:59 PM, H.J. Lu wrote:
> @@ -712,7 +715,16 @@ convert_modes (enum machine_mode mode, enum machine_mode oldmode, rtx x, int uns
>     if (GET_CODE (x) == SUBREG&&  SUBREG_PROMOTED_VAR_P (x)
>         &&  GET_MODE_SIZE (GET_MODE (SUBREG_REG (x)))>= GET_MODE_SIZE (mode)
>         &&  SUBREG_PROMOTED_UNSIGNED_P (x) == unsignedp)
> -    x = gen_lowpart (mode, x);
> +    {
> +      temp = rtl_hooks.gen_lowpart_no_emit (mode, x);
> +      if (temp)
> +	x = temp;
> +      else
> +	{
> +	  gcc_assert (!no_emit);
> +	  x = gen_lowpart (mode, x);
> +	}
> +    }

+    {
+       /* gen_lowpart_no_emit should always succeed here.  */
+       x = rtl_hooks.gen_lowpart_no_emit (mode, x);
+    }

>
>     if (GET_MODE (x) != VOIDmode)
>       oldmode = GET_MODE (x);
> @@ -776,6 +788,10 @@ convert_modes (enum machine_mode mode, enum machine_mode oldmode, rtx x, int uns
>   	  return gen_int_mode (val, mode);
>   	}
>
> +      temp = rtl_hooks.gen_lowpart_no_emit (mode, x);
> +      if (temp)
> +	return temp;
> +      gcc_assert (!no_emit);
>         return gen_lowpart (mode, x);

Right now, gen_lowpart_no_emit will never return NULL, so these tests in 
convert_modes are dead.  Instead, please include in your patch mine at 
http://permalink.gmane.org/gmane.comp.gcc.patches/242085 and adjust as 
follows.

+      temp = rtl_hooks.gen_lowpart_no_emit (mode, x);
+      if (no_emit)
+	 return rtl_hooks.gen_lowpart_no_emit (mode, x);
+      else
+        return gen_lowpart (mode, x);

>       }

If it does not work, PLEASE say why instead of posting another "updated 
patch".

Paolo

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: PING: PATCH [4/n]: Prepare x32: Permute the conversion and addition if one operand is a constant
  2011-07-29 13:29   ` H.J. Lu
@ 2011-07-28 18:23     ` Uros Bizjak
  2011-07-29  9:32     ` Paolo Bonzini
  1 sibling, 0 replies; 37+ messages in thread
From: Uros Bizjak @ 2011-07-28 18:23 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Paolo Bonzini, gcc-patches

On Thu, Jul 28, 2011 at 7:59 PM, H.J. Lu <hjl.tools@gmail.com> wrote:

>>> >  convert_memory_address_addr_space has a special PLUS/MULT case for
>>> >  POINTERS_EXTEND_UNSIGNED<  0. ?It turns out that it is also needed
>>> >  for all Pmode != ptr_mode cases. ?OK for trunk?
>>> >  2011-06-11 ?H.J. Lu ?<hongjiu.lu@intel.com>
>>> >
>>> >  ? ? ? ?PR middle-end/47727
>>> >  ? ? ? ?* explow.c (convert_memory_address_addr_space): Permute the
>>> >  ? ? ? ?conversion and addition if one operand is a constant.
>>>
>>> Do we still need this patch? With recent target changes the testcase
>>> from PR can be compiled without problems with a gcc from an unpatched
>>> trunk.
>>
>> Given the communication difficulties, I hope not...
>>
>> Paolo
>>
>
> Here is the updated patch.  OK for trunk?

Did you see the question two levels up the thread you are replying to?

Uros.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: PING: PATCH [4/n]: Prepare x32: Permute the conversion and addition if one operand is a constant
  2011-07-28 10:24 Uros Bizjak
@ 2011-07-28 10:27 ` Paolo Bonzini
  2011-07-29 13:29   ` H.J. Lu
  0 siblings, 1 reply; 37+ messages in thread
From: Paolo Bonzini @ 2011-07-28 10:27 UTC (permalink / raw)
  To: Uros Bizjak; +Cc: gcc-patches, H.J. Lu

On 07/28/2011 11:30 AM, Uros Bizjak wrote:
> >  convert_memory_address_addr_space has a special PLUS/MULT case for
> >  POINTERS_EXTEND_UNSIGNED<  0. ?It turns out that it is also needed
> >  for all Pmode != ptr_mode cases. ?OK for trunk?
> >  2011-06-11 ?H.J. Lu ?<hongjiu.lu@intel.com>
> >
> >  ? ? ? ?PR middle-end/47727
> >  ? ? ? ?* explow.c (convert_memory_address_addr_space): Permute the
> >  ? ? ? ?conversion and addition if one operand is a constant.
>
> Do we still need this patch? With recent target changes the testcase
> from PR can be compiled without problems with a gcc from an unpatched
> trunk.

Given the communication difficulties, I hope not...

Paolo

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: PING: PATCH [4/n]: Prepare x32: Permute the conversion and addition if one operand is a constant
@ 2011-07-28 10:24 Uros Bizjak
  2011-07-28 10:27 ` Paolo Bonzini
  0 siblings, 1 reply; 37+ messages in thread
From: Uros Bizjak @ 2011-07-28 10:24 UTC (permalink / raw)
  To: gcc-patches; +Cc: Paolo Bonzini, H.J. Lu

Hello!

> convert_memory_address_addr_space has a special PLUS/MULT case for
> POINTERS_EXTEND_UNSIGNED < 0. ?It turns out that it is also needed
> for all Pmode != ptr_mode cases. ?OK for trunk?

> 2011-06-11 ?H.J. Lu ?<hongjiu.lu@intel.com>
>
> ? ? ? ?PR middle-end/47727
> ? ? ? ?* explow.c (convert_memory_address_addr_space): Permute the
> ? ? ? ?conversion and addition if one operand is a constant.

Do we still need this patch? With recent target changes the testcase
from PR can be compiled without problems with a gcc from an unpatched
trunk.

Uros.

^ permalink raw reply	[flat|nested] 37+ messages in thread

end of thread, other threads:[~2014-05-30  7:18 UTC | newest]

Thread overview: 37+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-07-05 14:30 PING: PATCH [4/n]: Prepare x32: Permute the conversion and addition if one operand is a constant H.J. Lu
2011-07-09 21:22 ` H.J. Lu
2011-07-09 21:23 ` Paolo Bonzini
2011-07-09 21:41   ` H.J. Lu
2011-07-10 17:04     ` Paolo Bonzini
2011-07-10 21:16       ` H.J. Lu
2011-07-11  0:48         ` H.J. Lu
2011-07-11  1:14           ` H.J. Lu
2011-07-11  6:49             ` H.J. Lu
2011-07-11 11:09             ` Paolo Bonzini
2011-07-11 15:58               ` H.J. Lu
2011-07-11 16:57                 ` H.J. Lu
2011-07-11 17:26                   ` H.J. Lu
2011-07-13 16:24                 ` Paolo Bonzini
2011-07-13 16:52                   ` H.J. Lu
2011-07-13 16:55                     ` Paolo Bonzini
2011-07-13 16:58                       ` Paolo Bonzini
2011-07-13 18:42                         ` H.J. Lu
2011-07-25 10:34                           ` Paolo Bonzini
2011-07-27 18:18                             ` H.J. Lu
2011-07-27 22:41                               ` Paolo Bonzini
2011-07-28  3:11                                 ` H.J. Lu
2011-07-28  7:59                                   ` Paolo Bonzini
2014-05-29  4:52                     ` Andrew Pinski
2014-05-29 16:13                       ` H.J. Lu
2014-05-29 16:23                         ` pinskia
2014-05-29 17:09                           ` H.J. Lu
2014-05-29 17:20                             ` pinskia
2014-05-30  7:18                               ` Paolo Bonzini
2011-07-28 10:24 Uros Bizjak
2011-07-28 10:27 ` Paolo Bonzini
2011-07-29 13:29   ` H.J. Lu
2011-07-28 18:23     ` Uros Bizjak
2011-07-29  9:32     ` Paolo Bonzini
2011-07-29  9:37       ` Paolo Bonzini
2011-07-30  0:47       ` H.J. Lu
2011-07-30 16:35         ` Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).