public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Michael Matz <matz@suse.de>
To: Jakub Jelinek <jakub@redhat.com>
Cc: Andi Kleen <ak@linux.intel.com>,
	gcc-patches@gcc.gnu.org,
	 Richard Biener <richard.guenther@gmail.com>
Subject: Re: [PATCH v6 1/8] Improve must tail in RTL backend
Date: Tue, 4 Jun 2024 15:49:03 +0200 (CEST)	[thread overview]
Message-ID: <48d91e0f-129b-5cea-2f9e-edad9a023f23@suse.de> (raw)
In-Reply-To: <Zl36ww1xPsSsheqd@tucnak>

Hello,

On Mon, 3 Jun 2024, Jakub Jelinek wrote:

> > Hmm.  I count six tests in about 25 lines of code in 
> > tree-tailcall.cc:suitable_for_tail_opt_p and suitable_for_tail_call_opt_p.
> > 
> > Are you perhaps worrying about the sibcall discovery itself (i.e. much of 
> > find_tail_calls)?  Why would that be needed for musttail?  Is that 
> > attribute sometimes applied to calls that aren't in fact sibcall-able?
> > 
> > One thing I'm worried about is the need for a new sibcall pass at O0 just 
> > for sibcall discovery.  find_tail_calls isn't cheap, because it computes 
> > live local variables for the whole function, potentially being quadratic.
> 
> But the pass could be done only if there is at least one musttail call 
> in a function (remembered in some cfun flag).  If people use that 
> attribute, guess they are willing to pay for it.

Yeah, but I think the way the current proposal is doing it is mostly 
equivalent and fine enough, as Andi mentioned (in my worry I haven't 
considered that overall the backward walk stops fairly soon and then only 
does something when a musttail is there).

I still think that the tree pass being necessary for correctness is bad 
design, in the grand scheme of things, especially for those tests that are 
done for the call statement in isolation (i.e. tests about arguments like 
address-taken and suchlike, and return value, flags on the callee, and 
facts about the current function).  Those should all move to calls.cc or 
cfgexpand IMHO.

But I will yield on the discovery part that tree-tailcall is doing (i.e. 
those pieces that need to look at multiple statements, e.g. how the call 
result is used later); those are a bit harder to do in expand and how it's 
structured, and without getting rid of that part in tree-tailcall we have 
to run it at O0 anyway for musttail.  And moving only parts of the tests 
to calls.cc doesn't seem so worthwhile to hold up the patch.

So, I have no objections on the patch design anymore.


Ciao,
Michael.

  reply	other threads:[~2024-06-04 13:49 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-21 14:28 Musttail patchkit v6 Andi Kleen
2024-05-21 14:28 ` [PATCH v6 1/8] Improve must tail in RTL backend Andi Kleen
2024-05-29 13:39   ` Michael Matz
2024-05-31 18:00     ` Andi Kleen
2024-06-03 17:02       ` Michael Matz
2024-06-03 17:17         ` Jakub Jelinek
2024-06-04 13:49           ` Michael Matz [this message]
2024-06-03 17:31         ` Andi Kleen
2024-05-21 14:28 ` [PATCH v6 2/8] Add a musttail generic attribute to the c-attribs table Andi Kleen
2024-05-21 14:28 ` [PATCH v6 3/8] C++: Support clang compatible [[musttail]] (PR83324) Andi Kleen
2024-05-21 14:28 ` [PATCH v6 4/8] C: Implement musttail attribute for returns Andi Kleen
2024-05-21 14:28 ` [PATCH v6 5/8] Add tests for C/C++ musttail attributes Andi Kleen
2024-05-21 14:28 ` [PATCH v6 6/8] Enable musttail tail conversion even when not optimizing Andi Kleen
2024-05-21 14:28 ` [PATCH v6 7/8] Give better error messages for musttail Andi Kleen
2024-06-05  4:52   ` Andi Kleen
2024-05-21 14:28 ` [PATCH v6 8/8] Add documentation for musttail attribute Andi Kleen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=48d91e0f-129b-5cea-2f9e-edad9a023f23@suse.de \
    --to=matz@suse.de \
    --cc=ak@linux.intel.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jakub@redhat.com \
    --cc=richard.guenther@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).