On Thu, Mar 20, 2025 at 10:02 AM Andi Kleen <a...@firstfloor.org> wrote:
>
> On Thu, Mar 20, 2025 at 05:28:48PM +0100, Jakub Jelinek wrote:
> > On Thu, Mar 20, 2025 at 09:19:02AM -0700, Andi Kleen wrote:
> > > The inlining was just one of the issue, there are some related to
> > > different semantics of escaped locals. gcc always errors out while
> > > LLVM declares it undefined.
> > >
> > > But maybe we can fix that one too.
> >
> > I have 3 patches to be tested, the inline one, fnsplit one and ipa-icf one.
> > For the escaped locals, I guess we need to decide if [[clang::musttail]]
> > will be something different from [[gnu::musttail]] in GCC or not (and if
> > yes, what __attribute__((musttail)) will be).
>
> There were more differences. clang is better to handle various
> cases with structs at -O0 and then there are some target specific differences
> too (e.g. some of our targets always reject externs)
>
> But maybe it's good enough.
>
> I don't think multiple flavors of musttail are a good idea because
> it will be confusing to users. I guess we should just match what clang does
> even for gnu::musttail. Since they were the pioneer they get to chose
> the semantics.
>
>
> > The current difference in behavior is on
> > int foo (void);
> > void bar (int *);
> > int
> > baz (void)
> > {
> > int a;
> > bar (&a);
> > [[clang::musttail]] return foo ();
> > }
> > Clang makes the attribute not just a request to tail call it or fail,
> > but also changes behavior and says if the code ever accesses a from the
> > above during foo () (which without the attribute is completely valid), then
> > it is UB.
>
>
> So it could be as simple as that patch? It solves your test case at least
> for x86.
>
> diff --git a/gcc/tree-tailcall.cc b/gcc/tree-tailcall.cc
> index f97df31eb3cf..b87a92e95e3d 100644
> --- a/gcc/tree-tailcall.cc
> +++ b/gcc/tree-tailcall.cc
> @@ -722,8 +722,9 @@ find_tail_calls (basic_block bb, struct tailcall **ret,
> bool only_musttail,
> {
> if (local_live_vars)
> BITMAP_FREE (local_live_vars);
> - maybe_error_musttail (call,
> - _("call invocation refers to locals"));
> + /* Allow this for musttail to match clang semantics of
> musttail. */
> + if (gimple_call_must_tail_p (call))
> + continue;
It still would be a good thing to have a debug dump for this case.
Maybe even a warning.
Plus take:
```
int foo (int*);
void bar (int *);
int
baz (int*)
{
int a;
bar (&a);
int *b = &a;
[[clang::musttail]] return foo (b);
}
int
baz1 (int*)
{
int a;
bar (&a);
[[clang::musttail]] return foo (&a);
}
```
Note clang warns about baz1 and at this point GCC should be able to
warn about both cases too.
Thanks,
Andrew Pinski
> return;
> }
> else
> @@ -732,8 +733,9 @@ find_tail_calls (basic_block bb, struct tailcall **ret,
> bool only_musttail,
> if (bitmap_bit_p (local_live_vars, *v))
> {
> BITMAP_FREE (local_live_vars);
> - maybe_error_musttail (call,
> - _("call invocation refers to
> locals"));
> + /* Allow this for musttail to match clang semantics of
> musttail. */
> + if (gimple_call_must_tail_p (call))
> + continue;
> return;
> }
> }
