[PATCH] D15836: [libcxx] Fix undefined behavior in forward_list
EricWF created this revision.
EricWF added a reviewer: mclow.lists.
EricWF added a subscriber: cfe-commits.
This patch is similar to the fix but it has a few differences. This
patch doesn't use a `__link_pointer` typedef because we don't need to change
the linked list pointers because `forward_list` never stores a
`__forward_begin_node` in the linked list itself.
The issue with `forward_list` is that the iterators store pointers to
`__forward_list_node` and not `__forward_begin_node`. This is incorrect because
`before_begin()` and `cbefore_begin()` return iterators that point to a
`__forward_begin_node`. This means we incorrectly downcast the
`__forward_begin_node` pointer to a `__node_pointer`. This downcast itself is
sometimes UB but it cannot be safely removed until ABI v2. The more common
cause of UB is when we deference the downcast pointer. (for example
`__ptr_->__next_`). This can be fixed without an ABI break by upcasting
`__ptr_` before accessing it.
The fix is as follows:
1. Introduce a `__iter_node_pointer` typedef that works similar to
`__link_pointer` in the last patch. In ABI v2 it is always a typedef for
`__begin_node_pointer`.
2. Change the `__before_begin()` method to return the correct pointer type
(`__begin_node_pointer`),
Previously it incorrectly downcasted the `__forward_begin_node` to a
`__node_pointer` so it could be used to constructor the iterator types.
3. Change `__forward_list_iterator` and `__forward_list_const_iterator` in the
following way:
1. Change `__node_pointer __ptr_;` member to have the
`__iter_node_pointer` type instead.
2. Add additional private constructors that accept `__begin_node_pointer`
in addition to `__node_pointer` and then correctly cast them to the stored
`__iter_node_pointer` type.
3. Add `__get_begin()` and `__get_node_unchecked()` accessor methods that
correctly cast `__ptr_` to the expected pointer type. `__get_begin()` is always
safe to use and should be
preferred. `__get_node_unchecked()` can only be used on a deferencible
iterator.
4. Replace direct access to `__forward_list_iterator::__ptr_` with the safe
accessor methods.
http://reviews.llvm.org/D15836
Files:
include/__config
include/forward_list
Index: include/forward_list
===
--- include/forward_list
+++ include/forward_list
@@ -183,15 +183,57 @@
_LIBCPP_BEGIN_NAMESPACE_STD
template struct __forward_list_node;
+template struct __forward_begin_node;
+
+template
+struct __forward_node_traits {
+ typedef typename __rebind_pointer<_NodePtr, void>::type __void_pointer;
+ typedef typename pointer_traits<_NodePtr>::element_type __node;
+ typedef _NodePtr__node_pointer;
+ typedef __forward_begin_node<_NodePtr> __begin_node;
+ typedef typename __rebind_pointer<_NodePtr, __begin_node>::type
+ __begin_node_pointer;
+
+#if defined(_LIBCPP_ABI_FORWARD_LIST_REMOVE_NODE_POINTER_UB)
+ typedef __begin_node_pointer __iter_node_pointer;
+#else
+ typedef typename conditional<
+ is_pointer<__void_pointer>::value,
+ __begin_node_pointer,
+ __node_pointer
+>::type __iter_node_pointer;
+#endif
+
+ typedef typename conditional<
+ is_same<__iter_node_pointer, __node_pointer>::value,
+ __begin_node_pointer,
+ __node_pointer
+>::type __non_iter_node_pointer;
+
+ _LIBCPP_INLINE_VISIBILITY
+ static __iter_node_pointer __as_iter_node(__iter_node_pointer __p) {
+ return __p;
+ }
+ _LIBCPP_INLINE_VISIBILITY
+ static __iter_node_pointer __as_iter_node(__non_iter_node_pointer __p) {
+ return static_cast<__iter_node_pointer>(static_cast<__void_pointer>(__p));
+ }
+};
template
struct __forward_begin_node
{
typedef _NodePtr pointer;
+typedef typename __rebind_pointer<_NodePtr, __forward_begin_node>::type __begin_node_pointer;
pointer __next_;
- _LIBCPP_INLINE_VISIBILITY __forward_begin_node() : __next_(nullptr) {}
+_LIBCPP_INLINE_VISIBILITY __forward_begin_node() : __next_(nullptr) {}
+
+_LIBCPP_INLINE_VISIBILITY
+__begin_node_pointer __next_as_begin() const {
+return static_cast<__begin_node_pointer>(__next_);
+}
};
template
@@ -217,12 +259,35 @@
template
class _LIBCPP_TYPE_VIS_ONLY __forward_list_iterator
{
-typedef _NodePtr __node_pointer;
+typedef __forward_node_traits<_NodePtr> __traits;
+typedef typename __traits::__node_pointer __node_pointer;
+typedef typename __traits::__begin_node_pointer __begin_node_pointer;
+typedef typename __traits::__iter_node_pointer __iter_node_pointer;
+typedef typename __traits::__void_pointer __void_pointer;
+
+__iter_node_pointer __ptr_;
+
+_LIBCPP_INLINE_VISIBILITY
+__begin_node_pointer __get_begin() const {
+return static_cast<_
Re: [PATCH] D15836: [libcxx] Fix undefined behavior in forward_list
EricWF updated this revision to Diff 43824.
EricWF added a comment.
forward_list and it's iterators should allow incomplete types in C++17 and
beyond. I updated the patch to fix this issue as a drive-by.
http://reviews.llvm.org/D15836
Files:
include/__config
include/forward_list
test/std/containers/sequences/forwardlist/incomplete.pass.cpp
Index: test/std/containers/sequences/forwardlist/incomplete.pass.cpp
===
--- /dev/null
+++ test/std/containers/sequences/forwardlist/incomplete.pass.cpp
@@ -0,0 +1,53 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+//
+
+// forward_list()
+// forward_list::iterator()
+// forward_list::const_iterator()
+
+#include
+#include
+
+#include "test_macros.h"
+#include "min_allocator.h"
+
+struct A {
+ std::forward_list d;
+ std::forward_list::iterator it;
+ std::forward_list::const_iterator it2;
+};
+
+#if TEST_STD_VER >= 11
+struct B {
+ typedef std::forward_list> FList;
+ FList d;
+ FList::iterator it;
+ FList::const_iterator it2;
+};
+#endif
+
+int main()
+{
+ {
+A a;
+assert(a.d.empty());
+a.it = a.d.begin();
+a.it2 = a.d.cbefore_begin();
+ }
+#if TEST_STD_VER >= 11
+ {
+B b;
+assert(b.d.empty());
+b.it = b.d.begin();
+b.it2 = b.d.cbefore_begin();
+ }
+#endif
+}
Index: include/forward_list
===
--- include/forward_list
+++ include/forward_list
@@ -183,15 +183,69 @@
_LIBCPP_BEGIN_NAMESPACE_STD
template struct __forward_list_node;
+template struct __forward_begin_node;
+
+
+template
+struct __forward_list_node_value_type;
+
+template
+struct __forward_list_node_value_type<__forward_list_node<_Tp, _VoidPtr> > {
+ typedef _Tp type;
+};
+
+template
+struct __forward_node_traits {
+
+ typedef typename remove_cv<
+typename pointer_traits<_NodePtr>::element_type>::type __node;
+ typedef typename __forward_list_node_value_type<__node>::type __node_value_type;
+ typedef _NodePtr __node_pointer;
+ typedef __forward_begin_node<_NodePtr>__begin_node;
+ typedef typename __rebind_pointer<_NodePtr, __begin_node>::type
+__begin_node_pointer;
+ typedef typename __rebind_pointer<_NodePtr, void>::type __void_pointer;
+
+#if defined(_LIBCPP_ABI_FORWARD_LIST_REMOVE_NODE_POINTER_UB)
+ typedef __begin_node_pointer __iter_node_pointer;
+#else
+ typedef typename conditional<
+ is_pointer<__void_pointer>::value,
+ __begin_node_pointer,
+ __node_pointer
+>::type __iter_node_pointer;
+#endif
+
+ typedef typename conditional<
+ is_same<__iter_node_pointer, __node_pointer>::value,
+ __begin_node_pointer,
+ __node_pointer
+>::type __non_iter_node_pointer;
+
+ _LIBCPP_INLINE_VISIBILITY
+ static __iter_node_pointer __as_iter_node(__iter_node_pointer __p) {
+ return __p;
+ }
+ _LIBCPP_INLINE_VISIBILITY
+ static __iter_node_pointer __as_iter_node(__non_iter_node_pointer __p) {
+ return static_cast<__iter_node_pointer>(static_cast<__void_pointer>(__p));
+ }
+};
template
struct __forward_begin_node
{
typedef _NodePtr pointer;
+typedef typename __rebind_pointer<_NodePtr, __forward_begin_node>::type __begin_node_pointer;
pointer __next_;
- _LIBCPP_INLINE_VISIBILITY __forward_begin_node() : __next_(nullptr) {}
+_LIBCPP_INLINE_VISIBILITY __forward_begin_node() : __next_(nullptr) {}
+
+_LIBCPP_INLINE_VISIBILITY
+__begin_node_pointer __next_as_begin() const {
+return static_cast<__begin_node_pointer>(__next_);
+}
};
template
@@ -211,26 +265,49 @@
value_type __value_;
};
+
template > class _LIBCPP_TYPE_VIS_ONLY forward_list;
template class _LIBCPP_TYPE_VIS_ONLY __forward_list_const_iterator;
template
class _LIBCPP_TYPE_VIS_ONLY __forward_list_iterator
{
-typedef _NodePtr __node_pointer;
+typedef __forward_node_traits<_NodePtr> __traits;
+typedef typename __traits::__node_pointer __node_pointer;
+typedef typename __traits::__begin_node_pointer __begin_node_pointer;
+typedef typename __traits::__iter_node_pointer __iter_node_pointer;
+typedef typename __traits::__void_pointer __void_pointer;
+
+__iter_node_pointer __ptr_;
+
+_LIBCPP_INLINE_VISIBILITY
+__begin_node_pointer __get_begin() const {
+return static_cast<__begin_node_pointer>(
+static_cast<__void_pointer>(__ptr_));
+}
+_LIBCPP_INLINE_VISIBILITY
+__node_pointer
Re: [PATCH] D15709: [X86] Support 'interrupt' attribute for x86
ABataev added a comment.
Aaron, thanks for the review!
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2504
@@ -2493,3 +2503,3 @@
// Availability attribute
def warn_availability_unknown_platform : Warning<
aaron.ballman wrote:
> >> It would be good to model these new diagnostics after the MIPS interrupt
> >> diagnostics.
> >>
> >> def warn_mips_interrupt_attribute : Warning<
> >>"MIPS 'interrupt' attribute only applies to functions that have "
> >>"%select{no parameters|a 'void' return type}0">,
> >>InGroup;
>
> > Ok, will do
>
> This does not appear to have been completed yet.
Probably I did not quite understood your comment. I'll try to fix it and hope
this time I'll get it right. :)
Comment at: lib/Sema/SemaDeclAttr.cpp:4556
@@ +4555,3 @@
+ // e) The 2nd argument (if any) must be an unsigned integer.
+ if (!isFunctionOrMethod(D) || !hasFunctionProto(D) ||
+ !D->getDeclContext()->isFileContext()) {
aaron.ballman wrote:
> >> Yes, we allow any non-member functions.
> > Then we should have a test that shows this working with a named namespace
> > interrupt function. Also, it's a bit odd to me that members of named
> > namespaces are fine, but static member functions are not. This disallows
> > possibly-sensible code (like a private static member function of a class
> > for better encapsulation).
>
> I don't see a test using a named namespace with an interrupt function (though
> I do see one disallowing a static member function). Also, I am still
> wondering why there is a restriction against static member functions. (I
> looked at GCC's docs and they do not claim to support this attribute for
> these targets, so I'm not certain what this design is based on.)
See test/CodeGenCXX/attr-x86-interrupt.cpp. Function 'foo8' is declare in
'namespace S'.
Comment at: test/SemaCXX/attr-x86-interrupt.cpp:51
@@ +50,3 @@
+template
+void bar(T *a) {
+ foo9(a); // expected-error {{interrupt service routine can't be called
directly}}
aaron.ballman wrote:
> Ah, I'm sorry, I wasn't very clear with my template request. I was looking
> for something like (in addition to what you've added):
> ```
> template
> void bar(Fn F) {
> F(nullptr); // Should this diagnose? I expect not.
> }
>
> __attribute__((interrupt)) void foo(int *) {}
> void f() {
> bar(foo);
> }
> ```
Ok, I will add this test case.
http://reviews.llvm.org/D15709
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15710: [clang-tidy] Add non-inline function definition and variable definition check in header files.
hokein updated this revision to Diff 43826.
hokein added a comment.
Move unittest to lit test.
http://reviews.llvm.org/D15710
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/DefinitionsInHeadersCheck.cpp
clang-tidy/misc/DefinitionsInHeadersCheck.h
clang-tidy/misc/MiscTidyModule.cpp
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/misc-definitions-in-headers.rst
test/clang-tidy/check_clang_tidy.py
test/clang-tidy/misc-definitions-in-headers.hpp
test/lit.cfg
Index: test/lit.cfg
===
--- test/lit.cfg
+++ test/lit.cfg
@@ -43,7 +43,8 @@
config.test_format = lit.formats.ShTest(execute_external)
# suffixes: A list of file extensions to treat as test files.
-config.suffixes = ['.c', '.cpp', '.m', '.mm', '.cu', '.ll', '.cl', '.s', '.modularize', '.module-map-checker']
+config.suffixes = ['.c', '.cpp', '.m', '.mm', '.cu', '.ll', '.cl', '.s',
+ '.modularize', '.module-map-checker', '.hpp']
# Test-time dependencies located in directories called 'Inputs' are excluded
# from test suites; there won't be any lit tests within them.
Index: test/clang-tidy/misc-definitions-in-headers.hpp
===
--- /dev/null
+++ test/clang-tidy/misc-definitions-in-headers.hpp
@@ -0,0 +1,131 @@
+// RUN: %check_clang_tidy %s misc-definitions-in-headers %t
+
+int f() {
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function definition is not allowed in header file [misc-definitions-in-headers]
+// CHECK-FIXES: inline int f() {
+ return 1;
+}
+
+class CA {
+ void f1() {} // ok
+ void f2();
+ template
+ T f3() { // ok
+T a = 1;
+return a;
+ }
+ template
+ struct CAA {
+struct CAB {
+ void f4(); // ok
+};
+ };
+ static void f4(); // ok
+};
+
+void CA::f2() { }
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: function definition is not allowed in header file [misc-definitions-in-headers]
+// CHECK-FIXES: inline void CA::f2() {
+
+template <>
+int CA::f3() {
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: function definition is not allowed in header file [misc-definitions-in-headers]
+ int a = 1;
+ return a;
+}
+
+template
+void CA::CAA::CAB::f4() { // ok
+}
+
+template
+struct CB {
+ void f1();
+ struct CCA {
+void f2(T a);
+ };
+ struct CCB; // ok
+ static int a; // ok
+};
+
+template
+void CB::f1() { // ok
+}
+
+template
+void CB::CCA::f2(T a) { // ok
+}
+
+template
+struct CB::CCB {
+ void f3();
+};
+
+template
+void CB::CCB::f3() { // ok
+}
+
+template
+int CB::a = 2; // ok;
+
+template
+T tf() { // ok
+ T a;
+ return a;
+}
+
+
+namespace NA {
+ int f() { return 1; }
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: function definition is not allowed in header file [misc-definitions-in-headers]
+// CHECK-FIXES: inline int f() { return 1; }
+}
+
+template
+T f3() {
+ T a = 1;
+ return a;
+}
+
+template <>
+// CHECK-MESSAGES: :[[@LINE+1]]:5: warning: function definition is not allowed in header file [misc-definitions-in-headers]
+int f3() {
+ int a = 1;
+ return a;
+}
+
+int f5(); // ok
+inline int f6() { return 1; } // ok
+namespace {
+ int f7() { return 1; } // ok
+}
+
+int a = 1;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: variable definition is not allowed in header file [misc-definitions-in-headers]
+CA a1;
+// CHECK-MESSAGES: :[[@LINE-1]]:4: warning: variable definition is not allowed in header file [misc-definitions-in-headers]
+
+namespace NB {
+ int b = 1;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable definition is not allowed in header file [misc-definitions-in-headers]
+ const int c = 1; // ok;
+}
+
+class CC {
+ static int d;
+};
+
+int CC::d = 1;
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: variable definition is not allowed in header file [misc-definitions-in-headers]
+
+const char* ca = "foo";
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: variable definition is not allowed in header file [misc-definitions-in-headers]
+
+namespace {
+ int e = 2; // ok
+}
+
+const char* const g = "foo"; // ok
+static int h = 1; // ok
+const int i = 1; // ok
+extern int j; // ok
Index: test/clang-tidy/check_clang_tidy.py
===
--- test/clang-tidy/check_clang_tidy.py
+++ test/clang-tidy/check_clang_tidy.py
@@ -52,6 +52,8 @@
extension = '.cpp'
if (input_file_name.endswith('.c')):
extension = '.c'
+ if (input_file_name.endswith('.hpp')):
+extension = '.hpp'
temp_file_name = temp_file_name + extension
clang_tidy_extra_args = extra_args
Index: docs/clang-tidy/checks/misc-definitions-in-headers.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/misc-definitions-in-headers.rst
@@ -0,0 +1,36 @@
+misc-definitions-in-headers
+===
+
+Finds non-extern non-inline function and variable definitions in header files, which can lead to potential ODR violations.
+
+.. code::
Re: [PATCH] D15709: [X86] Support 'interrupt' attribute for x86
ABataev updated this revision to Diff 43828.
ABataev added a comment.
Fix after review.
After some discussion with gcc guys it was decided to allow to use static
member functions as interrupt handler.
http://reviews.llvm.org/D15709
Files:
include/clang/Basic/Attr.td
include/clang/Basic/AttrDocs.td
include/clang/Basic/DiagnosticSemaKinds.td
lib/CodeGen/TargetInfo.cpp
lib/Sema/SemaDeclAttr.cpp
lib/Sema/SemaExpr.cpp
test/CodeGen/attr-x86-interrupt.c
test/CodeGenCXX/attr-x86-interrupt.cpp
test/Sema/attr-x86-interrupt.c
test/SemaCXX/attr-x86-interrupt.cpp
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -1609,6 +1609,10 @@
llvm::AttributeSet::FunctionIndex,
B));
}
+if (FD->hasAttr()) {
+ llvm::Function *Fn = cast(GV);
+ Fn->setCallingConv(llvm::CallingConv::X86_INTR);
+}
}
}
@@ -1914,6 +1918,16 @@
('T' << 24);
return llvm::ConstantInt::get(CGM.Int32Ty, Sig);
}
+
+ void setTargetAttributes(const Decl *D, llvm::GlobalValue *GV,
+ CodeGen::CodeGenModule &CGM) const override {
+if (const FunctionDecl *FD = dyn_cast_or_null(D)) {
+ if (FD->hasAttr()) {
+llvm::Function *Fn = cast(GV);
+Fn->setCallingConv(llvm::CallingConv::X86_INTR);
+ }
+}
+ }
};
class PS4TargetCodeGenInfo : public X86_64TargetCodeGenInfo {
@@ -2031,6 +2045,13 @@
CodeGen::CodeGenModule &CGM) const {
TargetCodeGenInfo::setTargetAttributes(D, GV, CGM);
+ if (const FunctionDecl *FD = dyn_cast_or_null(D)) {
+if (FD->hasAttr()) {
+ llvm::Function *Fn = cast(GV);
+ Fn->setCallingConv(llvm::CallingConv::X86_INTR);
+}
+ }
+
addStackProbeSizeTargetAttribute(D, GV, CGM);
}
}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -5034,6 +5034,12 @@
FunctionDecl *FDecl = dyn_cast_or_null(NDecl);
unsigned BuiltinID = (FDecl ? FDecl->getBuiltinID() : 0);
+ // Functions with 'interrupt' attribute cannot be called directly.
+ if (FDecl && FDecl->hasAttr()) {
+Diag(Fn->getExprLoc(), diag::err_anyx86_interrupt_attribute) << 4;
+return ExprError();
+ }
+
// Promote the function operand.
// We special-case function promotion here because we only allow promoting
// builtin functions to function pointers in the callee of a call.
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -4545,17 +4545,75 @@
Attr.getLoc(), S.Context, Kind, Attr.getAttributeSpellingListIndex()));
}
+static void handleAnyX86InterruptAttr(Sema &S, Decl *D,
+ const AttributeList &Attr) {
+ // Semantic checks for a function with the 'interrupt' attribute.
+ // a) Must be a function.
+ // b) Must have the 'void' return type.
+ // c) Must take 1 or 2 arguments.
+ // d) The 1st argument must be a pointer.
+ // e) The 2nd argument (if any) must be an unsigned integer.
+ if (!isFunctionOrMethod(D) || !hasFunctionProto(D) || isInstanceMethod(D) ||
+ CXXMethodDecl::isStaticOverloadedOperator(
+ cast(D)->getDeclName().getCXXOverloadedOperator())) {
+S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)
+<< Attr.getName() << ExpectedFunctionWithProtoType;
+return;
+ }
+ // Interrupt handler must have void return type.
+ if (!getFunctionOrMethodResultType(D)->isVoidType()) {
+S.Diag(getFunctionOrMethodResultSourceRange(D).getBegin(),
+ diag::err_anyx86_interrupt_attribute) << 0;
+return;
+ }
+ // Interrupt handler must have 1 or 2 parameters.
+ unsigned NumParams = getFunctionOrMethodNumParams(D);
+ if (NumParams < 1 || NumParams > 2) {
+S.Diag(D->getLocStart(), diag::err_anyx86_interrupt_attribute) << 1;
+return;
+ }
+ // The first argument must be a pointer.
+ if (!getFunctionOrMethodParamType(D, 0)->isPointerType()) {
+S.Diag(getFunctionOrMethodParamRange(D, 0).getBegin(),
+ diag::err_anyx86_interrupt_attribute) << 2;
+return;
+ }
+ // The second argument, if present, must be an unsigned integer.
+ unsigned TypeSize =
+ S.Context.getTargetInfo().getTriple().getArch() == llvm::Triple::x86_64
+ ? 64
+ : 32;
+ if (NumParams == 2 &&
+ (!getFunctionOrMethodParamType(D, 1)->isUnsignedIntegerType() ||
+ S.Context.getTypeSize(getFunctionOrMethodParamType(D, 1)) != TypeSize)) {
+S.Diag(getFunctionOrMethodParamRange(D, 1).getBegin(),
+ diag::err_anyx86_interrupt_attribute)
+<< 3 << S.Context.getIntTypeForBitwidth(TypeSize, /*Signed=*/false);
+return;
+ }
+
Re: [PATCH] D15710: [clang-tidy] Add non-inline function definition and variable definition check in header files.
hokein marked 5 inline comments as done. hokein added a comment. http://reviews.llvm.org/D15710 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15805: [clang-tidy] Cleanup code in CERT module.
hokein added a comment. So shoud we need to rename to `cert` at this patch? Repository: rL LLVM http://reviews.llvm.org/D15805 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15639: [clang-format] Ensure Sort include is stable with negative Priority
jeanphilippeD abandoned this revision. jeanphilippeD added a comment. This was fixed by other patch. http://reviews.llvm.org/D15639 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14727: [Driver] Adapt Linux::GCCVersion::Parse to match GCC 5 installations
thiagomacieira added a comment. Ping? http://reviews.llvm.org/D14727 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14727: [Driver] Adapt Linux::GCCVersion::Parse to match GCC 5 installations
thiagomacieira added a comment. Ah, I see a comment. Sorry about that. I'll see if I can fix it, but I had no failures when I tried... http://reviews.llvm.org/D14727 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15266: [clang-format] Handle \n the same way as std::endl with stream operator.
jeanphilippeD added a comment. Ping. Is that patch of interest? http://reviews.llvm.org/D15266 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r256672 - [X86][PKU] add clang intrinsic for {RD|WR}PKRU
Author: abadouh
Date: Thu Dec 31 08:14:07 2015
New Revision: 256672
URL: http://llvm.org/viewvc/llvm-project?rev=256672&view=rev
Log:
[X86][PKU] add clang intrinsic for {RD|WR}PKRU
Differential Revision: http://reviews.llvm.org/D15837
Added:
cfe/trunk/lib/Headers/pkuintrin.h
cfe/trunk/test/CodeGen/pku.c
Modified:
cfe/trunk/include/clang/Basic/BuiltinsX86.def
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/lib/Basic/Targets.cpp
cfe/trunk/lib/Headers/CMakeLists.txt
cfe/trunk/lib/Headers/immintrin.h
Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=256672&r1=256671&r2=256672&view=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Thu Dec 31 08:14:07 2015
@@ -917,6 +917,9 @@ TARGET_BUILTIN(__builtin_ia32_xtest, "i"
BUILTIN(__builtin_ia32_rdpmc, "ULLii", "")
BUILTIN(__builtin_ia32_rdtsc, "ULLi", "")
BUILTIN(__builtin_ia32_rdtscp, "ULLiUi*", "")
+// PKU
+TARGET_BUILTIN(__builtin_ia32_rdpkru, "Ui", "", "pku")
+TARGET_BUILTIN(__builtin_ia32_wrpkru, "vUi", "", "pku")
// AVX-512
TARGET_BUILTIN(__builtin_ia32_sqrtpd512_mask, "V8dV8dV8dUcIi", "", "avx512f")
Modified: cfe/trunk/include/clang/Driver/Options.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=256672&r1=256671&r2=256672&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Thu Dec 31 08:14:07 2015
@@ -1369,6 +1369,7 @@ def mno_xsave : Flag<["-"], "mno-xsave">
def mno_xsaveopt : Flag<["-"], "mno-xsaveopt">, Group;
def mno_xsavec : Flag<["-"], "mno-xsavec">, Group;
def mno_xsaves : Flag<["-"], "mno-xsaves">, Group;
+def mno_pku : Flag<["-"], "mno-pku">, Group;
def munaligned_access : Flag<["-"], "munaligned-access">,
Group,
HelpText<"Allow memory accesses to be unaligned (AArch32/AArch64 only)">;
@@ -1520,6 +1521,7 @@ def mf16c : Flag<["-"], "mf16c">, Group<
def mrtm : Flag<["-"], "mrtm">, Group;
def mprfchw : Flag<["-"], "mprfchw">, Group;
def mrdseed : Flag<["-"], "mrdseed">, Group;
+def mpku : Flag<["-"], "mpku">, Group;
def madx : Flag<["-"], "madx">, Group;
def msha : Flag<["-"], "msha">, Group;
def mcx16 : Flag<["-"], "mcx16">, Group;
Modified: cfe/trunk/lib/Basic/Targets.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=256672&r1=256671&r2=256672&view=diff
==
--- cfe/trunk/lib/Basic/Targets.cpp (original)
+++ cfe/trunk/lib/Basic/Targets.cpp Thu Dec 31 08:14:07 2015
@@ -2095,6 +2095,7 @@ class X86TargetInfo : public TargetInfo
bool HasXSAVEOPT = false;
bool HasXSAVEC = false;
bool HasXSAVES = false;
+ bool HasPKU = false;
/// \brief Enumeration of all of the X86 CPUs supported by Clang.
///
@@ -2596,6 +2597,7 @@ bool X86TargetInfo::initFeatureMap(
setFeatureEnabledImpl(Features, "avx512vl", true);
setFeatureEnabledImpl(Features, "xsavec", true);
setFeatureEnabledImpl(Features, "xsaves", true);
+setFeatureEnabledImpl(Features, "pku", true);
// FALLTHROUGH
case CK_Broadwell:
setFeatureEnabledImpl(Features, "rdseed", true);
@@ -3021,6 +3023,8 @@ bool X86TargetInfo::handleTargetFeatures
HasXSAVEC = true;
} else if (Feature == "+xsaves") {
HasXSAVES = true;
+} else if (Feature == "+pku") {
+ HasPKU = true;
}
X86SSEEnum Level = llvm::StringSwitch(Feature)
@@ -3322,7 +3326,8 @@ void X86TargetInfo::getTargetDefines(con
Builder.defineMacro("__XSAVEC__");
if (HasXSAVES)
Builder.defineMacro("__XSAVES__");
-
+ if (HasPKU)
+Builder.defineMacro("__PKU__");
if (HasCX16)
Builder.defineMacro("__GCC_HAVE_SYNC_COMPARE_AND_SWAP_16");
@@ -3440,6 +3445,7 @@ bool X86TargetInfo::hasFeature(StringRef
.Case("xsavec", HasXSAVEC)
.Case("xsaves", HasXSAVES)
.Case("xsaveopt", HasXSAVEOPT)
+ .Case("pku", HasPKU)
.Default(false);
}
Modified: cfe/trunk/lib/Headers/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/CMakeLists.txt?rev=256672&r1=256671&r2=256672&view=diff
==
--- cfe/trunk/lib/Headers/CMakeLists.txt (original)
+++ cfe/trunk/lib/Headers/CMakeLists.txt Thu Dec 31 08:14:07 2015
@@ -12,6 +12,7 @@ set(files
avx512vlintrin.h
avx512dqintrin.h
avx512vldqintrin.h
+ pkuintrin.h
avxintrin.h
bmi2intrin.h
bmiintrin.h
Modified: cfe/trunk/lib/Headers/immintrin.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/immintrin.h?rev=256672&r1=256671&r2=256672&view=diff
===
Re: [PATCH] D15823: Support virtual-near-miss check.
congliu marked 2 inline comments as done.
Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:79
@@ +78,3 @@
+ if (BaseReturnType->isReferenceType() &&
DerivedReturnType->isReferenceType()){
+BaseReturnType = BaseReturnType.getNonReferenceType();
+DerivedReturnType = DerivedReturnType.getNonReferenceType();
alexfh wrote:
> You can call `.getNonReferenceType()` unconditionally to make the code
> shorter.
The condition is necessary, for the same reason I explained in the comment for
line 85.
Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:85
@@ +84,3 @@
+ }else {
+return false;
+ }
alexfh wrote:
> What if both return types are not references and are not pointers? Why do you
> return `false` in this case?
This is to deal with an special case of C++ override. Usually the return type
of the override and overriden should be the same. The exception is that the
return type ban be a reference (or pointer) to the class themselves. For
example,
class Base{
virtual Base* func();
};
class Derived : Base{
virtual Derived* func();
};
In this case, the Derived::func() does override Base::func(), even though the
return type are not the same.
So if the return types are not the same (line 72 assured that), and are not
both references (or pointers), we can rule out the possibility of override, and
therefore return false.
http://reviews.llvm.org/D15823
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D12834: add gcc abi_tag support
elizafox added a comment. In http://reviews.llvm.org/D12834#316988, @foutrelis wrote: > In http://reviews.llvm.org/D12834#314136, @elizafox wrote: > > > In http://reviews.llvm.org/D12834#311890, @foutrelis wrote: > > > > > We have received a few reports of clang crashes after applying the > > > abi_tag support patch to our llvm/clang package in Arch Linux. > > > > > > Why would you put a patch clearly marked as "needs review" into a > > distribution?!?!?!?! > > > We waited 6 months before switching to the new ABI in libstdc++. Ideally, we > would have waited until the patch was reviewed and merged but did not want to > wait much longer. I also (wrongly) considered the patch to be relative stable. What would make you believe this was stable?! This patch has known differences in ABI anyway. > > > > In any case, the recursion source seems obvious to me, but I don't know how > > to add patches to this reviewboard item. > > > If the correction is obvious as well and not very complex, would you mind > sharing it? I see the recursion source, but I don't know if my fix would be "correct," as I do not know enough about the new ABI nor GCC's internals (and therefore cross-checking it) to ensure it will behave correctly. I'm erring on the side of caution for now. Repository: rL LLVM http://reviews.llvm.org/D12834 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15805: [clang-tidy] Cleanup code in CERT module.
aaron.ballman added a comment. In http://reviews.llvm.org/D15805#318307, @hokein wrote: > So shoud we need to rename to `cert` at this patch? Yes, but just the namespaces as you do in this patch (not anything in user documentation, for instance). This patch LG; if you need me to commit on your behalf, I'm happy to do so. Repository: rL LLVM http://reviews.llvm.org/D15805 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15709: [X86] Support 'interrupt' attribute for x86
aaron.ballman added inline comments.
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:259
@@ +258,3 @@
+def err_anyx86_interrupt_attribute : Error<
+ "interrupt service routine %select{must have void return value|"
+ "can only have a pointer argument and an optional integer argument|"
interrupt service routine -> %select{x86|x86-64}0 'interrupt' attribute only
applies to functions that (then fix the other wordings accordingly)
have a void return value -> have a 'void' return type
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:262
@@ -258,1 +261,3 @@
+ "should have a pointer as the first argument|should have %1 type as the "
+ "second argument|can't be called directly}0">;
def warn_mips_interrupt_attribute : Warning<
I would split "can't be called directly" into its own diagnostic. The other
diagnostic is about the semantic requirements of the attribute, this is a
separate concept. Also, "can't be called directly" should be "cannot be called
directly".
Comment at: lib/Sema/SemaDeclAttr.cpp:4556
@@ +4555,3 @@
+ // e) The 2nd argument (if any) must be an unsigned integer.
+ if (!isFunctionOrMethod(D) || !hasFunctionProto(D) || isInstanceMethod(D) ||
+ CXXMethodDecl::isStaticOverloadedOperator(
> See test/CodeGenCXX/attr-x86-interrupt.cpp. Function 'foo8' is declare in
> 'namespace S'.
Ah, I was looking for that in the Sema tests because it's a semantic
requirement, not a codegen difference. Either place is reasonable, however.
Thank you for pointing it out!
http://reviews.llvm.org/D15709
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [libcxx] r249798 - Split out of .
On Wed, Dec 30, 2015 at 8:28 PM, Richard Smith
wrote:
> On Wed, Dec 30, 2015 at 1:17 PM, Nico Weber wrote:
>
>> One problem with this patch: stdio.h can be used in .c files, and when
>> building .c files with -gnu99 -pedantic,
>>
>
> Do you mean -std=gnu89?
>
Sorry, I meant -std=gnu99 -pedantic -ansi:
$ clang -c test.c -std=gnu99 -pedantic -ansi
test.c:1:1: warning: // comments are not allowed in this language
[-Wcomment]
// hi
^
Might be a clang bug?
>
>> clang will complain about // comments. Not only does this stdio.h have //
>> comments, it also pulls in some libc++ headers (__config) that have //
>> comments as well. I suppose all the comments in header files pulled in by C
>> headers need to become /* */ comments?
>>
>
> I suppose so too. Your configuration is probably somewhat broken if
> libc++'s headers are in your include path while building C code, but it
> doesn't seem unreasonable to properly support that mode, and my changes
> were already trying to do so.
>
Thanks!
>
> Eric, Marshall, what do you think about using only /*...*/-style comments
> in these headers, to handle the case where libc++ is somehow in the include
> path for a C89 compilation?
>
>
>> On Tue, Oct 13, 2015 at 7:34 PM, Richard Smith via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> On Tue, Oct 13, 2015 at 3:26 PM, Eric Fiselier wrote:
>>>
This change LGTM. Let's hold off on the using "_Static_assert" until we
understand how that would work with "-pedantic" when the macro is expanded
in user code.
>>>
>>> Committed as r250247, thanks.
>>>
>>>
/Eric
On Tue, Oct 13, 2015 at 4:19 PM, Richard Smith
wrote:
> On Tue, Oct 13, 2015 at 2:12 PM, Eric Fiselier via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> I would rather not do this if possible but I understand why we need
>> to do it.
>>
>> Richard is there a cost associated with the 'extern "C++"' construct?
>> or by forcing the compiler to switch modes in general?
>>
>
> Not a significant one compared to the cost of the code wrapped in the
> 'extern "C++"' here. (Also, this is wrapped in an #ifdef that only applies
> in C++98; we could further reduce the set of cases when this happens by
> using _Static_assert when available instead of this static_assert
> emulation.)
>
>
>> On Mon, Oct 12, 2015 at 12:27 PM, Richard Smith <
>> rich...@metafoo.co.uk> wrote:
>>
>>> On Mon, Oct 12, 2015 at 9:41 AM, Steven Wu via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>>
Hi Richard
Your splitting seems causing problem when using extern "C". Here is
a test case:
$ cat test.cpp
#ifdef __cplusplus
extern "C" {
#endif
#include
#ifdef __cplusplus
}
#endif
Error:
clang -fsyntax-only test.cpp
In file included from test.cpp:4:
In file included from /usr/bin/../include/c++/v1/stdio.h:102:
/usr/bin/../include/c++/v1/__config:593:1: error:
templates must have C++ linkage
template struct __static_assert_test;
^~~
/usr/bin/../include/c++/v1/__config:594:20: error:
explicit specialization of non-template struct
'__static_assert_test'
template <> struct __static_assert_test {};
^ ~~
/usr/bin/../include/c++/v1/__config:595:1: error:
templates must have C++ linkage
template struct __static_assert_check {};
^~~
3 errors generated.
Because the code is actually compiled in C++, the guard in the
header failed to exclude the templates. In the meantime, I don't know
if
there are ways to detect the header is in extern "C".
>>>
>>> This was supposed to work, but apparently I only tested it when
>>> compiling as C++11; the static_assert emulation in C++98 mode needs some
>>> massaging to cope with this.
>>>
>>> Eric, Marshall: Are you OK with the attached patch? The idea is to
>>> make <__config> be fine to include in extern "C" or extern "C++" modes
>>> (and
>>> likewise for the headers). This is something that comes up
>>> pretty
>>> often in practice (people wrap an include of a C header in 'extern "C"',
>>> and that C header includes a file that libc++ provides).
>>>
>>>
Steven
> On Oct 8, 2015, at 6:29 PM, Richard Smith via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
>
> Author: rsmith
> Date: Thu Oct 8 20:29:09 2015
> New Revision: 249798
>
> URL: http://llvm.org/viewvc/llvm-project?rev=249798&view=rev
> Log:
> Split out of .
[PATCH] D15839: Move _xgetbv to immintrin.h
ehsan created this revision.
ehsan added a reviewer: hansw.
ehsan added a subscriber: cfe-commits.
The documentation in https://msdn.microsoft.com/en-us/library/hh977022.aspx
says that this intrinsic is defined in immintrin.h, and that's the header
where MSVC declares this intrinsic, but clang-cl mistakenly provides it in
intrin.h.
http://reviews.llvm.org/D15839
Files:
lib/Headers/Intrin.h
lib/Headers/xsaveintrin.h
Index: lib/Headers/xsaveintrin.h
===
--- lib/Headers/xsaveintrin.h
+++ lib/Headers/xsaveintrin.h
@@ -53,6 +53,15 @@
}
#endif
+#if defined(__i386__) || defined(__x86_64__)
+static __inline__ unsigned __int64 __cdecl __DEFAULT_FN_ATTRS
+_xgetbv(unsigned int __xcr_no) {
+ unsigned int __eax, __edx;
+ __asm__ ("xgetbv" : "=a" (__eax), "=d" (__edx) : "c" (__xcr_no));
+ return ((unsigned __int64)__edx << 32) | __eax;
+}
+#endif
+
#undef __DEFAULT_FN_ATTRS
#endif
Index: lib/Headers/Intrin.h
===
--- lib/Headers/Intrin.h
+++ lib/Headers/Intrin.h
@@ -905,12 +905,6 @@
__asm__ ("cpuid" : "=a"(__info[0]), "=b" (__info[1]), "=c"(__info[2]),
"=d"(__info[3])
: "a"(__level), "c"(__ecx));
}
-static __inline__ unsigned __int64 __cdecl __DEFAULT_FN_ATTRS
-_xgetbv(unsigned int __xcr_no) {
- unsigned int __eax, __edx;
- __asm__ ("xgetbv" : "=a" (__eax), "=d" (__edx) : "c" (__xcr_no));
- return ((unsigned __int64)__edx << 32) | __eax;
-}
static __inline__ void __DEFAULT_FN_ATTRS
__halt(void) {
__asm__ volatile ("hlt");
Index: lib/Headers/xsaveintrin.h
===
--- lib/Headers/xsaveintrin.h
+++ lib/Headers/xsaveintrin.h
@@ -53,6 +53,15 @@
}
#endif
+#if defined(__i386__) || defined(__x86_64__)
+static __inline__ unsigned __int64 __cdecl __DEFAULT_FN_ATTRS
+_xgetbv(unsigned int __xcr_no) {
+ unsigned int __eax, __edx;
+ __asm__ ("xgetbv" : "=a" (__eax), "=d" (__edx) : "c" (__xcr_no));
+ return ((unsigned __int64)__edx << 32) | __eax;
+}
+#endif
+
#undef __DEFAULT_FN_ATTRS
#endif
Index: lib/Headers/Intrin.h
===
--- lib/Headers/Intrin.h
+++ lib/Headers/Intrin.h
@@ -905,12 +905,6 @@
__asm__ ("cpuid" : "=a"(__info[0]), "=b" (__info[1]), "=c"(__info[2]), "=d"(__info[3])
: "a"(__level), "c"(__ecx));
}
-static __inline__ unsigned __int64 __cdecl __DEFAULT_FN_ATTRS
-_xgetbv(unsigned int __xcr_no) {
- unsigned int __eax, __edx;
- __asm__ ("xgetbv" : "=a" (__eax), "=d" (__edx) : "c" (__xcr_no));
- return ((unsigned __int64)__edx << 32) | __eax;
-}
static __inline__ void __DEFAULT_FN_ATTRS
__halt(void) {
__asm__ volatile ("hlt");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15839: Move _xgetbv to immintrin.h
ehsan abandoned this revision. ehsan added a comment. Oops, sorry, I submitted this by mistake. http://reviews.llvm.org/D15839 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [libcxx] r249798 - Split out of .
On Thu, Dec 31, 2015 at 10:52 AM, Nico Weber wrote:
> On Wed, Dec 30, 2015 at 8:28 PM, Richard Smith
> wrote:
>
>> On Wed, Dec 30, 2015 at 1:17 PM, Nico Weber wrote:
>>
>>> One problem with this patch: stdio.h can be used in .c files, and when
>>> building .c files with -gnu99 -pedantic,
>>>
>>
>> Do you mean -std=gnu89?
>>
>
> Sorry, I meant -std=gnu99 -pedantic -ansi:
>
> $ clang -c test.c -std=gnu99 -pedantic -ansi
> test.c:1:1: warning: // comments are not allowed in this language
> [-Wcomment]
> // hi
> ^
>
> Might be a clang bug?
>
Ah, -ansi just means -std=c89, so passing both -std=gnu99 and -ansi doesn't
make a lot of sense. So not a clang bug :-)
> clang will complain about // comments. Not only does this stdio.h have //
>>> comments, it also pulls in some libc++ headers (__config) that have //
>>> comments as well. I suppose all the comments in header files pulled in by C
>>> headers need to become /* */ comments?
>>>
>>
>> I suppose so too. Your configuration is probably somewhat broken if
>> libc++'s headers are in your include path while building C code, but it
>> doesn't seem unreasonable to properly support that mode, and my changes
>> were already trying to do so.
>>
>
> Thanks!
>
>
>>
>> Eric, Marshall, what do you think about using only /*...*/-style comments
>> in these headers, to handle the case where libc++ is somehow in the include
>> path for a C89 compilation?
>>
>>
>>> On Tue, Oct 13, 2015 at 7:34 PM, Richard Smith via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>>
On Tue, Oct 13, 2015 at 3:26 PM, Eric Fiselier wrote:
> This change LGTM. Let's hold off on the using "_Static_assert" until
> we understand how that would work with "-pedantic" when the macro is
> expanded in user code.
>
Committed as r250247, thanks.
> /Eric
>
> On Tue, Oct 13, 2015 at 4:19 PM, Richard Smith
> wrote:
>
>> On Tue, Oct 13, 2015 at 2:12 PM, Eric Fiselier via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> I would rather not do this if possible but I understand why we need
>>> to do it.
>>>
>>> Richard is there a cost associated with the 'extern "C++"'
>>> construct? or by forcing the compiler to switch modes in general?
>>>
>>
>> Not a significant one compared to the cost of the code wrapped in the
>> 'extern "C++"' here. (Also, this is wrapped in an #ifdef that only
>> applies
>> in C++98; we could further reduce the set of cases when this happens by
>> using _Static_assert when available instead of this static_assert
>> emulation.)
>>
>>
>>> On Mon, Oct 12, 2015 at 12:27 PM, Richard Smith <
>>> rich...@metafoo.co.uk> wrote:
>>>
On Mon, Oct 12, 2015 at 9:41 AM, Steven Wu via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> Hi Richard
>
> Your splitting seems causing problem when using extern "C". Here
> is a test case:
>
> $ cat test.cpp
> #ifdef __cplusplus
> extern "C" {
> #endif
> #include
> #ifdef __cplusplus
> }
> #endif
>
> Error:
> clang -fsyntax-only test.cpp
> In file included from test.cpp:4:
> In file included from /usr/bin/../include/c++/v1/stdio.h:102:
> /usr/bin/../include/c++/v1/__config:593:1: error:
> templates must have C++ linkage
> template struct __static_assert_test;
> ^~~
> /usr/bin/../include/c++/v1/__config:594:20: error:
> explicit specialization of non-template struct
> '__static_assert_test'
> template <> struct __static_assert_test {};
>^ ~~
> /usr/bin/../include/c++/v1/__config:595:1: error:
> templates must have C++ linkage
> template struct __static_assert_check {};
> ^~~
> 3 errors generated.
>
> Because the code is actually compiled in C++, the guard in the
> header failed to exclude the templates. In the meantime, I don't know
> if
> there are ways to detect the header is in extern "C".
>
This was supposed to work, but apparently I only tested it when
compiling as C++11; the static_assert emulation in C++98 mode needs
some
massaging to cope with this.
Eric, Marshall: Are you OK with the attached patch? The idea is to
make <__config> be fine to include in extern "C" or extern "C++" modes
(and
likewise for the headers). This is something that comes up
pretty
often in practice (people wrap an include of a C header in 'extern
"C"',
and that C header includes a file that libc++ provides).
> Steven
>
>
>>>
r256683 - [TrailingObjects] Convert classes in OpenMPClause.h
Author: jyknight
Date: Thu Dec 31 18:38:24 2015
New Revision: 256683
URL: http://llvm.org/viewvc/llvm-project?rev=256683&view=rev
Log:
[TrailingObjects] Convert classes in OpenMPClause.h
Modified:
cfe/trunk/include/clang/AST/OpenMPClause.h
cfe/trunk/lib/AST/OpenMPClause.cpp
Modified: cfe/trunk/include/clang/AST/OpenMPClause.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/OpenMPClause.h?rev=256683&r1=256682&r2=256683&view=diff
==
--- cfe/trunk/include/clang/AST/OpenMPClause.h (original)
+++ cfe/trunk/include/clang/AST/OpenMPClause.h Thu Dec 31 18:38:24 2015
@@ -84,21 +84,15 @@ protected:
/// \brief Fetches list of variables associated with this clause.
MutableArrayRef getVarRefs() {
return MutableArrayRef(
-reinterpret_cast(
-reinterpret_cast(this) +
-llvm::RoundUpToAlignment(sizeof(T), llvm::alignOf())),
-NumVars);
+static_cast(this)->template getTrailingObjects(),
NumVars);
}
/// \brief Sets the list of variables for this clause.
void setVarRefs(ArrayRef VL) {
assert(VL.size() == NumVars &&
"Number of variables is not the same as the preallocated buffer");
-std::copy(
-VL.begin(), VL.end(),
-reinterpret_cast(
-reinterpret_cast(this) +
-llvm::RoundUpToAlignment(sizeof(T), llvm::alignOf(;
+std::copy(VL.begin(), VL.end(),
+ static_cast(this)->template getTrailingObjects());
}
/// \brief Build a clause with \a N variables
@@ -142,9 +136,7 @@ public:
/// \brief Fetches list of all variables in the clause.
ArrayRef getVarRefs() const {
return llvm::makeArrayRef(
-reinterpret_cast(
-reinterpret_cast(this) +
-llvm::RoundUpToAlignment(sizeof(T), llvm::alignOf())),
+static_cast(this)->template getTrailingObjects(),
NumVars);
}
};
@@ -1160,7 +1152,11 @@ public:
/// In this example directive '#pragma omp parallel' has clause 'private'
/// with the variables 'a' and 'b'.
///
-class OMPPrivateClause : public OMPVarListClause {
+class OMPPrivateClause final
+: public OMPVarListClause,
+ private llvm::TrailingObjects {
+ friend TrailingObjects;
+ friend class OMPVarListClause;
friend class OMPClauseReader;
/// \brief Build clause with number of variables \a N.
///
@@ -1252,7 +1248,11 @@ public:
/// In this example directive '#pragma omp parallel' has clause 'firstprivate'
/// with the variables 'a' and 'b'.
///
-class OMPFirstprivateClause : public OMPVarListClause {
+class OMPFirstprivateClause final
+: public OMPVarListClause,
+ private llvm::TrailingObjects {
+ friend TrailingObjects;
+ friend class OMPVarListClause;
friend class OMPClauseReader;
/// \brief Build clause with number of variables \a N.
@@ -1372,7 +1372,9 @@ public:
/// \endcode
/// In this example directive '#pragma omp simd' has clause 'lastprivate'
/// with the variables 'a' and 'b'.
-class OMPLastprivateClause : public OMPVarListClause {
+class OMPLastprivateClause final
+: public OMPVarListClause,
+ private llvm::TrailingObjects {
// There are 4 additional tail-allocated arrays at the end of the class:
// 1. Contains list of pseudo variables with the default initialization for
// each non-firstprivate variables. Used in codegen for initialization of
@@ -1390,6 +1392,8 @@ class OMPLastprivateClause : public OMPV
// Required for proper codegen of final assignment performed by the
// lastprivate clause.
//
+ friend TrailingObjects;
+ friend class OMPVarListClause;
friend class OMPClauseReader;
/// \brief Build clause with number of variables \a N.
@@ -1557,7 +1561,11 @@ public:
/// In this example directive '#pragma omp parallel' has clause 'shared'
/// with the variables 'a' and 'b'.
///
-class OMPSharedClause : public OMPVarListClause {
+class OMPSharedClause final
+: public OMPVarListClause,
+ private llvm::TrailingObjects {
+ friend TrailingObjects;
+ friend class OMPVarListClause;
/// \brief Build clause with number of variables \a N.
///
/// \param StartLoc Starting location of the clause.
@@ -1617,7 +1625,11 @@ public:
/// In this example directive '#pragma omp parallel' has clause 'reduction'
/// with operator '+' and the variables 'a' and 'b'.
///
-class OMPReductionClause : public OMPVarListClause {
+class OMPReductionClause final
+: public OMPVarListClause,
+ private llvm::TrailingObjects {
+ friend TrailingObjects;
+ friend class OMPVarListClause;
friend class OMPClauseReader;
/// \brief Location of ':'.
SourceLocation ColonLoc;
@@ -1819,7 +1831,11 @@ public:
/// In this example directive '#pragma omp simd' has clause 'linear'
/// with variables 'a', 'b' and linear step '2'.
///
-class OMPLinearClause : public OMPVarListClause {
+class OMPLinearClause final
+: public OMPVarLi
r256684 - [TrailingObjects] Fix "Convert classes in OpenMPClause.h" for MSVC
Author: jyknight
Date: Thu Dec 31 19:12:48 2015
New Revision: 256684
URL: http://llvm.org/viewvc/llvm-project?rev=256684&view=rev
Log:
[TrailingObjects] Fix "Convert classes in OpenMPClause.h" for MSVC
"friend class OMPVarListClause" -> "friend OMPVarListClause". It's a
template, not a class.
Modified:
cfe/trunk/include/clang/AST/OpenMPClause.h
Modified: cfe/trunk/include/clang/AST/OpenMPClause.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/OpenMPClause.h?rev=256684&r1=256683&r2=256684&view=diff
==
--- cfe/trunk/include/clang/AST/OpenMPClause.h (original)
+++ cfe/trunk/include/clang/AST/OpenMPClause.h Thu Dec 31 19:12:48 2015
@@ -1156,7 +1156,7 @@ class OMPPrivateClause final
: public OMPVarListClause,
private llvm::TrailingObjects {
friend TrailingObjects;
- friend class OMPVarListClause;
+ friend OMPVarListClause;
friend class OMPClauseReader;
/// \brief Build clause with number of variables \a N.
///
@@ -1252,7 +1252,7 @@ class OMPFirstprivateClause final
: public OMPVarListClause,
private llvm::TrailingObjects {
friend TrailingObjects;
- friend class OMPVarListClause;
+ friend OMPVarListClause;
friend class OMPClauseReader;
/// \brief Build clause with number of variables \a N.
@@ -1393,7 +1393,7 @@ class OMPLastprivateClause final
// lastprivate clause.
//
friend TrailingObjects;
- friend class OMPVarListClause;
+ friend OMPVarListClause;
friend class OMPClauseReader;
/// \brief Build clause with number of variables \a N.
@@ -1565,7 +1565,7 @@ class OMPSharedClause final
: public OMPVarListClause,
private llvm::TrailingObjects {
friend TrailingObjects;
- friend class OMPVarListClause;
+ friend OMPVarListClause;
/// \brief Build clause with number of variables \a N.
///
/// \param StartLoc Starting location of the clause.
@@ -1629,7 +1629,7 @@ class OMPReductionClause final
: public OMPVarListClause,
private llvm::TrailingObjects {
friend TrailingObjects;
- friend class OMPVarListClause;
+ friend OMPVarListClause;
friend class OMPClauseReader;
/// \brief Location of ':'.
SourceLocation ColonLoc;
@@ -1835,7 +1835,7 @@ class OMPLinearClause final
: public OMPVarListClause,
private llvm::TrailingObjects {
friend TrailingObjects;
- friend class OMPVarListClause;
+ friend OMPVarListClause;
friend class OMPClauseReader;
/// \brief Modifier of 'linear' clause.
OpenMPLinearClauseKind Modifier;
@@ -2059,7 +2059,7 @@ class OMPAlignedClause final
: public OMPVarListClause,
private llvm::TrailingObjects {
friend TrailingObjects;
- friend class OMPVarListClause;
+ friend OMPVarListClause;
friend class OMPClauseReader;
/// \brief Location of ':'.
SourceLocation ColonLoc;
@@ -2160,7 +2160,7 @@ class OMPCopyinClause final
// implicit threads.
friend TrailingObjects;
- friend class OMPVarListClause;
+ friend OMPVarListClause;
friend class OMPClauseReader;
/// \brief Build clause with number of variables \a N.
///
@@ -2310,7 +2310,7 @@ class OMPCopyprivateClause final
: public OMPVarListClause,
private llvm::TrailingObjects {
friend TrailingObjects;
- friend class OMPVarListClause;
+ friend OMPVarListClause;
friend class OMPClauseReader;
/// \brief Build clause with number of variables \a N.
///
@@ -2463,7 +2463,7 @@ class OMPFlushClause final
: public OMPVarListClause,
private llvm::TrailingObjects {
friend TrailingObjects;
- friend class OMPVarListClause;
+ friend OMPVarListClause;
/// \brief Build clause with number of variables \a N.
///
/// \param StartLoc Starting location of the clause.
@@ -2527,7 +2527,7 @@ class OMPDependClause final
: public OMPVarListClause,
private llvm::TrailingObjects {
friend TrailingObjects;
- friend class OMPVarListClause;
+ friend OMPVarListClause;
friend class OMPClauseReader;
/// \brief Dependency type (one of in, out, inout).
OpenMPDependClauseKind DepKind;
@@ -2734,7 +2734,7 @@ public:
class OMPMapClause final : public OMPVarListClause,
private llvm::TrailingObjects
{
friend TrailingObjects;
- friend class OMPVarListClause;
+ friend OMPVarListClause;
friend class OMPClauseReader;
/// \brief Map type modifier for the 'map' clause.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r256686 - Reimplement __readeflags and __writeeflags on top of intrinsics
Author: majnemer
Date: Fri Jan 1 00:50:08 2016
New Revision: 256686
URL: http://llvm.org/viewvc/llvm-project?rev=256686&view=rev
Log:
Reimplement __readeflags and __writeeflags on top of intrinsics
Lean on LLVM to provide this functionality now that it provides the
necessary intrinsics.
Modified:
cfe/trunk/include/clang/Basic/BuiltinsX86.def
cfe/trunk/lib/Headers/ia32intrin.h
Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=256686&r1=256685&r2=256686&view=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Fri Jan 1 00:50:08 2016
@@ -41,6 +41,13 @@ TARGET_BUILTIN(__builtin_ia32_undef128,
TARGET_BUILTIN(__builtin_ia32_undef256, "V4d", "nc", "")
TARGET_BUILTIN(__builtin_ia32_undef512, "V8d", "nc", "")
+// FLAGS
+//
+TARGET_BUILTIN(__builtin_ia32_readeflags_u32, "Ui", "n", "")
+TARGET_BUILTIN(__builtin_ia32_readeflags_u64, "ULLi", "n", "")
+TARGET_BUILTIN(__builtin_ia32_writeeflags_u32, "vUi", "n", "")
+TARGET_BUILTIN(__builtin_ia32_writeeflags_u64, "vULLi", "n", "")
+
// 3DNow!
//
TARGET_BUILTIN(__builtin_ia32_femms, "v", "", "3dnow")
Modified: cfe/trunk/lib/Headers/ia32intrin.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/ia32intrin.h?rev=256686&r1=256685&r2=256686&view=diff
==
--- cfe/trunk/lib/Headers/ia32intrin.h (original)
+++ cfe/trunk/lib/Headers/ia32intrin.h Fri Jan 1 00:50:08 2016
@@ -32,50 +32,26 @@
static __inline__ unsigned long long __attribute__((__always_inline__,
__nodebug__))
__readeflags(void)
{
- unsigned long long __res = 0;
- __asm__ __volatile__ ("pushf\n\t"
-"popq %0\n"
-:"=r"(__res)
-:
-:
- );
- return __res;
+ return __builtin_ia32_readeflags_u64();
}
static __inline__ void __attribute__((__always_inline__, __nodebug__))
__writeeflags(unsigned long long __f)
{
- __asm__ __volatile__ ("pushq %0\n\t"
-"popf\n"
-:
-:"r"(__f)
-:"flags"
- );
+ __builtin_ia32_writeeflags_u64(__f);
}
#else /* !__x86_64__ */
static __inline__ unsigned int __attribute__((__always_inline__, __nodebug__))
__readeflags(void)
{
- unsigned int __res = 0;
- __asm__ __volatile__ ("pushf\n\t"
-"popl %0\n"
-:"=r"(__res)
-:
-:
- );
- return __res;
+ return __builtin_ia32_readeflags_u32();
}
static __inline__ void __attribute__((__always_inline__, __nodebug__))
__writeeflags(unsigned int __f)
{
- __asm__ __volatile__ ("pushl %0\n\t"
-"popf\n"
-:
-:"r"(__f)
-:"flags"
- );
+ __builtin_ia32_writeeflags_u32(__f);
}
#endif /* !__x86_64__ */
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
