Fix PR69648: lra removes set of PIC reg, then introduces new use

Vladimir Makarov vmakarov@redhat.com
Fri Feb 12 20:22:00 GMT 2016


On 02/12/2016 01:49 PM, Bernd Schmidt wrote:
> This is a PR where LRA creates a read from an uninitialized stack 
> slot. That stack slot is supposed to hold the value of the PIC register.
>
> What seems to happen is that we have two passes making different choices:
>
>          Choosing alt 0 in insn 143:  (0) =x  (1) 0  (2) r {sse2_pinsrw}
> [...]
>          Choosing alt 1 in insn 143:  (0) x  (1) 0  (2) m {sse2_pinsrw}
>
> The second choice causes a constant to be placed in memory, and a new 
> reference to the PIC register is created. Unfortunately, before the 
> second pass, we seem to have deleted an insn that stores a reload reg 
> into the spilled pic register pseudo, because we think we've managed 
> to inherit the reload reg into all uses. The new use isn't taken into 
> account.
>
> I came up with the following, initially as a hack, but it seems to 
> work surprisingly well.  Bootstrapped and tested on x86_64-linux with 
> -fPIC; it seems to cure the problem for the testcase (by inspection, 
> it never crashed on my system).
>
> Since I was worried about code generation differences (adding 
> unnecessary stores) I've also run it against my collection of .i 
> files. With -m64 -fPIC, I did not observe any changes. I hadn't looked 
> at x86_64 pic generation previously, looks like we can use pc-relative 
> addressing and don't need the pic reg at all usually? With -m32 -fPIC, 
> I observe differences in 27 out of 4117 source files (taken from the 
> Linux kernel, gcc, and several other packages). That doesn't seem very 
> worrying, especially considering that several of these changes turn 
> out not to be redundant stores, just placing the PIC reg into a 
> different stack slot.
>
Thank you for checking this.  That is the only thing about the patch 
which I would worry too.
> So... ok everywhere?
>
>
Yes. Thanks, Bernd.
>
>     PR rtl-optimization/69648
>     * lra-constraints.c (update_ebb_live_info): Don't remove sets of
>     pic_offset_table_rtx.
>



More information about the Gcc-patches mailing list