From: Linus Torvalds <firstname.lastname@example.org>
To: Harry Wentland <email@example.com>
Cc: "Arnd Bergmann" <firstname.lastname@example.org>, "Leo Li" <email@example.com>,
"Alex Deucher" <firstname.lastname@example.org>,
"Christian König" <email@example.com>,
"Pan, Xinhui" <Xinhui.Pan@amd.com>,
"Nathan Chancellor" <firstname.lastname@example.org>,
"Guenter Roeck" <email@example.com>,
"Linux Kernel Mailing List" <firstname.lastname@example.org>,
"Nick Desaulniers" <email@example.com>,
Subject: Re: [PATCH] Enable '-Werror' by default for all kernel builds
Date: Tue, 7 Sep 2021 21:41:25 -0700 [thread overview]
Message-ID: <CAHk-=wjgi9jGiFHs7S2BPrGXKKOaVDqQsVPUsLWxkvZFAQ_WqQ@mail.gmail.com> (raw)
On Tue, Sep 7, 2021 at 8:52 PM Harry Wentland <firstname.lastname@example.org> wrote:
> Attached patches fix these x86_64 ones reported by Nick:
You didn't seem to fix up the calling convention for print__xyz(),
which still take those xyz structs as pass-by-value.
Obviously it would be good to do things incrementally, so if that
attached patch was just [1/N] I won't complain..
> I'm also seeing one more that might be more challenging to fix but is nearly at 1024:
> drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn21/display_mode_vba_21.c:3397:6: error: stack frame size of 1064 bytes in function 'dml21_ModeSupportAndSystemConfigurationFull' [-Werror,-Wframe-larger-than=]
Oh Gods, that function is truly something else..
Is there some reason why it's one humongous function, with the
occasional single-line comment?
Because it really looks to me like pretty much everywhere I see one of
those rare comments, I would go "this part should be a function of its
own", and then there would be one caller fuynction that just calls
each of those sub-functions one after the other.
That would - I think - make the code easier to read, and then it would
also make it very obvious where it magically uses a lot of stack.
My suspicion is actually "nowhere". The stack use is just hugely
spread out, and the compiler has just kept accumulating more spill
variables on the frame with no single big reason.
Yes, I see a couple of local structures:
but more than that I see several function calls that have basically 62
arguments. And I wish I was making that number up. I'm not. That
"CalculatePrefetchSchedule()" call literally has 62 arguments.
But *all* of the top-level loops in that function literally look like
they could - and should - be functions in their own right. Some of
them would be fairly complex even so (ie that code under the comment
would be quite the big function all of its own.
We have a coding style thing:
that says that you should strive to have functions that are "short and
sweet" and fit on one or two screenfuls of text.
That one function from hell is 1832 lines of code.
It really could be improved upon.
next prev parent reply other threads:[~2021-09-08 4:41 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-06 14:26 [PATCH] Enable '-Werror' by default for all kernel builds Guenter Roeck
2021-09-06 16:12 ` Linus Torvalds
2021-09-06 16:18 ` Linus Torvalds
2021-09-06 17:05 ` Guenter Roeck
2021-09-06 23:06 ` Linus Torvalds
2021-09-06 23:49 ` Guenter Roeck
2021-09-07 1:12 ` Linus Torvalds
2021-09-07 2:29 ` Guenter Roeck
2021-09-07 15:50 ` Guenter Roeck
2021-09-07 8:56 ` Arnd Bergmann
2021-09-08 4:28 ` Guenter Roeck
2021-09-08 4:48 ` Al Viro
2021-09-08 5:14 ` Guenter Roeck
2021-09-08 7:11 ` Geert Uytterhoeven
2021-09-08 9:50 ` Arnd Bergmann
2021-09-08 10:10 ` Geert Uytterhoeven
2021-09-08 10:21 ` Geert Uytterhoeven
2021-09-08 12:42 ` Guenter Roeck
2021-09-08 13:19 ` Al Viro
2021-09-08 13:54 ` Guenter Roeck
2021-09-08 14:47 ` David Laight
2021-09-08 4:55 ` Linus Torvalds
2021-09-08 5:46 ` Guenter Roeck
2021-09-07 5:32 ` Huang Rui
2021-09-07 6:15 ` Christian König
2021-09-07 6:58 ` Geert Uytterhoeven
2021-09-07 2:30 ` Nathan Chancellor
2021-09-07 9:11 ` Arnd Bergmann
2021-09-07 17:10 ` Linus Torvalds
2021-09-07 17:33 ` Linus Torvalds
2021-09-07 21:07 ` Harry Wentland
2021-09-08 3:52 ` Harry Wentland
2021-09-08 4:41 ` Linus Torvalds [this message]
2021-09-09 0:48 ` Harry Wentland
2021-09-07 17:48 ` Guenter Roeck
2021-09-07 19:12 ` Nathan Chancellor
2021-09-08 20:55 ` Nathan Chancellor
2021-09-08 21:16 ` Guenter Roeck
2021-09-08 21:58 ` Marco Elver
2021-09-09 5:58 ` Christoph Hellwig
2021-09-09 6:07 ` Guenter Roeck
2021-09-09 7:30 ` Christian König
2021-09-09 14:59 ` Guenter Roeck
2021-09-09 10:53 ` Marco Elver
2021-09-09 11:00 ` Arnd Bergmann
2021-09-09 11:43 ` Marco Elver
2021-09-09 12:55 ` Arnd Bergmann
2021-09-09 16:53 ` Linus Torvalds
2021-09-09 16:46 ` Linus Torvalds
2021-09-21 15:41 ` Arnd Bergmann
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:
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
* 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 NNTP newsgroup(s).