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

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

[Bernd Schmidt]

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.

So... ok everywhere?


Bernd

	PR rtl-optimization/69648
	* lra-constraints.c (update_ebb_live_info): Don't remove sets of
	pic_offset_table_rtx.

Index: lra-constraints.c
===================================================================
--- lra-constraints.c	(revision 232689)
+++ lra-constraints.c	(working copy)
@@ -5127,8 +5127,10 @@ update_ebb_live_info (rtx_insn *head, rt
        curr_id = lra_get_insn_recog_data (curr_insn);
        curr_static_id = curr_id->insn_static_data;
        remove_p = false;
-      if ((set = single_set (curr_insn)) != NULL_RTX && REG_P (SET_DEST 
(set))
+      if ((set = single_set (curr_insn)) != NULL_RTX
+	  && REG_P (SET_DEST (set))
  	  && (regno = REGNO (SET_DEST (set))) >= FIRST_PSEUDO_REGISTER
+	  && SET_DEST (set) != pic_offset_table_rtx
  	  && bitmap_bit_p (&check_only_regs, regno)
  	  && ! bitmap_bit_p (&live_regs, regno))
  	remove_p = true;

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

[Vladimir Makarov]

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.
>