diff mbox

[ARM,2/2] Fix DImode addressing

Message ID 4D1556DB.5020605@codesourcery.com
State New
Headers show

Commit Message

Jie Zhang Dec. 25, 2010, 2:28 a.m. UTC 
On 12/24/2010 12:28 AM, Ramana Radhakrishnan wrote:
>>>> +         if ((low != -8&&   low != -4&&   low != 0&&   low != 4)
>>>> +             || !multiple_operation_profitable_p (false, 2, val))
>>>>
>>>> should be moved into multiple_operation_profitable_p.
>>>
>>> Since the code is identical - why not move it in there today or is that
>>> what you are saying ? Ideally I think it would be better to move this in
>>> as a flag into the costs structure per core .
>>>
>> I'm not sure if it's good to move it into
>> multiple_operation_profitable_p since it's also called from some places.
>> Moving into it but not affecting other code needs more time thinking.
>> For now I just try to keep my patch as simple as possible to ease code
>> review. We can move it into multiple_operation_profitable_p later when
>> multiple_operation_profitable_p will be changed largely.
>
> Then I would suggest that you add a comment or a FIXME: that marks this
> as something that has to be looked at when we look at tuning
> multiple_operation_profitable_p .
>
Here is the updated patch with the comments added.

Regards,

Comments

Jie Zhang Jan. 17, 2011, 2:11 a.m. UTC | #1 
PING.

On 12/25/2010 10:28 AM, Jie Zhang wrote:
> On 12/24/2010 12:28 AM, Ramana Radhakrishnan wrote:
>>>>> + if ((low != -8&& low != -4&& low != 0&& low != 4)
>>>>> + || !multiple_operation_profitable_p (false, 2, val))
>>>>>
>>>>> should be moved into multiple_operation_profitable_p.
>>>>
>>>> Since the code is identical - why not move it in there today or is that
>>>> what you are saying ? Ideally I think it would be better to move
>>>> this in
>>>> as a flag into the costs structure per core .
>>>>
>>> I'm not sure if it's good to move it into
>>> multiple_operation_profitable_p since it's also called from some places.
>>> Moving into it but not affecting other code needs more time thinking.
>>> For now I just try to keep my patch as simple as possible to ease code
>>> review. We can move it into multiple_operation_profitable_p later when
>>> multiple_operation_profitable_p will be changed largely.
>>
>> Then I would suggest that you add a comment or a FIXME: that marks this
>> as something that has to be looked at when we look at tuning
>> multiple_operation_profitable_p .
>>
> Here is the updated patch with the comments added.
>
> Regards,
diff mbox

Patch


	* config/arm/arm.c (multiple_operation_profitable_p): Declare.
	(arm_legitimize_address): Adjust handling for DImode and DFmode.
	(arm_legitimize_reload_address): Likewise.

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 95bde4a..d084e50 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -250,6 +250,7 @@  static bool arm_builtin_support_vector_misalignment (enum machine_mode mode,
 						     bool is_packed);
 static void arm_conditional_register_usage (void);
 static reg_class_t arm_preferred_rename_class (reg_class_t rclass);
+static bool multiple_operation_profitable_p (bool, int, HOST_WIDE_INT);
 
 
 /* Table of machine attributes.  */
@@ -6230,11 +6231,14 @@  arm_legitimize_address (rtx x, rtx orig_x, enum machine_mode mode)
 	  rtx base_reg, val;
 	  n = INTVAL (xop1);
 
-	  /* VFP addressing modes actually allow greater offsets, but for
-	     now we just stick with the lowest common denominator.  */
 	  if (mode == DImode
 	      || ((TARGET_SOFT_FLOAT || TARGET_VFP) && mode == DFmode))
 	    {
+	      HOST_WIDE_INT orig_n = n;
+
+	      /* We first see if LDM is profitable.  If not, we will use
+		 the lowest common denominator suitable for two LDRs
+		 and VLDR, and for LDRD if TARGET_LDRD.  */
 	      low_n = n & 0x0f;
 	      n &= ~0x0f;
 	      if (low_n > 4)
@@ -6242,6 +6246,17 @@  arm_legitimize_address (rtx x, rtx orig_x, enum machine_mode mode)
 		  n += 16;
 		  low_n -= 16;
 		}
+	      /* The four conditionals about low_n should be moved into
+		 multiple_operation_profitable_p when we are going to
+		 tune multiple_operation_profitable_p.  */
+	      if ((low_n != -8 && low_n != -4 && low_n != 0 && low_n != 4)
+		  || !multiple_operation_profitable_p (false, 2, orig_n))
+		{
+		  HOST_WIDE_INT mask = (TARGET_LDRD ? 0xfc : 0x3fc);
+		  n = orig_n;
+		  low_n = (n >= 0 ? (n & mask) : -((-n) & mask));
+		  n -= low_n;
+		}
 	    }
 	  else
 	    {
@@ -6404,7 +6419,21 @@  arm_legitimize_reload_address (rtx *p,
       HOST_WIDE_INT low, high;
 
       if (mode == DImode || (mode == DFmode && TARGET_SOFT_FLOAT))
-	low = ((val & 0xf) ^ 0x8) - 0x8;
+	{
+	  /* We first see if LDM is profitable.  If not, we will use
+	     the lowest common denominator suitable for two LDRs
+	     and VLDR, and for LDRD if TARGET_LDRD.  */
+	  low = ((val & 0xf) ^ 0x8) - 0x8;
+	  /* The four conditionals about low should be moved into
+	     multiple_operation_profitable_p when we are going to
+	     tune multiple_operation_profitable_p.  */
+	  if ((low != -8 && low != -4 && low != 0 && low != 4)
+	      || !multiple_operation_profitable_p (false, 2, val))
+	    {
+	      HOST_WIDE_INT mask = (TARGET_LDRD ? 0xfc : 0x3fc);
+	      low = (val >= 0 ? (val & mask) : -((-val) & mask));
+	    }
+	}
       else if (TARGET_MAVERICK && TARGET_HARD_FLOAT)
 	/* Need to be careful, -256 is not a valid offset.  */
 	low = val >= 0 ? (val & 0xff) : -((-val) & 0xff);