Bug 118430 - [14 Regression] tail call vs IPA-VRP return value range with constant value
Summary: [14 Regression] tail call vs IPA-VRP return value range with constant value
Status: ASSIGNED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 15.0
: P3 enhancement
Target Milestone: 14.5
Assignee: Jakub Jelinek
URL:
Keywords: missed-optimization, rejects-valid, tail-call
Depends on:
Blocks: 118444
  Show dependency treegraph
 
Reported: 2025-01-12 07:01 UTC by Ken Jin
Modified: 2026-06-26 09:26 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2025-01-12 00:00:00


Attachments
testcase that shows this is a regression for -O2+ (189 bytes, text/plain)
2025-01-12 08:46 UTC, Drea Pinski
Details
gcc15-pr118430.patch (2.13 KB, patch)
2025-01-14 15:31 UTC, Jakub Jelinek
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ken Jin 2025-01-12 07:01:18 UTC
Dear GCC maintainers,

The following code gives a compilation error, when compiling with GCC 15.0 (experimental) commit c1729df6ec1eff4815a9cdd71392691ce21da028, on Ubuntu 22.04 AMD64. In Clang 19.1.0, this compiles successfully, producing working code:

test.c
```
#include "stdio.h"

__attribute__((noinline))
int tail_call2(int opcode) {
    printf("%d\n", opcode);
    return 1;
}

__attribute__((noinline))
int non_tail_call(int *ptr) {
    printf("%d\n", *ptr);
    return 2;
}

int tail_call1(int opcode) {
    int val;
    printf("%d\n", opcode);
    non_tail_call(&val); // Trigger the bug by passing in a local.
    [[gnu::musttail]]
    return tail_call2(opcode);
}


int main(int argc, char **argv) {
    tail_call1(argc);
}
```

Compilation error depends on options passed.

gcc test.c gives:

test.c: In function ‘tail_call1’:
test.c:19:12: error: cannot tail-call: call invocation refers to locals
   19 |     return tail_call2(opcode);
      |            ^~~~~~~~~~~~~~~~~~

gcc -O1 test.c passes


gcc -O2/3 test.c gives:

test.c: In function ‘tail_call1’:
test.c:19:12: error: cannot tail-call: call uses return slot
   19 |     return tail_call2(opcode);
      |            ^~~~~~~~~~~~~~~~~~

Output in first case and third case don't seem correct. In the first error, the tail call itself is not referring to a local, only a previous non-function tail call is. In the 3rd case, I don't see how it's using a return slot: the return value is not being assigned. I might be mistaken on what a "return slot" is in this case as I'm not a GCC expert though.

My background use case is implementing an optimization for CPython's interpreter. I have a PR that implements a nontrivial amount of code using musttail. The following PR works fully on clang-19, but fails to compile on GCC after applying patches to make it detect GCC: https://github.com/python/cpython/pull/128718

Thank you for your time and help.

Possibly related: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115979

Regards,
Ken Jin
Comment 1 Drea Pinski 2025-01-12 07:06:44 UTC
Must have decided that val escapes and can be referenced by the call tail_call2.

I am not 100% sure if this is valid or not.

Rewriting the code like:
int tail_call1(int opcode) {
{
    int val;
    printf("%d\n", opcode);
    non_tail_call(&val); // Trigger the bug by passing in a local.

}
    [[gnu::musttail]]
    return tail_call2(opcode);
}

Will cause the val not be escaping for tail_call2 as it goes out of scope before the call to tail_call2.
Comment 2 Ken Jin 2025-01-12 07:15:20 UTC
Thanks for the quick response.

Is the second case in the initial post on the return slot error expected?
Comment 3 Drea Pinski 2025-01-12 07:18:36 UTC
I have to double check but maybe the error message is just wrong.
Comment 4 Drea Pinski 2025-01-12 08:32:22 UTC
Oh for -O2, this is IPA VRP coming into play.

It is turning:
    [[gnu::musttail]]
    return tail_call2(opcode);

into

tail_call2(opcode);
return 2;


Which needs to go in the opposite direction and such.

Note if you use noipa instead of noinline, this testcase will work.

