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

["Fei Gao" <gaofei@eswincomputing.com>]

On 2024-06-09 04:36  Jeff Law <jeffreyalaw@gmail.com> wrote:
>
>
>
>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 
Hi Jeff
I should have made it clear that there're  li	a0,0 and use a0 insns instead of just li a0, 0 here.
>> 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. 
The DCE pass  after prologue generation may not help here for the following reasons:
1. The use a0 insn is not deletable, and then li a0,0 that defines a0 cannot be deleted.
2. We need to detect pattern (clear a0, use a0 and cm.popret) before generating cm.popretz. 
    I don't think DCE is a good place to put this piece of codes. 
    And I insist prologue and epilogue pass is a better place to do it with simplicity and clear logic
    as I explained earlier to Kito. Hook was added here safely without any impact on other targets.

Please let me know your idea. 

Thanks. 
Fei
>
>
>
>Jeff