[Bug c++/109378] New: improve __builtin_sqrt

[Bug c++/109378] New: improve __builtin_sqrt

[g.peterhoff@t-online.de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109378

            Bug ID: 109378
           Summary: improve __builtin_sqrt
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: g.peterhoff@t-online.de
  Target Milestone: ---

Hello gcc team,
https://godbolt.org/z/Wa1rfxrPo
when I write a function that contains std::sqrt, it always contains the nan(?)
tests for the argument. E.g. sqrtf64.
If I use my_sqrt the tests are done inside sqrt and not in the calling function
- clear (because noinline).

Wouldn't it be better to rewrite __builtin_sqrt so that these tests are done
inside __builtin_sqrt and not already in the calling context?
This would have the advantage that std::sqrt would not "contaminate" the
calling function with conditional jumps and thus inflate it.
I can make this clear with foo vs. bar.
And of course __builtin_sqrt must be able to be vectorized automatically and
must be inline for certain contexts (e.g. __FAST_MATH__).

regards
Gero

[Bug c++/109378] improve __builtin_sqrt

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109378

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
 -fno-math-errno

[Bug c++/109378] improve __builtin_sqrt

[g.peterhoff@t-online.de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109378

--- Comment #2 from g.peterhoff@t-online.de ---
But this is of no use if I want to compile something "normally" without
compiler specific options.

[Bug c++/109378] improve __builtin_sqrt

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109378

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|                            |*-linux-gnu

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to g.peterhoff from comment #2)
> But this is of no use if I want to compile something "normally" without
> compiler specific options.

Note some libc implementations don't implement setting errno on math functions
(e.g. Darwin) and for those targets,  -fno-math-errno is already the default.

So if you don't need to check errno, then you can use  -fno-math-errno. C/C++
allows for an implementation to set errno on an error for math functions even.
So this is more about you wanting something different from what the
implementation does ...

[Bug c++/109378] improve __builtin_sqrt

[g.peterhoff@t-online.de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109378

--- Comment #4 from g.peterhoff@t-online.de ---
Hm. Maybe we misunderstood each other or I don't understand. I don't want to
set -fno-math-errno or any other compiler-specific flag. My intention is that
__builtin_sqrt doesn't "contaminate" the calling context with internals of
__builtin_sqrt, but simply returns the result.

[Bug c++/109378] improve __builtin_sqrt

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109378

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to g.peterhoff from comment #4)
> Hm. Maybe we misunderstood each other or I don't understand. I don't want to
> set -fno-math-errno or any other compiler-specific flag. My intention is
> that __builtin_sqrt doesn't "contaminate" the calling context with internals
> of __builtin_sqrt, but simply returns the result.

That is not how the builtins work ...

[Bug c++/109378] new builtin like __builtin_sqrt but does not set errno

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109378

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |enhancement

[Bug c++/109378] new builtin like __builtin_sqrt but does not set errno

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109378

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
>Wouldn't it be better to rewrite __builtin_sqrt so that these tests are done inside __builtin_sqrt and not already in the calling context?

NO, in fact it is designed this way to better optimize sqrt in the normal case.

[Bug c++/109378] new builtin like __builtin_sqrt but does not set errno

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109378

--- Comment #7 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Also you could do:
template <typename Type>
inline constexpr Type   my_sqrt(const Type x)   noexcept
{
    if (std::isless(x, 0.0)) __builtin_unreachable();
    return std::sqrt(x);
}

And GCC will optimize away the check for negative and NAN correctly on the
trunk.

[Bug c++/109378] new builtin like __builtin_sqrt but does not set errno

[g.peterhoff@t-online.de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109378

--- Comment #8 from g.peterhoff@t-online.de ---
But I don't want and can't use a version of std::sqrt that requires compiler
specific flags/options/__builtins and injects internals of
std::sqrt/__builtin_sqrt into the calling context/function.
I just want to have a very dumb std::sqrt that does its error handling
internally.
Sorry, but is that too much to ask?

[Bug c++/109378] new builtin like __builtin_sqrt but does not set errno

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109378

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |INVALID
            Summary|improve __builtin_sqrt      |new builtin like
                   |                            |__builtin_sqrt but does not
                   |                            |set errno
             Status|UNCONFIRMED                 |RESOLVED

--- Comment #9 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Then there is nothing to be done here. GCC is doing the correct thing. It is
inlining the error path for better optimizations. 
Since you don't want to use another flag or another option, or even another
builtin, there is nothing to be done here.

It just sounds like you don't want to program in C/C++ because of these
constraints.

[Bug c++/109378] new builtin like __builtin_sqrt but does not set errno

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109378

--- Comment #10 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
>This would have the advantage that std::sqrt would not "contaminate" the calling function with conditional jumps and thus inflate it.

Also what do you mean by inflate it? do you mean code size? Or do you think
partial inlining the error part is causing branch misses?

[Bug c++/109378] new builtin like __builtin_sqrt but does not set errno

[g.peterhoff@t-online.de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109378

g.peterhoff@t-online.de changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|INVALID                     |FIXED

--- Comment #11 from g.peterhoff@t-online.de ---
Ok, in detail:
std::sqrt/__builtin_sqrt performs the check for nan in the calling context.
This causes the following problems:
* the calling context contains error handling/conditional jumps, which have
nothing to do there but have to be handled in the error handling of std::sqrt
* Because this does NOT happen in your implementation of std::sqrt, the code
gets bloated, at the latest when a function contains more than one std::sqrt.

Therefore
* do complete error handling in std::sqrt/__builtin_sqrt
* so there is only one exact call for std::sqrt, which can/must be vectorized.

[Bug c++/109378] new builtin like __builtin_sqrt but does not set errno

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109378

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|FIXED                       |INVALID

--- Comment #12 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to g.peterhoff from comment #11)
> Ok, in detail:
> std::sqrt/__builtin_sqrt performs the check for nan in the calling context.
> This causes the following problems:

It is a check for less than 0; not NAN.

> * the calling context contains error handling/conditional jumps, which have
> nothing to do there but have to be handled in the error handling of std::sqrt

> * Because this does NOT happen in your implementation of std::sqrt, the code
> gets bloated, at the latest when a function contains more than one std::sqrt.

There could be a check on errno later on even ... There is no way for GCC to
know that.

> 
> Therefore
> * do complete error handling in std::sqrt/__builtin_sqrt
> * so there is only one exact call for std::sqrt, which can/must be
> vectorized.

Again it still cannot be vectorized because it needs to be exact. It is also
hard to detect if errno is not being used too.