diff mbox

[ivopt] Try aligned offset when get_address_cost

Message ID 3153506.iERQeH6ODR@polaris
State New
Headers show

Commit Message

Eric Botcazou Feb. 4, 2015, 10:32 a.m. UTC 
> For some TARGET, like ARM THUMB1, the offset in load/store should be nature
> aligned. But in function get_address_cost, when computing max_offset, it
> only tries byte-aligned offsets:
> 
>   ((unsigned HOST_WIDE_INT) 1 << i) - 1
> 
> which can not meet thumb_legitimate_offset_p check called from
> thumb1_legitimate_address_p for HImode and SImode.
> 
> The patch adds additional try for aligned offset:
> 
>   ((unsigned HOST_WIDE_INT) 1 << i) - GET_MODE_SIZE (address_mode).

I think that the change is not fully correct, this must be mem_mode instead of 
address_mode since the alignment of the MEM is given by mem_mode:



This fixes unexpected differences in the result of the function between SPARC 
and SPARC64 for the same source code.  OK for mainline after testing?


	* tree-ssa-loop-ivopts.c (get_address_cost): Use proper mode for offset.

Comments

Richard Biener Feb. 4, 2015, 12:47 p.m. UTC | #1 
On February 4, 2015 11:32:54 AM CET, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> For some TARGET, like ARM THUMB1, the offset in load/store should be
>nature
>> aligned. But in function get_address_cost, when computing max_offset,
>it
>> only tries byte-aligned offsets:
>> 
>>   ((unsigned HOST_WIDE_INT) 1 << i) - 1
>> 
>> which can not meet thumb_legitimate_offset_p check called from
>> thumb1_legitimate_address_p for HImode and SImode.
>> 
>> The patch adds additional try for aligned offset:
>> 
>>   ((unsigned HOST_WIDE_INT) 1 << i) - GET_MODE_SIZE (address_mode).
>
>I think that the change is not fully correct, this must be mem_mode
>instead of 
>address_mode since the alignment of the MEM is given by mem_mode:
>
>Index: tree-ssa-loop-ivopts.c
>===================================================================
>--- tree-ssa-loop-ivopts.c      (revision 220343)
>+++ tree-ssa-loop-ivopts.c      (working copy)
>@@ -3324,12 +3324,12 @@ get_address_cost (bool symbol_present, b
>          XEXP (addr, 1) = gen_int_mode (off, address_mode);
>          if (memory_address_addr_space_p (mem_mode, addr, as))
>            break;
>-         /* For some TARGET, like ARM THUMB1, the offset should be
>nature
>-            aligned. Try an aligned offset if address_mode is not
>QImode.  */
>-         off = (address_mode == QImode)
>+         /* For some strict-alignment targets, the offset must be
>naturally
>+            aligned.  Try an aligned offset if mem_mode is not QImode.
> */
>+         off = mem_mode == QImode
>                ? 0
>                : ((unsigned HOST_WIDE_INT) 1 << i)
>-                   - GET_MODE_SIZE (address_mode);
>+                   - GET_MODE_SIZE (mem_mode);
>          if (off > 0)
>            {
>              XEXP (addr, 1) = gen_int_mode (off, address_mode);
>
>
>This fixes unexpected differences in the result of the function between
>SPARC 
>and SPARC64 for the same source code.  OK for mainline after testing?

OK.

Thanks,
Richard.

>
>	* tree-ssa-loop-ivopts.c (get_address_cost): Use proper mode for
>offset.
diff mbox

Patch

Index: tree-ssa-loop-ivopts.c
===================================================================
--- tree-ssa-loop-ivopts.c      (revision 220343)
+++ tree-ssa-loop-ivopts.c      (working copy)
@@ -3324,12 +3324,12 @@  get_address_cost (bool symbol_present, b
          XEXP (addr, 1) = gen_int_mode (off, address_mode);
          if (memory_address_addr_space_p (mem_mode, addr, as))
            break;
-         /* For some TARGET, like ARM THUMB1, the offset should be nature
-            aligned. Try an aligned offset if address_mode is not QImode.  */
-         off = (address_mode == QImode)
+         /* For some strict-alignment targets, the offset must be naturally
+            aligned.  Try an aligned offset if mem_mode is not QImode.  */
+         off = mem_mode == QImode
                ? 0
                : ((unsigned HOST_WIDE_INT) 1 << i)
-                   - GET_MODE_SIZE (address_mode);
+                   - GET_MODE_SIZE (mem_mode);
          if (off > 0)
            {
              XEXP (addr, 1) = gen_int_mode (off, address_mode);