[clang-cl] implement support for [[msvc::intrinsic]]

[nico]

See https://devblogs.microsoft.com/cppblog/improving-the-state-of-debug-performance-in-c/

"""
The new attribute will semantically replace a function call with a cast to that function’s return type if the function definition is decorated with [[msvc::intrinsic]]. You can see how we applied this new attribute in the STL: GH3182. The reason the compiler decided to go down the attribute route is that we want to eventually extend the scenarios it can cover and offer a data-driven approach to selectively decorate code with the new functionality. The latter is important for users of MSVC as well.
"""

[llvmbot]

@llvm/issue-subscribers-clang-frontend

[MaskRay]

https://learn.microsoft.com/en-us/cpp/cpp/attributes?view=msvc-170#msvcintrinsic says

When the attribute is present on a function definition, the compiler replaces all calls to that function with a simple cast.

So the number of possible useful functions is very small.

Clang 15 -O0 codegen is good as well.
It just hard codes std::move/std::move_if_noexcept/std::forward/std::as_const as builtin functions (https://reviews.llvm.org/D123345 thanks to @zygoloid ).
So perhaps at this point supporting a generic intrinsic (well, not that generic anyway) is no longer useful.

#include <utility>

int main()
{
    return std::move(0);
}

=>

clang 15 (before 15, there is a function call)
main:                                   # @main
        push    rbp
        mov     rbp, rsp
        mov     dword ptr [rbp - 4], 0
        mov     dword ptr [rbp - 8], 0
        mov     eax, dword ptr [rbp - 8]
        pop     rbp
        ret

Use -fno-builtin-std-move to get the previous behavior with a function call.

[frederick-vs-ja]

So the number of possible useful functions is very small.

I guess a generic attribute is still better than hard coding, as the number is not so that small (see microsoft/STL#3548).

[Swiftkill]

So the number of possible useful functions is very small.

Actually it mangles std::move behaviour.

#include<iostream>

struct A{
    ~A() { puts(__FUNCTION__); }
};

template <class Ty>//Added an attribute, behavior equivalent to std::move in /std:c++latest, non-standard, extended lifetime
[[msvc::intrinsic]] constexpr std::remove_reference_t<Ty>&& mmove(Ty&& Arg) noexcept {
    return static_cast<std::remove_reference_t<Ty>&&>(Arg);
}

template <class Ty>
constexpr std::remove_reference_t<Ty>&& nmove(Ty&& Arg) noexcept {
    return static_cast<std::remove_reference_t<Ty>&&>(Arg);
}

int main()
{
    {
        A&& rvalue1 = mmove(A{});
        puts(__FUNCTION__);
    }
    puts("------");
    {
        A&& rvalue2 = nmove(A{});
        puts(__FUNCTION__);
    }
}

std::move in MSVC 2022 currenty exhibits non-standard behaviour, because life time of rvalue1 is extended. Why? Because code is equivalent to

    {
        A&& rvalue1 = static_cast<A&&>(A{});
        puts(__FUNCTION__);
    }

Instead of binding an rvalue to a forwarding reference and then casting it to rvalue reference, only a binding of rvalue to rvalue reference occurs here. The output is:

main
A::~A()

It might be an intentional side-effect of attribute to "hack" around this limitation albeit documentation doesn't notify about it. Declaring std::move as [[msvc::intrinsic]] is a clear mistake, it breaches the as-if rule.

[frederick-vs-ja]

@Swiftkill Hi, I've reported the bug to Visual Studio Developer Community with your example and slight modifications.

[MaxEW707]

I have a PR up that went with a different approach. #76596.
This also helps us with vendor forks of clang on other platforms.

I know this doesn't exactly map to the [[msvc::intrinsic]] attribute but it gives us the same facility in clang including cl mode for the std meta functions and is easily wrapped behind a macro which we already have to do anyways to support VS2017-VS2022.

I went this route instead of an intrinsic cast attribute so that move and other std specific warnings still work with custom STL implementations. We may be able to extend it in the future as C++ continues to require the compiler to poke into the std namespace for things.

We have also noticed this non-standard behaviour with [[msvc::intrinsic]] lifetime extension in our unit tests but thankfully we haven't had to disable it yet because we also use plain static_cast in a lot of places for debug performance before we got this attribute. The debug performance is just too vital for us.

We also don't have worry about all the edge cases with just replacing the function call with static_cast<RetT>(arg) such as this bug report, https://developercommunity.visualstudio.com/t/Functions-defined-with-msvc::intrinsic/10305628?entry=problem&q=The+%E2%80%98ConversationsPackage%E2%80%99+package+did+not+load+correctly, with the direction I went in my PR.

In the future if we do add some intrinsic cast or go down the same reasoning as msvc, "The reason the compiler decided to go down the attribute route is that we want to eventually extend the scenarios it can cover", ideally it would also work with all small functions like to_integer(std::byte) that also just return its input argument or even all the operator[*/->]() on objects that just return a member variable. My 2c.