| Message ID | 20141124232539.GM1674@tucnak.redhat.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Nov 25, 2014 at 12:25 AM, Jakub Jelinek <jakub@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 <jakub@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. Uros. > > --- gcc/config/i386/i386.c.jj 2014-11-24 10:26:26.000000000 +0100 > +++ gcc/config/i386/i386.c 2014-11-24 20:17:27.659091709 +0100 > @@ -14848,7 +14848,7 @@ ix86_delegitimize_address (rtx x) > ... > movl foo@GOTOFF(%ecx), %edx > in which case we return (%ecx - %ebx) + foo > - or (%ecx - _GLOBAL_OFFSET_TABLE_) + foo if pseudo_pic_reg > + 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 ())) > @@ -14859,7 +14859,14 @@ ix86_delegitimize_address (rtx x) > { > 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); > + /* The order of the operands here is very important. find_base_term > + will only recurse into the first operand of PLUS if none of the > + arguments is REG with REG_POINTER_P set on it, and if that finds > + base term, it doesn't recurse into the second operand of PLUS. > + We don't want find_base_term to return the artificial > + _GLOBAL_OFFSET_TABLE_ symbol, so ensure it goes into the > + second operand. */ > + result = gen_rtx_PLUS (Pmode, result, tmp); > } > else > return orig_x; > > Jakub
--- gcc/config/i386/i386.c.jj 2014-11-24 10:26:26.000000000 +0100 +++ gcc/config/i386/i386.c 2014-11-24 20:17:27.659091709 +0100 @@ -14848,7 +14848,7 @@ ix86_delegitimize_address (rtx x) ... movl foo@GOTOFF(%ecx), %edx in which case we return (%ecx - %ebx) + foo - or (%ecx - _GLOBAL_OFFSET_TABLE_) + foo if pseudo_pic_reg + 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 ())) @@ -14859,7 +14859,14 @@ ix86_delegitimize_address (rtx x) { 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); + /* The order of the operands here is very important. find_base_term + will only recurse into the first operand of PLUS if none of the + arguments is REG with REG_POINTER_P set on it, and if that finds + base term, it doesn't recurse into the second operand of PLUS. + We don't want find_base_term to return the artificial + _GLOBAL_OFFSET_TABLE_ symbol, so ensure it goes into the + second operand. */ + result = gen_rtx_PLUS (Pmode, result, tmp); } else return orig_x;