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