Re: [PATCH V2] Disable sched1 in functions that call setjmp

[Richard Biener via Gcc-patches]

> Am 24.12.2022 um 10:54 schrieb Jose E. Marchesi <jose.march...@oracle.com>:
> 
> 
>>>> Am 24.12.2022 um 09:11 schrieb Alexander Monakov via Gcc-patches 
>>>> <gcc-patches@gcc.gnu.org>:
>>> 
>>> 
>>>> On Fri, 23 Dec 2022, Qing Zhao wrote:
>>>> 
>>>> BTW, Why sched1 is not enabled on x86 by default?
>>> 
>>> Register allocation is tricky on x86 due to small number of general-purpose
>>> registers, and sched1 can make it even more difficult. I think before 
>>> register
>>> pressure modeling was added, sched1 could not be enabled because then 
>>> allocation
>>> would sometimes fail, and now there's no incentive to enable it, as it is 
>>> not so
>>> important for modern x86 CPUs. Perhaps someone else has a more comprehensive
>>> answer.
>>> 
>>>> Another question is:  As discussed in the original bug PR57067:
>>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57067 The root cause of this
>>>> issue related to the abnormal control flow edges (from setjmp/longjmp) 
>>>> cannot
>>>> be represented correctly at RTL stage, shall we fix this root cause 
>>>> instead? 
>>> 
>>> You'd need an experienced reviewer to work with you, especially on 
>>> high-level
>>> design decisions such as "How ABNORMAL_DISPATCHER should be represented on 
>>> RTL".
>>> I'm afraid it's not just a matter of applying a small patch in one place.
>> 
>> For nonlocal goto we Thread the abnormal dispatcher.  Of course by
>> regenerating abnormal edges, not by keeping and modifying them.  We
>> cannot re-generate the (optimal) set of abnormal edges for setjmp so
>> we want to preserve those edges.  But as you say it’s a very
>> non-trivial change.
> 
> Allright, so we have two short-term alternatives for at least remove the
> possibility that GCC generates wrong code for valid C when the scheduler
> is turned on:
> 
> a) To disable sched1 in functions that call setjmp.
> 
> b) To change deps_analyze_insn so instructions are not moved across
>   function calls before register allocation (!reload_completed).
> 
> Both patches fix our particular use cases and are regression free in
> aarch64-linux-gnu.
> 
> However, there is something I don't understand: wouldn't sched2
> introduce the same problem when -fsched2-use-superblocks is specified?
> In that case, the option a) would need to be expanded to disable sched2
> as well, and b) wouldn't have effect (!after_reload)?
> 
> Using -fsched2-use-superblocks doesn't trigger the problem in our use
> case.

I’d go with b) and revisit sched2 when we run into a testcase that’s mishandled.

Richard