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

[Jeff Law <jeffreyalaw@gmail.com>]

On 6/5/24 1:47 AM, Fei Gao wrote:
> 
> On 2024-06-05 14:36  Kito Cheng <kito.cheng@gmail.com> wrote:
>>
>> Thanks for fixing this issue, and I am wondering doest it possible to
>> fix that without introduce target hook? I ask that because...GCC 14
>> also has this bug, but I am not sure it's OK to introduce new target
>> hook for release branch? or would you suggest we just revert patch to
>> fix that on GCC 14?
> 
> If hook is unacceptable in GCC14, I suggest to revert on GCC 14 the following commit.
> https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=b27d323a368033f0b37e93c57a57a35fd9997864
> 
> I started fixing this issue by adding changes in mach pass but abandoned it
> due to the following reasons:
> 1. more codes to detect location of epilogue in the whole insn list.
> 2. due to impact by scheduling pass, clear a0 and use a0 insns get reordered, resulting in more
>      codes.
> 3. data flow analysis is needed, but insn does't have bb info any more, so rescan actually does
>      nothing, which I guess there's some hidden issue in riscv_remove_unneeded_save_restore_calls
>      using dfa.
> 
> So I came up this hook based patch in prologue and epilogue pass to make the optimization
> happen as earlier as possible. It ends up with simplicity and clear logic.
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.

So let's get a good explanation of the problem and perhaps we'll find a 
better way to solve it.

jeff