Richard Henderson <richard.hender...@linaro.org> writes:
> On 8/8/25 20:39, Richard Sandiford wrote:
>> Richard Henderson <richard.hender...@linaro.org> writes:
>>> The save/restore_stack_nonlocal patterns passed a DImode rtx
>>> to gen_tbranch_neqi3 for a QImode compare.  The tbranch expander
>>> did not do what it said on the tin, that is: emit TBNZ.
>>> It only made it as far as AND+CMP+B.cond.
>> 
>> Yeah, that was done to allow ifcombine to have a go first
>> (g:17ae956c0fa6baac3d22764019d5dd5ebf5c2b11).  But the original
>> pattern was restricted in g:8e26ac4749c5ddf827e18a846b1010b091f76fa2
>> to "bit 0 of a QI or HI" that we have now.
>> 
>> If all tests pass without the expander then I suppose we at least need
>> a new test to justify the expander's existence.
>
> No, there are a few regressions on comparisons vs _Bool.  I didn't know where 
> they were 
> coming from and it didn't occur to me to check if tbranch was an optab.  (I 
> hesitate to 
> say "new", since my gcc lore is at least 10 years out of date.)

Ah, ok.  Good to know that the test coverage is still there. :)

So yeah, let's keep the optab and just change save/restore_stack_nonlocal.

> Thanks for the links, Richard.  And nice commit messages, Tamar.
>
>> It doesn't look like any other target has implemented the tbranch optab,
>> so if we do remove the expander, I suppose we should remove the optab
>> itself too (separately).
>> 
>> The fix for the save/restore_stack_nonlocal mode mismatch is ok
>> either way though.
>
> Presumably by retaining the change to CBNZ rather than passing gen_lowpart 
> (QImode, x16) 
> to tbranch?  The branch over the GCS stack manipulation isn't very 
> if-convertable.  :-)

Yeah, agreed.  The CBNZ approach seems more direct.

Richard

Reply via email to