[PATCH] D47344: LWG 2843 "Unclear behavior of std::pmr::memory_resource::do_allocate()"

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

@Quuxplusone if this fixes 2843 can you update it in `www/cxx2a_status.html`?


Repository:
  rCXX libc++

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D47344/new/

https://reviews.llvm.org/D47344



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47109: LWG 2969 "polymorphic_allocator::construct() shouldn't pass resource()"

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.
Herald added subscribers: libcxx-commits, ldionne.

We also need to mark this off in the status. I can do that in D58879 
 or create a separate patch.


Repository:
  rCXX libc++

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D47109/new/

https://reviews.llvm.org/D47109



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43159: Modernize: Use nullptr more.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added inline comments.



Comment at: include/memory:1259
 template  static __two __test(...);
 template  static char __test(typename _Xp::template 
rebind<_Up>::other* = 0);
 public:

This could be `nullptr` too. 


Repository:
  rCXX libc++

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D43159/new/

https://reviews.llvm.org/D43159



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44865: [libc++] Implement P0608R3 - A sane variant converting constructor

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

A few minor things:

- the spacing is different from the rest of libc++ (a lot of it was that way 
before).
- since this is a paper it should probably be guarded with `#if _LIBCPP_STD_VER 
> 17`
- update the status in `www`.

Also, the standard says explicitly, "A variant is permitted to hold the same 
type more than once, and to hold differently cv-qualified versions of the same 
type." But currently, that is not allowed — not an issue with this patch, but 
something that should be fixed.




