Re: [PATCH] tree-optimization/108691 - indirect calls to setjmp

[Richard Biener <rguenther@suse.de>]

On Mon, 13 Feb 2023, Jakub Jelinek wrote:

> On Mon, Feb 13, 2023 at 12:00:56PM +0100, Richard Biener wrote:
> > DCE now chokes on indirect setjmp calls becoming direct because
> > that exposes them too late to be subject to abnormal edge creation.
> > The following patch honors gimple_call_ctrl_altering for those and
> > _not_ treat formerly indirect calls to setjmp as calls to setjmp.
> > 
> > Unfortunately there's no way to have an indirect call to setjmp
> > properly annotated (the returns_twice attribute is ignored on types).
> > 
> > RTL expansion late discovers returns-twice for the purpose of
> > adding REG_SETJMP notes and also sets ->calls_setjmp
> > (instead of asserting it is set).  There's no good way to
> > transfer proper knowledge around here so I'm using ->calls_setjmp
> > as a flag to indicate whether gimple_call_ctrl_altering was set.
> > 
> > Comments on what's the most sensible thing to do here?  Supporting
> > returns_twice on indirect calls wouldn't be difficult, so we're
> > talking about how to handle this kind of "legacy" situation?
> 
> One thing is supporting returns_twice on function types, but another one
> is that initially none of the calls will be marked that way and even later,
> it is up to the user if they mark it or not.

Yep.

> Could we e.g. prevent turning such indirect calls into direct calls?

We do exactly have gimple_call_fntype and gimple_call_ctrl_altering_p
to not require special-casing indirect to direct call promotion here.

> Anyway, notice_special_calls is called in various spots, not just DCE,
> wouldn't it be better to simply not set calls_setjmp flag in there if
> the current function already has cfg and the call isn't ctrl altering?

I thought about changing gimple_call_flags instead, filtering out
ECF_RETURNS_TWICE.  I just didn't make up my mind on what
property to key at (and to require 'cfun' to be set to query it).
But sure, changing notice_special_calls also works - the only
other relevant caller is the inliner I think, and that could be
replaced by caller |= callee of the two flags tracked instead of
re-scanning each inlined stmt.

Would you be happy with changing notice_special_calls, dropping the
DCE hunk but keeping the cfgexpand/calls.cc hunks?

Thanks,
Richard.