| Message ID | 56BE2930.8050004@redhat.com |
|---|---|
| State | New |
| Headers | show |
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. >
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))
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. remove_p = true;