On Thu, 22 Sep 2016, Kyrill Tkachov wrote:
> > I don't follow. The macro used as a boolean in places changed by your patch
> > is H_F_P_IS_FRAME_POINTER, not H_F_P_REGNUM.
> >
> > Am I missing something?
>
> I'm following Bernd's proposed change from:
> https://gcc.gnu.org/ml/gcc-patches/2016-09/msg01330.html
I see - I was misled by the error in the ChangeLog and the fact that the first
hunk is not relevant to the issue:
> diff --git a/gcc/regrename.c b/gcc/regrename.c
> index
> 54c7768efa226139c340868e42b784fb011a19b9..a7339db441012e338de5697015f04c1fdb970164
> 100644
> --- a/gcc/regrename.c
> +++ b/gcc/regrename.c
> @@ -464,8 +464,7 @@ rename_chains (void)
> if (frame_pointer_needed)
> {
> add_to_hard_reg_set (&unavailable, Pmode, FRAME_POINTER_REGNUM);
> - if (!HARD_FRAME_POINTER_IS_FRAME_POINTER)
> - add_to_hard_reg_set (&unavailable, Pmode, HARD_FRAME_POINTER_REGNUM);
> + add_to_hard_reg_set (&unavailable, Pmode, HARD_FRAME_POINTER_REGNUM);
> }
There's no issue here: H_F_P_REGNUM is not used in the boolean context, only
H_F_P_IS_FRAME_POINTER is. There's no warning here, right?
> FOR_EACH_VEC_ELT (id_to_chain, i, this_head)
> @@ -479,10 +478,9 @@ rename_chains (void)
> continue;
>
> if (fixed_regs[reg] || global_regs[reg]
> - || (!HARD_FRAME_POINTER_IS_FRAME_POINTER && frame_pointer_needed
> - && reg == HARD_FRAME_POINTER_REGNUM)
> - || (HARD_FRAME_POINTER_REGNUM && frame_pointer_needed
> - && reg == FRAME_POINTER_REGNUM))
> + || (frame_pointer_needed
> + && (reg == HARD_FRAME_POINTER_REGNUM
> + || reg == FRAME_POINTER_REGNUM)))
> continue;
OK, so here's the real issue: due to a typo 'H_F_P_REGNUM &&
frame_pointer_needed' is used where H_F_P_IS_FRAME_POINTER was used in a
preprocessor #if previously.
A minimal fix would just change H_F_P_REGNUM back to H_F_P_IS_FRAME_POINTER.
I think that's what should be done in order to restore bootstrap: that's clearly
doing no more than restoring previous semantics. The change you've shown also
alters the meaning of the code: I think if that's desired, that should be a
separate patch.
> diff --git a/gcc/sel-sched.c b/gcc/sel-sched.c
> index
> 25a100ee34f6ceaceda2814ae281cadf8b29e688..4a2679c6701c256c5559fa1e9c156bdaad1c2891
> 100644
> --- a/gcc/sel-sched.c
> +++ b/gcc/sel-sched.c
> @@ -1183,10 +1183,8 @@ mark_unavailable_hard_regs (def_t def, struct
> reg_rename *reg_rename_p,
> frame pointer, or we could not discover its class. */
> if (fixed_regs[regno]
> || global_regs[regno]
> - || (!HARD_FRAME_POINTER_IS_FRAME_POINTER && frame_pointer_needed
> + || (frame_pointer_needed
> && regno == HARD_FRAME_POINTER_REGNUM)
> - || (HARD_FRAME_POINTER_REGNUM && frame_pointer_needed
> - && regno == FRAME_POINTER_REGNUM)
> || (reload_completed && cl == NO_REGS))
Originally this condition in sel-sched.c looked exactly like the above in
regrename.c (except the last line). Please keep them in sync: I think if both
H_F_P_REGNUM and F_P_REGNUM ought to be accepted in rename_chains (like your
patch does), so they should be here in mark_u_h_r.
Thanks.
Alexander
