Re: [PATCH 0/2] fix RISC-V zcmp popretz [PR113715]

[Jeff Law <jeffreyalaw@gmail.com>]

On 6/5/24 8:42 PM, Fei Gao wrote:

>> But let's back up and get a good explanation of what the problem is.
>> Based on patch 2/2 it looks like we have lost an assignment to the
>> return register.
>>
>> To someone not familiar with this code, it sounds to me like we've made
>> a mistake earlier and we're now defining a hook that lets us go back and
>> fix that earlier mistake.   I'm probably wrong, but so far that's what
>> it sounds like.
> Hi Jeff
> 
> You're right. Let me rephrase  patch 2/2 with more details. Search /* feigao to location the point I'm
> tring to explain.
> 
> code snippets from gcc/function.cc
> void
> thread_prologue_and_epilogue_insns (void)
> {
> ...
>    /*feigao:
>          targetm.gen_epilogue () is called here to generate epilogue sequence.
> 	https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=b27d323a368033f0b37e93c57a57a35fd9997864
> 	Commit above tries in targetm.gen_epilogue () to detect if
> 	there's li	a0,0 insn at the end of insn chain, if so, cm.popret
> 	is replaced by cm.popretz and li	a0,0 insn is deleted.
So that seems like the critical issue.  Generation of the 
prologue/epilogue really shouldn't be changing other instructions in the 
instruction stream.  I'm not immediately aware of another target that 
does that, an it seems like a rather risky thing to do.


It looks like the cm.popretz's RTL exposes the assignment to a0 and 
there's a DCE pass that runs after insertion of the prologue/epilogue. 
So I would suggest leaving the assignment to a0 in the RTL chain and see 
if the later DCE pass after prologue generation eliminates the redundant 
assignment.  That seems a lot cleaner.



Jeff