Re: [PATCH] Fix find_base_term in 32-bit -fpic code (PR lto/64025)

[Uros Bizjak]

On Tue, Nov 25, 2014 at 8:40 AM, Uros Bizjak <ubiz...@gmail.com> wrote:
> On Tue, Nov 25, 2014 at 12:25 AM, Jakub Jelinek <ja...@redhat.com> wrote:
>
>> The fallback delegitimization I've added as last option mainly for
>> debug info purposes, when we don't know if the base is a PIC register
>> or say a PIC register plus some addend, unfortunately in some tests
>> broke find_base_term, which for PLUS looks only at the first operand
>> and recursion on it finds a base term, it returns it immediately.
>> So, it found base term of _GLOBAL_OFFSET_TABLE_, when the right base
>> term is actually in the second operand.
>>
>> This patch fixes it by swapping the operands, debug info doesn't care about
>> the order, it won't match in any instruction anyway, but helps alias.c.
>>
>> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>>
>> 2014-11-24  Jakub Jelinek  <ja...@redhat.com>
>>
>>         PR lto/64025
>>         * config/i386/i386.c (ix86_delegitimize_address): Ensure result
>>         comes before (addend - _GLOBAL_OFFSET_TABLE_) term.
>
> Can you also swap operands of (%ecx - %ebx) + foo? There is no point
> digging into RTX involving registers only when we know that we are
> looking for foo. This will also be consistent with the code you
> patched below.

Something like attached prototype patch.

Uros.

Index: i386.c
===================================================================
--- i386.c      (revision 218037)
+++ i386.c      (working copy)
@@ -14847,19 +14847,20 @@ ix86_delegitimize_address (rtx x)
         leal (%ebx, %ecx, 4), %ecx
         ...
         movl foo@GOTOFF(%ecx), %edx
-        in which case we return (%ecx - %ebx) + foo
-        or (%ecx - _GLOBAL_OFFSET_TABLE_) + foo if pseudo_pic_reg
+        in which case we return foo + (%ecx - %ebx)
+        or foo + (%ecx - _GLOBAL_OFFSET_TABLE_) if pseudo_pic_reg
         and reload has completed.  */
       if (pic_offset_table_rtx
          && (!reload_completed || !ix86_use_pseudo_pic_reg ()))
-        result = gen_rtx_PLUS (Pmode, gen_rtx_MINUS (Pmode, copy_rtx (addend),
-                                                    pic_offset_table_rtx),
-                              result);
+        result = gen_rtx_PLUS (Pmode, result,
+                              gen_rtx_MINUS (Pmode, copy_rtx (addend),
+                                             pic_offset_table_rtx));
       else if (pic_offset_table_rtx && !TARGET_MACHO && !TARGET_VXWORKS_RTP)
        {
          rtx tmp = gen_rtx_SYMBOL_REF (Pmode, GOT_SYMBOL_NAME);
-         tmp = gen_rtx_MINUS (Pmode, copy_rtx (addend), tmp);
-         result = gen_rtx_PLUS (Pmode, tmp, result);
+         result = gen_rtx_PLUS (Pmode, result,
+                                gen_rtx_MINUS (Pmode, copy_rtx (addend),
+                                               tmp));
        }
       else
        return orig_x;