I also highly doubt this is representative of the CPython code either.
Comment 5 Drea Pinski 2025-01-12 08:40:31 UTC
(In reply to Andrew Pinski from comment #4)
> Oh for -O2, this is IPA VRP coming into play.
> 
> It is turning:
>     [[gnu::musttail]]
>     return tail_call2(opcode);
> 
> into
> 
> tail_call2(opcode);
> return 2;

s/2/1/ .

The error message is definitely wrong.

```
  /* We may proceed if there either is no return value, or the return value
     is identical to the call's return or if the return decl is an empty type
     variable and the call's return was not assigned. */
  if (ret_var
      && (ret_var != ass_var
          && !(is_empty_type (TREE_TYPE (ret_var)) && !ass_var)))
    {
      maybe_error_musttail (call, _("call uses return slot"));
      return;
    }
```

From EVRP:
Using return value range of tail_call2: [irange] int [1, 1]

tail call should be able to check the call's return value range to see if it same as the value here.

The -O0 case is due to -O0 does do any optimizations and I highly doubt it can handle musttail at all.
Comment 6 Drea Pinski 2025-01-12 08:46:39 UTC
Created attachment 60120 [details]
testcase that shows this is a regression for -O2+

In GCC 13, GCC would produce:
jmp     tail_call2

Now it does do a tail call due to proping the 1.
Comment 7 Drea Pinski 2025-01-12 08:50:46 UTC
>fails to compile on GCC after applying patches to make it detect GCC: https://github.com/python/cpython/pull/128718

Maybe the best thing is to file a bug report with a non-reduce testcase since -O2+ here is a specific issue with IPA VRP return values causing the tail call to be missed and I am 99% sure it is unrelated to what is going wrong with cpython.
Comment 8 Ken Jin 2025-01-12 08:57:13 UTC
Thanks a lot for your help on this! I think I've narrowed down what's happening with CPython. It seems what's happening in CPython is not a bug, but should be a feature request. I will file a separate feature request for that.

Thanks again for your time.
Comment 9 Drea Pinski 2025-01-12 20:12:03 UTC
This patch fixes the error message but not the missed optimization/reason why it is rejected:
```
diff --git a/gcc/tree-tailcall.cc b/gcc/tree-tailcall.cc
index d6d7eb4b47c..8e9fa0cca9f 100644
--- a/gcc/tree-tailcall.cc
+++ b/gcc/tree-tailcall.cc
@@ -833,7 +833,7 @@ find_tail_calls (basic_block bb, struct tailcall **ret, bool only_musttail,
       && (ret_var != ass_var
          && !(is_empty_type (TREE_TYPE (ret_var)) && !ass_var)))
     {
-      maybe_error_musttail (call, _("call uses return slot"));
+      maybe_error_musttail (call, _("call and return value are different"));
       return;
     }
```

I am still trying to figure out the place to get the range. 

To get the range from the call is simple as:
```
  tree callee = gimple_call_fndecl (call);
  if (callee
      && useless_type_conversion_p (TREE_TYPE (TREE_TYPE (callee)), type))
    {
      value_range val;
      if (ipa_return_value_range (val, callee)
          && val.singleton_p ())
	{
	  ....
	}
    }

```
But I don't see where the place to put this.
Comment 10 Jakub Jelinek 2025-01-14 12:36:58 UTC
I'm really surprised we used to tail call it at all in the #c6 testcase at all, must be some IPA optimization which proves that non_tail_call does not do say *ptr = 42; p = ptr; for some global int *p; and tail_call2 then say not checking if *p == 42 or similar, because then it can't be tail called.  If noipa is used on them, then we don't tail call for that very reason (correctly).
So, using musttail with val still in scope is a bad idea, you rely on inter-procedural optimizations figuring that out, better put val in a new {} scope and end that before the musttail call like in #c1.

That said, I'm afraid it is really hard to avoid the folding of uses of singleton ranges into constants, so perhaps better might be to check for this in the tailc pass and still keep there the constant in return, and then handle it right during expansion.
Comment 11 Jakub Jelinek 2025-01-14 15:31:12 UTC
Created attachment 60153 [details]
gcc15-pr118430.patch

Untested fix.
Comment 12 GCC Commits 2025-01-16 08:28:16 UTC
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:9c4397cafc5ded9b008a92a55d4e5207e1c2e4e4

commit r15-6943-g9c4397cafc5ded9b008a92a55d4e5207e1c2e4e4
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Jan 16 09:20:15 2025 +0100

    tailc: Virtually undo IPA-VRP return value optimization for tail calls [PR118430]
    
    When we have return somefn (whatever); where somefn is normally tail
    callable and IPA-VRP determines somefn returns a singleton range, VRP
    just changes the IL to
      somefn (whatever);
      return 42;
    (or whatever the value in that range is).  The introduction of IPA-VRP
    return value tracking then effectively regresses the tail call optimization.
    This is even more important if the call is [[gnu::musttail]].
    
    So, the following patch queries IPA-VRP whether a function returns singleton
    range and if so and the value returned is identical to that, marks the
    call as [tail call] anyway.  If expansion decides it can't use the tail
    call, we'll still expand the return 42; or similar statement, and if it
    decides it can use the tail call, that part will be ignored and we'll emit
    normal tail call.
    
    The reason it works is that the expand pass relies on the tailc pass to
    do its job properly.
    E.g. when we have
      <bb 2> [local count: 1073741824]:
      foo (x_2(D));
      baz (&v);
      v ={v} {CLOBBER(eos)};
      bar (x_2(D)); [tail call]
      return 1;
    when expand_gimple_basic_block handles the bar (x_2(D)); call, it uses
              if (call_stmt && gimple_call_tail_p (call_stmt))
                {
                  bool can_fallthru;
                  new_bb = expand_gimple_tailcall (bb, call_stmt, &can_fallthru);
                  if (new_bb)
                    {
                      if (can_fallthru)
                        bb = new_bb;
                      else
                        {
                          currently_expanding_gimple_stmt = NULL;
                          return new_bb;
                        }
                    }
                }
    As it is actually tail callable during expansion of the bar (x_2(D)); call
    stmt, expand_gimple_tailbb returns non-NULL and sets can_fallthru to false,
    plus emits
    ;; bar (x_2(D)); [tail call]
    
    (insn 11 10 12 2 (set (reg:SI 5 di)
            (reg/v:SI 99 [ x ])) "pr118430.c":35:10 -1
         (nil))
    
    (call_insn/j 12 11 13 2 (set (reg:SI 0 ax)
            (call (mem:QI (symbol_ref:DI ("bar") [flags 0x3]  <function_decl 0x7fb39020bd00 bar>) [0 bar S1 A8])
                (const_int 0 [0]))) "pr118430.c":35:10 -1
         (expr_list:REG_CALL_DECL (symbol_ref:DI ("bar") [flags 0x3]  <function_decl 0x7fb39020bd00 bar>)
            (expr_list:REG_EH_REGION (const_int 0 [0])
                (nil)))
        (expr_list:SI (use (reg:SI 5 di))
            (nil)))
    
    (barrier 13 12 0)
    Because it doesn't fallthru, no further statements in the same bb are
    expanded.  Now, if the bb with return happened to be in some other basic
    block from the [tail call], it could be expanded but because the bb with
    tail call ends with a barrier, it doesn't fall thru there and if nothing
    else could reach it, we'd remove the unreachable bb RSN.
    
    2025-01-16  Jakub Jelinek  <jakub@redhat.com>
                Andrew Pinski  <quic_apinski@quicinc.com>
    
            PR tree-optimization/118430
            * tree-tailcall.cc: Include gimple-range.h, alloc-pool.h, sreal.h,
            symbol-summary.h, ipa-cp.h and ipa-prop.h.
            (find_tail_calls): If ass_var is NULL and ret_var is not, check if
            IPA-VRP has not found singleton return range for it.  In that case,
            don't punt if ret_var is the only value in that range.  Adjust the
            maybe_error_musttail message otherwise to diagnose different value
            being returned from the caller and callee rather than using return
            slot.  Formatting fixes.
    
            * c-c++-common/musttail14.c: New test.
            * c-c++-common/pr118430.c: New test.
Comment 13 GCC Commits 2025-01-16 08:28:22 UTC
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:7f5adfd31b3af08924faec36679eaea40a98af19

commit r15-6944-g7f5adfd31b3af08924faec36679eaea40a98af19
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Jan 16 09:25:16 2025 +0100

    tree-ssa-propagate: Special case lhs of musttail calls in may_propagate_copy [PR118430]
    
    This patch ensures that VRP or similar passes don't replace the uses of lhs of
    [[gnu::musttail]] calls with some constant (e.g. if the call is known is known
    to return a singleton value range) etc. to make it more likely that it is actually
    tail callable.
    
    2025-01-16  Jakub Jelinek  <jakub@redhat.com>
    
            PR tree-optimization/118430
            * tree-ssa-propagate.cc (may_propagate_copy): Return false if dest
            is lhs of an [[gnu::musttail]] call.
            (substitute_and_fold_dom_walker::before_dom_children): Formatting fix.
    
            * c-c++-common/musttail14.c: Expect lhs on the must tail call calls.
Comment 14 Jakub Jelinek 2025-01-16 08:38:49 UTC
Fixed on the trunk.  Not sure about the backport, it is less important in 14 because there is no musttail and the patch is a new optimization which can e.g. introduce tail calls where they weren't before.
Comment 15 Richard Biener 2025-05-23 11:15:07 UTC
GCC 14.3 is being released, retargeting bugs to GCC 14.4.
Comment 16 Jakub Jelinek 2026-06-26 09:26:49 UTC
GCC 14.4 is being released, retargeting bugs to GCC 14.5.