invalid delayed slot filling

[Christian BRUEL]

Ian Lance Taylor wrote:
> Christian BRUEL <christian.bruel@st.com> writes:
> 
> 
>>that's the point, 'mark_target_live_regs' doesn't scan the insns until
>>the return statement, but until 'stop_insn' that is the first insns
>>after the delayed branch (the first insn in the fall thru block), so
>>'end_of_function_needs' wasn't looked at.
> 
> 
> That shouldn't matter.  mark_target_live_regs starts with the set of
> register live at the start of the block.  That set has already been
> computed and should be correct.  In particular, if FPSCR is live at
> the start of the block, it should be in that set.  Then
> mark_target_live_regs walks through the insns.  For each register it
> finds the dies, it removes it from the set of live registers.  For
> each register it finds that comes alive, it adds it to the set of live
> registers.
>
fpscr is livein at the start of the block. then mark_target_live_regs 
scans the insns, then it finds a call, then it is set dead by 
regs_invalidated_by_call.

> I don't know why you are focusing on the registers live in the
> epilogue.  The registers live in the epilogue of a function being
> called are not necessarily live in the function making the call.
> 

I was not talking about the called function. I was talking about the 
current function, for which end_of_function_needs pertains. If the 
register is live in the epilogue (of the current function, the one 
having the delayed branch) then there is possibly liverange between the 
begin (it was livein) and the end of the function.

Note that it has to be a conservative approach because it's possible to 
have interference between the prologue and the epilogue and it's 
possible than the value is not set with the same value at exit than in 
entry thanks to lcm. Also the value can be inherited from predecessors. 
So mark_target_live_regs must assume than the fpscr value is live after 
the call.

>>but ... I'm realizing that fpscr_reg is not in the global_regs set,
>>which is correctly looked at at the call stmt.
>>
>>So updating CONDITIONAL_REGISTER_USAGE in sh.md to have FPSCR_REG a
>>global_regs cleanly seem to fix this problem.
> 
> I'm sorry, that is not correct either.  The global_regs array exists
> only to hold global register variables declared by the user.  It would
> not be correct for a backend to add an entry to that array.
> 

ok, I figured out that they could also be used by the compiler to tell 
that the register is valid across calls. I saw some hack like this 
(arm.h or unicosmk.h) so I thought it was standard. I was wrong then :-)

> Let me ask again: why is the register considered to be dead?
> 

because it is marked in 'regs_invalidated_by_call', because it is in the 
CALL_USED_REGISTERS macro.

I tried to remove the fpscr register from CALL_REALLY_USED_REGNO_P but 
that gives very bad code (it is automatically saved on function entry 
and restored on function exit, as explained in the manual chapter 14.)

> Ian
> 

so to summarize there were the following solutions (for now)

- use define_delay and never slot this instruction. ok as a workaround

- mark calls with an explicit setting of fpscr. But what about extern 
declarations or preemptible functions ?  you need to assume than all 
calls as setting fpscr and you already have this information in 
EPILOGUE_USE. I'm sure there is something more central.

- in mark_target_live_regs, when scanning a CALL, merge EPILOGUE_USE 
with current_live_regs : we established it is a bad solution.

- in mark_target_live_regs, when scanning a CALL, merge 
end_of_function_needs AFTER registers are made dead by 
regs_invalidated_by_call. I like this one, is it ruled out ?

- use global_regs. I thought it was used at some other places. but ok, 
just for users :-)

Christian