Comment at: include/variant:1110
+template 
+struct __overload_bool : _Base {
+  using _Base::operator();

Is this structure 100% necessary? Couldn't `__overload` add an overload to take 
care of `bool`s? Maybe something like this:
 
```
  template
  auto operator()(_Tp, _Up&&) const
->  enable_if_t<
is_same_v<__uncvref_t<_Up>, bool>,
__identity<_Tp>
>;
```


Repository:
  rCXX libc++

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D44865/new/

https://reviews.llvm.org/D44865



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44865: [libc++] Implement P0608R3 - A sane variant converting constructor

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added inline comments.



Comment at: include/variant:1128
+template 
+struct __overload
+: __overload_bool<__overload<_Types...>, bool const volatile> {};

EricWF wrote:
> Do we even support volatile types in variant?
[[ http://eel.is/c++draft/variant.variant#2 | Yes. ]]

> All types in Types shall be (possibly cv-qualified) object types that are not 
> arrays.


Repository:
  rCXX libc++

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D44865/new/

https://reviews.llvm.org/D44865



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66862: Make lround builtin constexpr (and others)

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

Ping. Anything else I am missing?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66862/new/

https://reviews.llvm.org/D66862



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67588: Add builtin trait for add/remove cv (and similar)

[Zoe Carver via Phabricator via cfe-commits]

zoecarver created this revision.
zoecarver added reviewers: EricWF, eli.friedman, rsmith, craig.topper, 
mclow.lists.
zoecarver added projects: clang, libc++.
zoecarver updated this revision to Diff 220226.
zoecarver added a comment.

- diff from D67052 , not master


This patch adds six builtins: `__remove_cv`, `__remove_cosnt`, 
`__remove_volatile`, `__add_cv`, `__add_const`, and `__add_volatile`. I have 
added two stress tests to show the performace improvements. The `__remove_cv`  
test sees a 160% build time imporvement while the `__add_cv` test sees an 8% 
improvement.

This patch is based on D67052 .

I will submit a patch for implementing these in libc++ after this lands.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67588

Files:
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/DeclSpec.h
  clang/lib/Format/FormatToken.cpp
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaTemplateVariadic.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/SemaCXX/add_cv.cpp
  clang/test/SemaCXX/add_reference.cpp
  libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_add_cv.sh.cpp
  
libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_add_lvalue_reference.sh.cpp
  
libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_add_rvalue_reference.sh.cpp
  libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_remove_cv.sh.cpp
  
libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_remove_reference.sh.cpp

Index: libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_remove_reference.sh.cpp
===
--- /dev/null
+++ libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_remove_reference.sh.cpp
@@ -0,0 +1,63 @@
+//===--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This is a dummy feature that prevents this test from running by default.
+
+// The table below compares the compile time and object size for each of the
+// variants listed in the RUN script.
+//
+//  Impl   Compile Time  Object Size
+// --
+// new_remove_reference:   22.849 s  121 K
+// std::remove_reference:  25.643 s  121 K
+//
+// RUN: %cxx %flags %compile_flags -c %s -o %S/orig.o -ggdb  -ggnu-pubnames -ftemplate-depth=5000 -ftime-trace -std=c++17
+// RUN: %cxx %flags %compile_flags -c %s -o %S/new.o -ggdb  -ggnu-pubnames -ftemplate-depth=5000 -ftime-trace -std=c++17 -DTEST_NEW
+
+#include 
+#include 
+
+#include "test_macros.h"
+#include "template_cost_testing.h"
+
+template  struct Arg { enum { value = 1 }; };
+
+#ifdef TEST_NEW
+
+template 
+struct new_remove_reference
+{
+  typedef __remove_reference(T) type;
+};
+
+#define TEST_CASE_NOP()  new_remove_reference< Arg< __COUNTER__ > >{},
+#define TEST_CASE_TYPE() typename new_remove_reference< Arg< __COUNTER__ > >::type,
+
+#else
+
+#define TEST_CASE_NOP()  std::remove_reference< Arg< __COUNTER__ > >{},
+#define TEST_CASE_TYPE() typename std::remove_reference< Arg< __COUNTER__ > >::type,
+
+#endif
+
+int sink(...);
+
+int x = sink(
+  REPEAT_1(TEST_CASE_NOP)
+  REPEAT_1(TEST_CASE_NOP) 42
+);
+
+void Foo( REPEAT_1(TEST_CASE_TYPE) int) { }
+
+void escape() {
+
+sink(&x);
+sink(&Foo);
+}
+
+
Index: libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_remove_cv.sh.cpp
===
--- /dev/null
+++ libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_remove_cv.sh.cpp
@@ -0,0 +1,62 @@
+//===--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This is a dummy feature that prevents this test from running by default.
+
+// The table below compares the compile time and object size for each of the
+// variants listed in the RUN script.
+//
+//  Impl   Compile Time  Object Size
+// --
+// new_remove_cv:   23.594 s  121 K
+// std::remove_cv:  38.817 s  121 K
+

[PATCH] D67588: Add builtin trait for add/remove cv (and similar)

[Zoe Carver via Phabricator via cfe-commits]

zoecarver updated this revision to Diff 220226.
zoecarver added a comment.

- diff from D67052 , not master


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67588/new/

https://reviews.llvm.org/D67588

Files:
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/DeclSpec.h
  clang/lib/Format/FormatToken.cpp
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaTemplateVariadic.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/SemaCXX/add_cv.cpp
  clang/test/SemaCXX/add_reference.cpp
  libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_add_cv.sh.cpp
  
libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_add_lvalue_reference.sh.cpp
  
libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_add_rvalue_reference.sh.cpp
  libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_remove_cv.sh.cpp
  
libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_remove_reference.sh.cpp

Index: libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_remove_reference.sh.cpp
===
--- /dev/null
+++ libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_remove_reference.sh.cpp
@@ -0,0 +1,63 @@
+//===--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This is a dummy feature that prevents this test from running by default.
+
+// The table below compares the compile time and object size for each of the
+// variants listed in the RUN script.
+//
+//  Impl   Compile Time  Object Size
+// --
+// new_remove_reference:   22.849 s  121 K
+// std::remove_reference:  25.643 s  121 K
+//
+// RUN: %cxx %flags %compile_flags -c %s -o %S/orig.o -ggdb  -ggnu-pubnames -ftemplate-depth=5000 -ftime-trace -std=c++17
+// RUN: %cxx %flags %compile_flags -c %s -o %S/new.o -ggdb  -ggnu-pubnames -ftemplate-depth=5000 -ftime-trace -std=c++17 -DTEST_NEW
+
+#include 
+#include 
+
+#include "test_macros.h"
+#include "template_cost_testing.h"
+
+template  struct Arg { enum { value = 1 }; };
+
+#ifdef TEST_NEW
+
+template 
+struct new_remove_reference
+{
+  typedef __remove_reference(T) type;
+};
+
+#define TEST_CASE_NOP()  new_remove_reference< Arg< __COUNTER__ > >{},
+#define TEST_CASE_TYPE() typename new_remove_reference< Arg< __COUNTER__ > >::type,
+
+#else
+
+#define TEST_CASE_NOP()  std::remove_reference< Arg< __COUNTER__ > >{},
+#define TEST_CASE_TYPE() typename std::remove_reference< Arg< __COUNTER__ > >::type,
+
+#endif
+
+int sink(...);
+
+int x = sink(
+  REPEAT_1(TEST_CASE_NOP)
+  REPEAT_1(TEST_CASE_NOP) 42
+);
+
+void Foo( REPEAT_1(TEST_CASE_TYPE) int) { }
+
+void escape() {
+
+sink(&x);
+sink(&Foo);
+}
+
+
Index: libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_remove_cv.sh.cpp
===
--- /dev/null
+++ libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_remove_cv.sh.cpp
@@ -0,0 +1,62 @@
+//===--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This is a dummy feature that prevents this test from running by default.
+
+// The table below compares the compile time and object size for each of the
+// variants listed in the RUN script.
+//
+//  Impl   Compile Time  Object Size
+// --
+// new_remove_cv:   23.594 s  121 K
+// std::remove_cv:  38.817 s  121 K
+//
+// RUN: %cxx %flags %compile_flags -c %s -o %S/orig.o -ggdb  -ggnu-pubnames -ftemplate-depth=5000 -ftime-trace -std=c++17
+// RUN: %cxx %flags %compile_flags -c %s -o %S/new.o -ggdb  -ggnu-pubnames -ftemplate-depth=5000 -ftime-trace -std=c++17 -DTEST_NEW
+
+#include 
+
+#include "test_macros.h"
+#include "template_cost_testing.h"
+
+template  struct Arg { enum { value = 1 }; };
+
+#ifdef TEST_NEW
+
+template 
+struct new_remove_cv
+{
+  typedef __remove_cv(T) type;
+};
+
+#define TEST_CASE_NOP()  new_remove_cv< Arg< __COUNTER__ > >{},
+#d

[PATCH] D67588: Add builtin trait for add/remove cv (and similar)

[Zoe Carver via Phabricator via cfe-commits]

zoecarver updated this revision to Diff 220227.
zoecarver added a comment.

Generate diff based on D67052  (arc wasn't 
working so I had to do it manually this time).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67588/new/

https://reviews.llvm.org/D67588

Files:
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/DeclSpec.h
  clang/lib/Format/FormatToken.cpp
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaTemplateVariadic.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/SemaCXX/add_cv.cpp
  libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_add_cv.sh.cpp
  libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_remove_cv.sh.cpp

Index: libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_remove_cv.sh.cpp
===
--- /dev/null
+++ libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_remove_cv.sh.cpp
@@ -0,0 +1,62 @@
+//===--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This is a dummy feature that prevents this test from running by default.
+
+// The table below compares the compile time and object size for each of the
+// variants listed in the RUN script.
+//
+//  Impl   Compile Time  Object Size
+// --
+// new_remove_cv:   23.594 s  121 K
+// std::remove_cv:  38.817 s  121 K
+//
+// RUN: %cxx %flags %compile_flags -c %s -o %S/orig.o -ggdb  -ggnu-pubnames -ftemplate-depth=5000 -ftime-trace -std=c++17
+// RUN: %cxx %flags %compile_flags -c %s -o %S/new.o -ggdb  -ggnu-pubnames -ftemplate-depth=5000 -ftime-trace -std=c++17 -DTEST_NEW
+
+#include 
+
+#include "test_macros.h"
+#include "template_cost_testing.h"
+
+template  struct Arg { enum { value = 1 }; };
+
+#ifdef TEST_NEW
+
+template 
+struct new_remove_cv
+{
+  typedef __remove_cv(T) type;
+};
+
+#define TEST_CASE_NOP()  new_remove_cv< Arg< __COUNTER__ > >{},
+#define TEST_CASE_TYPE() typename new_remove_cv< Arg< __COUNTER__ > >::type,
+
+#else
+
+#define TEST_CASE_NOP()  std::remove_cv< Arg< __COUNTER__ > >{},
+#define TEST_CASE_TYPE() typename std::remove_cv< Arg< __COUNTER__ > >::type,
+
+#endif
+
+int sink(...);
+
+int x = sink(
+  REPEAT_1(TEST_CASE_NOP)
+  REPEAT_1(TEST_CASE_NOP) 42
+);
+
+void Foo( REPEAT_1(TEST_CASE_TYPE) int) { }
+
+void escape() {
+
+sink(&x);
+sink(&Foo);
+}
+
+
Index: libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_add_cv.sh.cpp
===
--- /dev/null
+++ libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_add_cv.sh.cpp
@@ -0,0 +1,62 @@
+//===--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This is a dummy feature that prevents this test from running by default.
+
+// The table below compares the compile time and object size for each of the
+// variants listed in the RUN script.
+//
+//  Impl   Compile Time  Object Size
+// --
+// new_add_cv:   22.710 s  121 K
+// std::add_cv:  25.643 s  121 K
+//
+// RUN: %cxx %flags %compile_flags -c %s -o %S/orig.o -ggdb  -ggnu-pubnames -ftemplate-depth=5000 -ftime-trace -std=c++17
+// RUN: %cxx %flags %compile_flags -c %s -o %S/new.o -ggdb  -ggnu-pubnames -ftemplate-depth=5000 -ftime-trace -std=c++17 -DTEST_NEW
+
+#include 
+
+#include "test_macros.h"
+#include "template_cost_testing.h"
+
+template  struct Arg { enum { value = 1 }; };
+
+#ifdef TEST_NEW
+
+template 
+struct new_add_cv
+{
+  typedef __add_cv(T) type;
+};
+
+#define TEST_CASE_NOP()  new_add_cv< Arg< __COUNTER__ > >{},
+#define TEST_CASE_TYPE() typename new_add_cv< Arg< __COUNTER__ > >::type,
+
+#else
+
+#define TEST_CASE_NOP()  std::add_cv< Arg< __COUNTER__ > >{},
+#define TEST_CASE_TYPE() typename std::add_cv< Arg< __COUNTER__ > >::type,
+
+#endif
+
+int sink(...);
+
+int x = sink(
+  REPEAT_1(TEST_CASE_NOP)
+  REPEAT_1(TEST_CASE_NOP) 42
+);
+
+void Foo( REPEAT_1(TEST_CASE_TYPE) int) { }

[PATCH] D67588: Add builtin trait for add/remove cv (and similar)

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

@lebedev.ri after adding _only_ these builtins to libc++ the type trait tests 
run several seconds faster. I think if we update _all_ the type traits to use 
builtins then, it could increase speed of the type trait tests by as much as 
50% (if not more).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67588/new/

https://reviews.llvm.org/D67588



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67052: Add reference type transformation builtins

[Zoe Carver via Phabricator via cfe-commits]

zoecarver marked an inline comment as done.
zoecarver added inline comments.



Comment at: clang/include/clang/Parse/Parser.h:2606
+  DeclSpec::TST ReferenceTransformTokToDeclSpec();
+  void ParseAddReferenceTypeSpecifier(DeclSpec &DS);
   void ParseAtomicSpecifier(DeclSpec &DS);

These methods are generalized more in D67588.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67052/new/

https://reviews.llvm.org/D67052



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67052: Add reference type transformation builtins

[Zoe Carver via Phabricator via cfe-commits]

zoecarver marked 5 inline comments as done.
zoecarver added inline comments.



Comment at: clang/lib/Sema/SemaType.cpp:1612
 break;
+  case DeclSpec::TST_addLValueReferenceType:
+  case DeclSpec::TST_addRValueReferenceType:

EricWF wrote:
> This should be merged with the above case. 
The reason I haven't done that is because `__underlying_type` will default to 
`IntTy` if it fails. We don't want that for the others (maybe we don't want 
that here either?). 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67052/new/

https://reviews.llvm.org/D67052



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67052: Add reference type transformation builtins

[Zoe Carver via Phabricator via cfe-commits]

zoecarver updated this revision to Diff 221014.
zoecarver added a comment.
Herald added a subscriber: erik.pilkington.

- address review comments
- fix warnings in build


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67052/new/

https://reviews.llvm.org/D67052

Files:
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/DeclSpec.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Format/FormatToken.cpp
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaTemplateVariadic.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/SemaCXX/add_reference.cpp
  
libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_add_lvalue_reference.sh.cpp
  
libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_add_rvalue_reference.sh.cpp
  
libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_remove_reference.sh.cpp

Index: libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_remove_reference.sh.cpp
===
--- /dev/null
+++ libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_remove_reference.sh.cpp
@@ -0,0 +1,63 @@
+//===--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This is a dummy feature that prevents this test from running by default.
+
+// The table below compares the compile time and object size for each of the
+// variants listed in the RUN script.
+//
+//  Impl   Compile Time  Object Size
+// --
+// new_remove_reference:   22.849 s  121 K
+// std::remove_reference:  25.643 s  121 K
+//
+// RUN: %cxx %flags %compile_flags -c %s -o %S/orig.o -ggdb  -ggnu-pubnames -ftemplate-depth=5000 -ftime-trace -std=c++17
+// RUN: %cxx %flags %compile_flags -c %s -o %S/new.o -ggdb  -ggnu-pubnames -ftemplate-depth=5000 -ftime-trace -std=c++17 -DTEST_NEW
+
+#include 
+#include 
+
+#include "test_macros.h"
+#include "template_cost_testing.h"
+
+template  struct Arg { enum { value = 1 }; };
+
+#ifdef TEST_NEW
+
+template 
+struct new_remove_reference
+{
+  typedef __remove_reference(T) type;
+};
+
+#define TEST_CASE_NOP()  new_remove_reference< Arg< __COUNTER__ > >{},
+#define TEST_CASE_TYPE() typename new_remove_reference< Arg< __COUNTER__ > >::type,
+
+#else
+
+#define TEST_CASE_NOP()  std::remove_reference< Arg< __COUNTER__ > >{},
+#define TEST_CASE_TYPE() typename std::remove_reference< Arg< __COUNTER__ > >::type,
+
+#endif
+
+int sink(...);
+
+int x = sink(
+  REPEAT_1(TEST_CASE_NOP)
+  REPEAT_1(TEST_CASE_NOP) 42
+);
+
+void Foo( REPEAT_1(TEST_CASE_TYPE) int) { }
+
+void escape() {
+
+sink(&x);
+sink(&Foo);
+}
+
+
Index: libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_add_rvalue_reference.sh.cpp
===
--- /dev/null
+++ libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_add_rvalue_reference.sh.cpp
@@ -0,0 +1,63 @@
+//===--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This is a dummy feature that prevents this test from running by default.
+
+// The table below compares the compile time and object size for each of the
+// variants listed in the RUN script.
+//
+//  Impl   Compile Time  Object Size
+// --
+// new_add_rvalue_reference:   56.398 s  171 K
+// std::add_rvalue_reference:  114.59 s  271 K
+//
+// RUN: %cxx %flags %compile_flags -c %s -o %S/orig.o -ggdb  -ggnu-pubnames -ftemplate-depth=5000 -ftime-trace -std=c++17
+// RUN: %cxx %flags %compile_flags -c %s -o %S/new.o -ggdb  -ggnu-pubnames -ftemplate-depth=5000 -ftime-trace -std=c++17 -DTEST_NEW
+
+#include 
+#include 
+
+#include "test_macros.h"
+#include "template_cost_testing.h"
+
+template  struct Arg { enum { value = 1 }; };
+
+#ifdef TEST_NEW
+
+template 
+struct new_add_rvalue_reference
+{
+  typedef __add_rvalue_reference(T) type;
+};
+
+#define TEST_CASE_

[PATCH] D67052: Add reference type transformation builtins

[Zoe Carver via Phabricator via cfe-commits]

zoecarver marked an inline comment as done.
zoecarver added inline comments.



Comment at: clang/lib/AST/ItaniumMangle.cpp:3412
 break;
+  case UnaryTransformType::RemoveReferenceType:
+Out << "3err";

Eric, I looked at your comment, but it seems that we no longer unary type 
transformations that way. @rsmith is this correct? 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67052/new/

https://reviews.llvm.org/D67052



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67897: Fix __is_signed builtin

[Zoe Carver via Phabricator via cfe-commits]

zoecarver created this revision.
zoecarver added reviewers: EricWF, rsmith, erichkeane, craig.topper, efriedma.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch fixes the __is_signed builtin type trait to work with floating point 
types. Now, the builtin will return true if it is passed a floating point type.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67897

Files:
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/type-traits.cpp

Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -56,14 +56,14 @@
 struct HasNoInheritedCons : HasCons {};
 struct HasCopyAssign { HasCopyAssign operator =(const HasCopyAssign&); };
 struct HasMoveAssign { HasMoveAssign operator =(const HasMoveAssign&&); };
-struct HasNoThrowMoveAssign { 
+struct HasNoThrowMoveAssign {
   HasNoThrowMoveAssign& operator=(
 const HasNoThrowMoveAssign&&) throw(); };
-struct HasNoExceptNoThrowMoveAssign { 
+struct HasNoExceptNoThrowMoveAssign {
   HasNoExceptNoThrowMoveAssign& operator=(
-const HasNoExceptNoThrowMoveAssign&&) noexcept; 
+const HasNoExceptNoThrowMoveAssign&&) noexcept;
 };
-struct HasThrowMoveAssign { 
+struct HasThrowMoveAssign {
   HasThrowMoveAssign& operator=(const HasThrowMoveAssign&&)
 #if __cplusplus <= 201402L
   throw(POD);
@@ -73,7 +73,7 @@
 };
 
 
-struct HasNoExceptFalseMoveAssign { 
+struct HasNoExceptFalseMoveAssign {
   HasNoExceptFalseMoveAssign& operator=(
 const HasNoExceptFalseMoveAssign&&) noexcept(false); };
 struct HasMoveCtor { HasMoveCtor(const HasMoveCtor&&); };
@@ -82,17 +82,17 @@
 struct HasStaticMemberMoveCtor { static HasMoveCtor member; };
 struct HasStaticMemberMoveAssign { static HasMoveAssign member; };
 struct HasMemberThrowMoveAssign { HasThrowMoveAssign member; };
-struct HasMemberNoExceptFalseMoveAssign { 
+struct HasMemberNoExceptFalseMoveAssign {
   HasNoExceptFalseMoveAssign member; };
 struct HasMemberNoThrowMoveAssign { HasNoThrowMoveAssign member; };
-struct HasMemberNoExceptNoThrowMoveAssign { 
+struct HasMemberNoExceptNoThrowMoveAssign {
   HasNoExceptNoThrowMoveAssign member; };
 
-struct HasDefaultTrivialCopyAssign { 
+struct HasDefaultTrivialCopyAssign {
   HasDefaultTrivialCopyAssign &operator=(
-const HasDefaultTrivialCopyAssign&) = default; 
+const HasDefaultTrivialCopyAssign&) = default;
 };
-struct TrivialMoveButNotCopy { 
+struct TrivialMoveButNotCopy {
   TrivialMoveButNotCopy &operator=(TrivialMoveButNotCopy&&) = default;
   TrivialMoveButNotCopy &operator=(const TrivialMoveButNotCopy&);
 };
@@ -361,7 +361,7 @@
 struct FinalClass final {
 };
 
-template 
+template
 struct PotentiallyFinal { };
 
 template
@@ -1419,12 +1419,12 @@
   int t04[T(__is_signed(short))];
   int t05[T(__is_signed(signed char))];
   int t06[T(__is_signed(wchar_t))];
+  int t07[T(__is_signed(float))];
+  int t08[T(__is_signed(double))];
+  int t09[T(__is_signed(long double))];
 
-  int t10[F(__is_signed(bool))];
-  int t11[F(__is_signed(cvoid))];
-  int t12[F(__is_signed(float))];
-  int t13[F(__is_signed(double))];
-  int t14[F(__is_signed(long double))];
+  int t13[F(__is_signed(bool))];
+  int t14[F(__is_signed(cvoid))];
   int t15[F(__is_signed(unsigned char))];
   int t16[F(__is_signed(unsigned int))];
   int t17[F(__is_signed(unsigned long long))];
@@ -2005,7 +2005,7 @@
 };
 
 template
-struct X0 { 
+struct X0 {
   template X0(const X0&);
 };
 
@@ -2766,7 +2766,7 @@
   char b[7];
 };
 
-static_assert(!has_unique_object_representations::value, 
+static_assert(!has_unique_object_representations::value,
   "non trivial");
 // Can be unique on Itanium, since the is child class' data is 'folded' into the
 // parent's tail padding.
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -4605,7 +4605,7 @@
   return RD->hasAttr();
 return false;
   case UTT_IsSigned:
-return T->isSignedIntegerType();
+return T->isFloatingType() || T->isSignedIntegerType();
   case UTT_IsUnsigned:
 return T->isUnsignedIntegerType();
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67897: Fix __is_signed builtin

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

In D67897#1678388 , @Quuxplusone wrote:

> But `std::is_signed_v` needs to yield `false`. Isn't it cleaner to 
> leave the compiler builtin matching the library type-trait, so that the 
> library doesn't have to check for integral types separately?


The idea is that these will match the standard. I don't see anywhere where the 
standard says that floating-point types yield `false`. The currently libc++ 
implementation also yields true for your example.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67897/new/

https://reviews.llvm.org/D67897



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67899: Fix __is_fundamental to accept nullptr_t

[Zoe Carver via Phabricator via cfe-commits]

zoecarver created this revision.
zoecarver added reviewers: rsmith, EricWF, efriedma, craig.topper, erichkeane.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch updates the __is_fundamental builtin type trait to return true for 
nullptr_t.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67899

Files:
  clang/include/clang/AST/Type.h
  clang/test/SemaCXX/type-traits.cpp

Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -56,14 +56,14 @@
 struct HasNoInheritedCons : HasCons {};
 struct HasCopyAssign { HasCopyAssign operator =(const HasCopyAssign&); };
 struct HasMoveAssign { HasMoveAssign operator =(const HasMoveAssign&&); };
-struct HasNoThrowMoveAssign { 
+struct HasNoThrowMoveAssign {
   HasNoThrowMoveAssign& operator=(
 const HasNoThrowMoveAssign&&) throw(); };
-struct HasNoExceptNoThrowMoveAssign { 
+struct HasNoExceptNoThrowMoveAssign {
   HasNoExceptNoThrowMoveAssign& operator=(
-const HasNoExceptNoThrowMoveAssign&&) noexcept; 
+const HasNoExceptNoThrowMoveAssign&&) noexcept;
 };
-struct HasThrowMoveAssign { 
+struct HasThrowMoveAssign {
   HasThrowMoveAssign& operator=(const HasThrowMoveAssign&&)
 #if __cplusplus <= 201402L
   throw(POD);
@@ -73,7 +73,7 @@
 };
 
 
-struct HasNoExceptFalseMoveAssign { 
+struct HasNoExceptFalseMoveAssign {
   HasNoExceptFalseMoveAssign& operator=(
 const HasNoExceptFalseMoveAssign&&) noexcept(false); };
 struct HasMoveCtor { HasMoveCtor(const HasMoveCtor&&); };
@@ -82,17 +82,17 @@
 struct HasStaticMemberMoveCtor { static HasMoveCtor member; };
 struct HasStaticMemberMoveAssign { static HasMoveAssign member; };
 struct HasMemberThrowMoveAssign { HasThrowMoveAssign member; };
-struct HasMemberNoExceptFalseMoveAssign { 
+struct HasMemberNoExceptFalseMoveAssign {
   HasNoExceptFalseMoveAssign member; };
 struct HasMemberNoThrowMoveAssign { HasNoThrowMoveAssign member; };
-struct HasMemberNoExceptNoThrowMoveAssign { 
+struct HasMemberNoExceptNoThrowMoveAssign {
   HasNoExceptNoThrowMoveAssign member; };
 
-struct HasDefaultTrivialCopyAssign { 
+struct HasDefaultTrivialCopyAssign {
   HasDefaultTrivialCopyAssign &operator=(
-const HasDefaultTrivialCopyAssign&) = default; 
+const HasDefaultTrivialCopyAssign&) = default;
 };
-struct TrivialMoveButNotCopy { 
+struct TrivialMoveButNotCopy {
   TrivialMoveButNotCopy &operator=(TrivialMoveButNotCopy&&) = default;
   TrivialMoveButNotCopy &operator=(const TrivialMoveButNotCopy&);
 };
@@ -361,7 +361,7 @@
 struct FinalClass final {
 };
 
-template 
+template
 struct PotentiallyFinal { };
 
 template
@@ -801,6 +801,7 @@
   int t23[T(__is_fundamental(unsigned long))];
   int t24[T(__is_fundamental(void))];
   int t25[T(__is_fundamental(cvoid))];
+  int t26[T(__is_fundamental(decltype(nullptr)))];
 
   int t30[F(__is_fundamental(Union))];
   int t31[F(__is_fundamental(UnionAr))];
@@ -2005,7 +2006,7 @@
 };
 
 template
-struct X0 { 
+struct X0 {
   template X0(const X0&);
 };
 
@@ -2766,7 +2767,7 @@
   char b[7];
 };
 
-static_assert(!has_unique_object_representations::value, 
+static_assert(!has_unique_object_representations::value,
   "non trivial");
 // Can be unique on Itanium, since the is child class' data is 'folded' into the
 // parent's tail padding.
Index: clang/include/clang/AST/Type.h
===
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -6353,6 +6353,7 @@
 /// \returns True for types specified in C++0x [basic.fundamental].
 inline bool Type::isFundamentalType() const {
   return isVoidType() ||
+ isNullPtrType() ||
  // FIXME: It's really annoying that we don't have an
  // 'isArithmeticType()' which agrees with the standard definition.
  (isArithmeticType() && !isEnumeralType());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67897: Fix __is_signed builtin

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

> (Can I interest you in fixing the misbehaviour for enumeration types too?)

Certainly. You mean that `__is_signed` should return false for enumeration 
types?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67897/new/

https://reviews.llvm.org/D67897



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67897: Fix __is_signed builtin

[Zoe Carver via Phabricator via cfe-commits]

zoecarver updated this revision to Diff 221247.
zoecarver added a comment.

- fix docs


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67897/new/

https://reviews.llvm.org/D67897

Files:
  clang/docs/LanguageExtensions.rst
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/type-traits.cpp

Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -56,14 +56,14 @@
 struct HasNoInheritedCons : HasCons {};
 struct HasCopyAssign { HasCopyAssign operator =(const HasCopyAssign&); };
 struct HasMoveAssign { HasMoveAssign operator =(const HasMoveAssign&&); };
-struct HasNoThrowMoveAssign { 
+struct HasNoThrowMoveAssign {
   HasNoThrowMoveAssign& operator=(
 const HasNoThrowMoveAssign&&) throw(); };
-struct HasNoExceptNoThrowMoveAssign { 
+struct HasNoExceptNoThrowMoveAssign {
   HasNoExceptNoThrowMoveAssign& operator=(
-const HasNoExceptNoThrowMoveAssign&&) noexcept; 
+const HasNoExceptNoThrowMoveAssign&&) noexcept;
 };
-struct HasThrowMoveAssign { 
+struct HasThrowMoveAssign {
   HasThrowMoveAssign& operator=(const HasThrowMoveAssign&&)
 #if __cplusplus <= 201402L
   throw(POD);
@@ -73,7 +73,7 @@
 };
 
 
-struct HasNoExceptFalseMoveAssign { 
+struct HasNoExceptFalseMoveAssign {
   HasNoExceptFalseMoveAssign& operator=(
 const HasNoExceptFalseMoveAssign&&) noexcept(false); };
 struct HasMoveCtor { HasMoveCtor(const HasMoveCtor&&); };
@@ -82,17 +82,17 @@
 struct HasStaticMemberMoveCtor { static HasMoveCtor member; };
 struct HasStaticMemberMoveAssign { static HasMoveAssign member; };
 struct HasMemberThrowMoveAssign { HasThrowMoveAssign member; };
-struct HasMemberNoExceptFalseMoveAssign { 
+struct HasMemberNoExceptFalseMoveAssign {
   HasNoExceptFalseMoveAssign member; };
 struct HasMemberNoThrowMoveAssign { HasNoThrowMoveAssign member; };
-struct HasMemberNoExceptNoThrowMoveAssign { 
+struct HasMemberNoExceptNoThrowMoveAssign {
   HasNoExceptNoThrowMoveAssign member; };
 
-struct HasDefaultTrivialCopyAssign { 
+struct HasDefaultTrivialCopyAssign {
   HasDefaultTrivialCopyAssign &operator=(
-const HasDefaultTrivialCopyAssign&) = default; 
+const HasDefaultTrivialCopyAssign&) = default;
 };
-struct TrivialMoveButNotCopy { 
+struct TrivialMoveButNotCopy {
   TrivialMoveButNotCopy &operator=(TrivialMoveButNotCopy&&) = default;
   TrivialMoveButNotCopy &operator=(const TrivialMoveButNotCopy&);
 };
@@ -361,7 +361,7 @@
 struct FinalClass final {
 };
 
-template 
+template
 struct PotentiallyFinal { };
 
 template
@@ -1419,12 +1419,12 @@
   int t04[T(__is_signed(short))];
   int t05[T(__is_signed(signed char))];
   int t06[T(__is_signed(wchar_t))];
+  int t07[T(__is_signed(float))];
+  int t08[T(__is_signed(double))];
+  int t09[T(__is_signed(long double))];
 
-  int t10[F(__is_signed(bool))];
-  int t11[F(__is_signed(cvoid))];
-  int t12[F(__is_signed(float))];
-  int t13[F(__is_signed(double))];
-  int t14[F(__is_signed(long double))];
+  int t13[F(__is_signed(bool))];
+  int t14[F(__is_signed(cvoid))];
   int t15[F(__is_signed(unsigned char))];
   int t16[F(__is_signed(unsigned int))];
   int t17[F(__is_signed(unsigned long long))];
@@ -2005,7 +2005,7 @@
 };
 
 template
-struct X0 { 
+struct X0 {
   template X0(const X0&);
 };
 
@@ -2766,7 +2766,7 @@
   char b[7];
 };
 
-static_assert(!has_unique_object_representations::value, 
+static_assert(!has_unique_object_representations::value,
   "non trivial");
 // Can be unique on Itanium, since the is child class' data is 'folded' into the
 // parent's tail padding.
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -4605,7 +4605,7 @@
   return RD->hasAttr();
 return false;
   case UTT_IsSigned:
-return T->isSignedIntegerType();
+return T->isFloatingType() || T->isSignedIntegerType();
   case UTT_IsUnsigned:
 return T->isUnsignedIntegerType();
 
Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -1162,9 +1162,7 @@
   Synonym for ``__is_final``.
 * ``__is_signed`` (C++, Embarcadero):
   Note that this currently returns true for enumeration types if the underlying
-  type is signed, and returns false for floating-point types, in violation of
-  the requirements for ``std::is_signed``. This behavior is likely to change in
-  a future version of Clang.
+  type is signed, and returns true for floating-point types.
 * ``__is_standard_layout`` (C++, GNU, Microsoft, Embarcadero)
 * ``__is_trivial`` (C++, GNU, Microsoft, Embarcadero)
 * ``__is_trivially_assignable`` (C++, GNU, Microsoft)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
ht

[PATCH] D67897: Fix __is_signed builtin

[Zoe Carver via Phabricator via cfe-commits]

zoecarver updated this revision to Diff 221253.
zoecarver added a comment.

- fix behavior when passed an enumeration type


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67897/new/

https://reviews.llvm.org/D67897

Files:
  clang/docs/LanguageExtensions.rst
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/type-traits.cpp

Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -12,6 +12,7 @@
 
 // PODs
 enum Enum { EV };
+enum SignedEnum : signed int { };
 struct POD { Enum e; int i; float f; NonPOD* p; };
 struct Empty {};
 struct IncompleteStruct;
@@ -56,14 +57,14 @@
 struct HasNoInheritedCons : HasCons {};
 struct HasCopyAssign { HasCopyAssign operator =(const HasCopyAssign&); };
 struct HasMoveAssign { HasMoveAssign operator =(const HasMoveAssign&&); };
-struct HasNoThrowMoveAssign { 
+struct HasNoThrowMoveAssign {
   HasNoThrowMoveAssign& operator=(
 const HasNoThrowMoveAssign&&) throw(); };
-struct HasNoExceptNoThrowMoveAssign { 
+struct HasNoExceptNoThrowMoveAssign {
   HasNoExceptNoThrowMoveAssign& operator=(
-const HasNoExceptNoThrowMoveAssign&&) noexcept; 
+const HasNoExceptNoThrowMoveAssign&&) noexcept;
 };
-struct HasThrowMoveAssign { 
+struct HasThrowMoveAssign {
   HasThrowMoveAssign& operator=(const HasThrowMoveAssign&&)
 #if __cplusplus <= 201402L
   throw(POD);
@@ -73,7 +74,7 @@
 };
 
 
-struct HasNoExceptFalseMoveAssign { 
+struct HasNoExceptFalseMoveAssign {
   HasNoExceptFalseMoveAssign& operator=(
 const HasNoExceptFalseMoveAssign&&) noexcept(false); };
 struct HasMoveCtor { HasMoveCtor(const HasMoveCtor&&); };
@@ -82,17 +83,17 @@
 struct HasStaticMemberMoveCtor { static HasMoveCtor member; };
 struct HasStaticMemberMoveAssign { static HasMoveAssign member; };
 struct HasMemberThrowMoveAssign { HasThrowMoveAssign member; };
-struct HasMemberNoExceptFalseMoveAssign { 
+struct HasMemberNoExceptFalseMoveAssign {
   HasNoExceptFalseMoveAssign member; };
 struct HasMemberNoThrowMoveAssign { HasNoThrowMoveAssign member; };
-struct HasMemberNoExceptNoThrowMoveAssign { 
+struct HasMemberNoExceptNoThrowMoveAssign {
   HasNoExceptNoThrowMoveAssign member; };
 
-struct HasDefaultTrivialCopyAssign { 
+struct HasDefaultTrivialCopyAssign {
   HasDefaultTrivialCopyAssign &operator=(
-const HasDefaultTrivialCopyAssign&) = default; 
+const HasDefaultTrivialCopyAssign&) = default;
 };
-struct TrivialMoveButNotCopy { 
+struct TrivialMoveButNotCopy {
   TrivialMoveButNotCopy &operator=(TrivialMoveButNotCopy&&) = default;
   TrivialMoveButNotCopy &operator=(const TrivialMoveButNotCopy&);
 };
@@ -361,7 +362,7 @@
 struct FinalClass final {
 };
 
-template 
+template
 struct PotentiallyFinal { };
 
 template
@@ -1419,12 +1420,12 @@
   int t04[T(__is_signed(short))];
   int t05[T(__is_signed(signed char))];
   int t06[T(__is_signed(wchar_t))];
+  int t07[T(__is_signed(float))];
+  int t08[T(__is_signed(double))];
+  int t09[T(__is_signed(long double))];
 
-  int t10[F(__is_signed(bool))];
-  int t11[F(__is_signed(cvoid))];
-  int t12[F(__is_signed(float))];
-  int t13[F(__is_signed(double))];
-  int t14[F(__is_signed(long double))];
+  int t13[F(__is_signed(bool))];
+  int t14[F(__is_signed(cvoid))];
   int t15[F(__is_signed(unsigned char))];
   int t16[F(__is_signed(unsigned int))];
   int t17[F(__is_signed(unsigned long long))];
@@ -1434,9 +1435,10 @@
   int t21[F(__is_signed(ClassType))];
   int t22[F(__is_signed(Derives))];
   int t23[F(__is_signed(Enum))];
-  int t24[F(__is_signed(IntArNB))];
-  int t25[F(__is_signed(Union))];
-  int t26[F(__is_signed(UnionAr))];
+  int t24[F(__is_signed(SignedEnum))];
+  int t25[F(__is_signed(IntArNB))];
+  int t26[F(__is_signed(Union))];
+  int t27[F(__is_signed(UnionAr))];
 }
 
 void is_unsigned()
@@ -2005,7 +2007,7 @@
 };
 
 template
-struct X0 { 
+struct X0 {
   template X0(const X0&);
 };
 
@@ -2766,7 +2768,7 @@
   char b[7];
 };
 
-static_assert(!has_unique_object_representations::value, 
+static_assert(!has_unique_object_representations::value,
   "non trivial");
 // Can be unique on Itanium, since the is child class' data is 'folded' into the
 // parent's tail padding.
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -4605,7 +4605,9 @@
   return RD->hasAttr();
 return false;
   case UTT_IsSigned:
-return T->isSignedIntegerType();
+// Enum types should always return false.
+// Floating points should always return true.
+return !T->isEnumeralType() && (T->isFloatingType() || T->isSignedIntegerType());
   case UTT_IsUnsigned:
 return T->isUnsignedIntegerType();
 
Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/Langua

[PATCH] D67897: Fix __is_signed builtin

[Zoe Carver via Phabricator via cfe-commits]

zoecarver marked an inline comment as done.
zoecarver added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:1165
   Note that this currently returns true for enumeration types if the underlying
-  type is signed, and returns false for floating-point types, in violation of
-  the requirements for ``std::is_signed``. This behavior is likely to change in
-  a future version of Clang.
+  type is signed, and returns true for floating-point types.
 * ``__is_standard_layout`` (C++, GNU, Microsoft, Embarcadero)

rsmith wrote:
> rsmith wrote:
> > I'd just drop the second half of this sentence, and keep the "likely to 
> > change" warning for the non-conformance for enumeration types until that's 
> > fixed. (We already have an introductory sentence that says we follow the 
> > standard-specified behavior for traits marked (C++) unless otherwise 
> > specified.)
> Now that this does the standard thing, you can delete this text entirely, or 
> add a "before Clang 10, returned true for enumeration types if the underlying 
> type was signed, and returned false for floating-point types".
I'll add that text before committing. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67897/new/

https://reviews.llvm.org/D67897



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67897: Fix __is_signed builtin

[Zoe Carver via Phabricator via cfe-commits]

zoecarver closed this revision.
zoecarver added a comment.

Resolved by rL372621 .


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67897/new/

https://reviews.llvm.org/D67897



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67899: Fix __is_fundamental to accept nullptr_t

[Zoe Carver via Phabricator via cfe-commits]

zoecarver closed this revision.
zoecarver added a comment.

Resolved by rL372624 .


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67899/new/

https://reviews.llvm.org/D67899



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69062: Resolve LWG issue 2426

[Zoe Carver via Phabricator via cfe-commits]

zoecarver created this revision.
zoecarver added reviewers: mclow.lists, EricWF, ldionne, rsmith.
Herald added subscribers: libcxx-commits, cfe-commits, dexonsmith, christof.
Herald added projects: clang, libc++.

This patch checks that `expected` is loaded before it is used. Libc++ already 
does this.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69062

Files:
  clang/test/CodeGen/atomic-ops.c
  libcxx/www/cxx1z_status.html


Index: libcxx/www/cxx1z_status.html
===
--- libcxx/www/cxx1z_status.html
+++ libcxx/www/cxx1z_status.html
@@ -318,7 +318,7 @@
https://wg21.link/LWG2328";>2328Rvalue 
stream extraction should use perfect 
forwardingOuluComplete
https://wg21.link/LWG2393";>2393std::function's Callable 
definition is brokenOuluComplete
https://wg21.link/LWG2422";>2422std::numeric_limits::is_modulo
 description: "most machines" errataOuluComplete
-   https://wg21.link/LWG2426";>2426Issue 
about compare_exchangeOulu
+   https://wg21.link/LWG2426";>2426Issue 
about compare_exchangeOuluComplete
https://wg21.link/LWG2436";>2436Comparators for associative 
containers should always be 
CopyConstructibleOuluComplete
https://wg21.link/LWG2441";>2441Exact-width atomic typedefs 
should be providedOuluComplete
https://wg21.link/LWG2451";>2451[fund.ts.v2] optional should 
'forward' T's implicit conversionsOuluNothing to do
Index: clang/test/CodeGen/atomic-ops.c
===
--- clang/test/CodeGen/atomic-ops.c
+++ clang/test/CodeGen/atomic-ops.c
@@ -466,6 +466,8 @@
   // CHECK: cmpxchg i32* {{%[0-9A-Za-z._]+}}, i32 {{%[0-9A-Za-z._]+}}, i32 
{{%[0-9A-Za-z_.]+}} acquire monotonic
 
   __c11_atomic_compare_exchange_weak(ptr, ptr2, 43, memory_order_seq_cst, 
memory_order_acquire);
+  // CHECK: load i32, i32* {{%[0-9A-Za-z._]+}}, align 4
+  // CHECK: load i32, i32* {{%[0-9A-Za-z._]+}}, align 4
   // CHECK: cmpxchg weak i32* {{%[0-9A-Za-z._]+}}, i32 {{%[0-9A-Za-z._]+}}, 
i32 {{%[0-9A-Za-z_.]+}} seq_cst acquire
 
   // Unknown ordering: conservatively pick strongest valid option (for now!).


Index: libcxx/www/cxx1z_status.html
===
--- libcxx/www/cxx1z_status.html
+++ libcxx/www/cxx1z_status.html
@@ -318,7 +318,7 @@
 	https://wg21.link/LWG2328";>2328Rvalue stream extraction should use perfect forwardingOuluComplete
 	https://wg21.link/LWG2393";>2393std::function's Callable definition is brokenOuluComplete
 	https://wg21.link/LWG2422";>2422std::numeric_limits::is_modulo description: "most machines" errataOuluComplete
-	https://wg21.link/LWG2426";>2426Issue about compare_exchangeOulu
+	https://wg21.link/LWG2426";>2426Issue about compare_exchangeOuluComplete
 	https://wg21.link/LWG2436";>2436Comparators for associative containers should always be CopyConstructibleOuluComplete
 	https://wg21.link/LWG2441";>2441Exact-width atomic typedefs should be providedOuluComplete
 	https://wg21.link/LWG2451";>2451[fund.ts.v2] optional should 'forward' T's implicit conversionsOuluNothing to do
Index: clang/test/CodeGen/atomic-ops.c
===
--- clang/test/CodeGen/atomic-ops.c
+++ clang/test/CodeGen/atomic-ops.c
@@ -466,6 +466,8 @@
   // CHECK: cmpxchg i32* {{%[0-9A-Za-z._]+}}, i32 {{%[0-9A-Za-z._]+}}, i32 {{%[0-9A-Za-z_.]+}} acquire monotonic
 
   __c11_atomic_compare_exchange_weak(ptr, ptr2, 43, memory_order_seq_cst, memory_order_acquire);
+  // CHECK: load i32, i32* {{%[0-9A-Za-z._]+}}, align 4
+  // CHECK: load i32, i32* {{%[0-9A-Za-z._]+}}, align 4
   // CHECK: cmpxchg weak i32* {{%[0-9A-Za-z._]+}}, i32 {{%[0-9A-Za-z._]+}}, i32 {{%[0-9A-Za-z_.]+}} seq_cst acquire
 
   // Unknown ordering: conservatively pick strongest valid option (for now!).
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67052: Add reference type transformation builtins

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.
Herald added a reviewer: mclow.lists.

Friendly ping. Other than type mangling, does anything need to be fixed?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67052/new/

https://reviews.llvm.org/D67052



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69363: [www] Change URLs to HTTPS.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

GitHub PRs can't come soon enough :) There was a round table meeting about that 
earlier today which I am anxious to hear about.




Comment at: clang/www/cxx_status.html:78
   Rvalue references
-  http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2006/n2118.html";>N2118
+  https://wg21.link/n2118";>N2118
   Clang 2.9

These will all now redirect. Maybe that's OK/better because they are now more 
uniform, though. 


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69363/new/

https://reviews.llvm.org/D69363



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69363: [www] Change URLs to HTTPS.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

Thanks for updating all these links! LGTM :)




Comment at: clang/www/cxx_status.html:78
   Rvalue references
-  http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2006/n2118.html";>N2118
+  https://wg21.link/n2118";>N2118
   Clang 2.9

STL_MSFT wrote:
> zoecarver wrote:
> > These will all now redirect. Maybe that's OK/better because they are now 
> > more uniform, though. 
> Yep, that's intentional for consistency with the newer papers that were 
> already using the official redirector. (It's my hope that open-std will 
> eventually support https so the destination is secure; if and when that 
> happens, these will automatically be enhanced.)
Makes sense, especially if (when?) papers are updated the links will be 
auto-updated too (like you said).


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69363/new/

https://reviews.llvm.org/D69363



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77519: Fix __is_pointer builtin type trait to work with Objective-C pointer types.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver created this revision.
Herald added projects: clang, libc++.
Herald added subscribers: libcxx-commits, cfe-commits.
Herald added a reviewer: libc++.
zoecarver added reviewers: ldionne, rsmith, EricWF.
Herald added a subscriber: dexonsmith.

5ade17e  
broke __is_pointer for Objective-C pointer types. This patch fixes the builtin 
and re-applies the change to type_traits.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77519

Files:
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaObjC/type-traits-is-pointer.mm
  libcxx/include/type_traits

Index: libcxx/include/type_traits
===
--- libcxx/include/type_traits
+++ libcxx/include/type_traits
@@ -897,6 +897,19 @@
 
 // is_pointer
 
+// In clang 10.0.0 and earlier __is_pointer didn't work with Objective-C types.
+#if __has_keyword(__is_pointer) && _LIBCPP_CLANG_VER > 1000
+
+template
+struct _LIBCPP_TEMPLATE_VIS is_pointer : _BoolConstant<__is_pointer(_Tp)> { };
+
+#if _LIBCPP_STD_VER > 14 && !defined(_LIBCPP_HAS_NO_VARIABLE_TEMPLATES)
+template 
+_LIBCPP_INLINE_VAR _LIBCPP_CONSTEXPR bool is_pointer_v = __is_pointer(_Tp);
+#endif
+
+#else // __has_keyword(__is_pointer)
+
 template  struct __libcpp_is_pointer   : public false_type {};
 template  struct __libcpp_is_pointer<_Tp*> : public true_type {};
 
@@ -917,6 +930,8 @@
 = is_pointer<_Tp>::value;
 #endif
 
+#endif // __has_keyword(__is_pointer)
+
 // is_reference
 
 #if __has_keyword(__is_lvalue_reference) && \
Index: clang/test/SemaObjC/type-traits-is-pointer.mm
===
--- /dev/null
+++ clang/test/SemaObjC/type-traits-is-pointer.mm
@@ -0,0 +1,52 @@
+// RUN: %clang -fobjc-arc %s
+
+template 
+void assert_is_pointer() {
+static_assert(__is_pointer(T), "");
+}
+
+template 
+void test_is_pointer() {
+assert_is_pointer();
+
+assert_is_pointer();
+assert_is_pointer();
+assert_is_pointer();
+assert_is_pointer();
+
+assert_is_pointer();
+assert_is_pointer();
+assert_is_pointer();
+assert_is_pointer();
+
+assert_is_pointer();
+assert_is_pointer();
+assert_is_pointer();
+assert_is_pointer();
+
+assert_is_pointer();
+assert_is_pointer();
+assert_is_pointer();
+assert_is_pointer();
+}
+
+@class Foo;
+
+int main(int, char**) {
+test_is_pointer();
+test_is_pointer();
+test_is_pointer();
+test_is_pointer();
+
+test_is_pointer();
+test_is_pointer();
+test_is_pointer();
+test_is_pointer();
+
+test_is_pointer();
+test_is_pointer();
+test_is_pointer();
+test_is_pointer();
+
+return 0;
+}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -4692,7 +4692,7 @@
   case UTT_IsArray:
 return T->isArrayType();
   case UTT_IsPointer:
-return T->isPointerType();
+return T->isAnyPointerType();
   case UTT_IsLvalueReference:
 return T->isLValueReferenceType();
   case UTT_IsRvalueReference:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77519: Fix __is_pointer builtin type trait to work with Objective-C pointer types.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver marked an inline comment as done.
zoecarver added a comment.

@ldionne of course!




Comment at: libcxx/include/type_traits:901
+// In clang 10.0.0 and earlier __is_pointer didn't work with Objective-C types.
+#if __has_keyword(__is_pointer) && _LIBCPP_CLANG_VER > 1000
+

ldionne wrote:
> Doesn't `__has_keyword` return a number that can be compared against? `#if 
> __has_keyword(__is_pointer) > some-number` would be better if feasible, 
> because it would handle Clang, AppleClang and any other potential derivative.
I don't think `__has_keyword` returns a comparable number. Libc++ defines 
`__has_keyword ` using `__is_identifier` [1] and, it seems like all the feature 
checking macros from clang (including `__is_identifier`) have bool return types 
[2]. I can add an AppleClang check too if you want. 

[1]:
```
#define __has_keyword(__x) !(__is_identifier(__x))
```

[2]: https://clang.llvm.org/docs/LanguageExtensions.html#feature-checking-macros


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77519/new/

https://reviews.llvm.org/D77519



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77519: Fix __is_pointer builtin type trait to work with Objective-C pointer types.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver updated this revision to Diff 255361.
zoecarver added a comment.

Fix based on review:

- Use static_assert directly in the test
- Update run command to work on non-objc platforms (e.g. linux)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77519/new/

https://reviews.llvm.org/D77519

Files:
  clang/test/SemaObjC/type-traits-is-pointer.mm
  clang/test/SemaObjCXX/type-traits-is-pointer.mm

Index: clang/test/SemaObjCXX/type-traits-is-pointer.mm
===
--- /dev/null
+++ clang/test/SemaObjCXX/type-traits-is-pointer.mm
@@ -0,0 +1,48 @@
+// RUN: %clang_cc1 -fsyntax-only -fobjc-arc -fobjc-runtime-has-weak -verify %s
+// expected-no-diagnostics
+
+template 
+void test_is_pointer() {
+static_assert(__is_pointer(T), "");
+
+static_assert(__is_pointer(T __weak), "");
+static_assert(__is_pointer(T __strong), "");
+static_assert(__is_pointer(T __autoreleasing), "");
+static_assert(__is_pointer(T __unsafe_unretained), "");
+
+static_assert(__is_pointer(T __weak const), "");
+static_assert(__is_pointer(T __strong const), "");
+static_assert(__is_pointer(T __autoreleasing const), "");
+static_assert(__is_pointer(T __unsafe_unretained const), "");
+
+static_assert(__is_pointer(T __weak volatile), "");
+static_assert(__is_pointer(T __strong volatile), "");
+static_assert(__is_pointer(T __autoreleasing volatile), "");
+static_assert(__is_pointer(T __unsafe_unretained volatile), "");
+
+static_assert(__is_pointer(T __weak const volatile), "");
+static_assert(__is_pointer(T __strong const volatile), "");
+static_assert(__is_pointer(T __autoreleasing const volatile), "");
+static_assert(__is_pointer(T __unsafe_unretained const volatile), "");
+}
+
+@class Foo;
+
+int main(int, char**) {
+test_is_pointer();
+test_is_pointer();
+test_is_pointer();
+test_is_pointer();
+
+test_is_pointer();
+test_is_pointer();
+test_is_pointer();
+test_is_pointer();
+
+test_is_pointer();
+test_is_pointer();
+test_is_pointer();
+test_is_pointer();
+
+return 0;
+}
Index: clang/test/SemaObjC/type-traits-is-pointer.mm
===
--- clang/test/SemaObjC/type-traits-is-pointer.mm
+++ /dev/null
@@ -1,52 +0,0 @@
-// RUN: %clang -fobjc-arc %s
-
-template 
-void assert_is_pointer() {
-static_assert(__is_pointer(T), "");
-}
-
-template 
-void test_is_pointer() {
-assert_is_pointer();
-
-assert_is_pointer();
-assert_is_pointer();
-assert_is_pointer();
-assert_is_pointer();
-
-assert_is_pointer();
-assert_is_pointer();
-assert_is_pointer();
-assert_is_pointer();
-
-assert_is_pointer();
-assert_is_pointer();
-assert_is_pointer();
-assert_is_pointer();
-
-assert_is_pointer();
-assert_is_pointer();
-assert_is_pointer();
-assert_is_pointer();
-}
-
-@class Foo;
-
-int main(int, char**) {
-test_is_pointer();
-test_is_pointer();
-test_is_pointer();
-test_is_pointer();
-
-test_is_pointer();
-test_is_pointer();
-test_is_pointer();
-test_is_pointer();
-
-test_is_pointer();
-test_is_pointer();
-test_is_pointer();
-test_is_pointer();
-
-return 0;
-}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77519: Fix __is_pointer builtin type trait to work with Objective-C pointer types.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver updated this revision to Diff 255362.
zoecarver added a comment.

Diff from master not last commit.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77519/new/

https://reviews.llvm.org/D77519

Files:
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaObjCXX/type-traits-is-pointer.mm
  libcxx/include/type_traits


Index: libcxx/include/type_traits
===
--- libcxx/include/type_traits
+++ libcxx/include/type_traits
@@ -897,6 +897,19 @@
 
 // is_pointer
 
+// In clang 10.0.0 and earlier __is_pointer didn't work with Objective-C types.
+#if __has_keyword(__is_pointer) && _LIBCPP_CLANG_VER > 1000
+
+template
+struct _LIBCPP_TEMPLATE_VIS is_pointer : _BoolConstant<__is_pointer(_Tp)> { };
+
+#if _LIBCPP_STD_VER > 14 && !defined(_LIBCPP_HAS_NO_VARIABLE_TEMPLATES)
+template 
+_LIBCPP_INLINE_VAR _LIBCPP_CONSTEXPR bool is_pointer_v = __is_pointer(_Tp);
+#endif
+
+#else // __has_keyword(__is_pointer)
+
 template  struct __libcpp_is_pointer   : public false_type {};
 template  struct __libcpp_is_pointer<_Tp*> : public true_type {};
 
@@ -917,6 +930,8 @@
 = is_pointer<_Tp>::value;
 #endif
 
+#endif // __has_keyword(__is_pointer)
+
 // is_reference
 
 #if __has_keyword(__is_lvalue_reference) && \
Index: clang/test/SemaObjCXX/type-traits-is-pointer.mm
===
--- /dev/null
+++ clang/test/SemaObjCXX/type-traits-is-pointer.mm
@@ -0,0 +1,48 @@
+// RUN: %clang_cc1 -fsyntax-only -fobjc-arc -fobjc-runtime-has-weak -verify %s
+// expected-no-diagnostics
+
+template 
+void test_is_pointer() {
+static_assert(__is_pointer(T), "");
+
+static_assert(__is_pointer(T __weak), "");
+static_assert(__is_pointer(T __strong), "");
+static_assert(__is_pointer(T __autoreleasing), "");
+static_assert(__is_pointer(T __unsafe_unretained), "");
+
+static_assert(__is_pointer(T __weak const), "");
+static_assert(__is_pointer(T __strong const), "");
+static_assert(__is_pointer(T __autoreleasing const), "");
+static_assert(__is_pointer(T __unsafe_unretained const), "");
+
+static_assert(__is_pointer(T __weak volatile), "");
+static_assert(__is_pointer(T __strong volatile), "");
+static_assert(__is_pointer(T __autoreleasing volatile), "");
+static_assert(__is_pointer(T __unsafe_unretained volatile), "");
+
+static_assert(__is_pointer(T __weak const volatile), "");
+static_assert(__is_pointer(T __strong const volatile), "");
+static_assert(__is_pointer(T __autoreleasing const volatile), "");
+static_assert(__is_pointer(T __unsafe_unretained const volatile), "");
+}
+
+@class Foo;
+
+int main(int, char**) {
+test_is_pointer();
+test_is_pointer();
+test_is_pointer();
+test_is_pointer();
+
+test_is_pointer();
+test_is_pointer();
+test_is_pointer();
+test_is_pointer();
+
+test_is_pointer();
+test_is_pointer();
+test_is_pointer();
+test_is_pointer();
+
+return 0;
+}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -4692,7 +4692,7 @@
   case UTT_IsArray:
 return T->isArrayType();
   case UTT_IsPointer:
-return T->isPointerType();
+return T->isAnyPointerType();
   case UTT_IsLvalueReference:
 return T->isLValueReferenceType();
   case UTT_IsRvalueReference:


Index: libcxx/include/type_traits
===
--- libcxx/include/type_traits
+++ libcxx/include/type_traits
@@ -897,6 +897,19 @@
 
 // is_pointer
 
+// In clang 10.0.0 and earlier __is_pointer didn't work with Objective-C types.
+#if __has_keyword(__is_pointer) && _LIBCPP_CLANG_VER > 1000
+
+template
+struct _LIBCPP_TEMPLATE_VIS is_pointer : _BoolConstant<__is_pointer(_Tp)> { };
+
+#if _LIBCPP_STD_VER > 14 && !defined(_LIBCPP_HAS_NO_VARIABLE_TEMPLATES)
+template 
+_LIBCPP_INLINE_VAR _LIBCPP_CONSTEXPR bool is_pointer_v = __is_pointer(_Tp);
+#endif
+
+#else // __has_keyword(__is_pointer)
+
 template  struct __libcpp_is_pointer   : public false_type {};
 template  struct __libcpp_is_pointer<_Tp*> : public true_type {};
 
@@ -917,6 +930,8 @@
 = is_pointer<_Tp>::value;
 #endif
 
+#endif // __has_keyword(__is_pointer)
+
 // is_reference
 
 #if __has_keyword(__is_lvalue_reference) && \
Index: clang/test/SemaObjCXX/type-traits-is-pointer.mm
===
--- /dev/null
+++ clang/test/SemaObjCXX/type-traits-is-pointer.mm
@@ -0,0 +1,48 @@
+// RUN: %clang_cc1 -fsyntax-only -fobjc-arc -fobjc-runtime-has-weak -verify %s
+// expected-no-diagnostics
+
+template 
+void test_is_pointer() {
+static_assert(__is_pointer(T), "");
+
+static_assert(__is_pointer(T __weak), "");
+static_assert(__is_pointer(T __strong), "");
+static_assert(__is_pointer(T __

[PATCH] D77519: Fix __is_pointer builtin type trait to work with Objective-C pointer types.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver marked 2 inline comments as done.
zoecarver added inline comments.



Comment at: clang/test/SemaObjCXX/type-traits-is-pointer.mm:1
+// RUN: %clang_cc1 -fsyntax-only -fobjc-arc -fobjc-runtime-has-weak -verify %s
+// expected-no-diagnostics

ldionne wrote:
> ldionne wrote:
> > Why do you run this through `-verify`? Can't you just drop that and the `// 
> > expected-no-diagnostics` bit too?
> Actually, never mind this. Someone pointed to me offline that using 
> `clang-verify` has the benefit that no other diagnostics are going to be 
> emitted, which is true. This is fine by me.
Great, thanks for the review. Most of these tests seem to use `-verify` so 
that's what I did. 



Comment at: libcxx/include/type_traits:901
+// In clang 10.0.0 and earlier __is_pointer didn't work with Objective-C types.
+#if __has_keyword(__is_pointer) && _LIBCPP_CLANG_VER > 1000
+

ldionne wrote:
> zoecarver wrote:
> > ldionne wrote:
> > > Doesn't `__has_keyword` return a number that can be compared against? 
> > > `#if __has_keyword(__is_pointer) > some-number` would be better if 
> > > feasible, because it would handle Clang, AppleClang and any other 
> > > potential derivative.
> > I don't think `__has_keyword` returns a comparable number. Libc++ defines 
> > `__has_keyword ` using `__is_identifier` [1] and, it seems like all the 
> > feature checking macros from clang (including `__is_identifier`) have bool 
> > return types [2]. I can add an AppleClang check too if you want. 
> > 
> > [1]:
> > ```
> > #define __has_keyword(__x) !(__is_identifier(__x))
> > ```
> > 
> > [2]: 
> > https://clang.llvm.org/docs/LanguageExtensions.html#feature-checking-macros
> Ah, you're right. There's no version of AppleClang to put here right now, so 
> I'd say status quo is OK. I can add one later.
Want me to add a TODO for you?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77519/new/

https://reviews.llvm.org/D77519



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77519: Fix __is_pointer builtin type trait to work with Objective-C pointer types.

[Zoe Carver via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb25ec45809fb: Fix __is_pointer builtin type trait to work 
with Objective-C pointer types. (authored by zoecarver).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77519/new/

https://reviews.llvm.org/D77519

Files:
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaObjCXX/type-traits-is-pointer.mm
  libcxx/include/type_traits


Index: libcxx/include/type_traits
===
--- libcxx/include/type_traits
+++ libcxx/include/type_traits
@@ -897,6 +897,19 @@
 
 // is_pointer
 
+// In clang 10.0.0 and earlier __is_pointer didn't work with Objective-C types.
+#if __has_keyword(__is_pointer) && _LIBCPP_CLANG_VER > 1000
+
+template
+struct _LIBCPP_TEMPLATE_VIS is_pointer : _BoolConstant<__is_pointer(_Tp)> { };
+
+#if _LIBCPP_STD_VER > 14 && !defined(_LIBCPP_HAS_NO_VARIABLE_TEMPLATES)
+template 
+_LIBCPP_INLINE_VAR _LIBCPP_CONSTEXPR bool is_pointer_v = __is_pointer(_Tp);
+#endif
+
+#else // __has_keyword(__is_pointer)
+
 template  struct __libcpp_is_pointer   : public false_type {};
 template  struct __libcpp_is_pointer<_Tp*> : public true_type {};
 
@@ -917,6 +930,8 @@
 = is_pointer<_Tp>::value;
 #endif
 
+#endif // __has_keyword(__is_pointer)
+
 // is_reference
 
 #if __has_keyword(__is_lvalue_reference) && \
Index: clang/test/SemaObjCXX/type-traits-is-pointer.mm
===
--- /dev/null
+++ clang/test/SemaObjCXX/type-traits-is-pointer.mm
@@ -0,0 +1,48 @@
+// RUN: %clang_cc1 -fsyntax-only -fobjc-arc -fobjc-runtime-has-weak -verify %s
+// expected-no-diagnostics
+
+template 
+void test_is_pointer() {
+static_assert(__is_pointer(T), "");
+
+static_assert(__is_pointer(T __weak), "");
+static_assert(__is_pointer(T __strong), "");
+static_assert(__is_pointer(T __autoreleasing), "");
+static_assert(__is_pointer(T __unsafe_unretained), "");
+
+static_assert(__is_pointer(T __weak const), "");
+static_assert(__is_pointer(T __strong const), "");
+static_assert(__is_pointer(T __autoreleasing const), "");
+static_assert(__is_pointer(T __unsafe_unretained const), "");
+
+static_assert(__is_pointer(T __weak volatile), "");
+static_assert(__is_pointer(T __strong volatile), "");
+static_assert(__is_pointer(T __autoreleasing volatile), "");
+static_assert(__is_pointer(T __unsafe_unretained volatile), "");
+
+static_assert(__is_pointer(T __weak const volatile), "");
+static_assert(__is_pointer(T __strong const volatile), "");
+static_assert(__is_pointer(T __autoreleasing const volatile), "");
+static_assert(__is_pointer(T __unsafe_unretained const volatile), "");
+}
+
+@class Foo;
+
+int main(int, char**) {
+test_is_pointer();
+test_is_pointer();
+test_is_pointer();
+test_is_pointer();
+
+test_is_pointer();
+test_is_pointer();
+test_is_pointer();
+test_is_pointer();
+
+test_is_pointer();
+test_is_pointer();
+test_is_pointer();
+test_is_pointer();
+
+return 0;
+}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -4692,7 +4692,7 @@
   case UTT_IsArray:
 return T->isArrayType();
   case UTT_IsPointer:
-return T->isPointerType();
+return T->isAnyPointerType();
   case UTT_IsLvalueReference:
 return T->isLValueReferenceType();
   case UTT_IsRvalueReference:


Index: libcxx/include/type_traits
===
--- libcxx/include/type_traits
+++ libcxx/include/type_traits
@@ -897,6 +897,19 @@
 
 // is_pointer
 
+// In clang 10.0.0 and earlier __is_pointer didn't work with Objective-C types.
+#if __has_keyword(__is_pointer) && _LIBCPP_CLANG_VER > 1000
+
+template
+struct _LIBCPP_TEMPLATE_VIS is_pointer : _BoolConstant<__is_pointer(_Tp)> { };
+
+#if _LIBCPP_STD_VER > 14 && !defined(_LIBCPP_HAS_NO_VARIABLE_TEMPLATES)
+template 
+_LIBCPP_INLINE_VAR _LIBCPP_CONSTEXPR bool is_pointer_v = __is_pointer(_Tp);
+#endif
+
+#else // __has_keyword(__is_pointer)
+
 template  struct __libcpp_is_pointer   : public false_type {};
 template  struct __libcpp_is_pointer<_Tp*> : public true_type {};
 
@@ -917,6 +930,8 @@
 = is_pointer<_Tp>::value;
 #endif
 
+#endif // __has_keyword(__is_pointer)
+
 // is_reference
 
 #if __has_keyword(__is_lvalue_reference) && \
Index: clang/test/SemaObjCXX/type-traits-is-pointer.mm
===
--- /dev/null
+++ clang/test/SemaObjCXX/type-traits-is-pointer.mm
@@ -0,0 +1,48 @@
+// RUN: %clang_cc1 -fsyntax-only -fobjc-arc -fobjc-runtime-has-weak -verify %s
+// expected-no-diagnostics
+
+template 
+void test_is_pointer() {
+static_assert(__is_pointer(T), "");
+
+static_assert(__is_pointer(T __

[PATCH] D82392: [CodeGen] Add public function to emit C++ destructor call.

[Zoe Carver via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe7c5da57a5f3: [CodeGen] Add public function to emit C++ 
destructor call. (authored by zoecarver).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82392/new/

https://reviews.llvm.org/D82392

Files:
  clang/include/clang/CodeGen/CodeGenABITypes.h
  clang/lib/CodeGen/ABIInfo.h
  clang/lib/CodeGen/CGCXXABI.h
  clang/lib/CodeGen/CodeGenABITypes.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/MicrosoftCXXABI.cpp

Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp
===
--- clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -259,6 +259,12 @@
bool ForVirtualBase,
bool Delegating) override;
 
+  llvm::Value *getCXXDestructorImplicitParam(CodeGenFunction &CGF,
+ const CXXDestructorDecl *DD,
+ CXXDtorType Type,
+ bool ForVirtualBase,
+ bool Delegating) override;
+
   void EmitDestructorCall(CodeGenFunction &CGF, const CXXDestructorDecl *DD,
   CXXDtorType Type, bool ForVirtualBase,
   bool Delegating, Address This,
@@ -1577,6 +1583,12 @@
   return AddedStructorArgs::suffix({{MostDerivedArg, getContext().IntTy}});
 }
 
+llvm::Value *MicrosoftCXXABI::getCXXDestructorImplicitParam(
+CodeGenFunction &CGF, const CXXDestructorDecl *DD, CXXDtorType Type,
+bool ForVirtualBase, bool Delegating) {
+  return nullptr;
+}
+
 void MicrosoftCXXABI::EmitDestructorCall(CodeGenFunction &CGF,
  const CXXDestructorDecl *DD,
  CXXDtorType Type, bool ForVirtualBase,
@@ -1603,8 +1615,11 @@
 BaseDtorEndBB = EmitDtorCompleteObjectHandler(CGF);
   }
 
+  llvm::Value *Implicit =
+  getCXXDestructorImplicitParam(CGF, DD, Type, ForVirtualBase,
+Delegating); // = nullptr
   CGF.EmitCXXDestructorCall(GD, Callee, This.getPointer(), ThisTy,
-/*ImplicitParam=*/nullptr,
+/*ImplicitParam=*/Implicit,
 /*ImplicitParamTy=*/QualType(), nullptr);
   if (BaseDtorEndBB) {
 // Complete object handler should continue to be the remaining
Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -228,6 +228,12 @@
bool ForVirtualBase,
bool Delegating) override;
 
+  llvm::Value *getCXXDestructorImplicitParam(CodeGenFunction &CGF,
+ const CXXDestructorDecl *DD,
+ CXXDtorType Type,
+ bool ForVirtualBase,
+ bool Delegating) override;
+
   void EmitDestructorCall(CodeGenFunction &CGF, const CXXDestructorDecl *DD,
   CXXDtorType Type, bool ForVirtualBase,
   bool Delegating, Address This,
@@ -1693,13 +1699,21 @@
   return AddedStructorArgs::prefix({{VTT, VTTTy}});
 }
 
+llvm::Value *ItaniumCXXABI::getCXXDestructorImplicitParam(
+CodeGenFunction &CGF, const CXXDestructorDecl *DD, CXXDtorType Type,
+bool ForVirtualBase, bool Delegating) {
+  GlobalDecl GD(DD, Type);
+  return CGF.GetVTTParameter(GD, ForVirtualBase, Delegating);
+}
+
 void ItaniumCXXABI::EmitDestructorCall(CodeGenFunction &CGF,
const CXXDestructorDecl *DD,
CXXDtorType Type, bool ForVirtualBase,
bool Delegating, Address This,
QualType ThisTy) {
   GlobalDecl GD(DD, Type);
-  llvm::Value *VTT = CGF.GetVTTParameter(GD, ForVirtualBase, Delegating);
+  llvm::Value *VTT =
+  getCXXDestructorImplicitParam(CGF, DD, Type, ForVirtualBase, Delegating);
   QualType VTTTy = getContext().getPointerType(getContext().VoidPtrTy);
 
   CGCallee Callee;
Index: clang/lib/CodeGen/CodeGenABITypes.cpp
===
--- clang/lib/CodeGen/CodeGenABITypes.cpp
+++ clang/lib/CodeGen/CodeGenABITypes.cpp
@@ -115,3 +115,16 @@
  const FieldDecl *FD) {
   return CGM.getTypes().getCGRecordLayout(RD).getLLVMFieldNo(FD);
 }
+
+llvm::Value *CodeGen::getCXXDestructorImplicitParam(
+CodeGenModule &CGM, llvm::BasicBlock *InsertBlock,
+llvm::BasicBlock::iterator Ins

[PATCH] D82392: [CodeGen] Add public function to emit C++ destructor call.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver created this revision.
zoecarver added reviewers: rjmccall, mboehme.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Adds `CodeGen::emitCXXDestructorCall`, a function that creates a 
CodeGenFunction using the arguments provided, then invokes 
CodeGenFunction::EmitCXXDestructorCall.

This will allow other frontends (Swift, for example) to easily emit calls to 
object destructors with correct ABI semantics and calling convetions.

This is needed for Swift C++ interop. Here's the corresponding Swift change: 
https://github.com/apple/swift/pull/32291


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D82392

Files:
  clang/include/clang/CodeGen/CodeGenABITypes.h
  clang/lib/CodeGen/ABIInfo.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenABITypes.cpp

Index: clang/lib/CodeGen/CodeGenABITypes.cpp
===
--- clang/lib/CodeGen/CodeGenABITypes.cpp
+++ clang/lib/CodeGen/CodeGenABITypes.cpp
@@ -115,3 +115,20 @@
  const FieldDecl *FD) {
   return CGM.getTypes().getCGRecordLayout(RD).getLLVMFieldNo(FD);
 }
+
+void CodeGen::emitCXXDestructorCall(CodeGenModule &CGM,
+llvm::BasicBlock *InsertBlock,
+llvm::BasicBlock::iterator InsertPoint,
+const CXXDestructorDecl *D,
+CXXDtorType Type, bool ForVirtualBase,
+bool Delegating, llvm::Value *This,
+CharUnits ThisAlign, QualType ThisTy) {
+  Address ThisAddr(This, ThisAlign);
+  CodeGenFunction CGF(CGM);
+  CGF.CurCodeDecl = D;
+  CGF.CurFuncDecl = D;
+  CGF.CurFn = InsertBlock->getParent();
+  CGF.Builder.SetInsertPoint(InsertBlock, InsertPoint);
+  CGF.EmitCXXDestructorCall(D, Type, ForVirtualBase, Delegating, ThisAddr,
+ThisTy);
+}
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -1462,9 +1462,10 @@
 case ABIArgInfo::Extend:
 case ABIArgInfo::Direct: {
   // FIXME: handle sseregparm someday...
-  llvm::StructType *STy = dyn_cast(AI.getCoerceToType());
-  if (AI.isDirect() && AI.getCanBeFlattened() && STy) {
-IRArgs.NumberOfArgs = STy->getNumElements();
+  llvm::Type *Ty = AI.getCoerceToType();
+  if (AI.isDirect() && AI.getCanBeFlattened() &&
+  isa(Ty)) {
+IRArgs.NumberOfArgs = cast(Ty)->getNumElements();
   } else {
 IRArgs.NumberOfArgs = 1;
   }
@@ -1644,8 +1645,9 @@
   // Fast-isel and the optimizer generally like scalar values better than
   // FCAs, so we flatten them if this is safe to do for this argument.
   llvm::Type *argType = ArgInfo.getCoerceToType();
-  llvm::StructType *st = dyn_cast(argType);
-  if (st && ArgInfo.isDirect() && ArgInfo.getCanBeFlattened()) {
+  if (ArgInfo.isDirect() && ArgInfo.getCanBeFlattened() &&
+  isa(argType)) {
+llvm::StructType *st = cast(argType);
 assert(NumIRArgs == st->getNumElements());
 for (unsigned i = 0, e = st->getNumElements(); i != e; ++i)
   ArgTypes[FirstIRArg + i] = st->getElementType(i);
Index: clang/lib/CodeGen/ABIInfo.h
===
--- clang/lib/CodeGen/ABIInfo.h
+++ clang/lib/CodeGen/ABIInfo.h
@@ -28,7 +28,6 @@
 
 namespace CodeGen {
   class ABIArgInfo;
-  class Address;
   class CGCXXABI;
   class CGFunctionInfo;
   class CodeGenFunction;
Index: clang/include/clang/CodeGen/CodeGenABITypes.h
===
--- clang/include/clang/CodeGen/CodeGenABITypes.h
+++ clang/include/clang/CodeGen/CodeGenABITypes.h
@@ -25,7 +25,9 @@
 
 #include "clang/AST/CanonicalType.h"
 #include "clang/AST/Type.h"
+#include "clang/Basic/ABI.h"
 #include "clang/CodeGen/CGFunctionInfo.h"
+#include "llvm/IR/BasicBlock.h"
 
 namespace llvm {
 class AttrBuilder;
@@ -40,6 +42,7 @@
 namespace clang {
 class ASTContext;
 class CXXConstructorDecl;
+class CXXDestructorDecl;
 class CXXRecordDecl;
 class CXXMethodDecl;
 class CodeGenOptions;
@@ -49,6 +52,7 @@
 class ObjCMethodDecl;
 class ObjCProtocolDecl;
 class PreprocessorOptions;
+class QualType;
 
 namespace CodeGen {
 class CGFunctionInfo;
@@ -90,6 +94,13 @@
 ImplicitCXXConstructorArgs
 getImplicitCXXConstructorArgs(CodeGenModule &CGM, const CXXConstructorDecl *D);
 
+void emitCXXDestructorCall(CodeGenModule &CGM, llvm::BasicBlock *InsertBlock,
+   llvm::BasicBlock::iterator InsertPoint,
+   const CXXDestructorDecl *D, CXXDtorType Type,
+   bool ForVirtualBase, bool Delegating,
+   llvm::Value *This, CharUnits ThisAlign,
+  

[PATCH] D82392: [CodeGen] Add public function to emit C++ destructor call.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver updated this revision to Diff 274616.
zoecarver added a comment.

- Remove `emitCXXDestructorCall` and add `getCXXDestructorImplicitParam`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82392/new/

https://reviews.llvm.org/D82392

Files:
  clang/include/clang/CodeGen/CodeGenABITypes.h
  clang/lib/CodeGen/ABIInfo.h
  clang/lib/CodeGen/CGCXXABI.h
  clang/lib/CodeGen/CodeGenABITypes.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/MicrosoftCXXABI.cpp

Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp
===
--- clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -259,6 +259,12 @@
bool ForVirtualBase,
bool Delegating) override;
 
+  llvm::Value *getCXXDestructorImplicitParam(CodeGenFunction &CGF,
+ const CXXDestructorDecl *DD,
+ CXXDtorType Type,
+ bool ForVirtualBase,
+ bool Delegating) override;
+
   void EmitDestructorCall(CodeGenFunction &CGF, const CXXDestructorDecl *DD,
   CXXDtorType Type, bool ForVirtualBase,
   bool Delegating, Address This,
@@ -1577,6 +1583,12 @@
   return AddedStructorArgs::suffix({{MostDerivedArg, getContext().IntTy}});
 }
 
+llvm::Value *MicrosoftCXXABI::getCXXDestructorImplicitParam(
+CodeGenFunction &CGF, const CXXDestructorDecl *DD, CXXDtorType Type,
+bool ForVirtualBase, bool Delegating) {
+  return nullptr;
+}
+
 void MicrosoftCXXABI::EmitDestructorCall(CodeGenFunction &CGF,
  const CXXDestructorDecl *DD,
  CXXDtorType Type, bool ForVirtualBase,
@@ -1603,8 +1615,11 @@
 BaseDtorEndBB = EmitDtorCompleteObjectHandler(CGF);
   }
 
+  llvm::Value *Implicit =
+  getCXXDestructorImplicitParam(CGF, DD, Type, ForVirtualBase,
+Delegating); // = nullptr
   CGF.EmitCXXDestructorCall(GD, Callee, This.getPointer(), ThisTy,
-/*ImplicitParam=*/nullptr,
+/*ImplicitParam=*/Implicit,
 /*ImplicitParamTy=*/QualType(), nullptr);
   if (BaseDtorEndBB) {
 // Complete object handler should continue to be the remaining
Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -228,6 +228,12 @@
bool ForVirtualBase,
bool Delegating) override;
 
+  llvm::Value *getCXXDestructorImplicitParam(CodeGenFunction &CGF,
+ const CXXDestructorDecl *DD,
+ CXXDtorType Type,
+ bool ForVirtualBase,
+ bool Delegating) override;
+
   void EmitDestructorCall(CodeGenFunction &CGF, const CXXDestructorDecl *DD,
   CXXDtorType Type, bool ForVirtualBase,
   bool Delegating, Address This,
@@ -1646,13 +1652,21 @@
   return AddedStructorArgs::prefix({{VTT, VTTTy}});
 }
 
+llvm::Value *ItaniumCXXABI::getCXXDestructorImplicitParam(
+CodeGenFunction &CGF, const CXXDestructorDecl *DD, CXXDtorType Type,
+bool ForVirtualBase, bool Delegating) {
+  GlobalDecl GD(DD, Type);
+  return CGF.GetVTTParameter(GD, ForVirtualBase, Delegating);
+}
+
 void ItaniumCXXABI::EmitDestructorCall(CodeGenFunction &CGF,
const CXXDestructorDecl *DD,
CXXDtorType Type, bool ForVirtualBase,
bool Delegating, Address This,
QualType ThisTy) {
   GlobalDecl GD(DD, Type);
-  llvm::Value *VTT = CGF.GetVTTParameter(GD, ForVirtualBase, Delegating);
+  llvm::Value *VTT =
+  getCXXDestructorImplicitParam(CGF, DD, Type, ForVirtualBase, Delegating);
   QualType VTTTy = getContext().getPointerType(getContext().VoidPtrTy);
 
   CGCallee Callee;
Index: clang/lib/CodeGen/CodeGenABITypes.cpp
===
--- clang/lib/CodeGen/CodeGenABITypes.cpp
+++ clang/lib/CodeGen/CodeGenABITypes.cpp
@@ -115,3 +115,16 @@
  const FieldDecl *FD) {
   return CGM.getTypes().getCGRecordLayout(RD).getLLVMFieldNo(FD);
 }
+
+llvm::Value *CodeGen::getCXXDestructorImplicitParam(
+CodeGenModule &CGM, llvm::BasicBlock *InsertBlock,
+llvm::BasicBlock::iterator InsertPoint, const CXXDestructorDecl *D,
+  

[PATCH] D82392: [CodeGen] Add public function to emit C++ destructor call.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

@rjmccall I updated this patch to introduce the function 
`getCXXDestructorImplicitParam` instead. This should be closer to D79942 
. It seems like there is never more than one 
implicit parameter so, I just have it return a single `llvm::Value*`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82392/new/

https://reviews.llvm.org/D82392



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67052: Add reference type transformation builtins

[Zoe Carver via Phabricator via cfe-commits]

zoecarver marked 6 inline comments as done.
zoecarver added a comment.

@EricWF and @miscco thanks for the review comments. Sorry for the delay, I 
forgot about this patch. All your comments have been addressed/fixed.




Comment at: clang/lib/AST/TypePrinter.cpp:1020
 
   switch (T->getUTTKind()) {
 case UnaryTransformType::EnumUnderlyingType:

miscco wrote:
> Couldn't we use `TextNodeDumper::VisitUnaryTransformType` to dump the string 
> and then simplify it to a single case.
> 
> Given the possibility that maybe some day one might add the `add_foo` builtins
> Couldn't we use TextNodeDumper::VisitUnaryTransformType to dump the string 
> and then simplify it to a single case.

Good call. Will do.

> Given the possibility that maybe some day one might add the add_foo builtins

I had a patch that added those builtins but, we decided that there was no real 
reason for them to exist.




Comment at: clang/lib/Sema/SemaType.cpp:1624
   }
+  case DeclSpec::TST_removeReferenceType:
+  case DeclSpec::TST_removeCV:

EricWF wrote:
> Why can't we share the implementation with `TST_underlyingType`?
Yep, fixed. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67052/new/

https://reviews.llvm.org/D67052



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67052: Add reference type transformation builtins

[Zoe Carver via Phabricator via cfe-commits]

zoecarver updated this revision to Diff 258836.
zoecarver added a comment.

- Format using clang-format


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67052/new/

https://reviews.llvm.org/D67052

Files:
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/DeclSpec.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Format/FormatToken.cpp
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaTemplateVariadic.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/SemaCXX/remove_cv.cpp
  clang/test/SemaCXX/remove_reference.cpp

Index: clang/test/SemaCXX/remove_reference.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/remove_reference.cpp
@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 -std=c++11 %s
+
+template
+struct test_remove_ref_trait
+{
+  typedef __remove_reference(T) type;
+};
+
+template
+struct test_remove_ref
+{
+  static const bool value = __is_same(typename test_remove_ref_trait::type, T)  &&
+__is_same(typename test_remove_ref_trait::type, T) &&
+__is_same(typename test_remove_ref_trait::type, T);
+};
+
+struct Foo { };
+
+template
+struct Bar { };
+
+template
+class Baz { };
+
+class Biz;
+
+void x() { }
+
+static_assert(test_remove_ref::value, "");
+static_assert(test_remove_ref::value, "");
+static_assert(test_remove_ref::value, "");
+static_assert(test_remove_ref::value, "");
+static_assert(test_remove_ref>::value, "");
+static_assert(test_remove_ref>::value, "");
+static_assert(test_remove_ref::value, "");
+static_assert(test_remove_ref::value, "");
+static_assert(test_remove_ref::value, "");
+static_assert(test_remove_ref::value, "");
+static_assert(test_remove_ref::value, "");
+
Index: clang/test/SemaCXX/remove_cv.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/remove_cv.cpp
@@ -0,0 +1,58 @@
+// RUN: %clang_cc1 -std=c++11 %s
+
+template
+struct remove_const
+{
+  typedef __remove_const(T) type;
+};
+
+template
+struct remove_volatile
+{
+  typedef __remove_volatile(T) type;
+};
+
+template
+struct remove_cv
+{
+  typedef __remove_cv(T) type;
+};
+
+template
+struct test
+{
+  static const bool value =
+__is_same(typename remove_const::type, T) &&
+__is_same(typename remove_const::type, volatile T) &&
+__is_same(typename remove_const::type, T) &&
+
+__is_same(typename remove_volatile::type, T) &&
+__is_same(typename remove_volatile::type, const T) &&
+__is_same(typename remove_volatile::type, T) &&
+
+__is_same(typename remove_cv::type, T) &&
+__is_same(typename remove_cv::type, T) &&
+__is_same(typename remove_cv::type, T) &&
+__is_same(typename remove_cv::type, T);
+};
+
+struct Foo { };
+
+template
+struct Bar { };
+
+template
+class Baz { };
+
+class Biz;
+
+void x() { }
+
+static_assert(test::value, "");
+static_assert(test::value, "");
+static_assert(test::value, "");
+static_assert(test::value, "");
+static_assert(test>::value, "");
+static_assert(test>::value, "");
+static_assert(test::value, "");
+static_assert(test::value, "");
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -1253,6 +1253,24 @@
   return OpenCLAccessAttr::Keyword_read_only;
 }
 
+static UnaryTransformType::UTTKind
+TSTToUnaryTransformType(DeclSpec::TST SwitchTST) {
+  switch (SwitchTST) {
+  case TST_removeCV:
+return UnaryTransformType::RemoveCV;
+  case TST_removeConst:
+return UnaryTransformType::RemoveConst;
+  case TST_removeVolatile:
+return UnaryTransformType::RemoveVolatile;
+  case TST_removeReferenceType:
+return UnaryTransformType::RemoveReferenceType;
+  case TST_underlyingType:
+return UnaryTransformType::EnumUnderlyingType;
+  default:
+llvm_unreachable("Cannot map TST to unary transform type");
+  }
+}
+
 static QualType ConvertConstrainedAutoDeclSpecToType(Sema &S, DeclSpec &DS,
  AutoTypeKeyword AutoKW) {
   assert(DS.isConstrainedAuto());
@@ -1605,16 +1623,23 @@
 break;
   }
   case DeclSpec::TST_underlyingType:
+  case DeclSpec::TST_removeReferenceType:
+  case DeclSpec::TST_removeCV:
+  case DeclSpec::TST_removeConst:
+  case DeclSpec::TST_removeVolatile: {
 Result = S.GetTypeFromParser(DS.getRepAsType());
-assert(!Result.isNull() && "Didn't get a type for __underlying_type?");
+assert(!Result.isNull() &&
+   "Type transform builtin may not have received a type.");
 Result = S.Bu

[PATCH] D67052: Add reference type transformation builtins

[Zoe Carver via Phabricator via cfe-commits]

zoecarver updated this revision to Diff 258834.
zoecarver marked 2 inline comments as done.
zoecarver added a comment.

- Rebase
- Fix based on review comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67052/new/

https://reviews.llvm.org/D67052

Files:
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/DeclSpec.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Format/FormatToken.cpp
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaTemplateVariadic.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/SemaCXX/remove_cv.cpp
  clang/test/SemaCXX/remove_reference.cpp

Index: clang/test/SemaCXX/remove_reference.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/remove_reference.cpp
@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 -std=c++11 %s
+
+template
+struct test_remove_ref_trait
+{
+  typedef __remove_reference(T) type;
+};
+
+template
+struct test_remove_ref
+{
+  static const bool value = __is_same(typename test_remove_ref_trait::type, T)  &&
+__is_same(typename test_remove_ref_trait::type, T) &&
+__is_same(typename test_remove_ref_trait::type, T);
+};
+
+struct Foo { };
+
+template
+struct Bar { };
+
+template
+class Baz { };
+
+class Biz;
+
+void x() { }
+
+static_assert(test_remove_ref::value, "");
+static_assert(test_remove_ref::value, "");
+static_assert(test_remove_ref::value, "");
+static_assert(test_remove_ref::value, "");
+static_assert(test_remove_ref>::value, "");
+static_assert(test_remove_ref>::value, "");
+static_assert(test_remove_ref::value, "");
+static_assert(test_remove_ref::value, "");
+static_assert(test_remove_ref::value, "");
+static_assert(test_remove_ref::value, "");
+static_assert(test_remove_ref::value, "");
+
Index: clang/test/SemaCXX/remove_cv.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/remove_cv.cpp
@@ -0,0 +1,58 @@
+// RUN: %clang_cc1 -std=c++11 %s
+
+template
+struct remove_const
+{
+  typedef __remove_const(T) type;
+};
+
+template
+struct remove_volatile
+{
+  typedef __remove_volatile(T) type;
+};
+
+template
+struct remove_cv
+{
+  typedef __remove_cv(T) type;
+};
+
+template
+struct test
+{
+  static const bool value =
+__is_same(typename remove_const::type, T) &&
+__is_same(typename remove_const::type, volatile T) &&
+__is_same(typename remove_const::type, T) &&
+
+__is_same(typename remove_volatile::type, T) &&
+__is_same(typename remove_volatile::type, const T) &&
+__is_same(typename remove_volatile::type, T) &&
+
+__is_same(typename remove_cv::type, T) &&
+__is_same(typename remove_cv::type, T) &&
+__is_same(typename remove_cv::type, T) &&
+__is_same(typename remove_cv::type, T);
+};
+
+struct Foo { };
+
+template
+struct Bar { };
+
+template
+class Baz { };
+
+class Biz;
+
+void x() { }
+
+static_assert(test::value, "");
+static_assert(test::value, "");
+static_assert(test::value, "");
+static_assert(test::value, "");
+static_assert(test>::value, "");
+static_assert(test>::value, "");
+static_assert(test::value, "");
+static_assert(test::value, "");
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -1253,6 +1253,24 @@
   return OpenCLAccessAttr::Keyword_read_only;
 }
 
+static UnaryTransformType::UTTKind
+TSTToUnaryTransformType(DeclSpec::TST SwitchTST) {
+  switch (SwitchTST) {
+  case TST_removeCV:
+return UnaryTransformType::RemoveCV;
+  case TST_removeConst:
+return UnaryTransformType::RemoveConst;
+  case TST_removeVolatile:
+return UnaryTransformType::RemoveVolatile;
+  case TST_removeReferenceType:
+return UnaryTransformType::RemoveReferenceType;
+  case TST_underlyingType:
+return UnaryTransformType::EnumUnderlyingType;
+  default:
+llvm_unreachable("Cannot map TST to unary transform type");
+  }
+}
+
 static QualType ConvertConstrainedAutoDeclSpecToType(Sema &S, DeclSpec &DS,
  AutoTypeKeyword AutoKW) {
   assert(DS.isConstrainedAuto());
@@ -1605,16 +1623,23 @@
 break;
   }
   case DeclSpec::TST_underlyingType:
+  case DeclSpec::TST_removeReferenceType:
+  case DeclSpec::TST_removeCV:
+  case DeclSpec::TST_removeConst:
+  case DeclSpec::TST_removeVolatile: {
 Result = S.GetTypeFromParser(DS.getRepAsType());
-assert(!Result.isNull() && "Didn't get a type for __underlying_type?");
+assert(!Result.isNull() &&
+   "Type transform bui

[PATCH] D129384: [objcxx] Fix `std::addressof` for `id`.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver created this revision.
Herald added a project: All.
zoecarver requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129384

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/test/SemaObjCXX/unsupported-signature-std-addressof-id.mm


Index: clang/test/SemaObjCXX/unsupported-signature-std-addressof-id.mm
===
--- /dev/null
+++ clang/test/SemaObjCXX/unsupported-signature-std-addressof-id.mm
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -fsyntax-only -fobjc-arc -fobjc-runtime-has-weak -verify %s
+// expected-no-diagnostics
+
+namespace std {
+template 
+T* addressof(T&);
+}
+
+void f(id obj) {
+(void)std::addressof(*obj);
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -2413,7 +2413,7 @@
 bool ReturnsPointer = BuiltinID == Builtin::BIaddressof ||
   BuiltinID == Builtin::BI__addressof;
 if (!(Param->isReferenceType() &&
-  (ReturnsPointer ? Result->isPointerType()
+  (ReturnsPointer ? Result->isAnyPointerType()
   : Result->isReferenceType()) &&
   Context.hasSameUnqualifiedType(Param->getPointeeType(),
  Result->getPointeeType( {


Index: clang/test/SemaObjCXX/unsupported-signature-std-addressof-id.mm
===
--- /dev/null
+++ clang/test/SemaObjCXX/unsupported-signature-std-addressof-id.mm
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -fsyntax-only -fobjc-arc -fobjc-runtime-has-weak -verify %s
+// expected-no-diagnostics
+
+namespace std {
+template 
+T* addressof(T&);
+}
+
+void f(id obj) {
+(void)std::addressof(*obj);
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -2413,7 +2413,7 @@
 bool ReturnsPointer = BuiltinID == Builtin::BIaddressof ||
   BuiltinID == Builtin::BI__addressof;
 if (!(Param->isReferenceType() &&
-  (ReturnsPointer ? Result->isPointerType()
+  (ReturnsPointer ? Result->isAnyPointerType()
   : Result->isReferenceType()) &&
   Context.hasSameUnqualifiedType(Param->getPointeeType(),
  Result->getPointeeType( {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129384: [objcxx] Fix `std::addressof` for `id`.

[Zoe Carver via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG22c7a6ec: [objcxx] Fix `std::addressof` for `id`. 
(authored by zoecarver).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129384/new/

https://reviews.llvm.org/D129384

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/test/SemaObjCXX/unsupported-signature-std-addressof-id.mm


Index: clang/test/SemaObjCXX/unsupported-signature-std-addressof-id.mm
===
--- /dev/null
+++ clang/test/SemaObjCXX/unsupported-signature-std-addressof-id.mm
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -fsyntax-only -fobjc-arc -fobjc-runtime-has-weak -verify %s
+// expected-no-diagnostics
+
+namespace std {
+template 
+T* addressof(T&);
+}
+
+void f(id obj) {
+(void)std::addressof(*obj);
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -2413,7 +2413,7 @@
 bool ReturnsPointer = BuiltinID == Builtin::BIaddressof ||
   BuiltinID == Builtin::BI__addressof;
 if (!(Param->isReferenceType() &&
-  (ReturnsPointer ? Result->isPointerType()
+  (ReturnsPointer ? Result->isAnyPointerType()
   : Result->isReferenceType()) &&
   Context.hasSameUnqualifiedType(Param->getPointeeType(),
  Result->getPointeeType( {


Index: clang/test/SemaObjCXX/unsupported-signature-std-addressof-id.mm
===
--- /dev/null
+++ clang/test/SemaObjCXX/unsupported-signature-std-addressof-id.mm
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -fsyntax-only -fobjc-arc -fobjc-runtime-has-weak -verify %s
+// expected-no-diagnostics
+
+namespace std {
+template 
+T* addressof(T&);
+}
+
+void f(id obj) {
+(void)std::addressof(*obj);
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -2413,7 +2413,7 @@
 bool ReturnsPointer = BuiltinID == Builtin::BIaddressof ||
   BuiltinID == Builtin::BI__addressof;
 if (!(Param->isReferenceType() &&
-  (ReturnsPointer ? Result->isPointerType()
+  (ReturnsPointer ? Result->isAnyPointerType()
   : Result->isReferenceType()) &&
   Context.hasSameUnqualifiedType(Param->getPointeeType(),
  Result->getPointeeType( {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106394: [clang][pp] adds '#pragma include_instead'

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

Can't wait to start using this! (Note: this is not a full review, just some 
thoughts.)

Do you have a test case where

  // private: header one
  *exists*
  
  // private: header two
  #include<__header_one.h>
  
  // public: header three
  #include<__header_two.h>

I think right now you might see a warning that says "please include either 
`__header_two.h` or `header_three.h`" which is not right.

This begs the question, would it not be better to have a second argument that 
specifies what header should be used as a replacement? This would also allow 
the person writing the library to decide whether it's better to use angle 
brackets or quotes.




Comment at: clang/include/clang/Lex/HeaderSearch.h:465
+getFileInfo(File).Aliases;
+auto InsertionPoint = llvm::lower_bound(Aliases, Alias);
+if (InsertionPoint != Aliases.end() && *InsertionPoint == Alias)

Is the logic here is just "if it's not already here add it?" If so, I think 
maybe you should just make `Aliases` a set (either a `StringSet` or 
`SmallSet`). Do you care about order? 



Comment at: clang/lib/Lex/PPLexerChange.cpp:429
+};
+std::string Aliases =
+std::accumulate(Info.Aliases.begin() + 1, Info.Aliases.end(),

Can we use `llvm::interleaveComma` or `interleave`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106394/new/

https://reviews.llvm.org/D106394

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130446: [apinotes] Upstream changes to `APINotesYAMLCompiler.cpp`.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver created this revision.
zoecarver added a reviewer: compnerd.
Herald added a project: All.
zoecarver requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130446

Files:
  clang/lib/APINotes/APINotesYAMLCompiler.cpp


Index: clang/lib/APINotes/APINotesYAMLCompiler.cpp
===
--- clang/lib/APINotes/APINotesYAMLCompiler.cpp
+++ clang/lib/APINotes/APINotesYAMLCompiler.cpp
@@ -289,6 +289,7 @@
   StringRef SwiftName;
   StringRef Type;
   StringRef ResultType;
+  Optional ImportAs;
 };
 
 typedef std::vector FunctionsSeq;
@@ -311,6 +312,7 @@
 IO.mapOptional("SwiftPrivate", F.SwiftPrivate);
 IO.mapOptional("SwiftName", F.SwiftName, StringRef(""));
 IO.mapOptional("ResultType", F.ResultType, StringRef(""));
+IO.mapOptional("ImportAs", F.ImportAs);
   }
 };
 } // namespace yaml
@@ -417,6 +419,10 @@
   Optional EnumExtensibility;
   Optional FlagEnum;
   Optional EnumConvenienceKind;
+  Optional ImportAs;
+  Optional RetainOp;
+  Optional ReleaseOp;
+  FunctionsSeq MemberFuncs;
 };
 
 typedef std::vector TagsSeq;
@@ -447,6 +453,10 @@
 IO.mapOptional("EnumExtensibility", T.EnumExtensibility);
 IO.mapOptional("FlagEnum", T.FlagEnum);
 IO.mapOptional("EnumKind", T.EnumConvenienceKind);
+IO.mapOptional("ImportAs", T.ImportAs);
+IO.mapOptional("Retain", T.RetainOp);
+IO.mapOptional("Release", T.ReleaseOp);
+IO.mapOptional("Methods", T.MemberFuncs);
   }
 };
 } // namespace yaml


Index: clang/lib/APINotes/APINotesYAMLCompiler.cpp
===
--- clang/lib/APINotes/APINotesYAMLCompiler.cpp
+++ clang/lib/APINotes/APINotesYAMLCompiler.cpp
@@ -289,6 +289,7 @@
   StringRef SwiftName;
   StringRef Type;
   StringRef ResultType;
+  Optional ImportAs;
 };
 
 typedef std::vector FunctionsSeq;
@@ -311,6 +312,7 @@
 IO.mapOptional("SwiftPrivate", F.SwiftPrivate);
 IO.mapOptional("SwiftName", F.SwiftName, StringRef(""));
 IO.mapOptional("ResultType", F.ResultType, StringRef(""));
+IO.mapOptional("ImportAs", F.ImportAs);
   }
 };
 } // namespace yaml
@@ -417,6 +419,10 @@
   Optional EnumExtensibility;
   Optional FlagEnum;
   Optional EnumConvenienceKind;
+  Optional ImportAs;
+  Optional RetainOp;
+  Optional ReleaseOp;
+  FunctionsSeq MemberFuncs;
 };
 
 typedef std::vector TagsSeq;
@@ -447,6 +453,10 @@
 IO.mapOptional("EnumExtensibility", T.EnumExtensibility);
 IO.mapOptional("FlagEnum", T.FlagEnum);
 IO.mapOptional("EnumKind", T.EnumConvenienceKind);
+IO.mapOptional("ImportAs", T.ImportAs);
+IO.mapOptional("Retain", T.RetainOp);
+IO.mapOptional("Release", T.ReleaseOp);
+IO.mapOptional("Methods", T.MemberFuncs);
   }
 };
 } // namespace yaml
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130446: [apinotes] Upstream changes to `APINotesYAMLCompiler.cpp`.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

> Can you please add a round trip test as well?

There is no testing infrastructure for this upstream. I have tests downstream.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130446/new/

https://reviews.llvm.org/D130446

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66822: Hardware cache line size builtins

[Zoe Carver via Phabricator via cfe-commits]

zoecarver created this revision.
Herald added a reviewer: jfb.
Herald added subscribers: cfe-commits, kbarton, aheejin, javed.absar, nemanjai.
Herald added a project: clang.

Creates the `__builtin_hardware_destructive_interference_size` and 
`__builtin_hardware_constructive_interference_size` builtins proposed by @jfb 
[[ here | http://lists.llvm.org/pipermail/cfe-dev/2018-May/058073.html ]]. 
These builtins can be used to implement [[ P0154 | http://wg21.link/P0154 ]] in 
libc++ and other standard libraries. My implementation switches on the target 
triple to get the max cache line size for that target. I am not sure if this is 
the best way to implement these builtins, but it will ensure that there is not 
an ABI break.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66822

Files:
  clang/include/clang/Basic/Builtins.def
  clang/lib/AST/ExprConstant.cpp
  clang/test/Sema/builtins.c
  clang/test/SemaCXX/builtin-hardware-interference-size-aarch.cpp
  clang/test/SemaCXX/builtin-hardware-interference-size-amd.cpp
  clang/test/SemaCXX/builtin-hardware-interference-size-arm.cpp
  clang/test/SemaCXX/builtin-hardware-interference-size-nvptx.cpp
  clang/test/SemaCXX/builtin-hardware-interference-size-ppc.cpp
  clang/test/SemaCXX/builtin-hardware-interference-size-unknown.cpp
  clang/test/SemaCXX/builtin-hardware-interference-size-x86.cpp

Index: clang/test/SemaCXX/builtin-hardware-interference-size-x86.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/builtin-hardware-interference-size-x86.cpp
@@ -0,0 +1,7 @@
+// expected-no-diagnostics
+// RUN: %clang_cc1 -std=c++2a -verify %s -fcxx-exceptions -triple=x86_64-linux-gnu
+// RUN: %clang_cc1 -std=c++2a -verify %s -fcxx-exceptions -triple=x86_64-unknown-unknown
+// RUN: %clang_cc1 -std=c++2a -verify %s -fcxx-exceptions -triple=x86_64-apple-darwin16.0.0
+
+static_assert(__builtin_hardware_constructive_interference_size() == 64);
+static_assert(__builtin_hardware_destructive_interference_size() == 64);
Index: clang/test/SemaCXX/builtin-hardware-interference-size-unknown.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/builtin-hardware-interference-size-unknown.cpp
@@ -0,0 +1,9 @@
+// expected-no-diagnostics
+// RUN: %clang_cc1 -std=c++2a -verify %s -fcxx-exceptions -triple=systemz-linux-gnu
+// RUN: %clang_cc1 -std=c++2a -verify %s -fcxx-exceptions -triple=wasm32-unknown-unknown
+// RUN: %clang_cc1 -std=c++2a -verify %s -fcxx-exceptions -triple=wasm64-unknown-unknown
+// RUN: %clang_cc1 -std=c++2a -verify %s -fcxx-exceptions -triple=hexagon-linux-gnu
+// RUN: %clang_cc1 -std=c++2a -verify %s -fcxx-exceptions -triple=systemz-apple-darwin16.0.0
+
+static_assert(__builtin_hardware_constructive_interference_size() == 0);
+static_assert(__builtin_hardware_destructive_interference_size() == 0);
Index: clang/test/SemaCXX/builtin-hardware-interference-size-ppc.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/builtin-hardware-interference-size-ppc.cpp
@@ -0,0 +1,8 @@
+// expected-no-diagnostics
+// RUN: %clang_cc1 -std=c++2a -verify %s -fcxx-exceptions -triple=ppc-linux-gnu
+// RUN: %clang_cc1 -std=c++2a -verify %s -fcxx-exceptions -triple=ppc64-linux-gnu
+// RUN: %clang_cc1 -std=c++2a -verify %s -fcxx-exceptions -triple=ppc64le-linux-gnu
+// RUN: %clang_cc1 -std=c++2a -verify %s -fcxx-exceptions -triple=ppc-apple-darwin16.0.0
+
+static_assert(__builtin_hardware_constructive_interference_size() == 128);
+static_assert(__builtin_hardware_destructive_interference_size() == 128);
Index: clang/test/SemaCXX/builtin-hardware-interference-size-nvptx.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/builtin-hardware-interference-size-nvptx.cpp
@@ -0,0 +1,7 @@
+// expected-no-diagnostics
+// RUN: %clang_cc1 -std=c++2a -verify %s -fcxx-exceptions -triple=nvptx-linux-gnu
+// RUN: %clang_cc1 -std=c++2a -verify %s -fcxx-exceptions -triple=nvptx64-linux-gnu
+// RUN: %clang_cc1 -std=c++2a -verify %s -fcxx-exceptions -triple=nvptx-apple-darwin16.0.0
+
+static_assert(__builtin_hardware_constructive_interference_size() == 64);
+static_assert(__builtin_hardware_destructive_interference_size() == 64);
Index: clang/test/SemaCXX/builtin-hardware-interference-size-arm.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/builtin-hardware-interference-size-arm.cpp
@@ -0,0 +1,9 @@
+// expected-no-diagnostics
+// RUN: %clang_cc1 -std=c++2a -verify %s -fcxx-exceptions -triple=arm-linux-gnu
+// RUN: %clang_cc1 -std=c++2a -verify %s -fcxx-exceptions -triple=armeb-linux-gnu
+// RUN: %clang_cc1 -std=c++2a -verify %s -fcxx-exceptions -triple=thumb-linux-gnu
+// RUN: %clang_cc1 -std=c++2a -verify %s -fcxx-exceptions -triple=thumbeb-linux-gnu
+// RUN: %clang_cc1 -std=c++2a -verify %s -fcxx-exceptions -tr

[PATCH] D66822: Hardware cache line size builtins

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

> Passing-by remark: i'm not sure it is possible to **guarantee** that this 
> will be always correct and that no ABI break will happen.

You're right. I should have said, "least-likely to cause an ABI break." And I 
completely agree that there is **no way** to gaurentee this is correct at 
compile time. `hardware_*_interference_size` certainly has the potential to do 
more harm than good but, I think that is another discussion.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66822/new/

https://reviews.llvm.org/D66822



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66822: Hardware cache line size builtins

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

It may be a good idea only to enable this when `-march=native`. And _maybe_ 
remove the constexpr requirement (though it would make this feature 
significantly less useful it would also make it significantly more accurate).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66822/new/

https://reviews.llvm.org/D66822



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66822: Hardware cache line size builtins

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

We should probably tell people never to use this, period. That being said, I 
like your idea of having it be a constant. The only issue would be when, in the 
next few years, people start shipping CPUs with 256-byte-wide cache lines.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66822/new/

https://reviews.llvm.org/D66822



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66822: Hardware cache line size builtins

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

> BTW, I note that facebook uses 128 bytes for x86

They also use 64 for arm which is interesting (the opposite of this patch).

Also, see this comment in the same snippet:

> We assume a cache line size of 64, so we use a cache line pair size of 128

Which would indicate that 64 is the correct number of bytes for x86. But maybe 
we should double that for `hardware_destructive_interference_size`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66822/new/

https://reviews.llvm.org/D66822



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66839: Fix stack address builtin for negative numbers

[Zoe Carver via Phabricator via cfe-commits]

zoecarver created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
zoecarver added reviewers: kparzysz, eli.friedman.
zoecarver marked an inline comment as done.
zoecarver added inline comments.



Comment at: clang/test/Sema/builtin-stackaddress.c:10
+// expected-error@+1 {{argument to '__builtin_return_address' must be a 
constant integer}}
+return __builtin_return_address(x);
 }

For some reason `lit` was still erroring here. 


This patch checks that an argument passed to either of the builtins 
`__builtin_frame_address` or `__builtin_return_address` is at least 0. This 
patch resolves the issue where these builtins would cause an infinite loop ( 
25497  ).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66839

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/Sema/builtin-stackaddress.c


Index: clang/test/Sema/builtin-stackaddress.c
===
--- clang/test/Sema/builtin-stackaddress.c
+++ clang/test/Sema/builtin-stackaddress.c
@@ -1,16 +1,30 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 %s
+
 void* a(unsigned x) {
 return __builtin_return_address(0);
 }
 
 void b(unsigned x) {
-return __builtin_return_address(x); // expected-error{{argument to 
'__builtin_return_address' must be a constant integer}}
+// TODO: lit doesn't catch this
+// expected-error@+1 {{argument to '__builtin_return_address' must be a 
constant integer}}
+return __builtin_return_address(x);
 }
 
 void* c(unsigned x) {
+// expected-error@+1 {{argument to '__builtin_return_address' must be at least 
0}}
+return __builtin_return_address(-1);
+}
+
+void* d(unsigned x) {
 return __builtin_frame_address(0);
 }
 
-void d(unsigned x) {
-return __builtin_frame_address(x); // expected-error{{argument to 
'__builtin_frame_address' must be a constant integer}}
+void e(unsigned x) {
+// expected-error@+1 {{argument to '__builtin_frame_address' must be a 
constant integer}}
+return __builtin_frame_address(x);
+}
+
+void* f(unsigned x) {
+// expected-error@+1 {{argument to '__builtin_frame_address' must be at least 
0}}
+return __builtin_frame_address(-1);
 }
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -2568,6 +2568,14 @@
   case Builtin::BI__builtin_return_address: {
 Value *Depth = ConstantEmitter(*this).emitAbstract(E->getArg(0),
getContext().UnsignedIntTy);
+llvm::ConstantInt* DepthConstVal = dyn_cast(Depth);
+if (!DepthConstVal)
+CGM.ErrorUnsupported(E,
+ "argument to '__builtin_return_address' must be a 
constant integer");
+if (DepthConstVal->getSExtValue() < 0)
+CGM.ErrorUnsupported(E,
+ "argument to '__builtin_return_address' must be 
at least 0");
+
 Function *F = CGM.getIntrinsic(Intrinsic::returnaddress);
 return RValue::get(Builder.CreateCall(F, Depth));
   }
@@ -2578,6 +2586,14 @@
   case Builtin::BI__builtin_frame_address: {
 Value *Depth = ConstantEmitter(*this).emitAbstract(E->getArg(0),
getContext().UnsignedIntTy);
+llvm::ConstantInt* DepthConstVal = dyn_cast(Depth);
+if (!DepthConstVal)
+CGM.ErrorUnsupported(E,
+ "argument to '__builtin_frame_address' must be a 
constant integer");
+if (DepthConstVal->getSExtValue() < 0)
+CGM.ErrorUnsupported(E,
+ "argument to '__builtin_frame_address' must be at 
least 0");
+
 Function *F = CGM.getIntrinsic(Intrinsic::frameaddress, AllocaInt8PtrTy);
 return RValue::get(Builder.CreateCall(F, Depth));
   }


Index: clang/test/Sema/builtin-stackaddress.c
===
--- clang/test/Sema/builtin-stackaddress.c
+++ clang/test/Sema/builtin-stackaddress.c
@@ -1,16 +1,30 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 %s
+
 void* a(unsigned x) {
 return __builtin_return_address(0);
 }
 
 void b(unsigned x) {
-return __builtin_return_address(x); // expected-error{{argument to '__builtin_return_address' must be a constant integer}}
+// TODO: lit doesn't catch this
+// expected-error@+1 {{argument to '__builtin_return_address' must be a constant integer}}
+return __builtin_return_address(x);
 }
 
 void* c(unsigned x) {
+// expected-error@+1 {{argument to '__builtin_return_address' must be at least 0}}
+return __builtin_return_address(-1);
+}
+
+void* d(unsigned x) {
 return __builtin_frame_address(0);
 }
 
-void d(unsigned x) {
-return __builtin_frame_address(x); // expected-error{{argument to '__builtin_frame_address' must be a constant integer}}
+void e(unsigned x) {
+// expected-error@+1 {

[PATCH] D66839: Fix stack address builtin for negative numbers

[Zoe Carver via Phabricator via cfe-commits]

zoecarver marked an inline comment as done.
zoecarver added inline comments.



Comment at: clang/test/Sema/builtin-stackaddress.c:10
+// expected-error@+1 {{argument to '__builtin_return_address' must be a 
constant integer}}
+return __builtin_return_address(x);
 }

For some reason `lit` was still erroring here. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66839/new/

https://reviews.llvm.org/D66839



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66836: [libc++] Add `__truncating_cast` for safely casting float types to integers

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

Seems like a very useful function. `__max_representable_int_for_float` also 
seems useful. Should this work in C++03? If so there are a few changes that 
need to be made. It would also be great if this could be a `constexpr` (but, 
obviously, not necessary).




Comment at: include/math.h:1556
 
+_LIBCPP_BEGIN_NAMESPACE_STD
+template ::digits > 
numeric_limits<_IntT>::digits)>
+struct __max_representable_int_for_float;

Nit: maybe qualify all the uses of `numeric_limits` and similar?



Comment at: include/math.h:1572
+  static_assert(is_integral<_IntT>::value, "must be an integral type");
+  enum { _Bits = numeric_limits<_IntT>::digits - 
numeric_limits<_FloatT>::digits };
+  static const _IntT value = numeric_limits<_IntT>::max() >> _Bits << _Bits;

What is the enum providing for you? Couldn't this just be `static const int 
_Bits  = ...`?



Comment at: include/math.h:1573
+  enum { _Bits = numeric_limits<_IntT>::digits - 
numeric_limits<_FloatT>::digits };
+  static const _IntT value = numeric_limits<_IntT>::max() >> _Bits << _Bits;
+};

What's the reasoning behind shifting something forward and back? Shouldn't this 
always negate the other operation? 



Comment at: include/math.h:1579
+_IntT __truncating_cast(_RealT __r) _NOEXCEPT {
+  using _Lim = std::numeric_limits<_IntT>;
+  const _IntT _MaxVal = __max_representable_int_for_float<_IntT, 
_RealT>::value;

This will not work before C++11.



Comment at: include/math.h:1582
+  const _RealT __trunc_r = __builtin_trunc(__r);
+  if (__trunc_r >= ::nextafter(static_cast<_RealT>(_MaxVal), INFINITY)) {
+return _Lim::max();

Maybe change `INFINITY` to `std::numeric_limits< _RealT >::infinity()`



Comment at: test/libcxx/numerics/truncating_cast.pass.cpp:10
+// __truncating_cast(RealT)
+
+// Test the conversion function that truncates floating point types to the

Is this supposed to work in C++03? If so, update this test and 
`__truncating_cast`. Otherwise, add an `#if` and a `// UNSUPPORTED: C++98, 
C++03`



Comment at: test/libcxx/numerics/truncating_cast.pass.cpp:25
+  struct {
+double Input;
+IntT Expect;

Maybe test with more than just `double`. `float`, `long double`,  others?



Comment at: test/libcxx/numerics/truncating_cast.pass.cpp:28
+bool IsRepresentable;
+  } TestCases[] = {
+  {0, 0, true},

C++03 will not like this :P


Repository:
  rCXX libc++

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66836/new/

https://reviews.llvm.org/D66836



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66839: Fix stack address builtin for negative numbers

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

Thanks for the explanation. Should I then not try to "fix" the issue? Or should 
I update sema checking?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66839/new/

https://reviews.llvm.org/D66839



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66862: Make lround builtin constexpr (and others)

[Zoe Carver via Phabricator via cfe-commits]

zoecarver created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66862

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/math-builtins.cpp


Index: clang/test/SemaCXX/math-builtins.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/math-builtins.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -std=c++11 %s
+
+int main()
+{
+  constexpr float f = 12345.6789;
+
+  static_assert(__builtin_llround(f) == 12346, "");
+  static_assert(__builtin_llroundf(f) == 12346, "");
+
+  static_assert(__builtin_llrint(f) == 12346, "");
+  static_assert(__builtin_llrintf(f) == 12346, "");
+
+  static_assert(__builtin_lrint(f) == 12346, "");
+  static_assert(__builtin_lrintf(f) == 12346, "");
+
+  static_assert(__builtin_lround(f) == 12346, "");
+  static_assert(__builtin_lroundf(f) == 12346, "");
+}
\ No newline at end of file
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -9609,6 +9609,58 @@
 return Success(N == Val.getBitWidth() ? 0 : N + 1, E);
   }
 
+  case Builtin::BIlround:
+  case Builtin::BI__builtin_lround: {
+APFloat Val(0.0);
+return EvaluateFloat(E->getArg(0), Val, Info) &&
+   Success(lround(Val.convertToDouble()), E);
+  }
+  case Builtin::BIlroundf:
+  case Builtin::BI__builtin_lroundf: {
+APFloat Val(0.0);
+return EvaluateFloat(E->getArg(0), Val, Info) &&
+   Success(lround(Val.convertToFloat()), E);
+  }
+
+  case Builtin::BIllround:
+  case Builtin::BI__builtin_llround: {
+APFloat Val(0.0);
+return EvaluateFloat(E->getArg(0), Val, Info) &&
+   Success(lround(Val.convertToDouble()), E);
+  }
+  case Builtin::BIllroundf:
+  case Builtin::BI__builtin_llroundf: {
+APFloat Val(0.0);
+return EvaluateFloat(E->getArg(0), Val, Info) &&
+   Success(llround(Val.convertToFloat()), E);
+  }
+
+  case Builtin::BIlrint:
+  case Builtin::BI__builtin_lrint: {
+APFloat Val(0.0);
+return EvaluateFloat(E->getArg(0), Val, Info) &&
+   Success(lround(Val.convertToDouble()), E);
+  }
+  case Builtin::BIlrintf:
+  case Builtin::BI__builtin_lrintf: {
+APFloat Val(0.0);
+return EvaluateFloat(E->getArg(0), Val, Info) &&
+   Success(lrint(Val.convertToFloat()), E);
+  }
+
+  case Builtin::BIllrint:
+  case Builtin::BI__builtin_llrint: {
+APFloat Val(0.0);
+return EvaluateFloat(E->getArg(0), Val, Info) &&
+   Success(lround(Val.convertToDouble()), E);
+  }
+  case Builtin::BIllrintf:
+  case Builtin::BI__builtin_llrintf: {
+APFloat Val(0.0);
+return EvaluateFloat(E->getArg(0), Val, Info) &&
+   Success(llrint(Val.convertToFloat()), E);
+  }
+
   case Builtin::BI__builtin_fpclassify: {
 APFloat Val(0.0);
 if (!EvaluateFloat(E->getArg(5), Val, Info))


Index: clang/test/SemaCXX/math-builtins.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/math-builtins.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -std=c++11 %s
+
+int main()
+{
+  constexpr float f = 12345.6789;
+
+  static_assert(__builtin_llround(f) == 12346, "");
+  static_assert(__builtin_llroundf(f) == 12346, "");
+
+  static_assert(__builtin_llrint(f) == 12346, "");
+  static_assert(__builtin_llrintf(f) == 12346, "");
+
+  static_assert(__builtin_lrint(f) == 12346, "");
+  static_assert(__builtin_lrintf(f) == 12346, "");
+
+  static_assert(__builtin_lround(f) == 12346, "");
+  static_assert(__builtin_lroundf(f) == 12346, "");
+}
\ No newline at end of file
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -9609,6 +9609,58 @@
 return Success(N == Val.getBitWidth() ? 0 : N + 1, E);
   }
 
+  case Builtin::BIlround:
+  case Builtin::BI__builtin_lround: {
+APFloat Val(0.0);
+return EvaluateFloat(E->getArg(0), Val, Info) &&
+   Success(lround(Val.convertToDouble()), E);
+  }
+  case Builtin::BIlroundf:
+  case Builtin::BI__builtin_lroundf: {
+APFloat Val(0.0);
+return EvaluateFloat(E->getArg(0), Val, Info) &&
+   Success(lround(Val.convertToFloat()), E);
+  }
+
+  case Builtin::BIllround:
+  case Builtin::BI__builtin_llround: {
+APFloat Val(0.0);
+return EvaluateFloat(E->getArg(0), Val, Info) &&
+   Success(lround(Val.convertToDouble()), E);
+  }
+  case Builtin::BIllroundf:
+  case Builtin::BI__builtin_llroundf: {
+APFloat Val(0.0);
+return EvaluateFloat(E->getArg(0), Val, Info) &&
+   Success(llround(Val.convertToFloat()), E);
+  }
+
+  case Builtin::BIlrint:
+  case Builtin::BI__builtin_lrint: {
+APFloat Val(0.0);
+return EvaluateFloat(E->getArg(0), Val, Info) &&
+   Success(lround(Val.co

[PATCH] D66836: [libc++] Add `__truncating_cast` for safely casting float types to integers

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added inline comments.



Comment at: include/math.h:1582
+  const _RealT __trunc_r = __builtin_trunc(__r);
+  if (__trunc_r >= ::nextafter(static_cast<_RealT>(_MaxVal), INFINITY)) {
+return _Lim::max();

scanon wrote:
> EricWF wrote:
> > scanon wrote:
> > > zoecarver wrote:
> > > > Maybe change `INFINITY` to `std::numeric_limits< _RealT >::infinity()`
> > > Why isn't this just `__trunc_r > _MaxVal`?
> > Consider `long long` and `double`. `MaxVal - numeric_limits > long>::max() == 1024`, and we want values between `MaxVal` and `::max()` to 
> > round down. So instead we essentially check for `__r >= numeric_limits > long>::max() + 1`. This approach seems more accurate.
> > Consider long long and double. MaxVal - numeric_limits::max() == 
> > 1024, and we want values between MaxVal and ::max() to round down. So 
> > instead we essentially check for __r >= numeric_limits::max() + 1
> 
> Yes, but there are no values between MaxVal and ::max() in the floating-point 
> format; if there were, they would be MaxVal instead. So you can ditch the 
> nextafter and just use `> static_cast<_FloatT>(MaxVal)`.
I thought the same thing, but that isn't necessarily true. Eric showed me this 
link https://godbolt.org/z/AjBHYqv which does a good job showing what happens 
when trying to compare an integer value and float value. See the precision loss:
> warning: implicit conversion from 'long long' to 'double' changes value from 
> 9223372036854775807 to 9223372036854775808 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66836/new/

https://reviews.llvm.org/D66836



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66836: [libc++] Add `__truncating_cast` for safely casting float types to integers

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

> Dead link.

Here: https://godbolt.org/z/AjBHYq


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66836/new/

https://reviews.llvm.org/D66836



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66836: [libc++] Add `__truncating_cast` for safely casting float types to integers

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

Yes, that sounds right. I can't think of any reason that the condition couldn't 
be `if (__r >  static_cast(numeric_limits::max()))`. The 
information lost from shifting the value around is never more than the 
information lost from static_casting the value (as far as I have been able to 
reason and test).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66836/new/

https://reviews.llvm.org/D66836



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66862: Make lround builtin constexpr (and others)

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

(from IRC) Add tests for:

- out of range
- nan
- rounding up _and_ down

Also, use the builtin APFloat methods.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66862/new/

https://reviews.llvm.org/D66862



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66862: Make lround builtin constexpr (and others)

[Zoe Carver via Phabricator via cfe-commits]

zoecarver updated this revision to Diff 217973.
zoecarver added a comment.

- remove rint updates (rint uses runtime-defined rounding method)
- add more tests (nan, overflow, round down)
- use APFloat rounding methods
- if rounding fails, bail out to runtime


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66862/new/

https://reviews.llvm.org/D66862

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/math-builtins.cpp


Index: clang/test/SemaCXX/math-builtins.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/math-builtins.cpp
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -std=c++11 %s
+
+int main()
+{
+  constexpr float f1 = 12345.6789;
+  constexpr float f2 = 12345.4321;
+
+  static_assert(__builtin_llround(f1) == 12346, "");
+  static_assert(__builtin_llroundf(f1) == 12346, "");
+
+  static_assert(__builtin_lround(f1) == 12346, "");
+  static_assert(__builtin_lroundf(f1) == 12346, "");
+
+  static_assert(__builtin_llround(f2) == 12345, "");
+  static_assert(__builtin_llroundf(f2) == 12345, "");
+
+  static_assert(__builtin_lround(f2) == 12345, "");
+  static_assert(__builtin_lroundf(f2) == 12345, "");
+
+  static_assert(__builtin_llround(float()) == 0, ""); // expected-error 
{{static_assert expression is not an integral constant expression}}
+  static_assert(__builtin_llroundf(float()) == 0, ""); // expected-error 
{{static_assert expression is not an integral constant expression}}
+
+  static_assert(__builtin_lround(float()) == 0, ""); // expected-error 
{{static_assert expression is not an integral constant expression}}
+  static_assert(__builtin_lroundf(float()) == 0, ""); // expected-error 
{{static_assert expression is not an integral constant expression}}
+
+  // TODO: expected-error not working
+  static_assert(__builtin_llround(__FLT_MAX__) == 0, ""); // expected-error 
{{static_assert expression is not an integral constant expression}}
+  static_assert(__builtin_llroundf(__FLT_MAX__) == 0, ""); // expected-error 
{{static_assert expression is not an integral constant expression}}
+
+  static_assert(__builtin_lround(__FLT_MAX__) == 0, ""); // expected-error 
{{static_assert expression is not an integral constant expression}}
+  static_assert(__builtin_lroundf(__FLT_MAX__) == 0, ""); // expected-error 
{{static_assert expression is not an integral constant expression}}
+}
\ No newline at end of file
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -9609,6 +9609,22 @@
 return Success(N == Val.getBitWidth() ? 0 : N + 1, E);
   }
 
+  case Builtin::BI__builtin_lround:
+  case Builtin::BI__builtin_lroundf:
+  case Builtin::BI__builtin_llround:
+  case Builtin::BI__builtin_llroundf: {
+APFloat FPVal(0.0);
+APSInt IVal(64, 0); // TODO: dynamically get correct size here?
+bool isExact = true;
+
+if(!EvaluateFloat(E->getArg(0), FPVal, Info)) return false;
+APFloat::opStatus status =
+FPVal.convertToInteger(IVal, APFloat::rmNearestTiesToAway, &isExact);
+if(status != APFloat::opInexact && status != APFloat::opOK) return false;
+
+return Success(IVal.getExtValue(), E);
+  }
+
   case Builtin::BI__builtin_fpclassify: {
 APFloat Val(0.0);
 if (!EvaluateFloat(E->getArg(5), Val, Info))


Index: clang/test/SemaCXX/math-builtins.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/math-builtins.cpp
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -std=c++11 %s
+
+int main()
+{
+  constexpr float f1 = 12345.6789;
+  constexpr float f2 = 12345.4321;
+
+  static_assert(__builtin_llround(f1) == 12346, "");
+  static_assert(__builtin_llroundf(f1) == 12346, "");
+
+  static_assert(__builtin_lround(f1) == 12346, "");
+  static_assert(__builtin_lroundf(f1) == 12346, "");
+
+  static_assert(__builtin_llround(f2) == 12345, "");
+  static_assert(__builtin_llroundf(f2) == 12345, "");
+
+  static_assert(__builtin_lround(f2) == 12345, "");
+  static_assert(__builtin_lroundf(f2) == 12345, "");
+
+  static_assert(__builtin_llround(float()) == 0, ""); // expected-error {{static_assert expression is not an integral constant expression}}
+  static_assert(__builtin_llroundf(float()) == 0, ""); // expected-error {{static_assert expression is not an integral constant expression}}
+
+  static_assert(__builtin_lround(float()) == 0, ""); // expected-error {{static_assert expression is not an integral constant expression}}
+  static_assert(__builtin_lroundf(float()) == 0, ""); // expected-error {{static_assert expression is not an integral constant expression}}
+
+  // TODO: expected-error not working
+  static_assert(__builtin_llround(__FLT_MAX__) == 0, ""); // expected-error {{static_assert expression is not an integral constant expression}}
+  static_assert(__builtin_llroundf(__FLT_MAX__) == 0, ""); // expected-e

[PATCH] D66862: Make lround builtin constexpr (and others)

[Zoe Carver via Phabricator via cfe-commits]

zoecarver marked 3 inline comments as done.
zoecarver added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:9612
 
+  case Builtin::BIlround:
+  case Builtin::BI__builtin_lround: {

rsmith wrote:
> It's non-conforming to accept calls to these non-`__builtin` functions in 
> constant expression evaluation, but it's fine to constant-fold them. Please 
> issue a `CCEDiag` diagnostic for them (take a look at what we do for 
> `Builtin::BIstrlen` or `Builtin::BIstrcmp` for an example; it would probably 
> be reasonable to factor out a local lambda to issue suitable diagnostics for 
> this case).
> 
> (For example, we are required to produce a diagnostic for `constexpr long n = 
> lround(1.2);`, but we are permitted to constant-fold `long n = lround(1.2);` 
> and emit it with static initialization.)
Thank you for pointing this out. I will do that in a separate patch.



Comment at: clang/lib/AST/ExprConstant.cpp:9616
+return EvaluateFloat(E->getArg(0), Val, Info) &&
+   Success(lround(Val.convertToDouble()), E);
+  }

rsmith wrote:
> This assumes that the host `double` has the same behavior as the target 
> `double`, which is not true in general. Generally the right thing to do is to 
> implement the relevant functionality on `APFloat`, taking care to produce the 
> right answer for the floating-point model being used by that `APFloat`. This 
> also assumes that the host `long` is the same size as the target `long`, 
> which again is not true in general. Finally, you need to catch the case where 
> the `APFloat` does not fit into the target type; in that case, the best 
> response is probably to treat the evaluation as non-constant and leave it to 
> the runtime library to implement whatever error response it chooses.
I've updated it to do just that: if there is an error, it will treat this 
evaluation as non-constant and leave it for the runtime library to handle. 

How should I determine the width of the APInt? Because it is run at 
compile-time, can I use the max size (long long)? Either way, how should I get 
the correct size, maybe from a builtin long type? 



Comment at: clang/test/SemaCXX/math-builtins.cpp:20
+
+  static_assert(__builtin_llround(float()) == 0, ""); // expected-error 
{{static_assert expression is not an integral constant expression}}
+  static_assert(__builtin_llroundf(float()) == 0, ""); // expected-error 
{{static_assert expression is not an integral constant expression}}

For some reasons, these `expected-error`s aren't being caught by lit. Any 
ideas? Do I need to put it in a `.fail.cpp` file or add something to the `RUN:` 
comment? 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66862/new/

https://reviews.llvm.org/D66862



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66862: Make lround builtin constexpr (and others)

[Zoe Carver via Phabricator via cfe-commits]

zoecarver marked 2 inline comments as done.
zoecarver added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:9616
+return EvaluateFloat(E->getArg(0), Val, Info) &&
+   Success(lround(Val.convertToDouble()), E);
+  }

lebedev.ri wrote:
> zoecarver wrote:
> > rsmith wrote:
> > > This assumes that the host `double` has the same behavior as the target 
> > > `double`, which is not true in general. Generally the right thing to do 
> > > is to implement the relevant functionality on `APFloat`, taking care to 
> > > produce the right answer for the floating-point model being used by that 
> > > `APFloat`. This also assumes that the host `long` is the same size as the 
> > > target `long`, which again is not true in general. Finally, you need to 
> > > catch the case where the `APFloat` does not fit into the target type; in 
> > > that case, the best response is probably to treat the evaluation as 
> > > non-constant and leave it to the runtime library to implement whatever 
> > > error response it chooses.
> > I've updated it to do just that: if there is an error, it will treat this 
> > evaluation as non-constant and leave it for the runtime library to handle. 
> > 
> > How should I determine the width of the APInt? Because it is run at 
> > compile-time, can I use the max size (long long)? Either way, how should I 
> > get the correct size, maybe from a builtin long type? 
> You probably want to look at the original callexpr, take it's return type, 
> and look it up in typesystem (i'm not sure exactly how to do that here)
Ah, good idea. Will do.



Comment at: clang/test/SemaCXX/math-builtins.cpp:1
+// RUN: %clang_cc1 -std=c++11 %s
+

lebedev.ri wrote:
> -verify ?
Yes, that worked. It was throwing me off the first time I tried because it 
doesn't expect the nan tests to error. Any ideas on generating a nan float? 
Thanks for the help :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66862/new/

https://reviews.llvm.org/D66862



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66862: Make lround builtin constexpr (and others)

[Zoe Carver via Phabricator via cfe-commits]

zoecarver marked an inline comment as done.
zoecarver added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:9616
+return EvaluateFloat(E->getArg(0), Val, Info) &&
+   Success(lround(Val.convertToDouble()), E);
+  }

craig.topper wrote:
> zoecarver wrote:
> > lebedev.ri wrote:
> > > zoecarver wrote:
> > > > rsmith wrote:
> > > > > This assumes that the host `double` has the same behavior as the 
> > > > > target `double`, which is not true in general. Generally the right 
> > > > > thing to do is to implement the relevant functionality on `APFloat`, 
> > > > > taking care to produce the right answer for the floating-point model 
> > > > > being used by that `APFloat`. This also assumes that the host `long` 
> > > > > is the same size as the target `long`, which again is not true in 
> > > > > general. Finally, you need to catch the case where the `APFloat` does 
> > > > > not fit into the target type; in that case, the best response is 
> > > > > probably to treat the evaluation as non-constant and leave it to the 
> > > > > runtime library to implement whatever error response it chooses.
> > > > I've updated it to do just that: if there is an error, it will treat 
> > > > this evaluation as non-constant and leave it for the runtime library to 
> > > > handle. 
> > > > 
> > > > How should I determine the width of the APInt? Because it is run at 
> > > > compile-time, can I use the max size (long long)? Either way, how 
> > > > should I get the correct size, maybe from a builtin long type? 
> > > You probably want to look at the original callexpr, take it's return 
> > > type, and look it up in typesystem (i'm not sure exactly how to do that 
> > > here)
> > Ah, good idea. Will do.
> Info.Ctx.getIntWidth(E->getType())I think.
@craig.topper fantastic, you're the best!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66862/new/

https://reviews.llvm.org/D66862



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66862: Make lround builtin constexpr (and others)

[Zoe Carver via Phabricator via cfe-commits]

zoecarver updated this revision to Diff 217977.
zoecarver added a comment.

- fix: formatting


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66862/new/

https://reviews.llvm.org/D66862

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/math-builtins.cpp


Index: clang/test/SemaCXX/math-builtins.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/math-builtins.cpp
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -std=c++11 -verify %s
+
+int main()
+{
+  constexpr float f1 = 12345.6789;
+  constexpr float f2 = 12345.4321;
+
+  static_assert(__builtin_llround(f1) == 12346, "");
+  static_assert(__builtin_llroundf(f1) == 12346, "");
+
+  static_assert(__builtin_lround(f1) == 12346, "");
+  static_assert(__builtin_lroundf(f1) == 12346, "");
+
+  static_assert(__builtin_llround(f2) == 12345, "");
+  static_assert(__builtin_llroundf(f2) == 12345, "");
+
+  static_assert(__builtin_lround(f2) == 12345, "");
+  static_assert(__builtin_lroundf(f2) == 12345, "");
+
+  static_assert(__builtin_llround(__builtin_nanf("")) == 0, ""); // 
expected-error {{static_assert expression is not an integral constant 
expression}}
+  static_assert(__builtin_llroundf(__builtin_nanf("")) == 0, ""); // 
expected-error {{static_assert expression is not an integral constant 
expression}}
+
+  static_assert(__builtin_lround(__builtin_nanf("")) == 0, ""); // 
expected-error {{static_assert expression is not an integral constant 
expression}}
+  static_assert(__builtin_lroundf(__builtin_nanf("")) == 0, ""); // 
expected-error {{static_assert expression is not an integral constant 
expression}}
+
+  static_assert(__builtin_llround(__FLT_MAX__) == 0, ""); // expected-error 
{{static_assert expression is not an integral constant expression}}
+  static_assert(__builtin_llroundf(__FLT_MAX__) == 0, ""); // expected-error 
{{static_assert expression is not an integral constant expression}}
+
+  static_assert(__builtin_lround(__FLT_MAX__) == 0, ""); // expected-error 
{{static_assert expression is not an integral constant expression}}
+  static_assert(__builtin_lroundf(__FLT_MAX__) == 0, ""); // expected-error 
{{static_assert expression is not an integral constant expression}}
+}
\ No newline at end of file
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -9609,6 +9609,22 @@
 return Success(N == Val.getBitWidth() ? 0 : N + 1, E);
   }
 
+  case Builtin::BI__builtin_lround:
+  case Builtin::BI__builtin_lroundf:
+  case Builtin::BI__builtin_llround:
+  case Builtin::BI__builtin_llroundf: {
+APFloat FPVal(0.0);
+APSInt IVal(Info.Ctx.getIntWidth(E->getType()), 0);
+bool isExact = true;
+
+if (!EvaluateFloat(E->getArg(0), FPVal, Info)) return false;
+APFloat::opStatus status =
+FPVal.convertToInteger(IVal, APFloat::rmNearestTiesToAway, &isExact);
+if (status != APFloat::opInexact && status != APFloat::opOK) return false;
+
+return Success(IVal.getExtValue(), E);
+  }
+
   case Builtin::BI__builtin_fpclassify: {
 APFloat Val(0.0);
 if (!EvaluateFloat(E->getArg(5), Val, Info))


Index: clang/test/SemaCXX/math-builtins.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/math-builtins.cpp
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -std=c++11 -verify %s
+
+int main()
+{
+  constexpr float f1 = 12345.6789;
+  constexpr float f2 = 12345.4321;
+
+  static_assert(__builtin_llround(f1) == 12346, "");
+  static_assert(__builtin_llroundf(f1) == 12346, "");
+
+  static_assert(__builtin_lround(f1) == 12346, "");
+  static_assert(__builtin_lroundf(f1) == 12346, "");
+
+  static_assert(__builtin_llround(f2) == 12345, "");
+  static_assert(__builtin_llroundf(f2) == 12345, "");
+
+  static_assert(__builtin_lround(f2) == 12345, "");
+  static_assert(__builtin_lroundf(f2) == 12345, "");
+
+  static_assert(__builtin_llround(__builtin_nanf("")) == 0, ""); // expected-error {{static_assert expression is not an integral constant expression}}
+  static_assert(__builtin_llroundf(__builtin_nanf("")) == 0, ""); // expected-error {{static_assert expression is not an integral constant expression}}
+
+  static_assert(__builtin_lround(__builtin_nanf("")) == 0, ""); // expected-error {{static_assert expression is not an integral constant expression}}
+  static_assert(__builtin_lroundf(__builtin_nanf("")) == 0, ""); // expected-error {{static_assert expression is not an integral constant expression}}
+
+  static_assert(__builtin_llround(__FLT_MAX__) == 0, ""); // expected-error {{static_assert expression is not an integral constant expression}}
+  static_assert(__builtin_llroundf(__FLT_MAX__) == 0, ""); // expected-error {{static_assert expression is not an integral constant expression}}
+
+  static_assert(__builtin_lround(__FLT_MAX__) == 0, ""); // expected-

[PATCH] D66862: Make lround builtin constexpr (and others)

[Zoe Carver via Phabricator via cfe-commits]

zoecarver updated this revision to Diff 217976.
zoecarver added a comment.

- fix expected error
- fix APInt width
- fix nan tests


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66862/new/

https://reviews.llvm.org/D66862

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/math-builtins.cpp


Index: clang/test/SemaCXX/math-builtins.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/math-builtins.cpp
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -std=c++11 -verify %s
+
+int main()
+{
+  constexpr float f1 = 12345.6789;
+  constexpr float f2 = 12345.4321;
+
+  static_assert(__builtin_llround(f1) == 12346, "");
+  static_assert(__builtin_llroundf(f1) == 12346, "");
+
+  static_assert(__builtin_lround(f1) == 12346, "");
+  static_assert(__builtin_lroundf(f1) == 12346, "");
+
+  static_assert(__builtin_llround(f2) == 12345, "");
+  static_assert(__builtin_llroundf(f2) == 12345, "");
+
+  static_assert(__builtin_lround(f2) == 12345, "");
+  static_assert(__builtin_lroundf(f2) == 12345, "");
+
+  static_assert(__builtin_llround(__builtin_nanf("")) == 0, ""); // 
expected-error {{static_assert expression is not an integral constant 
expression}}
+  static_assert(__builtin_llroundf(__builtin_nanf("")) == 0, ""); // 
expected-error {{static_assert expression is not an integral constant 
expression}}
+
+  static_assert(__builtin_lround(__builtin_nanf("")) == 0, ""); // 
expected-error {{static_assert expression is not an integral constant 
expression}}
+  static_assert(__builtin_lroundf(__builtin_nanf("")) == 0, ""); // 
expected-error {{static_assert expression is not an integral constant 
expression}}
+
+  static_assert(__builtin_llround(__FLT_MAX__) == 0, ""); // expected-error 
{{static_assert expression is not an integral constant expression}}
+  static_assert(__builtin_llroundf(__FLT_MAX__) == 0, ""); // expected-error 
{{static_assert expression is not an integral constant expression}}
+
+  static_assert(__builtin_lround(__FLT_MAX__) == 0, ""); // expected-error 
{{static_assert expression is not an integral constant expression}}
+  static_assert(__builtin_lroundf(__FLT_MAX__) == 0, ""); // expected-error 
{{static_assert expression is not an integral constant expression}}
+}
\ No newline at end of file
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -9609,6 +9609,22 @@
 return Success(N == Val.getBitWidth() ? 0 : N + 1, E);
   }
 
+  case Builtin::BI__builtin_lround:
+  case Builtin::BI__builtin_lroundf:
+  case Builtin::BI__builtin_llround:
+  case Builtin::BI__builtin_llroundf: {
+APFloat FPVal(0.0);
+APSInt IVal(Info.Ctx.getIntWidth(E->getType()), 0);
+bool isExact = true;
+
+if(!EvaluateFloat(E->getArg(0), FPVal, Info)) return false;
+APFloat::opStatus status =
+FPVal.convertToInteger(IVal, APFloat::rmNearestTiesToAway, &isExact);
+if(status != APFloat::opInexact && status != APFloat::opOK) return false;
+
+return Success(IVal.getExtValue(), E);
+  }
+
   case Builtin::BI__builtin_fpclassify: {
 APFloat Val(0.0);
 if (!EvaluateFloat(E->getArg(5), Val, Info))


Index: clang/test/SemaCXX/math-builtins.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/math-builtins.cpp
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -std=c++11 -verify %s
+
+int main()
+{
+  constexpr float f1 = 12345.6789;
+  constexpr float f2 = 12345.4321;
+
+  static_assert(__builtin_llround(f1) == 12346, "");
+  static_assert(__builtin_llroundf(f1) == 12346, "");
+
+  static_assert(__builtin_lround(f1) == 12346, "");
+  static_assert(__builtin_lroundf(f1) == 12346, "");
+
+  static_assert(__builtin_llround(f2) == 12345, "");
+  static_assert(__builtin_llroundf(f2) == 12345, "");
+
+  static_assert(__builtin_lround(f2) == 12345, "");
+  static_assert(__builtin_lroundf(f2) == 12345, "");
+
+  static_assert(__builtin_llround(__builtin_nanf("")) == 0, ""); // expected-error {{static_assert expression is not an integral constant expression}}
+  static_assert(__builtin_llroundf(__builtin_nanf("")) == 0, ""); // expected-error {{static_assert expression is not an integral constant expression}}
+
+  static_assert(__builtin_lround(__builtin_nanf("")) == 0, ""); // expected-error {{static_assert expression is not an integral constant expression}}
+  static_assert(__builtin_lroundf(__builtin_nanf("")) == 0, ""); // expected-error {{static_assert expression is not an integral constant expression}}
+
+  static_assert(__builtin_llround(__FLT_MAX__) == 0, ""); // expected-error {{static_assert expression is not an integral constant expression}}
+  static_assert(__builtin_llroundf(__FLT_MAX__) == 0, ""); // expected-error {{static_assert expression is not an integral constant expression}}
+
+  static_assert(__builtin_lround(_

[PATCH] D66862: Make lround builtin constexpr (and others)

[Zoe Carver via Phabricator via cfe-commits]

zoecarver marked an inline comment as done.
zoecarver added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:9617
+APFloat FPVal(0.0);
+APSInt IVal(Info.Ctx.getIntWidth(E->getType()), 0);
+bool isExact = true;

rsmith wrote:
> Please use `/*isUnsigned=*/false` for the second argument rather than `0`.
I'm using the default value of `isUnsigned` (false). Do you want me to specify 
it? The second argument (`0`) is the initial value (which is required). 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66862/new/

https://reviews.llvm.org/D66862



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66862: Make lround builtin constexpr (and others)

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

> Are you intentionally excluding __builtin_lroundl/__builtin_llroundl?

I //was// because `convertToDouble` could only return up to 64 bytes. But now 
that I am using the builtin APFloat round function, that works.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66862/new/

https://reviews.llvm.org/D66862



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66862: Make lround builtin constexpr (and others)

[Zoe Carver via Phabricator via cfe-commits]

zoecarver updated this revision to Diff 218231.
zoecarver added a comment.

- add roundl builtins
- add more tests
- address review comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66862/new/

https://reviews.llvm.org/D66862

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/math-builtins.cpp


Index: clang/test/SemaCXX/math-builtins.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/math-builtins.cpp
@@ -0,0 +1,58 @@
+// RUN: %clang_cc1 -std=c++11 -verify %s
+
+int main()
+{
+  constexpr float f1 = 12345.6789;
+  constexpr float f2 = 12345.4321;
+  constexpr float f3 = 0.5;
+  constexpr float f4 = -0.5;
+
+  static_assert(__builtin_llround(f1) == 12346, "");
+  static_assert(__builtin_llroundf(f1) == 12346, "");
+  static_assert(__builtin_llroundl(f1) == 12346, "");
+
+  static_assert(__builtin_lround(f1) == 12346, "");
+  static_assert(__builtin_lroundf(f1) == 12346, "");
+  static_assert(__builtin_lroundl(f1) == 12346, "");
+
+  static_assert(__builtin_llround(f2) == 12345, "");
+  static_assert(__builtin_llroundf(f2) == 12345, "");
+  static_assert(__builtin_llroundl(f2) == 12345, "");
+
+  static_assert(__builtin_lround(f2) == 12345, "");
+  static_assert(__builtin_lroundf(f2) == 12345, "");
+  static_assert(__builtin_lroundl(f2) == 12345, "");
+
+  static_assert(__builtin_llround(f3) == 1, "");
+  static_assert(__builtin_llroundf(f3) == 1, "");
+  static_assert(__builtin_llroundl(f3) == 1, "");
+
+  static_assert(__builtin_lround(f3) == 1, "");
+  static_assert(__builtin_lroundf(f3) == 1, "");
+  static_assert(__builtin_lroundl(f3) == 1, "");
+
+  static_assert(__builtin_llround(f4) == -1, "");
+  static_assert(__builtin_llroundf(f4) == -1, "");
+  static_assert(__builtin_llroundl(f4) == -1, "");
+
+  static_assert(__builtin_lround(f4) == -1, "");
+  static_assert(__builtin_lroundf(f4) == -1, "");
+  static_assert(__builtin_lroundl(f4) == -1, "");
+
+  static_assert(__builtin_llround(__builtin_nanf("")) == 0, ""); // 
expected-error {{static_assert expression is not an integral constant 
expression}}
+  static_assert(__builtin_llroundf(__builtin_nanf("")) == 0, ""); // 
expected-error {{static_assert expression is not an integral constant 
expression}}
+  static_assert(__builtin_llroundl(__builtin_nanf("")) == 0, ""); // 
expected-error {{static_assert expression is not an integral constant 
expression}}
+
+  static_assert(__builtin_lround(__builtin_nanf("")) == 0, ""); // 
expected-error {{static_assert expression is not an integral constant 
expression}}
+  static_assert(__builtin_lroundf(__builtin_nanf("")) == 0, ""); // 
expected-error {{static_assert expression is not an integral constant 
expression}}
+  static_assert(__builtin_lroundl(__builtin_nanf("")) == 0, ""); // 
expected-error {{static_assert expression is not an integral constant 
expression}}
+
+  static_assert(__builtin_llround(__FLT_MAX__) == 0, ""); // expected-error 
{{static_assert expression is not an integral constant expression}}
+  static_assert(__builtin_llroundf(__FLT_MAX__) == 0, ""); // expected-error 
{{static_assert expression is not an integral constant expression}}
+  static_assert(__builtin_llroundl(__FLT_MAX__) == 0, ""); // expected-error 
{{static_assert expression is not an integral constant expression}}
+
+  static_assert(__builtin_lround(__FLT_MAX__) == 0, ""); // expected-error 
{{static_assert expression is not an integral constant expression}}
+  static_assert(__builtin_lroundf(__FLT_MAX__) == 0, ""); // expected-error 
{{static_assert expression is not an integral constant expression}}
+  static_assert(__builtin_lroundl(__FLT_MAX__) == 0, ""); // expected-error 
{{static_assert expression is not an integral constant expression}}
+
+}
\ No newline at end of file
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -9609,6 +9609,24 @@
 return Success(N == Val.getBitWidth() ? 0 : N + 1, E);
   }
 
+  case Builtin::BI__builtin_lround:
+  case Builtin::BI__builtin_lroundf:
+  case Builtin::BI__builtin_lroundl:
+  case Builtin::BI__builtin_llround:
+  case Builtin::BI__builtin_llroundf:
+  case Builtin::BI__builtin_llroundl: {
+APFloat FPVal(0.0);
+APSInt IVal(Info.Ctx.getIntWidth(E->getType()), 0);
+bool IsExact = true;
+
+if (!EvaluateFloat(E->getArg(0), FPVal, Info)) return false;
+APFloat::opStatus RoundStatus =
+FPVal.convertToInteger(IVal, APFloat::rmNearestTiesToAway, &IsExact);
+if (RoundStatus != APFloat::opInexact && RoundStatus != APFloat::opOK) 
return false;
+
+return Success(IVal, E);
+  }
+
   case Builtin::BI__builtin_fpclassify: {
 APFloat Val(0.0);
 if (!EvaluateFloat(E->getArg(5), Val, Info))


Index: clang/test/SemaCXX/math-builtins.cpp
=

[PATCH] D67052: Add reference type transformation builtins

[Zoe Carver via Phabricator via cfe-commits]

zoecarver created this revision.
zoecarver added reviewers: EricWF, eli.friedman, rsmith, craig.topper.
Herald added subscribers: libcxx-commits, cfe-commits, jfb, christof.
Herald added projects: clang, libc++.

This patch adds builtin type traits to transform reference types. Specifically, 
it adds `__add_lvalue_reference`, `__add_rvalue_reference`, and 
`__remove_reference`. The first two builtins speed up builds by around 3x while 
the last builtin only sees small improvements (we may be able to optimize it 
more, though). Once added to the standard library, this should make libc++ (and 
other code) much faster to compile.

I tried to generalize as much of the builtin as possible so, the only 
functional difference between the three builtins is in the file 
`BuildUnaryTransformType`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67052

Files:
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/DeclSpec.h
  clang/lib/Format/FormatToken.cpp
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaTemplateVariadic.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/SemaCXX/add_reference.cpp
  libcxx/test/libcxx/utilities/meta/stress_tests/lit.site.cfg
  
libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_add_lvalue_reference.sh.cpp
  
libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_add_rvalue_reference.sh.cpp
  
libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_remove_reference.sh.cpp

Index: libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_remove_reference.sh.cpp
===
--- /dev/null
+++ libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_remove_reference.sh.cpp
@@ -0,0 +1,63 @@
+//===--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This is a dummy feature that prevents this test from running by default.
+
+// The table below compares the compile time and object size for each of the
+// variants listed in the RUN script.
+//
+//  Impl   Compile Time  Object Size
+// --
+// new_remove_reference:   22.849 s  121 K
+// std::remove_reference:  25.643 s  121 K
+//
+// RUN: %cxx %flags %compile_flags -c %s -o %S/orig.o -ggdb  -ggnu-pubnames -ftemplate-depth=5000 -ftime-trace -std=c++17
+// RUN: %cxx %flags %compile_flags -c %s -o %S/new.o -ggdb  -ggnu-pubnames -ftemplate-depth=5000 -ftime-trace -std=c++17 -DTEST_NEW
+
+#include 
+#include 
+
+#include "test_macros.h"
+#include "template_cost_testing.h"
+
+template  struct Arg { enum { value = 1 }; };
+
+#ifdef TEST_NEW
+
+template 
+struct new_remove_reference
+{
+  typedef __remove_reference(T) type;
+};
+
+#define TEST_CASE_NOP()  new_remove_reference< Arg< __COUNTER__ > >{},
+#define TEST_CASE_TYPE() typename new_remove_reference< Arg< __COUNTER__ > >::type,
+
+#else
+
+#define TEST_CASE_NOP()  std::remove_reference< Arg< __COUNTER__ > >{},
+#define TEST_CASE_TYPE() typename std::remove_reference< Arg< __COUNTER__ > >::type,
+
+#endif
+
+int sink(...);
+
+int x = sink(
+  REPEAT_1(TEST_CASE_NOP)
+  REPEAT_1(TEST_CASE_NOP) 42
+);
+
+void Foo( REPEAT_1(TEST_CASE_TYPE) int) { }
+
+void escape() {
+
+sink(&x);
+sink(&Foo);
+}
+
+
Index: libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_add_rvalue_reference.sh.cpp
===
--- /dev/null
+++ libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_add_rvalue_reference.sh.cpp
@@ -0,0 +1,63 @@
+//===--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This is a dummy feature that prevents this test from running by default.
+
+// The table below compares the compile time and object size for each of the
+// variants listed in the RUN script.
+//
+//  Impl   Compile Time  Object Size
+// --
+// new_add_rvalue_reference:   56.398 s  171 K
+// std::add_rvalue_reference:  114.59 s  271 K
+//
+// RUN: %cxx %flags %compile_flags -c %

[PATCH] D67052: Add reference type transformation builtins

[Zoe Carver via Phabricator via cfe-commits]

zoecarver marked an inline comment as done.
zoecarver added inline comments.



Comment at: 
libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_remove_reference.sh.cpp:13
+// variants listed in the RUN script.
+//
+//  Impl   Compile Time  Object Size

I think there is a good chance the reason that my added builtin isn't 
significantly faster is that `std::remove_reference` is already optimized 
somewhere.  Thoughts?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67052/new/

https://reviews.llvm.org/D67052



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67052: Add reference type transformation builtins

[Zoe Carver via Phabricator via cfe-commits]

zoecarver updated this revision to Diff 218276.
zoecarver added a comment.

- remove accedentally added file


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67052/new/

https://reviews.llvm.org/D67052

Files:
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/DeclSpec.h
  clang/lib/Format/FormatToken.cpp
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaTemplateVariadic.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/SemaCXX/add_reference.cpp
  
libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_add_lvalue_reference.sh.cpp
  
libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_add_rvalue_reference.sh.cpp
  
libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_remove_reference.sh.cpp

Index: libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_remove_reference.sh.cpp
===
--- /dev/null
+++ libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_remove_reference.sh.cpp
@@ -0,0 +1,63 @@
+//===--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This is a dummy feature that prevents this test from running by default.
+
+// The table below compares the compile time and object size for each of the
+// variants listed in the RUN script.
+//
+//  Impl   Compile Time  Object Size
+// --
+// new_remove_reference:   22.849 s  121 K
+// std::remove_reference:  25.643 s  121 K
+//
+// RUN: %cxx %flags %compile_flags -c %s -o %S/orig.o -ggdb  -ggnu-pubnames -ftemplate-depth=5000 -ftime-trace -std=c++17
+// RUN: %cxx %flags %compile_flags -c %s -o %S/new.o -ggdb  -ggnu-pubnames -ftemplate-depth=5000 -ftime-trace -std=c++17 -DTEST_NEW
+
+#include 
+#include 
+
+#include "test_macros.h"
+#include "template_cost_testing.h"
+
+template  struct Arg { enum { value = 1 }; };
+
+#ifdef TEST_NEW
+
+template 
+struct new_remove_reference
+{
+  typedef __remove_reference(T) type;
+};
+
+#define TEST_CASE_NOP()  new_remove_reference< Arg< __COUNTER__ > >{},
+#define TEST_CASE_TYPE() typename new_remove_reference< Arg< __COUNTER__ > >::type,
+
+#else
+
+#define TEST_CASE_NOP()  std::remove_reference< Arg< __COUNTER__ > >{},
+#define TEST_CASE_TYPE() typename std::remove_reference< Arg< __COUNTER__ > >::type,
+
+#endif
+
+int sink(...);
+
+int x = sink(
+  REPEAT_1(TEST_CASE_NOP)
+  REPEAT_1(TEST_CASE_NOP) 42
+);
+
+void Foo( REPEAT_1(TEST_CASE_TYPE) int) { }
+
+void escape() {
+
+sink(&x);
+sink(&Foo);
+}
+
+
Index: libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_add_rvalue_reference.sh.cpp
===
--- /dev/null
+++ libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_add_rvalue_reference.sh.cpp
@@ -0,0 +1,63 @@
+//===--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This is a dummy feature that prevents this test from running by default.
+
+// The table below compares the compile time and object size for each of the
+// variants listed in the RUN script.
+//
+//  Impl   Compile Time  Object Size
+// --
+// new_add_rvalue_reference:   56.398 s  171 K
+// std::add_rvalue_reference:  114.59 s  271 K
+//
+// RUN: %cxx %flags %compile_flags -c %s -o %S/orig.o -ggdb  -ggnu-pubnames -ftemplate-depth=5000 -ftime-trace -std=c++17
+// RUN: %cxx %flags %compile_flags -c %s -o %S/new.o -ggdb  -ggnu-pubnames -ftemplate-depth=5000 -ftime-trace -std=c++17 -DTEST_NEW
+
+#include 
+#include 
+
+#include "test_macros.h"
+#include "template_cost_testing.h"
+
+template  struct Arg { enum { value = 1 }; };
+
+#ifdef TEST_NEW
+
+template 
+struct new_add_rvalue_reference
+{
+  typedef __add_rvalue_reference(T) type;
+};
+
+#define TEST_CASE_NOP()  new_add_rvalue_reference< Arg< __COUNTER__ > >{},
+#define TEST_CASE_TYPE() typename new_add_rvalue_reference< Arg< __COUNTER__ > >::type,
+
+#else
+
+#define TEST_CASE_NOP()  std::add_rvalu

[PATCH] D67052: Add reference type transformation builtins

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

In D67052#1656319 , @mclow.lists wrote:

> If you're going to do `__add__lvalue_reference`, `__add_rvalue_reference`, 
> and `__remove_reference`, why not go all the way and add `__is_reference`, 
> `__is_lvalue_reference` and `__is_rvalue_reference`?


Those already exist :)

I am planning on adding others, `__remove_cv`, `__decay`, etc. but wanted to 
land this first and make sure I was doing it right.

I'll update `type_traits` in libc++ after this lands. I'm also planning on 
making a patch to update a few of the builtin type traits. That should 
significantly speed up build times. FYI here is all the "type trait primitives" 
clang supports. 
 At 
some point, we should try to support all of them.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67052/new/

https://reviews.llvm.org/D67052



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67052: Add reference type transformation builtins

[Zoe Carver via Phabricator via cfe-commits]

zoecarver marked an inline comment as done.
zoecarver added inline comments.



Comment at: clang/lib/Parse/ParseDeclCXX.cpp:1095
+  default:
+assert(false && "Not a reference type specifier");
+  }

Mordante wrote:
> Wouldn't it be better to use `llvm_unreachable("Not a reference type 
> specifier");`
> Maybe also move this line out of the switch, allowing the compiler to warn 
> about not handled enumeration values.
I'll update it to use `llvm_unreachable` but, I don't think we want the 
compiler to warn about unhandled enumeration values. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67052/new/

https://reviews.llvm.org/D67052



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67052: Add reference type transformation builtins

[Zoe Carver via Phabricator via cfe-commits]

zoecarver marked an inline comment as done.
zoecarver added inline comments.



Comment at: 
libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_remove_reference.sh.cpp:13
+// variants listed in the RUN script.
+//
+//  Impl   Compile Time  Object Size

zoecarver wrote:
> I think there is a good chance the reason that my added builtin isn't 
> significantly faster is that `std::remove_reference` is already optimized 
> somewhere.  Thoughts?
I think if I updated this test to actually remove a reference, it might show 
more of a difference. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67052/new/

https://reviews.llvm.org/D67052



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87974: Summary: [Builtin] Add __builtin_zero_non_value_bits.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
zoecarver requested review of this revision.

Adds `__builtin_zero_non_value_bits` to zero all padding bits of a
struct.

Currently does not support unions or bitfields.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87974

Files:
  clang/include/clang/Basic/Builtins.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGenCXX/builtin-zero-non-value-bits.cpp
  clang/test/SemaCXX/builtin-zero-non-value-bits.cpp

Index: clang/test/SemaCXX/builtin-zero-non-value-bits.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/builtin-zero-non-value-bits.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+struct Foo { };
+
+void test(int a, Foo b, void* c, int *d, Foo *e) {
+  __builtin_zero_non_value_bits(a); // expected-error {{passing 'int' to parameter of incompatible type structure pointer: type mismatch at 1st parameter ('int' vs structure pointer)}}
+  __builtin_zero_non_value_bits(b); // expected-error {{passing 'Foo' to parameter of incompatible type structure pointer: type mismatch at 1st parameter ('Foo' vs structure pointer)}}
+  __builtin_zero_non_value_bits(c); // expected-error {{passing 'void *' to parameter of incompatible type structure pointer: type mismatch at 1st parameter ('void *' vs structure pointer)}}
+  __builtin_zero_non_value_bits(d); // expected-error {{passing 'int *' to parameter of incompatible type structure pointer: type mismatch at 1st parameter ('int *' vs structure pointer)}}
+  __builtin_zero_non_value_bits(e); // This should not error.
+}
Index: clang/test/CodeGenCXX/builtin-zero-non-value-bits.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/builtin-zero-non-value-bits.cpp
@@ -0,0 +1,171 @@
+// RUN: mkdir -p %t
+// RUN: %clang++ %s -o %t/run
+// RUN: %t/run
+
+#include 
+#include 
+#include 
+
+template
+struct alignas(A1) BasicWithPadding {
+  T x;
+  alignas(A2) T y;
+};
+
+template
+struct alignas(A1) ArrWithPadding {
+  T x[N];
+  alignas(A2) char c;
+  T y[N];
+};
+
+template
+struct alignas(A1) PtrWithPadding {
+  T *x;
+  alignas(A2) T *y;
+};
+
+template
+struct alignas(A1) ThreeWithPadding {
+  T x;
+  alignas(A2) T y;
+  alignas(A3) T z;
+};
+
+template
+struct Normal {
+  T a;
+  T b;
+};
+
+template
+struct X {
+  T x;
+};
+
+template
+struct Z {
+  T z;
+};
+
+template
+struct YZ : public Z {
+  alignas(A) T y;
+};
+
+template
+struct alignas(A1) HasBase : public X, public YZ {
+  T a;
+  alignas(A2) T b;
+};
+
+template
+void testAllForType(T a, T b, T c, T d) {
+  using B = BasicWithPadding;
+  B basic1;
+  memset(&basic1, 0, sizeof(B));
+  basic1.x = a;
+  basic1.y = b;
+  B basic2;
+  memset(&basic2, 42, sizeof(B));
+  basic2.x = a;
+  basic2.y = b;
+  assert(memcmp(&basic1, &basic2, sizeof(B)) != 0);
+  __builtin_zero_non_value_bits(&basic2);
+  assert(memcmp(&basic1, &basic2, sizeof(B)) == 0);
+
+  using A = ArrWithPadding;
+  A arr1;
+  memset(&arr1, 0, sizeof(A));
+  arr1.x[0] = a;
+  arr1.x[1] = b;
+  arr1.y[0] = c;
+  arr1.y[1] = d;
+  A arr2;
+  memset(&arr2, 42, sizeof(A));
+  arr2.x[0] = a;
+  arr2.x[1] = b;
+  arr2.y[0] = c;
+  arr2.y[1] = d;
+  arr2.c = 0;
+  assert(memcmp(&arr1, &arr2, sizeof(A)) != 0);
+  __builtin_zero_non_value_bits(&arr2);
+  assert(memcmp(&arr1, &arr2, sizeof(A)) == 0);
+
+  using P = PtrWithPadding;
+  P ptr1;
+  memset(&ptr1, 0, sizeof(P));
+  ptr1.x = &a;
+  ptr1.y = &b;
+  P ptr2;
+  memset(&ptr2, 42, sizeof(P));
+  ptr2.x = &a;
+  ptr2.y = &b;
+  assert(memcmp(&ptr1, &ptr2, sizeof(P)) != 0);
+  __builtin_zero_non_value_bits(&ptr2);
+  assert(memcmp(&ptr1, &ptr2, sizeof(P)) == 0);
+
+  using Three = ThreeWithPadding;
+  Three three1;
+  memset(&three1, 0, sizeof(Three));
+  three1.x = a;
+  three1.y = b;
+  three1.z = c;
+  Three three2;
+  memset(&three2, 42, sizeof(Three));
+  three2.x = a;
+  three2.y = b;
+  three2.z = c;
+  __builtin_zero_non_value_bits(&three2);
+  assert(memcmp(&three1, &three2, sizeof(Three)) == 0);
+
+  using N = Normal;
+  N normal1;
+  memset(&normal1, 0, sizeof(N));
+  normal1.a = a;
+  normal1.b = b;
+  N normal2;
+  memset(&normal2, 42, sizeof(N));
+  normal2.a = a;
+  normal2.b = b;
+  __builtin_zero_non_value_bits(&normal2);
+  assert(memcmp(&normal1, &normal2, sizeof(N)) == 0);
+
+  using H = HasBase;
+  H base1;
+  memset(&base1, 0, sizeof(H));
+  base1.a = a;
+  base1.b = b;
+  base1.x = c;
+  base1.y = d;
+  base1.z = a;
+  H base2;
+  memset(&base2, 42, sizeof(H));
+  base2.a = a;
+  base2.b = b;
+  base2.x = c;
+  base2.y = d;
+  base2.z = a;
+  assert(memcmp(&base1, &base2, sizeof(H)) != 0);
+  __builtin_zero_non_value_bits(&base2);
+  unsigned i = 0;
+  assert(memcmp(&base1, &base2, sizeof(H)) == 0);
+}
+
+struct Foo {
+  int x;
+  int y;
+};
+
+int main() {
+  testAllForType<32, 16, char>(11,

[PATCH] D87974: Summary: [Builtin] Add __builtin_zero_non_value_bits.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added inline comments.



Comment at: clang/test/CodeGenCXX/builtin-zero-non-value-bits.cpp:160
+
+int main() {
+  testAllForType<32, 16, char>(11, 22, 33, 44);

jfb wrote:
> Usually CodeGen tests will use lit to check the emitted IR matches 
> expectations. I think that's what you want to do here.
> 
> Remember to test `volatile` qualified pointers, as well as address spaces too.
What's a good place for me to put this end-to-end test?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87974/new/

https://reviews.llvm.org/D87974

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver updated this revision to Diff 293606.
zoecarver edited the summary of this revision.
zoecarver added a comment.

- Add more test cases.
- Fix typo.
- Add codegen tests.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87974/new/

https://reviews.llvm.org/D87974

Files:
  clang/include/clang/Basic/Builtins.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGenCXX/builtin-zero-non-value-bits-codegen.cpp
  clang/test/CodeGenCXX/builtin-zero-non-value-bits.cpp
  clang/test/SemaCXX/builtin-zero-non-value-bits.cpp

Index: clang/test/SemaCXX/builtin-zero-non-value-bits.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/builtin-zero-non-value-bits.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+struct Foo { };
+
+struct Incomplete; // expected-note {{forward declaration of 'Incomplete'}}
+
+void test(int a, Foo b, void* c, int *d, Foo *e, const Foo *f, Incomplete *g) {
+  __builtin_zero_non_value_bits(a); // expected-error {{passing 'int' to parameter of incompatible type structure pointer: type mismatch at 1st parameter ('int' vs structure pointer)}}
+  __builtin_zero_non_value_bits(b); // expected-error {{passing 'Foo' to parameter of incompatible type structure pointer: type mismatch at 1st parameter ('Foo' vs structure pointer)}}
+  __builtin_zero_non_value_bits(c); // expected-error {{passing 'void *' to parameter of incompatible type structure pointer: type mismatch at 1st parameter ('void *' vs structure pointer)}}
+  __builtin_zero_non_value_bits(d); // expected-error {{passing 'int *' to parameter of incompatible type structure pointer: type mismatch at 1st parameter ('int *' vs structure pointer)}}
+  __builtin_zero_non_value_bits(e); // This should not error.
+  __builtin_zero_non_value_bits(f); // expected-error {{read-only variable is not assignable}}
+  __builtin_zero_non_value_bits(g); // expected-error {{variable has incomplete type 'Incomplete'}}
+}
Index: clang/test/CodeGenCXX/builtin-zero-non-value-bits.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/builtin-zero-non-value-bits.cpp
@@ -0,0 +1,249 @@
+// RUN: mkdir -p %t
+// RUN: %clang++ %s -o %t/run
+// RUN: %t/run
+
+#include 
+#include 
+#include 
+#include 
+
+template
+struct alignas(A1) BasicWithPadding {
+  T x;
+  alignas(A2) T y;
+};
+
+template
+struct alignas(A1) SpacedArrayMembers {
+  T x[N];
+  alignas(A2) char c;
+  T y[N];
+};
+
+template
+struct alignas(A1) PaddedPointerMembers {
+  T *x;
+  alignas(A2) T *y;
+};
+
+template
+struct alignas(A1) ThreeMembers {
+  T x;
+  alignas(A2) T y;
+  alignas(A3) T z;
+};
+
+template
+struct Normal {
+  T a;
+  T b;
+};
+
+template
+struct X {
+  T x;
+};
+
+template
+struct Z {
+  T z;
+};
+
+template
+struct YZ : public Z {
+  alignas(A) T y;
+};
+
+template
+struct alignas(A1) HasBase : public X, public YZ {
+  T a;
+  alignas(A2) T b;
+};
+
+template
+void testAllForType(T a, T b, T c, T d) {
+  using B = BasicWithPadding;
+  B basic1;
+  memset(&basic1, 0, sizeof(B));
+  basic1.x = a;
+  basic1.y = b;
+  B basic2;
+  memset(&basic2, 42, sizeof(B));
+  basic2.x = a;
+  basic2.y = b;
+  assert(memcmp(&basic1, &basic2, sizeof(B)) != 0);
+  __builtin_zero_non_value_bits(&basic2);
+  assert(memcmp(&basic1, &basic2, sizeof(B)) == 0);
+
+  using A = SpacedArrayMembers;
+  A arr1;
+  memset(&arr1, 0, sizeof(A));
+  arr1.x[0] = a;
+  arr1.x[1] = b;
+  arr1.y[0] = c;
+  arr1.y[1] = d;
+  A arr2;
+  memset(&arr2, 42, sizeof(A));
+  arr2.x[0] = a;
+  arr2.x[1] = b;
+  arr2.y[0] = c;
+  arr2.y[1] = d;
+  arr2.c = 0;
+  assert(memcmp(&arr1, &arr2, sizeof(A)) != 0);
+  __builtin_zero_non_value_bits(&arr2);
+  assert(memcmp(&arr1, &arr2, sizeof(A)) == 0);
+
+  using P = PaddedPointerMembers;
+  P ptr1;
+  memset(&ptr1, 0, sizeof(P));
+  ptr1.x = &a;
+  ptr1.y = &b;
+  P ptr2;
+  memset(&ptr2, 42, sizeof(P));
+  ptr2.x = &a;
+  ptr2.y = &b;
+  assert(memcmp(&ptr1, &ptr2, sizeof(P)) != 0);
+  __builtin_zero_non_value_bits(&ptr2);
+  assert(memcmp(&ptr1, &ptr2, sizeof(P)) == 0);
+
+  using Three = ThreeMembers;
+  Three three1;
+  memset(&three1, 0, sizeof(Three));
+  three1.x = a;
+  three1.y = b;
+  three1.z = c;
+  Three three2;
+  memset(&three2, 42, sizeof(Three));
+  three2.x = a;
+  three2.y = b;
+  three2.z = c;
+  __builtin_zero_non_value_bits(&three2);
+  assert(memcmp(&three1, &three2, sizeof(Three)) == 0);
+
+  using N = Normal;
+  N normal1;
+  memset(&normal1, 0, sizeof(N));
+  normal1.a = a;
+  normal1.b = b;
+  N normal2;
+  memset(&normal2, 42, sizeof(N));
+  normal2.a = a;
+  normal2.b = b;
+  __builtin_zero_non_value_bits(&normal2);
+  assert(memcmp(&normal1, &normal2, sizeof(N)) == 0);
+
+  using H = HasBase;
+  H base1;
+  memset(&base1, 0, sizeof(H));
+  base1.a = a;
+  base1.b = b;
+  base1.x = c;
+  base1.y = d;
+  base1.z = a;
+  H base2;
+  memse

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver updated this revision to Diff 294603.
zoecarver marked 3 inline comments as done.
zoecarver added a comment.

- Add UnsizedTail codegen test


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87974/new/

https://reviews.llvm.org/D87974

Files:
  clang/include/clang/Basic/Builtins.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGenCXX/builtin-zero-non-value-bits-codegen.cpp
  clang/test/CodeGenCXX/builtin-zero-non-value-bits.cpp
  clang/test/SemaCXX/builtin-zero-non-value-bits.cpp

Index: clang/test/SemaCXX/builtin-zero-non-value-bits.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/builtin-zero-non-value-bits.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+struct Foo { };
+
+struct Incomplete; // expected-note {{forward declaration of 'Incomplete'}}
+
+void test(int a, Foo b, void* c, int *d, Foo *e, const Foo *f, Incomplete *g) {
+  __builtin_zero_non_value_bits(a); // expected-error {{passing 'int' to parameter of incompatible type structure pointer: type mismatch at 1st parameter ('int' vs structure pointer)}}
+  __builtin_zero_non_value_bits(b); // expected-error {{passing 'Foo' to parameter of incompatible type structure pointer: type mismatch at 1st parameter ('Foo' vs structure pointer)}}
+  __builtin_zero_non_value_bits(c); // expected-error {{passing 'void *' to parameter of incompatible type structure pointer: type mismatch at 1st parameter ('void *' vs structure pointer)}}
+  __builtin_zero_non_value_bits(d); // expected-error {{passing 'int *' to parameter of incompatible type structure pointer: type mismatch at 1st parameter ('int *' vs structure pointer)}}
+  __builtin_zero_non_value_bits(e); // This should not error.
+  __builtin_zero_non_value_bits(f); // expected-error {{read-only variable is not assignable}}
+  __builtin_zero_non_value_bits(g); // expected-error {{variable has incomplete type 'Incomplete'}}
+}
Index: clang/test/CodeGenCXX/builtin-zero-non-value-bits.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/builtin-zero-non-value-bits.cpp
@@ -0,0 +1,249 @@
+// RUN: mkdir -p %t
+// RUN: %clang++ %s -o %t/run
+// RUN: %t/run
+
+#include 
+#include 
+#include 
+#include 
+
+template
+struct alignas(A1) BasicWithPadding {
+  T x;
+  alignas(A2) T y;
+};
+
+template
+struct alignas(A1) SpacedArrayMembers {
+  T x[N];
+  alignas(A2) char c;
+  T y[N];
+};
+
+template
+struct alignas(A1) PaddedPointerMembers {
+  T *x;
+  alignas(A2) T *y;
+};
+
+template
+struct alignas(A1) ThreeMembers {
+  T x;
+  alignas(A2) T y;
+  alignas(A3) T z;
+};
+
+template
+struct Normal {
+  T a;
+  T b;
+};
+
+template
+struct X {
+  T x;
+};
+
+template
+struct Z {
+  T z;
+};
+
+template
+struct YZ : public Z {
+  alignas(A) T y;
+};
+
+template
+struct alignas(A1) HasBase : public X, public YZ {
+  T a;
+  alignas(A2) T b;
+};
+
+template
+void testAllForType(T a, T b, T c, T d) {
+  using B = BasicWithPadding;
+  B basic1;
+  memset(&basic1, 0, sizeof(B));
+  basic1.x = a;
+  basic1.y = b;
+  B basic2;
+  memset(&basic2, 42, sizeof(B));
+  basic2.x = a;
+  basic2.y = b;
+  assert(memcmp(&basic1, &basic2, sizeof(B)) != 0);
+  __builtin_zero_non_value_bits(&basic2);
+  assert(memcmp(&basic1, &basic2, sizeof(B)) == 0);
+
+  using A = SpacedArrayMembers;
+  A arr1;
+  memset(&arr1, 0, sizeof(A));
+  arr1.x[0] = a;
+  arr1.x[1] = b;
+  arr1.y[0] = c;
+  arr1.y[1] = d;
+  A arr2;
+  memset(&arr2, 42, sizeof(A));
+  arr2.x[0] = a;
+  arr2.x[1] = b;
+  arr2.y[0] = c;
+  arr2.y[1] = d;
+  arr2.c = 0;
+  assert(memcmp(&arr1, &arr2, sizeof(A)) != 0);
+  __builtin_zero_non_value_bits(&arr2);
+  assert(memcmp(&arr1, &arr2, sizeof(A)) == 0);
+
+  using P = PaddedPointerMembers;
+  P ptr1;
+  memset(&ptr1, 0, sizeof(P));
+  ptr1.x = &a;
+  ptr1.y = &b;
+  P ptr2;
+  memset(&ptr2, 42, sizeof(P));
+  ptr2.x = &a;
+  ptr2.y = &b;
+  assert(memcmp(&ptr1, &ptr2, sizeof(P)) != 0);
+  __builtin_zero_non_value_bits(&ptr2);
+  assert(memcmp(&ptr1, &ptr2, sizeof(P)) == 0);
+
+  using Three = ThreeMembers;
+  Three three1;
+  memset(&three1, 0, sizeof(Three));
+  three1.x = a;
+  three1.y = b;
+  three1.z = c;
+  Three three2;
+  memset(&three2, 42, sizeof(Three));
+  three2.x = a;
+  three2.y = b;
+  three2.z = c;
+  __builtin_zero_non_value_bits(&three2);
+  assert(memcmp(&three1, &three2, sizeof(Three)) == 0);
+
+  using N = Normal;
+  N normal1;
+  memset(&normal1, 0, sizeof(N));
+  normal1.a = a;
+  normal1.b = b;
+  N normal2;
+  memset(&normal2, 42, sizeof(N));
+  normal2.a = a;
+  normal2.b = b;
+  __builtin_zero_non_value_bits(&normal2);
+  assert(memcmp(&normal1, &normal2, sizeof(N)) == 0);
+
+  using H = HasBase;
+  H base1;
+  memset(&base1, 0, sizeof(H));
+  base1.a = a;
+  base1.b = b;
+  base1.x = c;
+  base1.y = d;
+  base1.z = a;
+  H base2;
+  memset(&base2, 42, sizeof(H));
+ 

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver marked 3 inline comments as done.
zoecarver added inline comments.



Comment at: clang/test/CodeGenCXX/builtin-zero-non-value-bits-codegen.cpp:16
+  Bar f;
+};
+

jfb wrote:
> It would be helpful to have a comment with the final layout of the struct, 
> including padding. Give each padding field a name, and reference them in the 
> IR check below.
Done. I've named each padding field as "PAD_X" so below it should be clear what 
fields are being stored without a comment. 



Comment at: clang/test/CodeGenCXX/builtin-zero-non-value-bits-codegen.cpp:26
+// CHECK: [[FOO_RAW_PTR:%.*]] = bitcast %struct.Foo* [[FOO_BASE]] to i8*
+// CHECK: [[PAD_1:%.*]] = getelementptr i8, i8* [[FOO_RAW_PTR]], i32 1
+// CHECK: store i8 0, i8* [[PAD_1]]

jfb wrote:
> It would help read the tests if you had a comment on top of each store, for 
> example here "padding byte X".
See above comment. 



Comment at: clang/test/CodeGenCXX/builtin-zero-non-value-bits-codegen.cpp:46
+void test(Baz *baz) {
+  __builtin_zero_non_value_bits(baz);
+}

jfb wrote:
> It would be useful to see a test for arrays with a type that contains tail 
> padding.
Hmm, this test case doesn't seem to be working. I'll investigate further. 



Comment at: clang/test/CodeGenCXX/builtin-zero-non-value-bits.cpp:160
+
+int main() {
+  testAllForType<32, 16, char>(11, 22, 33, 44);

jfb wrote:
> zoecarver wrote:
> > jfb wrote:
> > > Usually CodeGen tests will use lit to check the emitted IR matches 
> > > expectations. I think that's what you want to do here.
> > > 
> > > Remember to test `volatile` qualified pointers, as well as address spaces 
> > > too.
> > What's a good place for me to put this end-to-end test?
> I'm not sure, I don't usually add this type of test :)
Even if it's a bit unconventional, I think it would be good to have this type 
of test. I think we should try to cover as many test cases as possible because 
it's important that this builtin both doesn't zero non-padding bits and does 
zero all padding bits. And it wouldn't be practical to add the 100+ test cases 
covered here as codegen tests. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87974/new/

https://reviews.llvm.org/D87974

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

In D87974#2438682 , @BillyONeal wrote:

>> Are they actually the same, with the same handling of corner cases like 
>> unions and tail padding?
>> There's more to this than just the name, and if they aren't the same, it 
>> seems better to have two names.
>
> They are both implementing the same C++ feature, with the same desired 
> semantics of zeroing out any bits in the object representation that aren't in 
> the value representation. If they differ, one or the other would have a bug.

I agree, they either need to be identical (including corner cases) or there 
needs to be two of them (i.e., GCC ships both `__builtin_zero_non_value_bits` 
and `__builtin_clear_padding` and the first is the same as MSVC, Clang, and 
NVCC).

>> Is there a specification for __builtin_zero_non_value_bits available 
>> somewhere?
>
> I don't know if there is a formal spec for it beyond the actual C++ standard.

I think P0528 is the relevant paper but other than that, no, there's not a 
spec. I think that's going to be the most time sensitive part of implementing 
this: coming up with the spec and making sure all the tests pass on all the 
implementations.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87974/new/

https://reviews.llvm.org/D87974

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

> If the implementation-specific builtins don't match on these, then maybe they 
> should have different names, is his argument I think.

That's a fair point. And I agree, if they don't match, maybe it would be best 
to have different names. I'm hoping that we can all agree on how to handle 
these gray areas, though.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87974/new/

https://reviews.llvm.org/D87974

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92361: [trivial-abi] Support types without a copy or move constructor.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver updated this revision to Diff 38.
zoecarver added a comment.

- Add lots of tests for S0.
- Implement discussed change for handling S0.
- Update docs to reflect change.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92361/new/

https://reviews.llvm.org/D92361

Files:
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CodeGenCXX/trivial_abi.cpp
  clang/test/SemaCXX/attr-trivial-abi.cpp
  clang/test/SemaObjCXX/attr-trivial-abi.mm

Index: clang/test/SemaObjCXX/attr-trivial-abi.mm
===
--- clang/test/SemaObjCXX/attr-trivial-abi.mm
+++ clang/test/SemaObjCXX/attr-trivial-abi.mm
@@ -101,34 +101,3 @@
 };
 
 S17 s17;
-
-namespace deletedCopyMoveConstructor {
-  struct __attribute__((trivial_abi)) CopyMoveDeleted { // expected-warning {{'trivial_abi' cannot be applied to 'CopyMoveDeleted'}} expected-note {{copy constructors and move constructors are all deleted}}
-CopyMoveDeleted(const CopyMoveDeleted &) = delete;
-CopyMoveDeleted(CopyMoveDeleted &&) = delete;
-  };
-
-  struct __attribute__((trivial_abi)) S18 { // expected-warning {{'trivial_abi' cannot be applied to 'S18'}} expected-note {{copy constructors and move constructors are all deleted}}
-CopyMoveDeleted a;
-  };
-
-  struct __attribute__((trivial_abi)) CopyDeleted {
-CopyDeleted(const CopyDeleted &) = delete;
-CopyDeleted(CopyDeleted &&) = default;
-  };
-
-  struct __attribute__((trivial_abi)) MoveDeleted {
-MoveDeleted(const MoveDeleted &) = default;
-MoveDeleted(MoveDeleted &&) = delete;
-  };
-
-  struct __attribute__((trivial_abi)) S19 { // expected-warning {{'trivial_abi' cannot be applied to 'S19'}} expected-note {{copy constructors and move constructors are all deleted}}
-CopyDeleted a;
-MoveDeleted b;
-  };
-
-  // This is fine since the move constructor isn't deleted.
-  struct __attribute__((trivial_abi)) S20 {
-int &&a; // a member of rvalue reference type deletes the copy constructor.
-  };
-}
Index: clang/test/SemaCXX/attr-trivial-abi.cpp
===
--- clang/test/SemaCXX/attr-trivial-abi.cpp
+++ clang/test/SemaCXX/attr-trivial-abi.cpp
@@ -34,6 +34,21 @@
   S3_2 s32;
 };
 
+struct NonTrivial {
+  NonTrivial(const NonTrivial &) {}
+  ~NonTrivial() {}
+};
+
+struct __attribute__((trivial_abi)) S18 { // expected-warning {{'trivial_abi' cannot be applied to 'S18'}} expected-note {{has a field of a non-trivial class type}}
+  NonTrivial n;
+  ~S18() {}
+};
+
+struct __attribute__((trivial_abi)) S19 : NonTrivial { // expected-warning {{'trivial_abi' cannot be applied to 'S19'}} expected-note {{'trivial_abi' is disallowed on 'S19' because it has a base of a non-trivial class type}}
+  int n;
+  ~S19() {}
+};
+
 struct S4 {
   int a;
 };
@@ -79,34 +94,3 @@
 };
 
 S17 s17;
-
-namespace deletedCopyMoveConstructor {
-struct __attribute__((trivial_abi)) CopyMoveDeleted { // expected-warning {{'trivial_abi' cannot be applied to 'CopyMoveDeleted'}} expected-note {{copy constructors and move constructors are all deleted}}
-  CopyMoveDeleted(const CopyMoveDeleted &) = delete;
-  CopyMoveDeleted(CopyMoveDeleted &&) = delete;
-};
-
-struct __attribute__((trivial_abi)) S18 { // expected-warning {{'trivial_abi' cannot be applied to 'S18'}} expected-note {{copy constructors and move constructors are all deleted}}
-  CopyMoveDeleted a;
-};
-
-struct __attribute__((trivial_abi)) CopyDeleted {
-  CopyDeleted(const CopyDeleted &) = delete;
-  CopyDeleted(CopyDeleted &&) = default;
-};
-
-struct __attribute__((trivial_abi)) MoveDeleted {
-  MoveDeleted(const MoveDeleted &) = default;
-  MoveDeleted(MoveDeleted &&) = delete;
-};
-
-struct __attribute__((trivial_abi)) S19 { // expected-warning {{'trivial_abi' cannot be applied to 'S19'}} expected-note {{copy constructors and move constructors are all deleted}}
-  CopyDeleted a;
-  MoveDeleted b;
-};
-
-// This is fine since the move constructor isn't deleted.
-struct __attribute__((trivial_abi)) S20 {
-  int &&a; // a member of rvalue reference type deletes the copy constructor.
-};
-} // namespace deletedCopyMoveConstructor
Index: clang/test/CodeGenCXX/trivial_abi.cpp
===
--- clang/test/CodeGenCXX/trivial_abi.cpp
+++ clang/test/CodeGenCXX/trivial_abi.cpp
@@ -1,10 +1,17 @@
-// RUN: %clang_cc1 -triple arm64-apple-ios11 -std=c++11 -fcxx-exceptions -fexceptions -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple arm64-apple-ios11 -std=c++11 -fcxx-exceptions -fexceptions -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,CHECK-NEW-ABI
 // RUN: %clang_cc1 -triple arm64-apple-ios11 -std=c++11 -fcxx-exceptions -fexceptions -fclang-abi-compat=4.0 -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple arm64-apple-ios11 -std=c++20 -fcxx-exc

[PATCH] D92361: [trivial-abi] Support types without a copy or move constructor.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

Sorry for the slow update. This patch now implements the suggested change (to 
"inherit" the trivial-abi attribute if there are no user-defined special 
members and its copy/move constructor is deleted solely because of a subobject 
with the attribute). Let me know what you think/if there are any other review 
comments.




Comment at: clang/lib/Sema/SemaDeclCXX.cpp:6518
+  D->isAggregate())
+return true;
+

I don't really love this implementation, to be honest. Mainly because it makes 
this function recursive and adds another special-case. 

I tried a few other implementations, but none of those worked very well. Let me 
know if you have alternative implementation strategies that you prefer and I'm 
happy to use those instead. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92361/new/

https://reviews.llvm.org/D92361

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92361: [trivial-abi] Support types without a copy or move constructor.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

> I think that as long as the class leaves a copy/move constructor defaulted, 
> there's no need for a new trivial_abi attribute.

Sorry, I'm not sure I follow. Could you elaborate a bit or provide an example? 
What do you mean by "new" trivial_abi attribute?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92361/new/

https://reviews.llvm.org/D92361

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92361: [trivial-abi] Support types without a copy or move constructor.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added inline comments.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:6502
+  // except that it has a non-trivial member *with* the trivial_abi attribute.
+  for (auto Base : D->bases()) {
+if (auto CxxRecord = Base.getType()->getAsCXXRecordDecl())

ahatanak wrote:
> It looks like this patch changes the way `D` is passed in the following code:
> 
> ```
> struct B {
>   int i[4];
>   B();
>   B(const B &) = default;
>   B(B &&);
> };
> 
> struct D : B {
>   D();
>   D(const D &) = default;
>   D(D &&) = delete;
> };
> 
> void testB(B a);
> void testD(D a);
> 
> void testCallB() {
>   B b;
>   testB(b);
> }
> 
> void testCallD() {
>   D d;
>   testD(d);
> }
> ```
> 
> `B` cannot be passed in registers because it has a non-trivial move 
> constructor, whereas `D` can be passed in registers because the move 
> constructor is deleted and the copy constructor is trivial.
> 
> I'm not sure what the best way to handle this is, but I just wanted to point 
> this out.
Hmm. Good catch. One way to fix this would be to simply create a 
`HasPassableSubobject` variable and add that to the conditions below (instead 
of returning false here). But, it seems that `D` isn't passed by registers 
(even though, maybe it should be) on ToT: https://godbolt.org/z/4xevW5 

Given that, do you think it's OK to return false here, or should I update this 
patch to use the logic I just described (even though that would be a nfc)? 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92361/new/

https://reviews.llvm.org/D92361

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92361: [trivial-abi] Support types without a copy or move constructor.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added inline comments.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:6502
+  // except that it has a non-trivial member *with* the trivial_abi attribute.
+  for (auto Base : D->bases()) {
+if (auto CxxRecord = Base.getType()->getAsCXXRecordDecl())

ahatanak wrote:
> zoecarver wrote:
> > ahatanak wrote:
> > > It looks like this patch changes the way `D` is passed in the following 
> > > code:
> > > 
> > > ```
> > > struct B {
> > >   int i[4];
> > >   B();
> > >   B(const B &) = default;
> > >   B(B &&);
> > > };
> > > 
> > > struct D : B {
> > >   D();
> > >   D(const D &) = default;
> > >   D(D &&) = delete;
> > > };
> > > 
> > > void testB(B a);
> > > void testD(D a);
> > > 
> > > void testCallB() {
> > >   B b;
> > >   testB(b);
> > > }
> > > 
> > > void testCallD() {
> > >   D d;
> > >   testD(d);
> > > }
> > > ```
> > > 
> > > `B` cannot be passed in registers because it has a non-trivial move 
> > > constructor, whereas `D` can be passed in registers because the move 
> > > constructor is deleted and the copy constructor is trivial.
> > > 
> > > I'm not sure what the best way to handle this is, but I just wanted to 
> > > point this out.
> > Hmm. Good catch. One way to fix this would be to simply create a 
> > `HasPassableSubobject` variable and add that to the conditions below 
> > (instead of returning false here). But, it seems that `D` isn't passed by 
> > registers (even though, maybe it should be) on ToT: 
> > https://godbolt.org/z/4xevW5 
> > 
> > Given that, do you think it's OK to return false here, or should I update 
> > this patch to use the logic I just described (even though that would be a 
> > nfc)? 
> The argument is byval, so `D` is passed directly. If you remove `-O3` and add 
> `-target aarch64`, you'll see that `[2 x i64]` is being passed
Ah, I see now. Great. Thanks. I'll update the patch. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92361/new/

https://reviews.llvm.org/D92361

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92361: [trivial-abi] Support types without a copy or move constructor.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

In D92361#2459655 , @rjmccall wrote:

> In D92361#2459513 , @zoecarver wrote:
>
>>> I think that as long as the class leaves a copy/move constructor defaulted, 
>>> there's no need for a new trivial_abi attribute.
>>
>> Sorry, I'm not sure I follow. Could you elaborate a bit or provide an 
>> example? What do you mean by "new" trivial_abi attribute?
>
> Sorry, I mean that I think Akira's example should be passed directly.  It 
> shouldn't require its own trivial_abi attribute in order to get the treatment.

No worries. I understand now. The problem we're discussing actually has nothing 
to do with the trivial-abi attribute. We just need to make sure that 
non-trival-abi types are not affected by this change (and it appears they were).




Comment at: clang/lib/Sema/SemaDeclCXX.cpp:6502
+  // except that it has a non-trivial member *with* the trivial_abi attribute.
+  for (auto Base : D->bases()) {
+if (auto CxxRecord = Base.getType()->getAsCXXRecordDecl())

Quuxplusone wrote:
> zoecarver wrote:
> > ahatanak wrote:
> > > zoecarver wrote:
> > > > ahatanak wrote:
> > > > > It looks like this patch changes the way `D` is passed in the 
> > > > > following code:
> > > > > 
> > > > > ```
> > > > > struct B {
> > > > >   int i[4];
> > > > >   B();
> > > > >   B(const B &) = default;
> > > > >   B(B &&);
> > > > > };
> > > > > 
> > > > > struct D : B {
> > > > >   D();
> > > > >   D(const D &) = default;
> > > > >   D(D &&) = delete;
> > > > > };
> > > > > 
> > > > > void testB(B a);
> > > > > void testD(D a);
> > > > > 
> > > > > void testCallB() {
> > > > >   B b;
> > > > >   testB(b);
> > > > > }
> > > > > 
> > > > > void testCallD() {
> > > > >   D d;
> > > > >   testD(d);
> > > > > }
> > > > > ```
> > > > > 
> > > > > `B` cannot be passed in registers because it has a non-trivial move 
> > > > > constructor, whereas `D` can be passed in registers because the move 
> > > > > constructor is deleted and the copy constructor is trivial.
> > > > > 
> > > > > I'm not sure what the best way to handle this is, but I just wanted 
> > > > > to point this out.
> > > > Hmm. Good catch. One way to fix this would be to simply create a 
> > > > `HasPassableSubobject` variable and add that to the conditions below 
> > > > (instead of returning false here). But, it seems that `D` isn't passed 
> > > > by registers (even though, maybe it should be) on ToT: 
> > > > https://godbolt.org/z/4xevW5 
> > > > 
> > > > Given that, do you think it's OK to return false here, or should I 
> > > > update this patch to use the logic I just described (even though that 
> > > > would be a nfc)? 
> > > The argument is byval, so `D` is passed directly. If you remove `-O3` and 
> > > add `-target aarch64`, you'll see that `[2 x i64]` is being passed
> > Ah, I see now. Great. Thanks. I'll update the patch. 
> Akira's example is legal C++ with no Clang-specific attributes, so its 
> behavior is governed by the appropriate platform's ABI doc — there exists one 
> correct answer.
> At least on x86-64 with the Itanium ABI, GCC and ICC and Clang ToT all agree 
> on the answer: `B` and `D` have exactly the same passing convention. If your 
> patch breaks that, that's a problem.
> Contrariwise, it appears that `B` and `D` have different passing conventions 
> on "armv8" according to Clang, and the same passing convention on "ARM64" 
> according to GCC: https://godbolt.org/z/j9jzYG
> Of course if the programmer adds `[[clang::trivial_abi]]` to one of them, 
> then all bets are off, standards-wise, and you're free to figure out a way to 
> pass it in registers if you want to. But otherwise I think you have to follow 
> what the ABI says.
> Bear in mind that I don't really know ABIs other than Itanium/x86-64. Maybe 
> the problem here is that other platforms don't have well-defined ABIs and so 
> we get to make one up? Maybe everyone except me is already aware that that's 
> what we're doing? :)
> At least on x86-64 with the Itanium ABI, GCC and ICC and Clang ToT all agree 
> on the answer: B and D have exactly the same passing convention. If your 
> patch breaks that, that's a problem.

IIUC (and that's a big "if") being trivial for the purposes of a call, does not 
mean it will _always_ be passed through registers. (For example, we'd never 
pass a 512-byte type through registers.) And, I don't think they do have the 
same calling convention, though they are both passed indirectly. As Akira 
rightly pointed out, on ToT `D` has the `byval` attribute indicating that it 
_could_ be passed directly (according to the ABI/standard at least). While it 
might not be an observable change in the assembly, my patch (or at least the 
current version on phab) removes the byval argument. I have a local version 
that fixes this, though, and I'll upload that shortly. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST

[PATCH] D92361: [trivial-abi] Support types without a copy or move constructor.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver updated this revision to Diff 312629.
zoecarver added a comment.

- Fix base-derived case.
- Rebase.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92361/new/

https://reviews.llvm.org/D92361

Files:
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CodeGenCXX/trivial_abi.cpp
  clang/test/SemaCXX/attr-trivial-abi.cpp
  clang/test/SemaObjCXX/attr-trivial-abi.mm

Index: clang/test/SemaObjCXX/attr-trivial-abi.mm
===
--- clang/test/SemaObjCXX/attr-trivial-abi.mm
+++ clang/test/SemaObjCXX/attr-trivial-abi.mm
@@ -101,34 +101,3 @@
 };
 
 S17 s17;
-
-namespace deletedCopyMoveConstructor {
-  struct __attribute__((trivial_abi)) CopyMoveDeleted { // expected-warning {{'trivial_abi' cannot be applied to 'CopyMoveDeleted'}} expected-note {{copy constructors and move constructors are all deleted}}
-CopyMoveDeleted(const CopyMoveDeleted &) = delete;
-CopyMoveDeleted(CopyMoveDeleted &&) = delete;
-  };
-
-  struct __attribute__((trivial_abi)) S18 { // expected-warning {{'trivial_abi' cannot be applied to 'S18'}} expected-note {{copy constructors and move constructors are all deleted}}
-CopyMoveDeleted a;
-  };
-
-  struct __attribute__((trivial_abi)) CopyDeleted {
-CopyDeleted(const CopyDeleted &) = delete;
-CopyDeleted(CopyDeleted &&) = default;
-  };
-
-  struct __attribute__((trivial_abi)) MoveDeleted {
-MoveDeleted(const MoveDeleted &) = default;
-MoveDeleted(MoveDeleted &&) = delete;
-  };
-
-  struct __attribute__((trivial_abi)) S19 { // expected-warning {{'trivial_abi' cannot be applied to 'S19'}} expected-note {{copy constructors and move constructors are all deleted}}
-CopyDeleted a;
-MoveDeleted b;
-  };
-
-  // This is fine since the move constructor isn't deleted.
-  struct __attribute__((trivial_abi)) S20 {
-int &&a; // a member of rvalue reference type deletes the copy constructor.
-  };
-}
Index: clang/test/SemaCXX/attr-trivial-abi.cpp
===
--- clang/test/SemaCXX/attr-trivial-abi.cpp
+++ clang/test/SemaCXX/attr-trivial-abi.cpp
@@ -34,6 +34,21 @@
   S3_2 s32;
 };
 
+struct NonTrivial {
+  NonTrivial(const NonTrivial &) {}
+  ~NonTrivial() {}
+};
+
+struct __attribute__((trivial_abi)) S18 { // expected-warning {{'trivial_abi' cannot be applied to 'S18'}} expected-note {{has a field of a non-trivial class type}}
+  NonTrivial n;
+  ~S18() {}
+};
+
+struct __attribute__((trivial_abi)) S19 : NonTrivial { // expected-warning {{'trivial_abi' cannot be applied to 'S19'}} expected-note {{'trivial_abi' is disallowed on 'S19' because it has a base of a non-trivial class type}}
+  int n;
+  ~S19() {}
+};
+
 struct S4 {
   int a;
 };
@@ -79,34 +94,3 @@
 };
 
 S17 s17;
-
-namespace deletedCopyMoveConstructor {
-struct __attribute__((trivial_abi)) CopyMoveDeleted { // expected-warning {{'trivial_abi' cannot be applied to 'CopyMoveDeleted'}} expected-note {{copy constructors and move constructors are all deleted}}
-  CopyMoveDeleted(const CopyMoveDeleted &) = delete;
-  CopyMoveDeleted(CopyMoveDeleted &&) = delete;
-};
-
-struct __attribute__((trivial_abi)) S18 { // expected-warning {{'trivial_abi' cannot be applied to 'S18'}} expected-note {{copy constructors and move constructors are all deleted}}
-  CopyMoveDeleted a;
-};
-
-struct __attribute__((trivial_abi)) CopyDeleted {
-  CopyDeleted(const CopyDeleted &) = delete;
-  CopyDeleted(CopyDeleted &&) = default;
-};
-
-struct __attribute__((trivial_abi)) MoveDeleted {
-  MoveDeleted(const MoveDeleted &) = default;
-  MoveDeleted(MoveDeleted &&) = delete;
-};
-
-struct __attribute__((trivial_abi)) S19 { // expected-warning {{'trivial_abi' cannot be applied to 'S19'}} expected-note {{copy constructors and move constructors are all deleted}}
-  CopyDeleted a;
-  MoveDeleted b;
-};
-
-// This is fine since the move constructor isn't deleted.
-struct __attribute__((trivial_abi)) S20 {
-  int &&a; // a member of rvalue reference type deletes the copy constructor.
-};
-} // namespace deletedCopyMoveConstructor
Index: clang/test/CodeGenCXX/trivial_abi.cpp
===
--- clang/test/CodeGenCXX/trivial_abi.cpp
+++ clang/test/CodeGenCXX/trivial_abi.cpp
@@ -1,10 +1,19 @@
-// RUN: %clang_cc1 -triple arm64-apple-ios11 -std=c++11 -fcxx-exceptions -fexceptions -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple arm64-apple-ios11 -std=c++11 -fcxx-exceptions -fexceptions -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,CHECK-NEW-ABI
 // RUN: %clang_cc1 -triple arm64-apple-ios11 -std=c++11 -fcxx-exceptions -fexceptions -fclang-abi-compat=4.0 -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple arm64-apple-ios11 -std=c++20 -fcxx-exceptions -fexceptions -emit-llvm -o - %s | FileCheck --check-prefix=CHECK

[PATCH] D92361: [trivial-abi] Support types without a copy or move constructor.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

@ahatanak this patch should now be a nfc for types that don't have anything to 
do with the trivial-abi attribute.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92361/new/

https://reviews.llvm.org/D92361

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92361: [trivial-abi] Support types without a copy or move constructor.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

> We could probably do something like what this patch is doing and determine 
> whether a class can be passed in registers based on whether its subobjects 
> can be passed in registers. If all of the subobjects can be passed in 
> registers, the current class can be passed in registers too unless something 
> declared in the current class forces it to be passed indirectly (e.g., a 
> virtual function is declared).

That's where I'm getting confused because on main both `B0` and `B1` are passed 
directly, so why isn't `D` also getting passed directly? There's nothing 
declared in that class other than the two members.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92361/new/

https://reviews.llvm.org/D92361

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver marked an inline comment as done.
zoecarver added inline comments.



Comment at: clang/test/CodeGenCXX/builtin-zero-non-value-bits-codegen.cpp:46
+void test(Baz *baz) {
+  __builtin_zero_non_value_bits(baz);
+}

zoecarver wrote:
> jfb wrote:
> > It would be useful to see a test for arrays with a type that contains tail 
> > padding.
> Hmm, this test case doesn't seem to be working. I'll investigate further. 
OK, I've added that. Just to clarify, you mean a type that contains a constant 
array type of types with tail padding (i.e., `Bar [2]`)?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87974/new/

https://reviews.llvm.org/D87974

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver updated this revision to Diff 295445.
zoecarver added a comment.

- Support constant arrays
- Format changes with clang-format


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87974/new/

https://reviews.llvm.org/D87974

Files:
  clang/include/clang/Basic/Builtins.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGenCXX/builtin-zero-non-value-bits-codegen.cpp
  clang/test/CodeGenCXX/builtin-zero-non-value-bits.cpp
  clang/test/SemaCXX/builtin-zero-non-value-bits.cpp

Index: clang/test/SemaCXX/builtin-zero-non-value-bits.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/builtin-zero-non-value-bits.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+struct Foo {};
+
+struct Incomplete; // expected-note {{forward declaration of 'Incomplete'}}
+
+void test(int a, Foo b, void *c, int *d, Foo *e, const Foo *f, Incomplete *g) {
+  __builtin_zero_non_value_bits(a); // expected-error {{passing 'int' to parameter of incompatible type structure pointer: type mismatch at 1st parameter ('int' vs structure pointer)}}
+  __builtin_zero_non_value_bits(b); // expected-error {{passing 'Foo' to parameter of incompatible type structure pointer: type mismatch at 1st parameter ('Foo' vs structure pointer)}}
+  __builtin_zero_non_value_bits(c); // expected-error {{passing 'void *' to parameter of incompatible type structure pointer: type mismatch at 1st parameter ('void *' vs structure pointer)}}
+  __builtin_zero_non_value_bits(d); // expected-error {{passing 'int *' to parameter of incompatible type structure pointer: type mismatch at 1st parameter ('int *' vs structure pointer)}}
+  __builtin_zero_non_value_bits(e); // This should not error.
+  __builtin_zero_non_value_bits(f); // expected-error {{read-only variable is not assignable}}
+  __builtin_zero_non_value_bits(g); // expected-error {{variable has incomplete type 'Incomplete'}}
+}
Index: clang/test/CodeGenCXX/builtin-zero-non-value-bits.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/builtin-zero-non-value-bits.cpp
@@ -0,0 +1,249 @@
+// RUN: mkdir -p %t
+// RUN: %clang++ %s -o %t/run
+// RUN: %t/run
+
+#include 
+#include 
+#include 
+#include 
+
+template 
+struct alignas(A1) BasicWithPadding {
+  T x;
+  alignas(A2) T y;
+};
+
+template 
+struct alignas(A1) SpacedArrayMembers {
+  T x[N];
+  alignas(A2) char c;
+  T y[N];
+};
+
+template 
+struct alignas(A1) PaddedPointerMembers {
+  T *x;
+  alignas(A2) T *y;
+};
+
+template 
+struct alignas(A1) ThreeMembers {
+  T x;
+  alignas(A2) T y;
+  alignas(A3) T z;
+};
+
+template 
+struct Normal {
+  T a;
+  T b;
+};
+
+template 
+struct X {
+  T x;
+};
+
+template 
+struct Z {
+  T z;
+};
+
+template 
+struct YZ : public Z {
+  alignas(A) T y;
+};
+
+template 
+struct alignas(A1) HasBase : public X, public YZ {
+  T a;
+  alignas(A2) T b;
+};
+
+template 
+void testAllForType(T a, T b, T c, T d) {
+  using B = BasicWithPadding;
+  B basic1;
+  memset(&basic1, 0, sizeof(B));
+  basic1.x = a;
+  basic1.y = b;
+  B basic2;
+  memset(&basic2, 42, sizeof(B));
+  basic2.x = a;
+  basic2.y = b;
+  assert(memcmp(&basic1, &basic2, sizeof(B)) != 0);
+  __builtin_zero_non_value_bits(&basic2);
+  assert(memcmp(&basic1, &basic2, sizeof(B)) == 0);
+
+  using A = SpacedArrayMembers;
+  A arr1;
+  memset(&arr1, 0, sizeof(A));
+  arr1.x[0] = a;
+  arr1.x[1] = b;
+  arr1.y[0] = c;
+  arr1.y[1] = d;
+  A arr2;
+  memset(&arr2, 42, sizeof(A));
+  arr2.x[0] = a;
+  arr2.x[1] = b;
+  arr2.y[0] = c;
+  arr2.y[1] = d;
+  arr2.c = 0;
+  assert(memcmp(&arr1, &arr2, sizeof(A)) != 0);
+  __builtin_zero_non_value_bits(&arr2);
+  assert(memcmp(&arr1, &arr2, sizeof(A)) == 0);
+
+  using P = PaddedPointerMembers;
+  P ptr1;
+  memset(&ptr1, 0, sizeof(P));
+  ptr1.x = &a;
+  ptr1.y = &b;
+  P ptr2;
+  memset(&ptr2, 42, sizeof(P));
+  ptr2.x = &a;
+  ptr2.y = &b;
+  assert(memcmp(&ptr1, &ptr2, sizeof(P)) != 0);
+  __builtin_zero_non_value_bits(&ptr2);
+  assert(memcmp(&ptr1, &ptr2, sizeof(P)) == 0);
+
+  using Three = ThreeMembers;
+  Three three1;
+  memset(&three1, 0, sizeof(Three));
+  three1.x = a;
+  three1.y = b;
+  three1.z = c;
+  Three three2;
+  memset(&three2, 42, sizeof(Three));
+  three2.x = a;
+  three2.y = b;
+  three2.z = c;
+  __builtin_zero_non_value_bits(&three2);
+  assert(memcmp(&three1, &three2, sizeof(Three)) == 0);
+
+  using N = Normal;
+  N normal1;
+  memset(&normal1, 0, sizeof(N));
+  normal1.a = a;
+  normal1.b = b;
+  N normal2;
+  memset(&normal2, 42, sizeof(N));
+  normal2.a = a;
+  normal2.b = b;
+  __builtin_zero_non_value_bits(&normal2);
+  assert(memcmp(&normal1, &normal2, sizeof(N)) == 0);
+
+  using H = HasBase;
+  H base1;
+  memset(&base1, 0, sizeof(H));
+  base1.a = a;
+  base1.b = b;
+  base1.x = c;
+  base1.y = d;
+  base1.z = a;
+  H base2;
+  memset(&base2, 42, sizeof(H));
+  base

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:1652
+auto Element = CGF.Builder.CreateGEP(I8Ptr, Index);
+CGF.Builder.CreateAlignedStore(Zero, Element, MaybeAlign());
+  };

jfb wrote:
> You should use `alignmentAtOffset` here.
I'm using `CharUnits::One().alignmentAtOffset` to here because this type will 
always have a size of 1 because it's an `i8` ptr.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87974/new/

https://reviews.llvm.org/D87974

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:1704
+->getArrayElementTypeNoTypeQual()
+->isRecordType()) {
+  auto FieldElement = CGF.Builder.CreateStructGEP(Ptr, Index);

Is it OK to possibly create hundreds of stores here? I assume later 
optimizations will catch this and turn it into a loop or a call to memset or 
something. But this could potentially be harmful to code size.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87974/new/

https://reviews.llvm.org/D87974

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

In D87974#2408119 , @jwakely wrote:

> As of a few hours ago, GCC has `__builtin_clear_padding`, see 
> https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-_005f_005fbuiltin_005fclear_005fpadding
>  for the docs.

Great. Then I'll update this to be named `__builtin_clear_padding`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87974/new/

https://reviews.llvm.org/D87974

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

> Let's make sure that we follow the same semantics that GCC does, particularly 
> w.r.t. union, bitfields, and padding at the end of an object (whether it's in 
> an array or not).

Agreed. I'm planning to run some tests tomorrow once the nightly build has 
updated.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87974/new/

https://reviews.llvm.org/D87974

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

> How should I check for support? Is it going to be e.g. 
> __has_feature(__builtin_clear_padding)?

I'm not sure `__has_feature` will work but `__has_builtin` should work on both 
Clang and GCC.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87974/new/

https://reviews.llvm.org/D87974

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

@jwakely It looks like `UnsizedTail` causes a crash.

Other than that all the tests in this PR pass. It also looks like there's (at 
least) some support for unions and bitfields. This patch doesn't support those 
but I'm planning to add support as a follow-up.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87974/new/

https://reviews.llvm.org/D87974

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

> Filed https://gcc.gnu.org/PR97943 for that. Avoiding the crash is trivial, 
> deciding what we want to do exactly for flexible array members (which are 
> beyond the C++ standard) is harder.

Yes, and we should try to do the same thing in this case. Currently, this 
implementation clears any padding bits that might come before the flexible 
array member but doesn't attempt to clear any of the array's padding bits 
(which I'm pretty sure wouldn't be feasible). So, what we're really deciding is 
whether or not to error for these types.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87974/new/

https://reviews.llvm.org/D87974

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92361: [trivial-abi] Support types without a copy or move constructor.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver created this revision.
Herald added a reviewer: aaron.ballman.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
zoecarver requested review of this revision.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92361

Files:
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CodeGenCXX/trivial_abi.cpp
  clang/test/SemaCXX/attr-trivial-abi.cpp
  clang/test/SemaObjCXX/attr-trivial-abi.mm

Index: clang/test/SemaObjCXX/attr-trivial-abi.mm
===
--- clang/test/SemaObjCXX/attr-trivial-abi.mm
+++ clang/test/SemaObjCXX/attr-trivial-abi.mm
@@ -101,34 +101,3 @@
 };
 
 S17 s17;
-
-namespace deletedCopyMoveConstructor {
-  struct __attribute__((trivial_abi)) CopyMoveDeleted { // expected-warning {{'trivial_abi' cannot be applied to 'CopyMoveDeleted'}} expected-note {{copy constructors and move constructors are all deleted}}
-CopyMoveDeleted(const CopyMoveDeleted &) = delete;
-CopyMoveDeleted(CopyMoveDeleted &&) = delete;
-  };
-
-  struct __attribute__((trivial_abi)) S18 { // expected-warning {{'trivial_abi' cannot be applied to 'S18'}} expected-note {{copy constructors and move constructors are all deleted}}
-CopyMoveDeleted a;
-  };
-
-  struct __attribute__((trivial_abi)) CopyDeleted {
-CopyDeleted(const CopyDeleted &) = delete;
-CopyDeleted(CopyDeleted &&) = default;
-  };
-
-  struct __attribute__((trivial_abi)) MoveDeleted {
-MoveDeleted(const MoveDeleted &) = default;
-MoveDeleted(MoveDeleted &&) = delete;
-  };
-
-  struct __attribute__((trivial_abi)) S19 { // expected-warning {{'trivial_abi' cannot be applied to 'S19'}} expected-note {{copy constructors and move constructors are all deleted}}
-CopyDeleted a;
-MoveDeleted b;
-  };
-
-  // This is fine since the move constructor isn't deleted.
-  struct __attribute__((trivial_abi)) S20 {
-int &&a; // a member of rvalue reference type deletes the copy constructor.
-  };
-}
Index: clang/test/SemaCXX/attr-trivial-abi.cpp
===
--- clang/test/SemaCXX/attr-trivial-abi.cpp
+++ clang/test/SemaCXX/attr-trivial-abi.cpp
@@ -79,34 +79,3 @@
 };
 
 S17 s17;
-
-namespace deletedCopyMoveConstructor {
-struct __attribute__((trivial_abi)) CopyMoveDeleted { // expected-warning {{'trivial_abi' cannot be applied to 'CopyMoveDeleted'}} expected-note {{copy constructors and move constructors are all deleted}}
-  CopyMoveDeleted(const CopyMoveDeleted &) = delete;
-  CopyMoveDeleted(CopyMoveDeleted &&) = delete;
-};
-
-struct __attribute__((trivial_abi)) S18 { // expected-warning {{'trivial_abi' cannot be applied to 'S18'}} expected-note {{copy constructors and move constructors are all deleted}}
-  CopyMoveDeleted a;
-};
-
-struct __attribute__((trivial_abi)) CopyDeleted {
-  CopyDeleted(const CopyDeleted &) = delete;
-  CopyDeleted(CopyDeleted &&) = default;
-};
-
-struct __attribute__((trivial_abi)) MoveDeleted {
-  MoveDeleted(const MoveDeleted &) = default;
-  MoveDeleted(MoveDeleted &&) = delete;
-};
-
-struct __attribute__((trivial_abi)) S19 { // expected-warning {{'trivial_abi' cannot be applied to 'S19'}} expected-note {{copy constructors and move constructors are all deleted}}
-  CopyDeleted a;
-  MoveDeleted b;
-};
-
-// This is fine since the move constructor isn't deleted.
-struct __attribute__((trivial_abi)) S20 {
-  int &&a; // a member of rvalue reference type deletes the copy constructor.
-};
-} // namespace deletedCopyMoveConstructor
Index: clang/test/CodeGenCXX/trivial_abi.cpp
===
--- clang/test/CodeGenCXX/trivial_abi.cpp
+++ clang/test/CodeGenCXX/trivial_abi.cpp
@@ -5,6 +5,9 @@
 // CHECK: %[[STRUCT_LARGE:.*]] = type { i32*, [128 x i32] }
 // CHECK: %[[STRUCT_TRIVIAL:.*]] = type { i32 }
 // CHECK: %[[STRUCT_NONTRIVIAL:.*]] = type { i32 }
+// CHECK: [[STRUCT_COPY_MOVE_DELETED:%.*]] = type { i8 }
+// CHECK: [[STRUCT_MOVE_LIKE_1:%.*]] = type { i8 }
+// CHECK: [[STRUCT_MOVE_LIKE_2:%.*]] = type { i8 }
 
 struct __attribute__((trivial_abi)) Small {
   int *p;
@@ -55,6 +58,26 @@
   Small m0() override;
 };
 
+struct __attribute__((trivial_abi)) CopyMoveDeleted {
+  CopyMoveDeleted(CopyMoveDeleted const&) = delete;
+  CopyMoveDeleted(CopyMoveDeleted &&) = delete;
+  CopyMoveDeleted(int) {}
+  ~CopyMoveDeleted() {}
+};
+
+struct __attribute__((trivial_abi)) TemplatedMoveLikeCtor {
+  TemplatedMoveLikeCtor(TemplatedMoveLikeCtor const&) = delete;
+  TemplatedMoveLikeCtor(TemplatedMoveLikeCtor &&) = delete;
+  template TemplatedMoveLikeCtor(T&&) { }
+  ~TemplatedMoveLikeCtor() {}
+};
+
+struct __attribute__((trivial_abi)) TemplatedMoveLikeCtor2 {
+  TemplatedMoveLikeCtor2(TemplatedMoveLikeCtor2 const&) = delete;
+  template TemplatedMoveLikeCtor2(TemplatedMoveLikeCtor2&&) { }
+  ~TemplatedMoveLikeCtor2() {}
+};
+
 // CHECK-LAB