Hi,
On Tue, Jan 28 2025, Richard Biener wrote:
> On Mon, 27 Jan 2025, Martin Jambor wrote:
>
>> Hi,
>>
>> Zhendong Su and Michal Jireš found out that our gimple DSE pass can,
>> under fairly specific conditions, remove a noreturn call which then
>> leaves behind a "normal" BB with no successor edges which following
>> passes do not expect. This patch simply tells the pass to leave such
>> calls alone even when they otherwise appear to be dead.
>>
>> Interestingly, our CFG verifier does not report this. I'll put on my
>> todo list to add a test for it in the next stage 1.
>>
>> Bootstrapped and tested on x86_64-linux, OK for master?
>>
>> Thanks,
>>
>> Martin
>>
>>
>> gcc/ChangeLog:
>>
>> 2025-01-27 Martin Jambor <mjam...@suse.cz>
>>
>> PR tree-optimization/117892
>> * tree-ssa-dse.cc (dse_optimize_call): Leave noreturn calls alone.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2025-01-27 Martin Jambor <mjam...@suse.cz>
>>
>> PR tree-optimization/117892
>> * gcc.dg/tree-ssa/pr117892.c: New test.
>> * gcc.dg/tree-ssa/pr118517.c: Likewise.
>>
>> co-authored-by: Michal Jireš <mji...@suse.cz>
>> ---
>> gcc/testsuite/gcc.dg/tree-ssa/pr117892.c | 17 +++++++++++++++++
>> gcc/testsuite/gcc.dg/tree-ssa/pr118517.c | 11 +++++++++++
>> gcc/tree-ssa-dse.cc | 5 +++--
>> 3 files changed, 31 insertions(+), 2 deletions(-)
>> create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr117892.c
>> create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr118517.c
>>
>> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr117892.c
>> b/gcc/testsuite/gcc.dg/tree-ssa/pr117892.c
>> new file mode 100644
>> index 00000000000..d9b9c15095f
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr117892.c
>> @@ -0,0 +1,17 @@
>> +/* { dg-do compile } */
>> +/* { dg-options "-O1" } */
>> +
>> +
>> +volatile int a;
>> +void b(int *c) {
>> + int *d = 0;
>> + *c = 0;
>> + *d = 0;
>> + __builtin_abort();
>> +}
>> +int main() {
>> + int f;
>> + if (a)
>> + b(&f);
>> + return 0;
>> +}
>> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr118517.c
>> b/gcc/testsuite/gcc.dg/tree-ssa/pr118517.c
>> new file mode 100644
>> index 00000000000..3a34f6788a9
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr118517.c
>> @@ -0,0 +1,11 @@
>> +/* { dg-do compile } */
>> +/* { dg-options "-O1 -fno-ipa-pure-const" } */
>> +
>> +void __attribute__((noreturn)) bar(void) {
>> + __builtin_unreachable ();
>> +}
>> +
>> +int p;
>> +void foo() {
>> + if (p) bar();
>> +}
>> diff --git a/gcc/tree-ssa-dse.cc b/gcc/tree-ssa-dse.cc
>> index 753d7ef148b..492080a7eb0 100644
>> --- a/gcc/tree-ssa-dse.cc
>> +++ b/gcc/tree-ssa-dse.cc
>> @@ -1396,8 +1396,9 @@ dse_optimize_call (gimple_stmt_iterator *gsi, sbitmap
>> live_bytes)
>> if (!node)
>> return false;
>>
>> - if (stmt_could_throw_p (cfun, stmt)
>> - && !cfun->can_delete_dead_exceptions)
>> + if ((stmt_could_throw_p (cfun, stmt)
>> + && !cfun->can_delete_dead_exceptions)
>> + || gimple_call_noreturn_p (stmt))
>
> Please use the same test as is used in tree-ssa-dce.cc:
>
> /* Never elide a noreturn call we pruned control-flow for. */
> if ((gimple_call_flags (call) & ECF_NORETURN)
> && gimple_call_ctrl_altering_p (call))
> {
> mark_stmt_necessary (call, true);
> return;
> }
OK, but just to make sure I understand: You do want me to leave the
!cfun->can_delete_dead_exceptions check there, right, so the condition
would become:
if ((stmt_could_throw_p (cfun, stmt)
&& !cfun->can_delete_dead_exceptions)
|| (gimple_call_flags (call) & ECF_NORETURN)
&& gimple_call_ctrl_altering_p (call))
Right?
On x86_64 at least we do not have a testcase that would exercise the
can_delete_dead_exceptions part of the condition - not even an Ada
testcase - so it is not easy to think about it for me.
And I also cannot stop wondering: Is explicitely checking for the
ECF_NORETURN bit really better than gimple_call_noreturn_p which does
exactly the same thing?
And perhaps more importantly, are there noreturn calls which are not
gimple_call_ctrl_altering_p? What are they?
Thanks,
Martin
