This revision was automatically updated to reflect the committed changes.
Closed by commit rC349195: [Clang] Add __builtin_launder (authored by EricWF,
committed by ).
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D40218/new/
https://reviews.llvm.org/D40218
EricWF updated this revision to Diff 178267.
EricWF added a comment.
Merging with upstream. Preparing to commit.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D40218/new/
https://reviews.llvm.org/D40218
Files:
include/clang/Basic/Builtins.def
EricWF updated this revision to Diff 177445.
EricWF marked 6 inline comments as done.
EricWF added a comment.
Address review comments.
@rsmith I think this is good to go. Just need your input on one change.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D40218/new/
EricWF marked 22 inline comments as done.
EricWF added inline comments.
Comment at: lib/CodeGen/CGBuiltin.cpp:1425-1426
+
+ // FIXME: We either have an incomplete class type, or we have a class
template
+ // whose instantiation has not been forced. Example:
+ //
rsmith added inline comments.
Comment at: lib/CodeGen/CGBuiltin.cpp:1425-1426
+
+ // FIXME: We either have an incomplete class type, or we have a class
template
+ // whose instantiation has not been forced. Example:
+ //
I think it's a bug that `launder`
EricWF added a comment.
In https://reviews.llvm.org/D40218#1298943, @Romain-Geissler-1A wrote:
> Is there any news on this code review ? Is it ready to land ?
I think so, but there were some changes regarding incomplete types since
Richard last looked at it.
I wanted him to sign off on the
EricWF updated this revision to Diff 174109.
EricWF added a comment.
Merge with upstream.
https://reviews.llvm.org/D40218
Files:
include/clang/Basic/Builtins.def
include/clang/Basic/DiagnosticSemaKinds.td
lib/AST/ExprConstant.cpp
lib/CodeGen/CGBuiltin.cpp
lib/Sema/SemaChecking.cpp
Romain-Geissler-1A added a comment.
Hi,
Is there any news on this code review ? Is it ready to land ?
Cheers,
Romain
https://reviews.llvm.org/D40218
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
erichkeane added a comment.
Bump! I don't see anything here that seems questionable (and this has
extensive testing), but I presume @rsmith should be the final one to approve.
https://reviews.llvm.org/D40218
___
cfe-commits mailing list
EricWF updated this revision to Diff 147639.
EricWF added a comment.
- Handle array types as requested.
- Attempt to handle incomplete class types. They are always considered in need
of laundering. Including class templates who's instantiation hasn't been forced
yet. @rsmith is this the
rsmith added a comment.
Also we probably want to hold off on landing this until PR37458 is fixed,
otherwise `std::launder` will start miscompiling code.
https://reviews.llvm.org/D40218
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
LGTM with a fix (and test) for pointer-to-array-of-dynamic-class-type handling.
Comment at: lib/CodeGen/CGBuiltin.cpp:936-938
+ const auto *Record =
EricWF added a comment.
Ping.
https://reviews.llvm.org/D40218
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
EricWF updated this revision to Diff 139363.
EricWF marked an inline comment as done.
EricWF added a comment.
- Launder types with subobjects of dynamic class type.
- Improve diagnostic selection using `llvm::Optional`.
- Add comment about LTO ABI concerns to test file.
- Merge with master.
EricWF marked 2 inline comments as done.
EricWF added inline comments.
Comment at: test/CodeGenCXX/builtin-launder.cpp:93-96
+/// The test cases in this namespace technically need to be laundered according
+/// to the language in the standard (ie they have const or reference
EricWF marked an inline comment as done.
EricWF added inline comments.
Comment at: lib/CodeGen/CGBuiltin.cpp:1947-1948
+const auto *Record = ArgTy->getAsCXXRecordDecl();
+if (CGM.getCodeGenOpts().StrictVTablePointers && Record &&
+Record->isDynamicClass())
+
rsmith added inline comments.
Comment at: lib/CodeGen/CGBuiltin.cpp:1947-1948
+const auto *Record = ArgTy->getAsCXXRecordDecl();
+if (CGM.getCodeGenOpts().StrictVTablePointers && Record &&
+Record->isDynamicClass())
+ Ptr =
EricWF updated this revision to Diff 134986.
EricWF added a comment.
Ping.
https://reviews.llvm.org/D40218
Files:
include/clang/Basic/Builtins.def
include/clang/Basic/DiagnosticSemaKinds.td
lib/AST/ExprConstant.cpp
lib/CodeGen/CGBuiltin.cpp
lib/Sema/SemaChecking.cpp
EricWF updated this revision to Diff 133290.
EricWF added a comment.
- Rebase off master.
https://reviews.llvm.org/D40218
Files:
include/clang/Basic/Builtins.def
include/clang/Basic/DiagnosticSemaKinds.td
lib/AST/ExprConstant.cpp
lib/CodeGen/CGBuiltin.cpp
lib/Sema/SemaChecking.cpp
EricWF added a comment.
Ping.
https://reviews.llvm.org/D40218
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
EricWF updated this revision to Diff 129742.
EricWF added a comment.
- Improve diagnostic handling.
https://reviews.llvm.org/D40218
Files:
include/clang/Basic/Builtins.def
include/clang/Basic/DiagnosticSemaKinds.td
lib/AST/ExprConstant.cpp
lib/CodeGen/CGBuiltin.cpp
EricWF updated this revision to Diff 129740.
EricWF marked an inline comment as done.
EricWF added a comment.
- Address inline comments about missing diagnostics for void pointers and
function pointers.
- Address inline comments about only enabling when `-fstrict-vtable-pointers`
is specified,
EricWF marked 2 inline comments as done.
EricWF added inline comments.
Comment at: lib/CodeGen/CGBuiltin.cpp:1674
+Value *Ptr = EmitScalarExpr(E->getArg(0));
+Ptr = Builder.CreateInvariantGroupBarrier(Ptr);
+return RValue::get(Ptr);
rsmith wrote:
>
rsmith added inline comments.
Comment at: lib/CodeGen/CGBuiltin.cpp:1674
+Value *Ptr = EmitScalarExpr(E->getArg(0));
+Ptr = Builder.CreateInvariantGroupBarrier(Ptr);
+return RValue::get(Ptr);
It would be nice to avoid this for types that contain no
EricWF updated this revision to Diff 123627.
EricWF added a comment.
- Improve quality of tests.
- Format code.
https://reviews.llvm.org/D40218
Files:
include/clang/Basic/Builtins.def
include/clang/Basic/DiagnosticSemaKinds.td
lib/AST/ExprConstant.cpp
lib/CodeGen/CGBuiltin.cpp
EricWF updated this revision to Diff 123516.
EricWF added a comment.
- Fix argument initialization.
- Make constexpr.
https://reviews.llvm.org/D40218
Files:
include/clang/Basic/Builtins.def
include/clang/Basic/DiagnosticSemaKinds.td
lib/AST/ExprConstant.cpp
lib/CodeGen/CGBuiltin.cpp
jroelofs added inline comments.
Comment at: include/clang/Basic/Builtins.def:480
BUILTIN(__builtin_thread_pointer, "v*", "nc")
+BUILTIN(__builtin_launder, "v*v*", "nt")
EricWF wrote:
> jroelofs wrote:
> > GCC's is type-generic:
> >
> > ```
> > #include
> >
EricWF added inline comments.
Comment at: include/clang/Basic/Builtins.def:480
BUILTIN(__builtin_thread_pointer, "v*", "nc")
+BUILTIN(__builtin_launder, "v*v*", "nt")
jroelofs wrote:
> GCC's is type-generic:
>
> ```
> #include
>
> bool is_type_generic() {
jroelofs added inline comments.
Comment at: include/clang/Basic/Builtins.def:480
BUILTIN(__builtin_thread_pointer, "v*", "nc")
+BUILTIN(__builtin_launder, "v*v*", "nt")
GCC's is type-generic:
```
#include
bool is_type_generic() {
int v;
return
majnemer added a comment.
A test with restrict and __restrict might be interesting.
https://reviews.llvm.org/D40218
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
EricWF updated this revision to Diff 123484.
EricWF added a comment.
- Remove incorrect FIXME comment.
https://reviews.llvm.org/D40218
Files:
include/clang/Basic/Builtins.def
include/clang/Basic/DiagnosticSemaKinds.td
lib/CodeGen/CGBuiltin.cpp
lib/Sema/SemaChecking.cpp
EricWF created this revision.
Herald added a subscriber: Prazek.
This patch adds `__builtin_launder`, which is required to implement
`std::launder`. Additionally GCC provides `__builtin_launder`, so thing brings
Clang in-line with GCC.
I'm not exactly sure what magic `__builtin_launder`
32 matches
Mail list logo
