Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

Clang-cl support for MSVC overload of __builtin_assume_aligned #41233

Open Quuxplusone opened 5 years ago

Quuxplusone commented 5 years ago
Bugzilla Link PR42263
Status CONFIRMED
Importance P normal
Reported by Kirsten Lee (kile@microsoft.com)
Reported on 2019-06-12 17:33:02 -0700
Last modified on 2019-08-15 20:58:30 -0700
Version unspecified
Hardware PC Windows NT
CC Casey@Carter.net, htmldeveloper@gmail.com, llvm-bugs@lists.llvm.org, neeilans@live.com, richard-llvm@metafoo.co.uk, rnk@google.com
Fixed by commit(s)
Attachments bug.cpp (234 bytes, text/plain)
Blocks
Blocked by
See also
Created attachment 22087
example

As part of the C++20 work for std::assume_aligned, MSVC has implemented an
intrinsic that has an already existing counterpart on clang, gcc, etc. The
declaration is almost identical to those except that it's constexpr. Function
declaration:
constexpr void* __builtin_assume_aligned(const void*, size_t, ... ) noexcept;

Clang/GCC delcaration:
void* __builtin_assume_aligned(const void*, size_t, ... ) noexcept;

Currently the MSVC intrinsic isn't accepted due to it's constexprness (or at
least on clang-cl it's not right now) but we'd like this to work on that front
end at some point.

Current error:
error: constexpr declaration of '__builtin_assume_aligned' follows non-
constexpr declaration
constexpr void* __builtin_assume_aligned(const void*, size_t, ... /* size_t
misalignmentOffset */) noexcept;
                ^
bug.cpp(2,17): note: previous declaration is here
Quuxplusone commented 5 years ago

Attached bug.cpp (234 bytes, text/plain): example

Quuxplusone commented 5 years ago
This strawman approach simply makes __builtin_assume_aligned constexpr in all
C++ compilations:

$ git diff
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 03b6cc91657..0706de3e59e 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -2004,6 +2004,7 @@ NamedDecl *Sema::LazilyCreateBuiltin(IdentifierInfo *II,
unsigned ID,
     return nullptr;

   DeclContext *Parent = Context.getTranslationUnitDecl();
+  ConstexprSpecKind CSK = CSK_unspecified;
   if (getLangOpts().CPlusPlus) {
     LinkageSpecDecl *CLinkageDecl =
         LinkageSpecDecl::Create(Context, Parent, Loc, Loc,
@@ -2011,14 +2012,14 @@ NamedDecl *Sema::LazilyCreateBuiltin(IdentifierInfo
*II, unsigned ID,
     CLinkageDecl->setImplicit();
     Parent->addDecl(CLinkageDecl);
     Parent = CLinkageDecl;
+
+    if (ID == Builtin::BI__builtin_assume_aligned)
+      CSK = CSK_constexpr;
   }

-  FunctionDecl *New = FunctionDecl::Create(Context,
-                                           Parent,
-                                           Loc, Loc, II, R, /*TInfo=*/nullptr,
-                                           SC_Extern,
-                                           false,
-                                           R->isFunctionProtoType());
+  FunctionDecl *New = FunctionDecl::Create(
+      Context, Parent, Loc, Loc, II, R, /*TInfo=*/nullptr, SC_Extern,
+      /*IsInlineSpecified=*/false, R->isFunctionProtoType(), CSK);
   New->setImplicit();

   // Create Decl objects for each parameter, adding them to the

It solves the redeclaration error immediately, but it's an awkward special case
in an important common codepath. Should we add a code to Builtins.def for this?
Are there are other builtins that we might wish to mark constexpr? The constant
evaluator seems to know a few.