On 22/09/16 16:24, Alexander Monakov wrote:
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 for the suggestions.
From what I understand
(https://gcc.gnu.org/ml/gcc-patches/2016-09/msg01569.html)
the warning won't trigger anymore for macros, so there shouldn't be a bootstrap
failure any more. I'm re-running an arm bootstrap with latest trunk now to
confirm.
If that is the case, then this is not as urgent to fix.
However it's still a problem we should fix now that it has been exposed.
I'll implement your suggestions and do the usual testing.
Thanks,
Kyrill
Thanks.
Alexander
