Re: [RS6000] Correct save_reg_p
On Fri, Feb 08, 2019 at 04:18:52PM +, Iain Sandoe wrote: > > > On 8 Feb 2019, at 16:16, Segher Boessenkool > > wrote: > > > > On Fri, Feb 08, 2019 at 10:19:40PM +1030, Alan Modra wrote: > >> That one regressed gcc.dg/20020312-2.c, due to my "cleverness" in > >> simplifying the ABI_V4 case. This one passes regression testing. > >> OK to apply? > > > > I think this is correct. Thanks! Okay for trunk. Does it need backports? > > Yes, we reverted the original fix on 7 and 8. Then I hope we can make a working 8.3 :-/ Alan, to increase the chances of that, please backport it to 8 as soon as you are confident trunk works? Have tested it, I mean :-) Segher
Re: [RS6000] Correct save_reg_p
> On 8 Feb 2019, at 16:16, Segher Boessenkool
> wrote:
>
> On Fri, Feb 08, 2019 at 10:19:40PM +1030, Alan Modra wrote:
>> That one regressed gcc.dg/20020312-2.c, due to my "cleverness" in
>> simplifying the ABI_V4 case. This one passes regression testing.
>> OK to apply?
>
> I think this is correct. Thanks! Okay for trunk. Does it need backports?
Yes, we reverted the original fix on 7 and 8.
Iain
>
>
> Segher
>
>
>> PR target/88343
>> * config/rs6000/rs6000.c (save_reg_p): Correct calls_eh_return
>> case. Match logic in rs6000_emit_prologue emitting pic_offset_table
>> setup.
>>
>> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
>> index cced90bb518..b4908c4f795 100644
>> --- a/gcc/config/rs6000/rs6000.c
>> +++ b/gcc/config/rs6000/rs6000.c
>> @@ -24008,21 +24008,30 @@ rs6000_split_multireg_move (rtx dst, rtx src)
>> static bool
>> save_reg_p (int reg)
>> {
>> - /* We need to mark the PIC offset register live for the same conditions
>> - as it is set up, or otherwise it won't be saved before we clobber it.
>> */
>> -
>> if (reg == RS6000_PIC_OFFSET_TABLE_REGNUM && !TARGET_SINGLE_PIC_BASE)
>> {
>> /* When calling eh_return, we must return true for all the cases
>> where conditional_register_usage marks the PIC offset reg
>> - call used. */
>> + call used or fixed. */
>> + if (crtl->calls_eh_return
>> + && ((DEFAULT_ABI == ABI_V4 && flag_pic)
>> + || (DEFAULT_ABI == ABI_DARWIN && flag_pic)
>> + || (TARGET_TOC && TARGET_MINIMAL_TOC)))
>> +return true;
>> +
>> + /* We need to mark the PIC offset register live for the same
>> + conditions as it is set up in rs6000_emit_prologue, or
>> + otherwise it won't be saved before we clobber it. */
>> if (TARGET_TOC && TARGET_MINIMAL_TOC
>> - && (crtl->calls_eh_return
>> - || df_regs_ever_live_p (reg)
>> - || !constant_pool_empty_p ()))
>> + && !constant_pool_empty_p ())
>> +return true;
>> +
>> + if (DEFAULT_ABI == ABI_V4
>> + && (flag_pic == 1 || (flag_pic && TARGET_SECURE_PLT))
>> + && df_regs_ever_live_p (RS6000_PIC_OFFSET_TABLE_REGNUM))
>> return true;
>>
>> - if ((DEFAULT_ABI == ABI_V4 || DEFAULT_ABI == ABI_DARWIN)
>> + if (DEFAULT_ABI == ABI_DARWIN
>>&& flag_pic && crtl->uses_pic_offset_table)
>> return true;
>> }
Re: [RS6000] Correct save_reg_p
On Fri, Feb 08, 2019 at 10:19:40PM +1030, Alan Modra wrote:
> That one regressed gcc.dg/20020312-2.c, due to my "cleverness" in
> simplifying the ABI_V4 case. This one passes regression testing.
> OK to apply?
I think this is correct. Thanks! Okay for trunk. Does it need backports?
Segher
> PR target/88343
> * config/rs6000/rs6000.c (save_reg_p): Correct calls_eh_return
> case. Match logic in rs6000_emit_prologue emitting pic_offset_table
> setup.
>
> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> index cced90bb518..b4908c4f795 100644
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -24008,21 +24008,30 @@ rs6000_split_multireg_move (rtx dst, rtx src)
> static bool
> save_reg_p (int reg)
> {
> - /* We need to mark the PIC offset register live for the same conditions
> - as it is set up, or otherwise it won't be saved before we clobber it.
> */
> -
>if (reg == RS6000_PIC_OFFSET_TABLE_REGNUM && !TARGET_SINGLE_PIC_BASE)
> {
>/* When calling eh_return, we must return true for all the cases
>where conditional_register_usage marks the PIC offset reg
> - call used. */
> + call used or fixed. */
> + if (crtl->calls_eh_return
> + && ((DEFAULT_ABI == ABI_V4 && flag_pic)
> + || (DEFAULT_ABI == ABI_DARWIN && flag_pic)
> + || (TARGET_TOC && TARGET_MINIMAL_TOC)))
> + return true;
> +
> + /* We need to mark the PIC offset register live for the same
> + conditions as it is set up in rs6000_emit_prologue, or
> + otherwise it won't be saved before we clobber it. */
>if (TARGET_TOC && TARGET_MINIMAL_TOC
> - && (crtl->calls_eh_return
> - || df_regs_ever_live_p (reg)
> - || !constant_pool_empty_p ()))
> + && !constant_pool_empty_p ())
> + return true;
> +
> + if (DEFAULT_ABI == ABI_V4
> + && (flag_pic == 1 || (flag_pic && TARGET_SECURE_PLT))
> + && df_regs_ever_live_p (RS6000_PIC_OFFSET_TABLE_REGNUM))
> return true;
>
> - if ((DEFAULT_ABI == ABI_V4 || DEFAULT_ABI == ABI_DARWIN)
> + if (DEFAULT_ABI == ABI_DARWIN
> && flag_pic && crtl->uses_pic_offset_table)
> return true;
> }
Re: [RS6000] Correct save_reg_p
On Fri, Feb 08, 2019 at 11:13:50AM +1030, Alan Modra wrote:
> PR target/88343
> * config/rs6000/rs6000.c (save_reg_p): Correct calls_eh_return
> case. Match logic in rs6000_emit_prologue emitting pic_offset_table
> setup, simplifying ABI_V4 case to df_regs_ever_live_p.
That one regressed gcc.dg/20020312-2.c, due to my "cleverness" in
simplifying the ABI_V4 case. This one passes regression testing.
OK to apply?
PR target/88343
* config/rs6000/rs6000.c (save_reg_p): Correct calls_eh_return
case. Match logic in rs6000_emit_prologue emitting pic_offset_table
setup.
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index cced90bb518..b4908c4f795 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -24008,21 +24008,30 @@ rs6000_split_multireg_move (rtx dst, rtx src)
static bool
save_reg_p (int reg)
{
- /* We need to mark the PIC offset register live for the same conditions
- as it is set up, or otherwise it won't be saved before we clobber it. */
-
if (reg == RS6000_PIC_OFFSET_TABLE_REGNUM && !TARGET_SINGLE_PIC_BASE)
{
/* When calling eh_return, we must return true for all the cases
where conditional_register_usage marks the PIC offset reg
-call used. */
+call used or fixed. */
+ if (crtl->calls_eh_return
+ && ((DEFAULT_ABI == ABI_V4 && flag_pic)
+ || (DEFAULT_ABI == ABI_DARWIN && flag_pic)
+ || (TARGET_TOC && TARGET_MINIMAL_TOC)))
+ return true;
+
+ /* We need to mark the PIC offset register live for the same
+conditions as it is set up in rs6000_emit_prologue, or
+otherwise it won't be saved before we clobber it. */
if (TARGET_TOC && TARGET_MINIMAL_TOC
- && (crtl->calls_eh_return
- || df_regs_ever_live_p (reg)
- || !constant_pool_empty_p ()))
+ && !constant_pool_empty_p ())
+ return true;
+
+ if (DEFAULT_ABI == ABI_V4
+ && (flag_pic == 1 || (flag_pic && TARGET_SECURE_PLT))
+ && df_regs_ever_live_p (RS6000_PIC_OFFSET_TABLE_REGNUM))
return true;
- if ((DEFAULT_ABI == ABI_V4 || DEFAULT_ABI == ABI_DARWIN)
+ if (DEFAULT_ABI == ABI_DARWIN
&& flag_pic && crtl->uses_pic_offset_table)
return true;
}
--
Alan Modra
Australia Development Lab, IBM
[RS6000] Correct save_reg_p
Fixes lack of r30 save/restore on
// -m32 -fpic -ftls-model=initial-exec
__thread char* p;
char** f1 (void) { return }
and
// -m32 -fpic -msecure-plt
extern int foo (int);
int f1 (int x) { return foo (x); }
These are both caused by save_reg_p returning false when the pic
offset table reg (r30 for ABI_V4) was used, due to the logic not
exactly matching that in rs6000_emit_prologue to set up r30. I've
simplified the ABI_V4 logic down to df_regs_ever_live_p since that is
obviously correct for all ABIs.
I also noticed that save_reg_p isn't following the comment regarding
calls_eh_return (since svn 267049, git 0edf78b1b2a0), and the comment
needs tweaking too. For why the revised comment is correct, grep for
saves_all_registers in lra.c, and yes, we do want to save the pic
offset table reg for eh_return.
Bootstrap and regression test on powerpc64-linux biarch in progress.
OK assuming no regressions? Segher this patch supercedes
https://gcc.gnu.org/ml/gcc-patches/2019-02/msg00307.html as there's no
need to set crtl->uses_pic_offset_table for those cases now with the
df_regs_ever_live_p test in save_reg_p.
PR target/88343
* config/rs6000/rs6000.c (save_reg_p): Correct calls_eh_return
case. Match logic in rs6000_emit_prologue emitting pic_offset_table
setup, simplifying ABI_V4 case to df_regs_ever_live_p.
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index cced90bb518..9933d4443f7 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -24013,18 +24013,26 @@ save_reg_p (int reg)
if (reg == RS6000_PIC_OFFSET_TABLE_REGNUM && !TARGET_SINGLE_PIC_BASE)
{
+ if (df_regs_ever_live_p (reg))
+ return true;
+
/* When calling eh_return, we must return true for all the cases
where conditional_register_usage marks the PIC offset reg
-call used. */
+call used or fixed. */
+ if (crtl->calls_eh_return
+ && ((DEFAULT_ABI == ABI_V4 && flag_pic)
+ || (DEFAULT_ABI == ABI_DARWIN && flag_pic)
+ || (TARGET_TOC && TARGET_MINIMAL_TOC)))
+ return true;
+
if (TARGET_TOC && TARGET_MINIMAL_TOC
- && (crtl->calls_eh_return
- || df_regs_ever_live_p (reg)
- || !constant_pool_empty_p ()))
+ && !constant_pool_empty_p ())
return true;
- if ((DEFAULT_ABI == ABI_V4 || DEFAULT_ABI == ABI_DARWIN)
+ if (DEFAULT_ABI == ABI_DARWIN
&& flag_pic && crtl->uses_pic_offset_table)
return true;
+ return false;
}
return !call_used_regs[reg] && df_regs_ever_live_p (reg);
--
Alan Modra
Australia Development Lab, IBM
