All of lore.kernel.org
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Sebastian Reichel <sre@kernel.org>,
	Nathan Chancellor <nathan@kernel.org>,
	Nick Desaulniers <ndesaulniers@google.com>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Linux PM <linux-pm@vger.kernel.org>
Subject: Re: [GIT PULL] power-supply fixes for 5.16
Date: Sat, 8 Jan 2022 12:10:25 -0800	[thread overview]
Message-ID: <CAHk-=wh55h__rC6+RTH7pLLTbnBuuOXPNzGGswVDGN7C6NW1pQ@mail.gmail.com> (raw)
In-Reply-To: <20220108112229.v3d7enmuibypa5tm@earth.universe>

On Sat, Jan 8, 2022 at 3:22 AM Sebastian Reichel <sre@kernel.org> wrote:
>
> 2. Replace 1E6L with NSEC_PER_MSEC to avoid floating point calculation
>    in LLVM resulting in a build failure

So I think the patch is obviously the right thing to do regardless,
but that still makes me go "WTF?"

Those uses of 1E6L were perhaps strange, but they were only used in
constant compile-time expressions that were cast to 'unsigned long' in
the end, so how the heck did llvm end up with any floating point in
there?

In this case there was really no excuse not to just use a better
constant, but there are other situations where it might well be quite
reasonable to use floating point calculations to create an integer
constant (eg maybe some spec describes an algorithm in FP, but the
implementation uses fixed-point arithmetic and has initializers that
do the conversion).

Imagine for a moment that you want to do fixed-point math (perhaps
because you have a microcontroller without an FP unit - it's not
limited to just "the kernel doesn't do FP"). Further imagine just for
a moment that you still want some fundamental constants like PI in
that fixed-point format.

The sane way to generate said constants is to do something like

      #define FIXPT_1 (1u << FIXPT_SHIFT)
      #define FIXPT_FP(x) ((fixpt_t) (((x)*FIXPT_1)+0.5))
      #define FIXPT_PI FIXPT_FP(3.14159265)

rather than have to do something incredibly stupid and illogical and
unreadable like

    #define FIXPT_PI 205887

So honestly, this seems to be just llvm being completely stupid. The
fact that you don't want to generate floating point code has *NOTHING*
to do with floating point literals for constant expressions.

In fact, even if you don't want to generate floating point code -
again, maybe you don't have a FP unit - doesn't mean that you might
not want to generate normal floating point constants. You may end up
having explicit software floating point, and doing things like passing
the floating point values around manually, ie

        union fp {
            uint64_t val;
            double fp_val;
       };

and having code like

        static const union fp sqrt2 = { .fp_val = 1.4142.... };

and then doing all the math in 'uint64_t' just because you wrote a
couple of math routines yourself:

        fp_mul(&res,  sqrt2.val, x);

again, there's no floating point *code* generated, but that doesn't
mean that the compiler shouldn't allow users to use floating point
*values*.

Sadly, I see in the LLVM discussions that the llvm people seem to be
completely out to lunch and in denial, and claim "maybe this is a gcc
bug".

No. Gcc just isn't being stupid.

Nathan, Nick, please talk some sense into the llvm people. The two
examples above are very much not about the Linux kernel (although I
think we actually have a couple of places that do things exactly like
that) they are about generic coding issues.

              Linus

  reply	other threads:[~2022-01-08 20:10 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-08 11:22 [GIT PULL] power-supply fixes for 5.16 Sebastian Reichel
2022-01-08 20:10 ` Linus Torvalds [this message]
2022-01-08 20:19 ` pr-tracker-bot

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='CAHk-=wh55h__rC6+RTH7pLLTbnBuuOXPNzGGswVDGN7C6NW1pQ@mail.gmail.com' \
    --to=torvalds@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=nathan@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=sre@kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.