[PATCH] D46439: Fix incorrect packed aligned structure layout
chill updated this revision to Diff 145687.
https://reviews.llvm.org/D46439
Files:
lib/Sema/SemaDecl.cpp
test/Layout/itanium-pack-and-align.cpp
Index: test/Layout/itanium-pack-and-align.cpp
===
--- /dev/null
+++ test/Layout/itanium-pack-and-align.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm-only
-fdump-record-layouts %s \
+// RUN:| FileCheck %s
+
+struct S {
+ char x;
+ int y;
+} __attribute__((packed, aligned(8)));
+
+struct alignas(8) T {
+ char x;
+ int y;
+} __attribute__((packed));
+
+S s;
+T t;
+// CHECK: 0 | struct T
+// CHECK-NEXT: 0 | char x
+// CHECK-NEXT: 1 | int y
+// CHECK-NEXT:| [sizeof=8, dsize=8, align=8,
+// CHECK-NEXT:| nvsize=8, nvalign=8]
+
+// CHECK: 0 | struct S
+// CHECK-NEXT: 0 | char x
+// CHECK-NEXT: 1 | int y
+// CHECK-NEXT:| [sizeof=8, dsize=8, align=8,
+// CHECK-NETX:| nvsize=8, nvalign=8]
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -15573,6 +15573,10 @@
if (!Completed)
Record->completeDefinition();
+// Handle attributes before checking the layout.
+if (Attr)
+ ProcessDeclAttributeList(S, Record, Attr);
+
// We may have deferred checking for a deleted destructor. Check now.
if (CXXRecordDecl *CXXRecord = dyn_cast(Record)) {
auto *Dtor = CXXRecord->getDestructor();
@@ -15703,9 +15707,6 @@
CDecl->setIvarRBraceLoc(RBrac);
}
}
-
- if (Attr)
-ProcessDeclAttributeList(S, Record, Attr);
}
/// \brief Determine whether the given integral value is representable within
Index: test/Layout/itanium-pack-and-align.cpp
===
--- /dev/null
+++ test/Layout/itanium-pack-and-align.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm-only -fdump-record-layouts %s \
+// RUN:| FileCheck %s
+
+struct S {
+ char x;
+ int y;
+} __attribute__((packed, aligned(8)));
+
+struct alignas(8) T {
+ char x;
+ int y;
+} __attribute__((packed));
+
+S s;
+T t;
+// CHECK: 0 | struct T
+// CHECK-NEXT: 0 | char x
+// CHECK-NEXT: 1 | int y
+// CHECK-NEXT:| [sizeof=8, dsize=8, align=8,
+// CHECK-NEXT:| nvsize=8, nvalign=8]
+
+// CHECK: 0 | struct S
+// CHECK-NEXT: 0 | char x
+// CHECK-NEXT: 1 | int y
+// CHECK-NEXT:| [sizeof=8, dsize=8, align=8,
+// CHECK-NETX:| nvsize=8, nvalign=8]
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -15573,6 +15573,10 @@
if (!Completed)
Record->completeDefinition();
+// Handle attributes before checking the layout.
+if (Attr)
+ ProcessDeclAttributeList(S, Record, Attr);
+
// We may have deferred checking for a deleted destructor. Check now.
if (CXXRecordDecl *CXXRecord = dyn_cast(Record)) {
auto *Dtor = CXXRecord->getDestructor();
@@ -15703,9 +15707,6 @@
CDecl->setIvarRBraceLoc(RBrac);
}
}
-
- if (Attr)
-ProcessDeclAttributeList(S, Record, Attr);
}
/// \brief Determine whether the given integral value is representable within
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46439: Fix incorrect packed aligned structure layout
chill marked an inline comment as done.
chill added a comment.
Update: updated comment, added a test.
Comment at: lib/Sema/SemaDecl.cpp:15651
}
} else {
ObjCIvarDecl **ClsFields =
rsmith wrote:
> Do we need to do any attribute processing in this Objective-C interface /
> implementation / category case?
I think we didn't need it, or else `ProcessDeclAttributeList` would have been
called with a null `Record` argument.
https://reviews.llvm.org/D46439
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43778: [clang-tidy] Adding RestrictIncludes check to Fuchsia module
aaron.ballman added inline comments.
Comment at: clang-tidy/fuchsia/FuchsiaTidyModule.cpp:41
+CheckFactories.registerCheck(
+"fuchsia-restrict-includes");
CheckFactories.registerCheck(
I think this should be named `fuchsia-restrict-system-includes`.
Comment at: clang-tidy/fuchsia/RestrictIncludesCheck.cpp:60
+StringRef SearchPath, StringRef RelativePath, const Module *Imported) {
+ if (!llvm::is_contained(Check.getAllowedIncludes(), FileName) && IsAngled)
+// Bucket the allowed include directives by the id of the file they were
I'm not certain that `IsAngled` is sufficient. For instance, say the user
prohibits use of `vector`, nothing prevents the user from writing `#include
"vector"` to access the system header (assuming there is no user header with
the same name). e.g., https://godbolt.org/g/Scfv3U
Perhaps we could use `SourceManager::isInSystemHeader()` or extract some logic
from it to test whether the actual included file is in the system header search
path?
Comment at: docs/ReleaseNotes.rst:116
+
+ Checks for allowed includes and suggests removal of any others. If no
includes
+ are specified, the check will exit without issuing any warnings.
Be explicit that this only impacts system headers.
Comment at: docs/clang-tidy/checks/fuchsia-restrict-includes.rst:6-7
+
+Checks for allowed includes and suggests removal of any others. If no includes
+are specified, the check will exit without issuing any warnings.
+
Should also be explicit about system headers.
Comment at: test/clang-tidy/Inputs/fuchsia-restrict-includes/system/r.h:2
+void f() {}
\ No newline at end of file
Please add in the EOF.
Comment at:
test/clang-tidy/Inputs/fuchsia-restrict-includes/system/transitive.h:2
+#include
\ No newline at end of file
Please add in the EOF.
Comment at: test/clang-tidy/fuchsia-restrict-includes.cpp:31
+}
\ No newline at end of file
Please add in the EOF.
https://reviews.llvm.org/D43778
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46000: [AST] Added a helper to extract a user-friendly text of a comment.
ilya-biryukov updated this revision to Diff 145691.
ilya-biryukov added a comment.
- Fixed infinite loop with comments that contain doxygen commands
Repository:
rC Clang
https://reviews.llvm.org/D46000
Files:
include/clang/AST/CommentLexer.h
include/clang/AST/RawCommentList.h
lib/AST/CommentLexer.cpp
lib/AST/RawCommentList.cpp
unittests/AST/CMakeLists.txt
unittests/AST/CommentTextTest.cpp
Index: unittests/AST/CommentTextTest.cpp
===
--- /dev/null
+++ unittests/AST/CommentTextTest.cpp
@@ -0,0 +1,128 @@
+//===- unittest/AST/CommentTextTest.cpp - Comment text extraction test ===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// Tests for user-friendly output formatting of comments, i.e.
+// RawComment::getFormattedText().
+//
+//===--===//
+
+#include "clang/AST/RawCommentList.h"
+#include "clang/Basic/CommentOptions.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/FileSystemOptions.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/VirtualFileSystem.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include
+
+namespace clang {
+
+class CommentTextTest : public ::testing::Test {
+protected:
+ std::string formatComment(llvm::StringRef CommentText) {
+llvm::IntrusiveRefCntPtr EmptyFS(
+new vfs::InMemoryFileSystem);
+FileSystemOptions Opts;
+FileManager FileMgr(Opts, EmptyFS);
+
+DiagnosticsEngine Diags(new DiagnosticIDs, new DiagnosticOptions);
+SourceManager SourceMgr(Diags, FileMgr);
+
+auto File = SourceMgr.createFileID(
+llvm::MemoryBuffer::getMemBuffer(CommentText, "test-comment"));
+
+auto CommentStartOffset = CommentText.find("/");
+assert(CommentStartOffset != llvm::StringRef::npos);
+
+SourceRange CommentRange(
+SourceMgr.getLocForStartOfFile(File).getLocWithOffset(
+CommentStartOffset),
+SourceMgr.getLocForEndOfFile(File));
+CommentOptions EmptyOpts;
+// FIXME: technically, merged that we set here is incorrect, but that
+// shouldn't matter.
+RawComment Comment(SourceMgr, CommentRange, EmptyOpts, /*Merged=*/true);
+return Comment.getFormattedText(SourceMgr, Diags);
+ }
+};
+
+TEST_F(CommentTextTest, FormattedText) {
+ // clang-format off
+ auto ExpectedOutput =
+R"(This function does this and that.
+For example,
+ Runnning it in that case will give you
+ this result.
+That's about it.)";
+ // Two-slash comments.
+ EXPECT_EQ(ExpectedOutput, formatComment(
+R"cpp(
+// This function does this and that.
+// For example,
+//Runnning it in that case will give you
+//this result.
+// That's about it.)cpp"));
+
+ // Three-slash comments.
+ EXPECT_EQ(ExpectedOutput, formatComment(
+R"cpp(
+/// This function does this and that.
+/// For example,
+///Runnning it in that case will give you
+///this result.
+/// That's about it.)cpp"));
+
+ // Block comments.
+ EXPECT_EQ(ExpectedOutput, formatComment(
+R"cpp(
+/* This function does this and that.
+ * For example,
+ *Runnning it in that case will give you
+ *this result.
+ * That's about it.*/)cpp"));
+
+ // Doxygen-style block comments.
+ EXPECT_EQ(ExpectedOutput, formatComment(
+R"cpp(
+/** This function does this and that.
+ * For example,
+ *Runnning it in that case will give you
+ *this result.
+ * That's about it.*/)cpp"));
+
+ // Weird indentation.
+ EXPECT_EQ(ExpectedOutput, formatComment(
+R"cpp(
+ // This function does this and that.
+ // For example,
+ // Runnning it in that case will give you
+// this result.
+ // That's about it.)cpp"));
+ // clang-format on
+}
+
+TEST_F(CommentTextTest, KeepsDoxygenControlSeqs) {
+ // clang-format off
+ auto ExpectedOutput =
+R"(\brief This is the brief part of the comment.
+\param a something about a.
+@param b something about b.)";
+
+ EXPECT_EQ(ExpectedOutput, formatComment(
+R"cpp(
+/// \brief This is the brief part of the comment.
+/// \param a something about a.
+/// @param b something about b.)cpp"));
+ // clang-format on
+}
+
+} // namespace clang
Index: unittests/AST/CMakeLists.txt
===
--- unittests/AST/CMakeLists.txt
+++ unittests/AST/CMakeLists.txt
@@ -9,6 +9,7 @@
ASTVectorTest.cpp
CommentLexer.cpp
CommentParser.cpp
+ CommentTextTest.cpp
DataCollectionTest.cpp
DeclPrinterTest.cpp
DeclTest.cpp
Index: lib/AST/RawCommentList.cpp
===
--- lib/AST/RawCommentLi
[PATCH] D41537: Optionally add code completion results for arrow instead of dot
ilya-biryukov added inline comments. Comment at: include/clang-c/Index.h:5237 +/** + * \brief FixIts that *must* be applied before inserting the text for the + * corresponding completion item. Completion items with non-empty fixits will This seems too large for a brief comment :-) Maybe break with a newline after the first sentence. Comment at: include/clang/Sema/CodeCompleteConsumer.h:564 + + /// \brief For this completion result correction is required. + std::vector Corrections; yvvan wrote: > yvvan wrote: > > ilya-biryukov wrote: > > > Adding some docs here would be useful. These fix-its could be interpreted > > > too broadly at this point. > > > I suggest the following semantics and the comment: > > > > > > ``` > > > /// \brief FixIts that *must* be applied before inserting the text for > > > the corresponding completion item. > > > /// Completion items with non-empty fixits will not be returned by > > > default, they should be explicitly requested by setting > > > CompletionOptions::IncludeCorrections. > > > /// For the editors to be able to compute position of the cursor for the > > > completion item itself, the following conditions are guaranteed to hold > > > for RemoveRange of the stored fixits: > > > /// - Ranges in the fixits are guaranteed to never contain the > > > completion point (or identifier under completion point, if any) inside > > > them, except at the start or at the end of the range. > > > /// - If a fixit range starts or ends with completion point (or starts > > > or ends after the identifier under completion point), it will contain at > > > least one character. It allows to unambiguously recompute completion > > > point after applying the fixit. > > > /// The intuition is that provided fixits change code around the > > > identifier we complete, but are not allowed to touch the identifier > > > itself or the completion point. > > > /// One example of completion items with corrections are the ones > > > replacing '.' with '->' and vice versa: > > > /// std::unique_ptr> vec_ptr; > > > /// vec_ptr.^ // completion returns an item 'push_back', replacing > > > '.' with '->' > > > /// vec_ptr->^ // completion returns an item 'release', replacing > > > '->' with '.'let's put > > > ``` > > > > > > Do those invariants sound reasonable? Could we add asserts that they hold > > > when constructing the completion results? > > Thanks! I usually feel the lack of patience when writing descriptions :) > "Could we add asserts that they hold when constructing the completion results" > > The current way of creating them actually assures that by default. And to > check it once again it's required to change many things. > Starting with the fact that there is no SourceRange contains() methos or > something similar, > The current way of creating them actually assures that by default. And to > check it once again it's required to change many things. It's still nice to have a sanity check there, mostly for the sake of new checks that are gonna get added. But even the current one is tricky, since it may actually contain a range that ends exactly at the cursor (when completing right after the dot/arrow `foo->|`). > Starting with the fact that there is no SourceRange contains() methos or > something similar, Moreover, `SourceRange` doesn't have clear semantics AFAIK. It can either be a half-open or closed range. Let's add a FIXME, at least, that we should add those asserts later. Comment at: include/clang/Sema/CodeCompleteConsumer.h:564 + + /// \brief FixIts that *must* be applied before inserting the text for the + /// corresponding completion item. Completion items with non-empty fixits will I suggest moving this doc comment to `getFixits` method, as it is the public interface. Comment at: include/clang/Sema/CodeCompleteConsumer.h:592 + StringRef ParentName, const char *BriefComment, + std::vector FixIts); ~CodeCompletionString() = default; We can't store fixits in `CodeCompletionString`, it should not contain source locatins, which are only valid for the lifetime of SourceManager. Note that CCS has a lifetime independent of both the SourceManager and the AST. Storing them in CodeCompletionResult should be good enough for all our use-cases. Comment at: include/clang/Sema/CodeCompleteConsumer.h:814 + /// \brief FixIts that *must* be applied before inserting the text for the + /// corresponding completion item. Completion items with non-empty fixits will We shouldn't duplicate such a large comment in too many places, it would be impossible to keep it in sync. I would suggest only keeping it in `CodeCompletionResult` and add a reference to it in other places. libclang is a bit tricky, though. It is distributed without other LLV
[PATCH] D46540: [X86] ptwrite intrinsic
craig.topper accepted this revision. craig.topper added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D46540 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46541: [CodeGen] Improve diagnostics related to target attributes
craig.topper added inline comments. Comment at: lib/CodeGen/CodeGenFunction.cpp:2346 + return Feat.substr(1) == F.getKey(); + })) +ReqFeatures.insert(ReqFeatures.begin(), F.getKey()); This and the next line are indented funny. https://reviews.llvm.org/D46541 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers
smeenai added a comment. In https://reviews.llvm.org/D42933#1091234, @aaron.ballman wrote: > In https://reviews.llvm.org/D42933#1090268, @jfb wrote: > > > I was just looking at this, and I think @arphaman's patch is pretty much > > the right approach (with 2 suggested fixes below). > > > > I don't think the code we're currently warning on is broken: a user code > > has `NSInteger` with `%zd` or `NSUInteger` with `%zu`, and on all platforms > > which support those types the implementor has guaranteed that > > `(sizeof(size_t) == sizeof(NSInteger)) && (sizeof(ssize_t) == > > sizeof(NSUInteger))`. > > > Yes, but is this guaranteed to be the case or does it happen to be the case? > I'm worried about the less mainstream uses where you might find ObjC code > where this does not hold but -Wformat points out a portability concern. Also keep in mind that Obj-C might be used on non-Apple platforms, e.g. there's an active review going on for GNUstep ABI v2 right now (https://reviews.llvm.org/D46052), and WinObjC is also a thing. Those platforms might not necessarily provide the same guarantees or consistency that Apple's do. >> I agree that, if we're playing C++ pedant and look at the typedefs, then >> it's undefined behavior and the code is broken. > > This is reason alone to diagnose the code, because the optimizer is welcome > to use that UB to perform transformations the programmer did not expect. > However, I'm not certain our optimizer is currently paying attention to that > UB directly, so this may not be an immediate issue (but could be a pitfall > for the future) but I'm not certain about other optimizers for which > portability would still be a concern. I would honestly find it a bit surprising (and scary) if the optimizer actually took advantage of UB in the case where the size and alignment of the specifier and the actual type matches. Repository: rC Clang https://reviews.llvm.org/D42933 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46475: [HIP] Set proper triple and offload kind for the toolchain
yaxunl updated this revision to Diff 145699.
yaxunl marked an inline comment as done.
yaxunl added a comment.
Revised by John's comments.
https://reviews.llvm.org/D46475
Files:
include/clang/Basic/DiagnosticDriverKinds.td
include/clang/Driver/Options.td
include/clang/Driver/Types.h
lib/Driver/Driver.cpp
lib/Driver/Types.cpp
test/Driver/Inputs/hip_multiple_inputs/a.cu
test/Driver/Inputs/hip_multiple_inputs/b.hip
test/Driver/hip-inputs.hip
Index: test/Driver/hip-inputs.hip
===
--- /dev/null
+++ test/Driver/hip-inputs.hip
@@ -0,0 +1,23 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang -ccc-print-phases -target x86_64-linux-gnu \
+// RUN: -x hip --cuda-gpu-arch=gfx803 -c \
+// RUN: %S/Inputs/hip_multiple_inputs/a.cu \
+// RUN: %S/Inputs/hip_multiple_inputs/b.hip 2>&1 \
+// RUN: | FileCheck %s
+
+// RUN: not %clang -ccc-print-phases -target x86_64-linux-gnu \
+// RUN: --cuda-gpu-arch=gfx803 -c \
+// RUN: %S/Inputs/hip_multiple_inputs/a.cu \
+// RUN: %S/Inputs/hip_multiple_inputs/b.hip 2>&1 \
+// RUN: | FileCheck -check-prefix=MIX %s
+
+// RUN: not %clang -ccc-print-phases -target x86_64-linux-gnu \
+// RUN: --cuda-gpu-arch=gfx803 -c \
+// RUN: --hip-link %S/Inputs/hip_multiple_inputs/a.cu 2>&1 \
+// RUN: | FileCheck -check-prefix=MIX %s
+
+// CHECK-NOT: error: Mixed Cuda and HIP compilation is not supported.
+// MIX: error: Mixed Cuda and HIP compilation is not supported.
Index: lib/Driver/Types.cpp
===
--- lib/Driver/Types.cpp
+++ lib/Driver/Types.cpp
@@ -172,6 +172,15 @@
case TY_CUDA:
case TY_PP_CUDA:
case TY_CUDA_DEVICE:
+return true;
+ }
+}
+
+bool types::isHIP(ID Id) {
+ switch (Id) {
+ default:
+return false;
+
case TY_HIP:
case TY_PP_HIP:
case TY_HIP_DEVICE:
@@ -230,6 +239,7 @@
.Case("fpp", TY_Fortran)
.Case("FPP", TY_Fortran)
.Case("gch", TY_PCH)
+ .Case("hip", TY_HIP)
.Case("hpp", TY_CXXHeader)
.Case("iim", TY_PP_CXXModule)
.Case("lib", TY_Object)
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -538,24 +538,46 @@
InputList &Inputs) {
//
- // CUDA
+ // CUDA/HIP
//
- // We need to generate a CUDA toolchain if any of the inputs has a CUDA type.
- if (llvm::any_of(Inputs, [](std::pair &I) {
+ // We need to generate a CUDA toolchain if any of the inputs has a CUDA
+ // or HIP type. However, mixed CUDA/HIP compilation is not supported.
+ bool IsCuda =
+ llvm::any_of(Inputs, [](std::pair &I) {
return types::isCuda(I.first);
- })) {
+ });
+ bool IsHIP =
+ llvm::any_of(Inputs,
+ [](std::pair &I) {
+ return types::isHIP(I.first);
+ }) ||
+ C.getInputArgs().hasArg(options::OPT_hip_link);
+ if (IsCuda && IsHIP) {
+Diag(clang::diag::err_drv_mix_cuda_hip);
+return;
+ }
+ if (IsCuda || IsHIP) {
const ToolChain *HostTC = C.getSingleOffloadToolChain();
const llvm::Triple &HostTriple = HostTC->getTriple();
-llvm::Triple CudaTriple(HostTriple.isArch64Bit() ? "nvptx64-nvidia-cuda"
- : "nvptx-nvidia-cuda");
-// Use the CUDA and host triples as the key into the ToolChains map, because
-// the device toolchain we create depends on both.
+StringRef DeviceTripleStr;
+auto OFK = IsHIP ? Action::OFK_HIP : Action::OFK_Cuda;
+if (IsHIP) {
+ // HIP is only supported on amdgcn.
+ DeviceTripleStr = "amdgcn-amd-amdhsa";
+} else {
+ // CUDA is only supported on nvptx.
+ DeviceTripleStr = HostTriple.isArch64Bit() ? "nvptx64-nvidia-cuda"
+ : "nvptx-nvidia-cuda";
+}
+llvm::Triple CudaTriple(DeviceTripleStr);
+// Use the CUDA/HIP and host triples as the key into the ToolChains map,
+// because the device toolchain we create depends on both.
auto &CudaTC = ToolChains[CudaTriple.str() + "/" + HostTriple.str()];
if (!CudaTC) {
CudaTC = llvm::make_unique(
- *this, CudaTriple, *HostTC, C.getInputArgs(), Action::OFK_Cuda);
+ *this, CudaTriple, *HostTC, C.getInputArgs(), OFK);
}
-C.addOffloadDeviceToolChain(CudaTC.get(), Action::OFK_Cuda);
+C.addOffloadDeviceToolChain(CudaTC.get(), OFK);
}
//
Index: include/clang/Driver/Types.h
===
--- include/clang/Driver/Types.h
+++ include/clang/Driver/Types.h
@@ -77,6 +77,9 @@
/// isCuda - Is this a CUDA input.
bool isCuda(ID Id);
+ /// isHIP - Is this a HIP input.
+ bool isHIP(ID Id);
+
/// isObjC - Is
[PATCH] D45093: [AST] Fix -ast-print for _Bool when have diagnostics
jdenny updated this revision to Diff 145711.
jdenny edited the summary of this revision.
jdenny added a comment.
Made the suggested changes.
https://reviews.llvm.org/D45093
Files:
include/clang/Sema/Sema.h
lib/Frontend/ASTConsumers.cpp
lib/Sema/Sema.cpp
test/Misc/ast-print-bool.c
Index: test/Misc/ast-print-bool.c
===
--- /dev/null
+++ test/Misc/ast-print-bool.c
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 -verify -ast-print %s -xc -DDEF_BOOL_CBOOL \
+// RUN: | FileCheck %s --check-prefixes=BOOL-AS-CBOOL,CBOOL
+//
+// RUN: %clang_cc1 -verify -ast-print %s -xc -DDEF_BOOL_CBOOL -DDIAG \
+// RUN: | FileCheck %s --check-prefixes=BOOL-AS-CBOOL,CBOOL
+//
+// RUN: %clang_cc1 -verify -ast-print %s -xc -DDEF_BOOL_INT \
+// RUN: | FileCheck %s --check-prefixes=BOOL-AS-INT,CBOOL
+//
+// RUN: %clang_cc1 -verify -ast-print %s -xc -DDEF_BOOL_INT -DDIAG \
+// RUN: | FileCheck %s --check-prefixes=BOOL-AS-INT,CBOOL
+//
+// RUN: %clang_cc1 -verify -ast-print %s -xc++ \
+// RUN: | FileCheck %s --check-prefixes=BOOL-AS-BOOL
+//
+// RUN: %clang_cc1 -verify -ast-print %s -xc++ -DDIAG \
+// RUN: | FileCheck %s --check-prefixes=BOOL-AS-BOOL
+
+#if DEF_BOOL_CBOOL
+# define bool _Bool
+#elif DEF_BOOL_INT
+# define bool int
+#endif
+
+// BOOL-AS-CBOOL: _Bool i;
+// BOOL-AS-INT: int i;
+// BOOL-AS-BOOL: bool i;
+bool i;
+
+#ifndef __cplusplus
+// CBOOL: _Bool j;
+_Bool j;
+#endif
+
+// Induce a diagnostic (and verify we actually managed to do so), which used to
+// permanently alter the -ast-print printing policy for _Bool. How bool is
+// defined by the preprocessor is examined only once per compilation, when the
+// diagnostic is emitted, and it used to affect the entirety of -ast-print, so
+// test only one definition of bool per compilation.
+#if DIAG
+void fn() { 1; } // expected-warning {{expression result unused}}
+#else
+// expected-no-diagnostics
+#endif
Index: lib/Sema/Sema.cpp
===
--- lib/Sema/Sema.cpp
+++ lib/Sema/Sema.cpp
@@ -52,8 +52,8 @@
PrintingPolicy Sema::getPrintingPolicy(const ASTContext &Context,
const Preprocessor &PP) {
PrintingPolicy Policy = Context.getPrintingPolicy();
- // Our printing policy is copied over the ASTContext printing policy whenever
- // a diagnostic is emitted, so recompute it.
+ // In diagnostics, we print _Bool as bool if the latter is defined as the
+ // former.
Policy.Bool = Context.getLangOpts().Bool;
if (!Policy.Bool) {
if (const MacroInfo *BoolMacro = PP.getMacroInfo(Context.getBoolName())) {
@@ -1284,7 +1284,8 @@
}
}
- // Set up the context's printing policy based on our current state.
+ // Copy the diagnostic printing policy over the ASTContext printing policy.
+ // TODO: Stop doing that. See: https://reviews.llvm.org/D45093#1090292
Context.setPrintingPolicy(getPrintingPolicy());
// Emit the diagnostic.
Index: lib/Frontend/ASTConsumers.cpp
===
--- lib/Frontend/ASTConsumers.cpp
+++ lib/Frontend/ASTConsumers.cpp
@@ -87,9 +87,10 @@
<< DC->getPrimaryContext() << "\n";
} else
Out << "Not a DeclContext\n";
- } else if (OutputKind == Print)
-D->print(Out, /*Indentation=*/0, /*PrintInstantiation=*/true);
- else if (OutputKind != None)
+ } else if (OutputKind == Print) {
+PrintingPolicy Policy(D->getASTContext().getLangOpts());
+D->print(Out, Policy, /*Indentation=*/0, /*PrintInstantiation=*/true);
+ } else if (OutputKind != None)
D->dump(Out, OutputKind == DumpFull);
}
Index: include/clang/Sema/Sema.h
===
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -2111,12 +2111,12 @@
void checkPartialSpecializationVisibility(SourceLocation Loc,
NamedDecl *Spec);
- /// \brief Retrieve a suitable printing policy.
+ /// \brief Retrieve a suitable printing policy for diagnostics.
PrintingPolicy getPrintingPolicy() const {
return getPrintingPolicy(Context, PP);
}
- /// \brief Retrieve a suitable printing policy.
+ /// \brief Retrieve a suitable printing policy for diagnostics.
static PrintingPolicy getPrintingPolicy(const ASTContext &Ctx,
const Preprocessor &PP);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers
jfb added a comment. In https://reviews.llvm.org/D42933#1091234, @aaron.ballman wrote: > In https://reviews.llvm.org/D42933#1090268, @jfb wrote: > > > I was just looking at this, and I think @arphaman's patch is pretty much > > the right approach (with 2 suggested fixes below). > > > > I don't think the code we're currently warning on is broken: a user code > > has `NSInteger` with `%zd` or `NSUInteger` with `%zu`, and on all platforms > > which support those types the implementor has guaranteed that > > `(sizeof(size_t) == sizeof(NSInteger)) && (sizeof(ssize_t) == > > sizeof(NSUInteger))`. > > > Yes, but is this guaranteed to be the case or does it happen to be the case? > I'm worried about the less mainstream uses where you might find ObjC code > where this does not hold but -Wformat points out a portability concern. For Darwin platform, yes. That's why this diagnostic should only be squelched for Darwin platforms. >> However the implementation provided more guarantees than C++ does through >> its platform typedefs so pendantry need not apply (or rather: clang should >> look no further than the typedef, and trust the platform in this particular >> case). >> >> We of course should still warn on sign mismatch. >> >> I don't think this should even be a warning with pedantic on: there's no >> portability issue at all because all OSes and architectures where this could >> ever fire are guaranteed to do the right thing. > > Can you point me to this guarantee, as I was unable to find one (but that > doesn't mean it doesn't exist)? Once this patch is committed I'll update the corresponding documentation on developer.apple.com In https://reviews.llvm.org/D42933#1091411, @smeenai wrote: > In https://reviews.llvm.org/D42933#1091234, @aaron.ballman wrote: > > > In https://reviews.llvm.org/D42933#1090268, @jfb wrote: > > > > > I was just looking at this, and I think @arphaman's patch is pretty much > > > the right approach (with 2 suggested fixes below). > > > > > > I don't think the code we're currently warning on is broken: a user code > > > has `NSInteger` with `%zd` or `NSUInteger` with `%zu`, and on all > > > platforms which support those types the implementor has guaranteed that > > > `(sizeof(size_t) == sizeof(NSInteger)) && (sizeof(ssize_t) == > > > sizeof(NSUInteger))`. > > > > > > Yes, but is this guaranteed to be the case or does it happen to be the > > case? I'm worried about the less mainstream uses where you might find ObjC > > code where this does not hold but -Wformat points out a portability concern. > > > Also keep in mind that Obj-C might be used on non-Apple platforms, e.g. > there's an active review going on for GNUstep ABI v2 right now > (https://reviews.llvm.org/D46052), and WinObjC is also a thing. Those > platforms might not necessarily provide the same guarantees or consistency > that Apple's do. Indeed, I want to make sure that this change only affects Darwin platforms. >>> I agree that, if we're playing C++ pedant and look at the typedefs, then >>> it's undefined behavior and the code is broken. >> >> This is reason alone to diagnose the code, because the optimizer is welcome >> to use that UB to perform transformations the programmer did not expect. >> However, I'm not certain our optimizer is currently paying attention to that >> UB directly, so this may not be an immediate issue (but could be a pitfall >> for the future) but I'm not certain about other optimizers for which >> portability would still be a concern. > > I would honestly find it a bit surprising (and scary) if the optimizer > actually took advantage of UB in the case where the size and alignment of the > specifier and the actual type matches. Hear, hear! I'm happy to fix any such optimization out of their misguided optimism :-) Repository: rC Clang https://reviews.llvm.org/D42933 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44435: CUDA ctor/dtor Module-Unique Symbol Name
tra added a comment. Great! Let's close this review then. And good luck with cling. https://reviews.llvm.org/D44435 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46541: [CodeGen] Improve diagnostics related to target attributes
craig.topper added inline comments.
Comment at: lib/CodeGen/CodeGenFunction.cpp:2342
// Only positive features are "required".
- if (F.getValue())
+ if (F.getValue()) {
+if (std::any_of(ParsedAttr.Features.begin(), ParsedAttr.Features.end(),
Rather than walking the ParsedAttr.Features for each feature in the map. And
having to shift the ReqFeatures vectors sometimes. How about doing this
-Walk through all features in ParsedAttr, for each feature with a +, query the
callee feature map. If it's enabled there, push it to ReqFeatures.
-Walk through all features in the callee feature map and if enabled push those.
This will lead to duplicates in the list, but all the explicitly mentioned
features will be listed first.
https://reviews.llvm.org/D46541
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46320: Remove \brief commands from doxygen comments.
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm! (Back from a week long vacation) https://reviews.llvm.org/D46320 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46540: [X86] ptwrite intrinsic
Hahnfeld added a comment. Could you maybe add some short summaries to your patches? It's hard for non-Intel employees to guess what all these instructions do... https://reviews.llvm.org/D46540 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46241: [CodeGen] Recognize more cases of zero initialization
sepavloff updated this revision to Diff 145735.
sepavloff added a comment.
Added treatment of structures/unions
Repository:
rC Clang
https://reviews.llvm.org/D46241
Files:
include/clang/AST/Expr.h
lib/AST/ExprConstant.cpp
lib/CodeGen/CGExprConstant.cpp
test/CodeGen/const-init.c
test/CodeGen/designated-initializers.c
test/CodeGen/union-init2.c
test/CodeGenCXX/cxx11-initializer-aggregate.cpp
test/CodeGenCXX/cxx1z-initializer-aggregate.cpp
test/SemaCXX/large-array-init.cpp
Index: test/SemaCXX/large-array-init.cpp
===
--- test/SemaCXX/large-array-init.cpp
+++ /dev/null
@@ -1,10 +0,0 @@
-// RUN: %clang_cc1 -S -o %t.ll -mllvm -debug-only=exprconstant %s 2>&1 | \
-// RUN: FileCheck %s
-// REQUIRES: asserts
-
-struct S { int i; };
-
-static struct S arr[1] = {{ 0 }};
-// CHECK: The number of elements to initialize: 1.
-
-struct S *foo() { return arr; }
Index: test/CodeGenCXX/cxx1z-initializer-aggregate.cpp
===
--- test/CodeGenCXX/cxx1z-initializer-aggregate.cpp
+++ test/CodeGenCXX/cxx1z-initializer-aggregate.cpp
@@ -17,14 +17,14 @@
C c1 = {};
C c2 = {1};
- // CHECK: @_ZN8Constant2c1E = global { i8 } zeroinitializer, align 1
+ // CHECK: @_ZN8Constant2c1E = global %"struct.Constant::C" zeroinitializer, align 1
// CHECK: @_ZN8Constant2c2E = global { i8 } { i8 1 }, align 1
// Test packing bases into tail padding.
D d1 = {};
D d2 = {1, 2, 3};
D d3 = {1};
- // CHECK: @_ZN8Constant2d1E = global { i32, i8, i8 } zeroinitializer, align 4
+ // CHECK: @_ZN8Constant2d1E = global %"struct.Constant::D" zeroinitializer, align 4
// CHECK: @_ZN8Constant2d2E = global { i32, i8, i8 } { i32 1, i8 2, i8 3 }, align 4
// CHECK: @_ZN8Constant2d3E = global { i32, i8, i8 } { i32 1, i8 0, i8 0 }, align 4
Index: test/CodeGenCXX/cxx11-initializer-aggregate.cpp
===
--- test/CodeGenCXX/cxx11-initializer-aggregate.cpp
+++ test/CodeGenCXX/cxx11-initializer-aggregate.cpp
@@ -51,3 +51,30 @@
// meaningful.
B b[30] = {};
}
+
+namespace ZeroInit {
+ enum { Zero, One };
+ constexpr int zero() { return 0; }
+ constexpr int *null() { return nullptr; }
+ struct Filler {
+int x;
+Filler();
+ };
+ struct S1 {
+int x;
+ };
+
+ // These declarations, if implemented elementwise, require huge
+ // amout of memory and compiler time.
+ unsigned char data_1[1024 * 1024 * 1024 * 2u] = { 0 };
+ unsigned char data_2[1024 * 1024 * 1024 * 2u] = { Zero };
+ unsigned char data_3[1024][1024][1024] = {{{0}}};
+ unsigned char data_4[1024 * 1024 * 1024 * 2u] = { zero() };
+ int *data_5[1024 * 1024 * 512] = { nullptr };
+ int *data_6[1024 * 1024 * 512] = { null() };
+ struct S1 data_7[1024 * 1024 * 512] = {{0}};
+
+ // This variable must be initialized elementwise.
+ Filler data_e1[1024] = {};
+ // CHECK: getelementptr inbounds {{.*}} @_ZN8ZeroInit7data_e1E
+}
Index: test/CodeGen/union-init2.c
===
--- test/CodeGen/union-init2.c
+++ test/CodeGen/union-init2.c
@@ -5,7 +5,7 @@
union x {long long b;union x* a;} r = {.a = &r};
-// CHECK: global { [3 x i8], [5 x i8] } { [3 x i8] zeroinitializer, [5 x i8] undef }
+// CHECK: global %union.z zeroinitializer
union z {
char a[3];
long long b;
Index: test/CodeGen/designated-initializers.c
===
--- test/CodeGen/designated-initializers.c
+++ test/CodeGen/designated-initializers.c
@@ -8,7 +8,7 @@
// CHECK: @u = global %union.anon zeroinitializer
union { int i; float f; } u = { };
-// CHECK: @u2 = global { i32, [4 x i8] } { i32 0, [4 x i8] undef }
+// CHECK: @u2 = global %union.anon.0 zeroinitializer
union { int i; double f; } u2 = { };
// CHECK: @u3 = global %union.anon.1 zeroinitializer
Index: test/CodeGen/const-init.c
===
--- test/CodeGen/const-init.c
+++ test/CodeGen/const-init.c
@@ -167,7 +167,7 @@
int : 1;
int x;
} a = {};
- // CHECK: @g30.a = internal global %struct.anon.1 <{ i8 undef, i32 0 }>, align 1
+ // CHECK: @g30.a = internal global %struct.anon.1 zeroinitializer, align 1
#pragma pack()
}
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -1392,20 +1392,37 @@
return type;
}
+/// Checks if the specified initializer is equivalent to zero initialization.
+static bool isZeroInitializer(ConstantEmitter &CE, const Expr *Init) {
+ if (auto *E = dyn_cast_or_null(Init)) {
+CXXConstructorDecl *CD = E->getConstructor();
+return CD->isDefaultConstructor() && CD->isTrivial();
+ }
+
+ if (auto *IL = dyn_cast_or_null(Init)) {
+for (auto I : IL->inits())
+ if (!isZero
[PATCH] D46241: [CodeGen] Recognize more cases of zero initialization
sepavloff added a comment.
> Hmm. In C++, a static object which isn't constant-initialized is
> zero-initialized, which is required to set any padding bits to zero (N4640
> [dcl.init]p6) in both structs and unions. In C, a static object which doesn't
> have an initializer also has all any padding bits set to zero (N1548
> 6.7.9p10). Now, this object has an initializer (that acts as a
> constant-initializer in C++), so those rules don't directly apply; but I
> would argue that the intent of the standards is not that padding bits become
> undefined when you happen to have an initializer. So I actually think the
> zeroinitializer emission is more correct.
Using undefined values instead of zero initialization was implemented in
https://reviews.llvm.org/rL101535. There is no much info about the reason of
the implementation. Clang uses undefined values for padding bits, in particular
in unions, when the first member is not widest. The code:
union C1 {
char sel;
double dval;
};
union C1 val_1 = { 0 };
produces:
@val_1 = dso_local global { i8, [7 x i8] } { i8 0, [7 x i8] undef }, align 8
Another case is unnamed bit fields.
struct C2 {
int : 4;
int x;
};
struct C2 val_2 = { 0 };
produces:
@val_2 = dso_local global { [4 x i8], i32 } { [4 x i8] undef, i32 0 }, align 4
Strictly speaking, this IR does not mean violation of the standard, but it can
modify code generation in some cases. If we decided to use zeroinitializer in
this case, we probably would need to revise using undefined values in
initializers, otherwise similar declarations like:
union C1 val_1a = { 0 };
union C1 val_1b = { 1 };
would produce different IR representations, with and without undefined values.
The test `SemaCXX/large-array-init.cpp` is removed in this change. This test
was added in https://reviews.llvm.org/rL325120 to solve
https://bugs.llvm.org/show_bug.cgi?id=18978, which describes the same problem,
as solved by this patch. This patch presents more efficient solution, with it
the tests compiles 50 time faster. If r325120 does not solve additional
problems, it can be reverted.
Repository:
rC Clang
https://reviews.llvm.org/D46241
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r331786 - [lit] Fix running tests that require 'examples'.
Author: zturner
Date: Tue May 8 11:20:10 2018
New Revision: 331786
URL: http://llvm.org/viewvc/llvm-project?rev=331786&view=rev
Log:
[lit] Fix running tests that require 'examples'.
Differential Revision: https://reviews.llvm.org/D46514
Patch by Nikolai Kosjar.
Modified:
cfe/trunk/test/lit.cfg.py
Modified: cfe/trunk/test/lit.cfg.py
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/lit.cfg.py?rev=331786&r1=331785&r2=331786&view=diff
==
--- cfe/trunk/test/lit.cfg.py (original)
+++ cfe/trunk/test/lit.cfg.py Tue May 8 11:20:10 2018
@@ -63,6 +63,7 @@ tools = [
]
if config.clang_examples:
+config.available_features.add('examples')
tools.append('clang-interpreter')
llvm_config.add_tool_substitutions(tools, tool_dirs)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46593: Allow copy elision in path concatenation
xbolva00 updated this revision to Diff 145740.
Herald added a subscriber: cfe-commits.
Repository:
rCXX libc++
https://reviews.llvm.org/D46593
Files:
include/experimental/filesystem
Index: include/experimental/filesystem
===
--- include/experimental/filesystem
+++ include/experimental/filesystem
@@ -1140,7 +1140,9 @@
inline _LIBCPP_INLINE_VISIBILITY
path operator/(const path& __lhs, const path& __rhs) {
-return path(__lhs) /= __rhs;
+path __result(__lhs);
+__result /= __rhs;
+return __result;
}
template
Index: include/experimental/filesystem
===
--- include/experimental/filesystem
+++ include/experimental/filesystem
@@ -1140,7 +1140,9 @@
inline _LIBCPP_INLINE_VISIBILITY
path operator/(const path& __lhs, const path& __rhs) {
-return path(__lhs) /= __rhs;
+path __result(__lhs);
+__result /= __rhs;
+return __result;
}
template
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46593: Allow copy elision in path concatenation
xbolva00 updated this revision to Diff 145742.
Herald added a subscriber: christof.
https://reviews.llvm.org/D46593
Files:
libcxx/trunk/include/experimental/filesystem
Index: libcxx/trunk/include/experimental/filesystem
===
--- libcxx/trunk/include/experimental/filesystem
+++ libcxx/trunk/include/experimental/filesystem
@@ -1140,7 +1140,9 @@
inline _LIBCPP_INLINE_VISIBILITY
path operator/(const path& __lhs, const path& __rhs) {
-return path(__lhs) /= __rhs;
+path __result(__lhs);
+__result /= __rhs;
+return __result;
}
template
Index: libcxx/trunk/include/experimental/filesystem
===
--- libcxx/trunk/include/experimental/filesystem
+++ libcxx/trunk/include/experimental/filesystem
@@ -1140,7 +1140,9 @@
inline _LIBCPP_INLINE_VISIBILITY
path operator/(const path& __lhs, const path& __rhs) {
-return path(__lhs) /= __rhs;
+path __result(__lhs);
+__result /= __rhs;
+return __result;
}
template
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46593: Allow copy elision in path concatenation
xbolva00 updated this revision to Diff 145745.
xbolva00 added a comment.
Can anybody give me advice from where to generate patch? I do it from libcxx.
https://reviews.llvm.org/D46593
Files:
include/experimental/filesystem
Index: include/experimental/filesystem
===
--- include/experimental/filesystem
+++ include/experimental/filesystem
@@ -1140,7 +1140,9 @@
inline _LIBCPP_INLINE_VISIBILITY
path operator/(const path& __lhs, const path& __rhs) {
-return path(__lhs) /= __rhs;
+path __result(__lhs);
+__result /= __rhs;
+return __result;
}
template
Index: include/experimental/filesystem
===
--- include/experimental/filesystem
+++ include/experimental/filesystem
@@ -1140,7 +1140,9 @@
inline _LIBCPP_INLINE_VISIBILITY
path operator/(const path& __lhs, const path& __rhs) {
-return path(__lhs) /= __rhs;
+path __result(__lhs);
+__result /= __rhs;
+return __result;
}
template
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46593: Allow copy elision in path concatenation
smeenai added a comment. Generating the patch from libc++ is fine (and this patch looks like it has sane paths). https://reviews.llvm.org/D46593 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46593: Allow copy elision in path concatenation
xbolva00 added a comment. In https://reviews.llvm.org/D46593#1091732, @smeenai wrote: > Generating the patch from libc++ is fine (and this patch looks like it has > sane paths). Thank you https://reviews.llvm.org/D46593 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46471: [HIP] Add hip offload kind
tra accepted this revision. tra added a comment. Small nit. LGTM otherwise. Comment at: lib/Driver/ToolChains/Clang.cpp:133-135 Work(*C.getSingleOffloadToolChain()); + if (JA.isHostOffloading(Action::OFK_HIP)) CUDA and HIP are mutually exclusive, so this should probably be `else if` https://reviews.llvm.org/D46471 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46535: Correct warning on Float->Integer conversions.
efriedma added a comment.
The check for whether an input is "out of range" doesn't handle fractions
correctly. Testcase:
int a() { return 2147483647.5; }
unsigned b() { return -.5; }
Repository:
rC Clang
https://reviews.llvm.org/D46535
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers
jyknight added a comment.
In https://reviews.llvm.org/D42933#1090384, @jfb wrote:
> In https://reviews.llvm.org/D42933#1090286, @smeenai wrote:
>
> > I'd be fine with adding an option to relax the printf checking if the size
> > and alignment of the specifier and the actual type match, even if the types
> > themselves differ (`-Wformat-relaxed` or something similar), so that you'd
> > still get warnings on cases where the specifier mismatch could cause
> > runtime issues.
>
>
> What are the cases that you're worried about? The only ones I'm trying to
> capture here are `NSInteger` with `%zd` and `NSUInteger` with `%zu`, are
> there others?
>
> > I think that would be preferable to special-casing the Apple types.
>
> If there are more that should be captured and a similar point solution
> doesn't apply, agreed. However I'd like to understand if we agree on the
> guarantees that the platform offers for the two specific cases I'm targeting.
I also think that special casing these two specifiers doesn't make sense. The
problem is a general issue -- and one I've often found irritating. This exact
same situation comes up all the time in non-Darwin contexts too.
E.g. one I find particularly annoying is "%lld" vs "%ld" in printf.
Some 64-bit platforms define e.g. `int64_t` as `long long`, and others as
`long`. Although both types are size 8, align 8, and mean exactly the same
thing, they are still distinct. And so, users write code like `int64_t x = 0;
printf("%ld", x);`...which then emits warnings on the platform that defines
int64_t as a `long long`. Obviously, the code _could_ be using `PRId64`
instead...but it often doesn't. So it'd sure be nice if you could restrict the
warning to only warn about actual problems, and suppress these
superficially-incompatible-but-not-really warnings.
So, +1 from me for the general-purpose `-Wformat-relaxed` flag (or whatever
other flag name is appropriate).
Repository:
rC Clang
https://reviews.llvm.org/D42933
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46349: [X86] Mark builtins 'const' where possible
craig.topper added a comment. Ping https://reviews.llvm.org/D46349 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers
jfb added a comment. In https://reviews.llvm.org/D42933#1091809, @jyknight wrote: > I also think that special casing these two specifiers doesn't make sense. The > problem is a general issue -- and one I've often found irritating. This exact > same situation comes up all the time in non-Darwin contexts too. I don't think that's true. In this very specific case the platform guarantees that `(sizeof(size_t) == sizeof(NSInteger)) && (sizeof(ssize_t) == sizeof(NSUInteger))` for all architectures this platform supports. This exact same situation does not come up all the time in other contexts because the `int` / `long` / `long long` distinction isn't backed by a portability guarantee. The warning is there to say "this code isn't portable!", but in the very specific case of `NSInteger` and `NSUInteger` it is and users rely on it so it cannot be broken. The warning is therefore spurious, users therefore rightly complain. Repository: rC Clang https://reviews.llvm.org/D42933 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers
aaron.ballman added a reviewer: rjmccall. aaron.ballman added a subscriber: rjmccall. aaron.ballman added a comment. In https://reviews.llvm.org/D42933#1091502, @jfb wrote: > In https://reviews.llvm.org/D42933#1091234, @aaron.ballman wrote: > > > In https://reviews.llvm.org/D42933#1090268, @jfb wrote: > > > > > I was just looking at this, and I think @arphaman's patch is pretty much > > > the right approach (with 2 suggested fixes below). > > > > > > I don't think the code we're currently warning on is broken: a user code > > > has `NSInteger` with `%zd` or `NSUInteger` with `%zu`, and on all > > > platforms which support those types the implementor has guaranteed that > > > `(sizeof(size_t) == sizeof(NSInteger)) && (sizeof(ssize_t) == > > > sizeof(NSUInteger))`. > > > > > > Yes, but is this guaranteed to be the case or does it happen to be the > > case? I'm worried about the less mainstream uses where you might find ObjC > > code where this does not hold but -Wformat points out a portability concern. > > > For Darwin platform, yes. That's why this diagnostic should only be squelched > for Darwin platforms. Ah, I missed the fact that you were proposing this only for Darwin. I agree that, if we're playing C++ pedant and look at the typedefs, then it's undefined behavior and the code is broken. >>> >>> This is reason alone to diagnose the code, because the optimizer is welcome >>> to use that UB to perform transformations the programmer did not expect. >>> However, I'm not certain our optimizer is currently paying attention to >>> that UB directly, so this may not be an immediate issue (but could be a >>> pitfall for the future) but I'm not certain about other optimizers for >>> which portability would still be a concern. >> >> I would honestly find it a bit surprising (and scary) if the optimizer >> actually took advantage of UB in the case where the size and alignment of >> the specifier and the actual type matches. > > Hear, hear! I'm happy to fix any such optimization out of their misguided > optimism :-) People used to think the same thing about many other optimizations that had non-local effects, but we've come to learn that these are not uncommon and incredibly hard for users to track down when it happens. Some enterprising soul could lower the call with an undef on the wrongly-typed argument and then who knows what happens. However, it's also reasonable for us to define undefined behavior for a given platform and that sounds like exactly this case. If we're looking at only a specific change for Darwin, I think it's reasonable to make it to `-Wformat` rather than require `-Wformat-relaxed` (though we may still want to assert that the size and alignment of the underlying type match expectations). I've added @rjmccall as a reviewer to see what his opinions are on this, but I'm inclined to say this is a reasonable change to make for Darwin targets. Repository: rC Clang https://reviews.llvm.org/D42933 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45517: [analyzer] WIP: False positive refutation with Z3
rnkovacs updated this revision to Diff 145762.
rnkovacs marked 4 inline comments as done.
rnkovacs edited the summary of this revision.
rnkovacs added a comment.
Expression chaining is fixed. The visitor now collects constraints that are
about to disappear along the bug path and checks them once in the end.
https://reviews.llvm.org/D45517
Files:
include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
lib/StaticAnalyzer/Core/BugReporter.cpp
lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
lib/StaticAnalyzer/Core/ProgramState.cpp
lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
Index: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
===
--- lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
+++ lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
@@ -7,6 +7,7 @@
//
//===--===//
+#include "RangedConstraintManager.h"
#include "clang/Basic/TargetInfo.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
@@ -915,6 +916,13 @@
void print(ProgramStateRef St, raw_ostream &Out, const char *nl,
const char *sep) override;
+ void reset() override;
+
+ bool isModelFeasible() override;
+
+ void addRangeConstraints(ConstraintRangeTy PrevCR, ConstraintRangeTy SuccCR,
+ bool OnlyPurged) override;
+
//===--===//
// Implementation for interface from SimpleConstraintManager.
//===--===//
@@ -1235,6 +1243,57 @@
return State->set(CZ);
}
+void Z3ConstraintManager::reset() { Solver.reset(); }
+
+bool Z3ConstraintManager::isModelFeasible() {
+ return Solver.check() != Z3_L_FALSE;
+}
+
+void Z3ConstraintManager::addRangeConstraints(ConstraintRangeTy PrevCR,
+ ConstraintRangeTy SuccCR,
+ bool OnlyPurged) {
+ if (OnlyPurged && PrevCR.isEmpty())
+return;
+ if (!OnlyPurged && SuccCR.isEmpty())
+return;
+ ConstraintRangeTy CR = OnlyPurged ? PrevCR : SuccCR;
+
+ for (ConstraintRangeTy::iterator I = CR.begin(), E = CR.end(); I != E; ++I) {
+SymbolRef Sym = I.getKey();
+
+if (OnlyPurged && SuccCR.contains(Sym))
+ continue;
+
+Z3Expr Constraints = Z3Expr::fromBoolean(false);
+
+for (const auto &Range : I.getData()) {
+ const llvm::APSInt &From = Range.From();
+ const llvm::APSInt &To = Range.To();
+
+ assert((getAPSIntType(From) == getAPSIntType(To)) &&
+ "Range values have different types!");
+ QualType RangeTy = getAPSIntType(From);
+ // Skip ranges whose endpoints cannot be converted to APSInts with
+ // a valid APSIntType.
+ if (RangeTy.isNull())
+continue;
+
+ QualType SymTy;
+ Z3Expr Exp = getZ3Expr(Sym, &SymTy);
+ bool IsSignedTy = SymTy->isSignedIntegerOrEnumerationType();
+
+ Z3Expr FromExp = Z3Expr::fromAPSInt(From);
+ Z3Expr ToExp = Z3Expr::fromAPSInt(To);
+
+ Z3Expr LHS = getZ3BinExpr(Exp, SymTy, BO_GE, FromExp, RangeTy, nullptr);
+ Z3Expr RHS = getZ3BinExpr(Exp, SymTy, BO_LE, ToExp, RangeTy, nullptr);
+ Z3Expr SymRange = Z3Expr::fromBinOp(LHS, BO_LAnd, RHS, IsSignedTy);
+ Constraints = Z3Expr::fromBinOp(Constraints, BO_LOr, SymRange, IsSignedTy);
+}
+Solver.addConstraint(Constraints);
+ }
+}
+
//===--===//
// Internal implementation.
//===--===//
Index: lib/StaticAnalyzer/Core/ProgramState.cpp
===
--- lib/StaticAnalyzer/Core/ProgramState.cpp
+++ lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -13,6 +13,8 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
#include "clang/Analysis/CFG.h"
+#include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h"
@@ -78,6 +80,10 @@
CallEventMgr(new CallEventManager(alloc)), Alloc(alloc) {
StoreMgr = (*CreateSMgr)(*this);
ConstraintMgr = (*CreateCMgr)(*this, SubEng);
+ AnalyzerOptions &Opts = SubEng->getAnalysisManager().getAnalyzerOptions();
+ RefutationMgr = Opts.shouldCrosscheckWithZ3()
+ ? CreateZ3ConstraintManager(*thi
[PATCH] D45517: [analyzer] WIP: False positive refutation with Z3
rnkovacs added inline comments.
Comment at: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:2342
+BugReport &BR) {
+ if (isInvalidated)
+return nullptr;
george.karpenkov wrote:
> Is this field actually necessary? Do we ever check the same bug report with
> the same visitor multiple times?
I believe this function is called for each node on the bug path. I have a
similar field to indicate the first visited node in the new version, but there
may exist a better solution for that as well.
Comment at: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:2351
+
+ if (!RefutationMgr.checkRangedStateConstraints(Succ->getState())) {
+const LocationContext *LC = Succ->getLocationContext();
george.karpenkov wrote:
> For the initial version I would just do all work in the visitor, but that's a
> matter of taste.
I think that doing all the work in the visitor would need exposing even more of
`Z3ConstraintManager`'s internals as of `RangedConstraintManager`. I tried to
keep such changes minimal.
Comment at: lib/StaticAnalyzer/Core/ProgramState.cpp:86
+ ? CreateZ3ConstraintManager(*this, SubEng)
+ : nullptr;
}
george.karpenkov wrote:
> Would then we crash on NPE if `getRefutationManager` is called? Getters
> should preferably not cause crashes.
Um, currently yes, it will give a backend error if clang isn't built with Z3,
but the option is on.
https://reviews.llvm.org/D45517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46535: Correct warning on Float->Integer conversions.
erichkeane added a comment.
In https://reviews.llvm.org/D46535#1091787, @efriedma wrote:
> The check for whether an input is "out of range" doesn't handle fractions
> correctly. Testcase:
>
> int a() { return 2147483647.5; }
> unsigned b() { return -.5; }
>
Hrm... For some reaosn I had it in my head that both of those were UB...
Interestingly, EDG/ICC thinks the second one is. I'll look into this more to
see if I can refine the warning.
Repository:
rC Clang
https://reviews.llvm.org/D46535
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46475: [HIP] Set proper triple and offload kind for the toolchain
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. Thanks, LGTM. https://reviews.llvm.org/D46475 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46471: [HIP] Add hip offload kind
yaxunl marked an inline comment as done. yaxunl added inline comments. Comment at: lib/Driver/ToolChains/Clang.cpp:133-135 Work(*C.getSingleOffloadToolChain()); + if (JA.isHostOffloading(Action::OFK_HIP)) tra wrote: > CUDA and HIP are mutually exclusive, so this should probably be `else if` Will do when committing. https://reviews.llvm.org/D46471 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r331805 - Fix Wdocumentation warning. NFCI.
Author: rksimon Date: Tue May 8 13:24:45 2018 New Revision: 331805 URL: http://llvm.org/viewvc/llvm-project?rev=331805&view=rev Log: Fix Wdocumentation warning. NFCI. Modified: clang-tools-extra/trunk/clang-tidy/ClangTidy.h Modified: clang-tools-extra/trunk/clang-tidy/ClangTidy.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidy.h?rev=331805&r1=331804&r2=331805&view=diff == --- clang-tools-extra/trunk/clang-tidy/ClangTidy.h (original) +++ clang-tools-extra/trunk/clang-tidy/ClangTidy.h Tue May 8 13:24:45 2018 @@ -222,8 +222,8 @@ ClangTidyOptions::OptionMap getCheckOpti /// \brief Run a set of clang-tidy checks on a set of files. /// -/// \param Profile if provided, it enables check profile collection in -/// MatchFinder, and will contain the result of the profile. +/// \param EnableCheckProfile If provided, it enables check profile collection +/// in MatchFinder, and will contain the result of the profile. void runClangTidy(clang::tidy::ClangTidyContext &Context, const tooling::CompilationDatabase &Compilations, ArrayRef InputFiles, ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46386: Adding __atomic_fetch_min/max intrinsics to clang
rjmccall added inline comments. Comment at: docs/LanguageExtensions.rst:1998 +``__ATOMIC_CONSUME``, ``__ATOMIC_ACQUIRE``, ``__ATOMIC_RELEASE``, +``__ATOMIC_ACQ_REL``, or ``__ATOMIC_SEQ_CST`` following C++11 memory model semantics. + Thank you for adding this documentation. Please do clarify what the memory ordering semantics actually are when the atomic object does not need to be updated, though, and verify that target code generation actually obeys that ordering. For example, if the memory ordering makes this a release operation, `__atomic_fetch_min` must always store the result back to the atomic object, even if the new value was actually greater than the stored value; I believe that would not be required with a relaxed operation. Repository: rC Clang https://reviews.llvm.org/D46386 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46349: [X86] Mark builtins 'const' where possible
efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. I'm pretty sure this has zero visible effect, but I guess it makes sense as documentation. LGTM. https://reviews.llvm.org/D46349 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46601: [OpenCL] Fix typos in emitted enqueue kernel function names
yaxunl created this revision.
yaxunl added reviewers: Anastasia, b-sumner.
Two typos:
vaarg => vararg
get_kernel_preferred_work_group_multiple =>
get_kernel_preferred_work_group_size_multiple
https://reviews.llvm.org/D46601
Files:
lib/CodeGen/CGBuiltin.cpp
test/CodeGenOpenCL/cl20-device-side-enqueue.cl
Index: test/CodeGenOpenCL/cl20-device-side-enqueue.cl
===
--- test/CodeGenOpenCL/cl20-device-side-enqueue.cl
+++ test/CodeGenOpenCL/cl20-device-side-enqueue.cl
@@ -88,7 +88,7 @@
// B64: %[[TMP:.*]] = alloca [1 x i64]
// B64: %[[TMP1:.*]] = getelementptr [1 x i64], [1 x i64]* %[[TMP]], i32 0, i32 0
// B64: store i64 256, i64* %[[TMP1]], align 8
- // COMMON-LABEL: call i32 @__enqueue_kernel_vaargs(
+ // COMMON-LABEL: call i32 @__enqueue_kernel_varargs(
// COMMON-SAME: %opencl.queue_t{{.*}}* [[DEF_Q]], i32 [[FLAGS]], %struct.ndrange_t* [[NDR]]{{([0-9]+)?}},
// COMMON-SAME: i8 addrspace(4)* addrspacecast (i8* bitcast ({{.*}} [[INVGK1:[^ ]+_kernel]] to i8*) to i8 addrspace(4)*),
// COMMON-SAME: i8 addrspace(4)* addrspacecast (i8 addrspace(1)* bitcast ({ i32, i32 } addrspace(1)* [[BLG1]] to i8 addrspace(1)*) to i8 addrspace(4)*), i32 1,
@@ -109,7 +109,7 @@
// B64: %[[TMP:.*]] = alloca [1 x i64]
// B64: %[[TMP1:.*]] = getelementptr [1 x i64], [1 x i64]* %[[TMP]], i32 0, i32 0
// B64: store i64 %{{.*}}, i64* %[[TMP1]], align 8
- // COMMON-LABEL: call i32 @__enqueue_kernel_vaargs(
+ // COMMON-LABEL: call i32 @__enqueue_kernel_varargs(
// COMMON-SAME: %opencl.queue_t{{.*}}* [[DEF_Q]], i32 [[FLAGS]], %struct.ndrange_t* [[NDR]]{{([0-9]+)?}},
// COMMON-SAME: i8 addrspace(4)* addrspacecast (i8* bitcast ({{.*}} [[INVGK2:[^ ]+_kernel]] to i8*) to i8 addrspace(4)*),
// COMMON-SAME: i8 addrspace(4)* addrspacecast (i8 addrspace(1)* bitcast ({ i32, i32 } addrspace(1)* [[BLG2]] to i8 addrspace(1)*) to i8 addrspace(4)*), i32 1,
@@ -133,7 +133,7 @@
// B64: %[[TMP:.*]] = alloca [1 x i64]
// B64: %[[TMP1:.*]] = getelementptr [1 x i64], [1 x i64]* %[[TMP]], i32 0, i32 0
// B64: store i64 256, i64* %[[TMP1]], align 8
- // COMMON-LABEL: call i32 @__enqueue_kernel_events_vaargs
+ // COMMON-LABEL: call i32 @__enqueue_kernel_events_varargs
// COMMON-SAME: (%opencl.queue_t{{.*}}* [[DEF_Q]], i32 [[FLAGS]], %struct.ndrange_t* {{.*}}, i32 2, %opencl.clk_event_t{{.*}} [[WAIT_EVNT]], %opencl.clk_event_t{{.*}} [[EVNT]],
// COMMON-SAME: i8 addrspace(4)* addrspacecast (i8* bitcast ({{.*}} [[INVGK3:[^ ]+_kernel]] to i8*) to i8 addrspace(4)*),
// COMMON-SAME: i8 addrspace(4)* addrspacecast (i8 addrspace(1)* bitcast ({ i32, i32 } addrspace(1)* [[BLG3]] to i8 addrspace(1)*) to i8 addrspace(4)*), i32 1,
@@ -157,7 +157,7 @@
// B64: %[[TMP:.*]] = alloca [1 x i64]
// B64: %[[TMP1:.*]] = getelementptr [1 x i64], [1 x i64]* %[[TMP]], i32 0, i32 0
// B64: store i64 %{{.*}}, i64* %[[TMP1]], align 8
- // COMMON-LABEL: call i32 @__enqueue_kernel_events_vaargs
+ // COMMON-LABEL: call i32 @__enqueue_kernel_events_varargs
// COMMON-SAME: (%opencl.queue_t{{.*}}* [[DEF_Q]], i32 [[FLAGS]], %struct.ndrange_t* {{.*}}, i32 2, %opencl.clk_event_t{{.*}}* addrspace(4)* [[WAIT_EVNT]], %opencl.clk_event_t{{.*}}* addrspace(4)* [[EVNT]],
// COMMON-SAME: i8 addrspace(4)* addrspacecast (i8* bitcast ({{.*}} [[INVGK4:[^ ]+_kernel]] to i8*) to i8 addrspace(4)*),
// COMMON-SAME: i8 addrspace(4)* addrspacecast (i8 addrspace(1)* bitcast ({ i32, i32 } addrspace(1)* [[BLG4]] to i8 addrspace(1)*) to i8 addrspace(4)*), i32 1,
@@ -179,7 +179,7 @@
// B64: %[[TMP:.*]] = alloca [1 x i64]
// B64: %[[TMP1:.*]] = getelementptr [1 x i64], [1 x i64]* %[[TMP]], i32 0, i32 0
// B64: store i64 %{{.*}}, i64* %[[TMP1]], align 8
- // COMMON-LABEL: call i32 @__enqueue_kernel_vaargs
+ // COMMON-LABEL: call i32 @__enqueue_kernel_varargs
// COMMON-SAME: (%opencl.queue_t{{.*}}* [[DEF_Q]], i32 [[FLAGS]], %struct.ndrange_t* [[NDR]]{{([0-9]+)?}},
// COMMON-SAME: i8 addrspace(4)* addrspacecast (i8* bitcast ({{.*}} [[INVGK5:[^ ]+_kernel]] to i8*) to i8 addrspace(4)*),
// COMMON-SAME: i8 addrspace(4)* addrspacecast (i8 addrspace(1)* bitcast ({ i32, i32 } addrspace(1)* [[BLG5]] to i8 addrspace(1)*) to i8 addrspace(4)*), i32 1,
@@ -208,7 +208,7 @@
// B64: store i64 2, i64* %[[TMP2]], align 8
// B64: %[[TMP3:.*]] = getelementptr [3 x i64], [3 x i64]* %[[TMP]], i32 0, i32 2
// B64: store i64 4, i64* %[[TMP3]], align 8
- // COMMON-LABEL: call i32 @__enqueue_kernel_vaargs
+ // COMMON-LABEL: call i32 @__enqueue_kernel_varargs
// COMMON-SAME: (%opencl.queue_t{{.*}}* [[DEF_Q]], i32 [[FLAGS]], %struct.ndrange_t* [[NDR]]{{([0-9]+)?}},
// COMMON-SAME: i8 addrspace(4)* addrspacecast (i8* bitcast ({{.*}} [[INVGK6:[^ ]+_kernel]] to i8*) to i8 addrspace(4)*),
// COMMON-SAME: i8 addrspace(4)* addrspacecast (i8 addrspace(1)* bitcast ({ i32, i32 } addrspace(1)* [[BLG6]] to i8 addrspace(1)*) to i8 addrspace(4)*), i32 3,
@@ -229,7 +229,7 @@
[PATCH] D46601: [OpenCL] Fix typos in emitted enqueue kernel function names
b-sumner added a comment. Thanks! Looks good to me. https://reviews.llvm.org/D46601 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r331806 - Change -foutline to -moutline
Author: paquette
Date: Tue May 8 13:53:19 2018
New Revision: 331806
URL: http://llvm.org/viewvc/llvm-project?rev=331806&view=rev
Log:
Change -foutline to -moutline
Nitpicky, but the MachineOutliner is a machine-level pass, and so we should
reflect that by using "m" instead of "n".
Figured we should get this in before people get used to the letter f. :)
Modified:
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/test/Driver/aarch64-outliner.c
Modified: cfe/trunk/include/clang/Driver/Options.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=331806&r1=331805&r2=331806&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Tue May 8 13:53:19 2018
@@ -1317,8 +1317,6 @@ def fno_exceptions : Flag<["-"], "fno-ex
def fno_gnu_keywords : Flag<["-"], "fno-gnu-keywords">, Group,
Flags<[CC1Option]>;
def fno_inline_functions : Flag<["-"], "fno-inline-functions">,
Group, Flags<[CC1Option]>;
def fno_inline : Flag<["-"], "fno-inline">, Group,
Flags<[CC1Option]>;
-def foutline : Flag<["-"], "foutline">, Group,
Flags<[CC1Option]>,
-HelpText<"Enable function outlining (AArch64 only)">;
def fno_experimental_isel : Flag<["-"], "fno-experimental-isel">,
Group,
HelpText<"Disables the experimental global instruction selector">;
def fno_experimental_new_pass_manager : Flag<["-"],
"fno-experimental-new-pass-manager">,
@@ -1908,6 +1906,8 @@ def mmacos_version_min_EQ : Joined<["-"]
Group, Alias;
def mms_bitfields : Flag<["-"], "mms-bitfields">, Group,
Flags<[CC1Option]>,
HelpText<"Set the default structure layout to be compatible with the
Microsoft compiler standard">;
+def moutline : Flag<["-"], "moutline">, Group,
Flags<[CC1Option]>,
+HelpText<"Enable function outlining (AArch64 only)">;
def mno_ms_bitfields : Flag<["-"], "mno-ms-bitfields">, Group,
HelpText<"Do not set the default structure layout to be compatible with the
Microsoft compiler standard">;
def mstackrealign : Flag<["-"], "mstackrealign">, Group,
Flags<[CC1Option]>,
Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=331806&r1=331805&r2=331806&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Tue May 8 13:53:19 2018
@@ -1485,7 +1485,7 @@ void Clang::AddAArch64TargetArgs(const A
CmdArgs.push_back("-aarch64-enable-global-merge=true");
}
- if (Args.getLastArg(options::OPT_foutline)) {
+ if (Args.getLastArg(options::OPT_moutline)) {
CmdArgs.push_back("-mllvm");
CmdArgs.push_back("-enable-machine-outliner");
}
Modified: cfe/trunk/test/Driver/aarch64-outliner.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/aarch64-outliner.c?rev=331806&r1=331805&r2=331806&view=diff
==
--- cfe/trunk/test/Driver/aarch64-outliner.c (original)
+++ cfe/trunk/test/Driver/aarch64-outliner.c Tue May 8 13:53:19 2018
@@ -1,4 +1,4 @@
// REQUIRES: aarch64-registered-target
-// RUN: %clang -target aarch64 -foutline -S %s -### 2>&1 | FileCheck %s
+// RUN: %clang -target aarch64 -moutline -S %s -### 2>&1 | FileCheck %s
// CHECK: "-mllvm" "-enable-machine-outliner"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r331807 - [Driver] Don't add -dwarf-column-info when using -gcodeview on non-msvc targets
Author: mstorsjo
Date: Tue May 8 13:55:23 2018
New Revision: 331807
URL: http://llvm.org/viewvc/llvm-project?rev=331807&view=rev
Log:
[Driver] Don't add -dwarf-column-info when using -gcodeview on non-msvc targets
-dwarf-column-info is omitted if -gcodeview is specified for msvc
targets at the moment, but since -gcodeview is an option that can be
specified for any target, there's little reason to restrict this
handling to msvc targets.
This allows getting proper codeview debug info by passing -gcodeview
for e.g. MinGW targets as well.
Differential Revision: https://reviews.llvm.org/D46287
Modified:
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/test/Driver/codeview-column-info.c
Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=331807&r1=331806&r2=331807&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Tue May 8 13:55:23 2018
@@ -3000,7 +3000,7 @@ static void RenderDebugOptions(const Too
// debuggers don't handle missing end columns well, so it's better not to
// include any column info.
if (Args.hasFlag(options::OPT_gcolumn_info, options::OPT_gno_column_info,
- /*Default=*/!(IsWindowsMSVC && EmitCodeView) &&
+ /*Default=*/!EmitCodeView &&
DebuggerTuning != llvm::DebuggerKind::SCE))
CmdArgs.push_back("-dwarf-column-info");
Modified: cfe/trunk/test/Driver/codeview-column-info.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/codeview-column-info.c?rev=331807&r1=331806&r2=331807&view=diff
==
--- cfe/trunk/test/Driver/codeview-column-info.c (original)
+++ cfe/trunk/test/Driver/codeview-column-info.c Tue May 8 13:55:23 2018
@@ -6,6 +6,8 @@
// RUN: FileCheck < %t1 %s
// RUN: %clangxx -### --target=x86_64-windows-msvc -c -g -gcodeview %s 2> %t2
// RUN: FileCheck < %t2 %s
+// RUN: %clangxx -### --target=x86_64-windows-gnu -c -g -gcodeview %s 2> %t2
+// RUN: FileCheck < %t2 %s
// RUN: %clang_cl -### --target=x86_64-windows-msvc /c /Z7 -- %s 2> %t2
// RUN: FileCheck < %t2 %s
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46287: [Driver] Don't add -dwarf-column-info when using -gcodeview on non-msvc targets
This revision was automatically updated to reflect the committed changes.
Closed by commit rL331807: [Driver] Don't add -dwarf-column-info when
using -gcodeview on non-msvc targets (authored by mstorsjo, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D46287?vs=144630&id=145776#toc
Repository:
rL LLVM
https://reviews.llvm.org/D46287
Files:
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/test/Driver/codeview-column-info.c
Index: cfe/trunk/test/Driver/codeview-column-info.c
===
--- cfe/trunk/test/Driver/codeview-column-info.c
+++ cfe/trunk/test/Driver/codeview-column-info.c
@@ -6,6 +6,8 @@
// RUN: FileCheck < %t1 %s
// RUN: %clangxx -### --target=x86_64-windows-msvc -c -g -gcodeview %s 2> %t2
// RUN: FileCheck < %t2 %s
+// RUN: %clangxx -### --target=x86_64-windows-gnu -c -g -gcodeview %s 2> %t2
+// RUN: FileCheck < %t2 %s
// RUN: %clang_cl -### --target=x86_64-windows-msvc /c /Z7 -- %s 2> %t2
// RUN: FileCheck < %t2 %s
Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp
@@ -3000,7 +3000,7 @@
// debuggers don't handle missing end columns well, so it's better not to
// include any column info.
if (Args.hasFlag(options::OPT_gcolumn_info, options::OPT_gno_column_info,
- /*Default=*/!(IsWindowsMSVC && EmitCodeView) &&
+ /*Default=*/!EmitCodeView &&
DebuggerTuning != llvm::DebuggerKind::SCE))
CmdArgs.push_back("-dwarf-column-info");
Index: cfe/trunk/test/Driver/codeview-column-info.c
===
--- cfe/trunk/test/Driver/codeview-column-info.c
+++ cfe/trunk/test/Driver/codeview-column-info.c
@@ -6,6 +6,8 @@
// RUN: FileCheck < %t1 %s
// RUN: %clangxx -### --target=x86_64-windows-msvc -c -g -gcodeview %s 2> %t2
// RUN: FileCheck < %t2 %s
+// RUN: %clangxx -### --target=x86_64-windows-gnu -c -g -gcodeview %s 2> %t2
+// RUN: FileCheck < %t2 %s
// RUN: %clang_cl -### --target=x86_64-windows-msvc /c /Z7 -- %s 2> %t2
// RUN: FileCheck < %t2 %s
Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp
@@ -3000,7 +3000,7 @@
// debuggers don't handle missing end columns well, so it's better not to
// include any column info.
if (Args.hasFlag(options::OPT_gcolumn_info, options::OPT_gno_column_info,
- /*Default=*/!(IsWindowsMSVC && EmitCodeView) &&
+ /*Default=*/!EmitCodeView &&
DebuggerTuning != llvm::DebuggerKind::SCE))
CmdArgs.push_back("-dwarf-column-info");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r331810 - Add a mno-outline flag to disable the MachineOutliner
Author: paquette
Date: Tue May 8 13:58:32 2018
New Revision: 331810
URL: http://llvm.org/viewvc/llvm-project?rev=331810&view=rev
Log:
Add a mno-outline flag to disable the MachineOutliner
Since we're working on turning the MachineOutliner by default under -Oz for
AArch64, it makes sense to have an -mno-outline flag available. This currently
doesn't do much (it basically just undoes -moutline).
When the MachineOutliner is on by default under AArch64, this flag should
set -mllvm -enable-machine-outliner=never.
Modified:
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/test/Driver/aarch64-outliner.c
Modified: cfe/trunk/include/clang/Driver/Options.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=331810&r1=331809&r2=331810&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Tue May 8 13:58:32 2018
@@ -1908,6 +1908,8 @@ def mms_bitfields : Flag<["-"], "mms-bit
HelpText<"Set the default structure layout to be compatible with the
Microsoft compiler standard">;
def moutline : Flag<["-"], "moutline">, Group,
Flags<[CC1Option]>,
HelpText<"Enable function outlining (AArch64 only)">;
+def mno_outline : Flag<["-"], "mno-outline">, Group,
Flags<[CC1Option]>,
+HelpText<"Disable function outlining (AArch64 only)">;
def mno_ms_bitfields : Flag<["-"], "mno-ms-bitfields">, Group,
HelpText<"Do not set the default structure layout to be compatible with the
Microsoft compiler standard">;
def mstackrealign : Flag<["-"], "mstackrealign">, Group,
Flags<[CC1Option]>,
Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=331810&r1=331809&r2=331810&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Tue May 8 13:58:32 2018
@@ -1485,7 +1485,8 @@ void Clang::AddAArch64TargetArgs(const A
CmdArgs.push_back("-aarch64-enable-global-merge=true");
}
- if (Args.getLastArg(options::OPT_moutline)) {
+ if (!Args.hasArg(options::OPT_mno_outline) &&
+ Args.getLastArg(options::OPT_moutline)) {
CmdArgs.push_back("-mllvm");
CmdArgs.push_back("-enable-machine-outliner");
}
Modified: cfe/trunk/test/Driver/aarch64-outliner.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/aarch64-outliner.c?rev=331810&r1=331809&r2=331810&view=diff
==
--- cfe/trunk/test/Driver/aarch64-outliner.c (original)
+++ cfe/trunk/test/Driver/aarch64-outliner.c Tue May 8 13:58:32 2018
@@ -1,4 +1,9 @@
// REQUIRES: aarch64-registered-target
-// RUN: %clang -target aarch64 -moutline -S %s -### 2>&1 | FileCheck %s
-// CHECK: "-mllvm" "-enable-machine-outliner"
+// RUN: %clang -target aarch64 -moutline -S %s -### 2>&1 | FileCheck %s
-check-prefix=ON
+// ON: "-mllvm" "-enable-machine-outliner"
+
+// RUN: %clang -target aarch64 -moutline -mno-outline -S %s -### 2>&1 |
FileCheck %s -check-prefix=OFF1
+// RUN: %clang -target aarch64 -mno-outline -moutline -S %s -### 2>&1 |
FileCheck %s -check-prefix=OFF2
+// OFF1-NOT: "-mllvm" "-enable-machine-outliner"
+// OFF2-NOT: "-mllvm" "-enable-machine-outliner"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r331811 - [HIP] Add hip offload kind
Author: yaxunl
Date: Tue May 8 14:02:12 2018
New Revision: 331811
URL: http://llvm.org/viewvc/llvm-project?rev=331811&view=rev
Log:
[HIP] Add hip offload kind
There are quite differences in HIP action builder and action job creation,
which justifies to define a separate offload kind.
Differential Revision: https://reviews.llvm.org/D46471
Modified:
cfe/trunk/include/clang/Driver/Action.h
cfe/trunk/lib/Driver/Action.cpp
cfe/trunk/lib/Driver/Compilation.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
Modified: cfe/trunk/include/clang/Driver/Action.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Action.h?rev=331811&r1=331810&r2=331811&view=diff
==
--- cfe/trunk/include/clang/Driver/Action.h (original)
+++ cfe/trunk/include/clang/Driver/Action.h Tue May 8 14:02:12 2018
@@ -88,6 +88,7 @@ public:
// The device offloading tool chains - one bit for each programming model.
OFK_Cuda = 0x02,
OFK_OpenMP = 0x04,
+OFK_HIP = 0x08,
};
static const char *getClassName(ActionClass AC);
Modified: cfe/trunk/lib/Driver/Action.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Action.cpp?rev=331811&r1=331810&r2=331811&view=diff
==
--- cfe/trunk/lib/Driver/Action.cpp (original)
+++ cfe/trunk/lib/Driver/Action.cpp Tue May 8 14:02:12 2018
@@ -96,6 +96,8 @@ std::string Action::getOffloadingKindPre
return "device-cuda";
case OFK_OpenMP:
return "device-openmp";
+ case OFK_HIP:
+return "device-hip";
// TODO: Add other programming models here.
}
@@ -104,8 +106,13 @@ std::string Action::getOffloadingKindPre
return {};
std::string Res("host");
+ assert(!((ActiveOffloadKindMask & OFK_Cuda) &&
+ (ActiveOffloadKindMask & OFK_HIP)) &&
+ "Cannot offload CUDA and HIP at the same time");
if (ActiveOffloadKindMask & OFK_Cuda)
Res += "-cuda";
+ if (ActiveOffloadKindMask & OFK_HIP)
+Res += "-hip";
if (ActiveOffloadKindMask & OFK_OpenMP)
Res += "-openmp";
@@ -142,6 +149,8 @@ StringRef Action::GetOffloadKindName(Off
return "cuda";
case OFK_OpenMP:
return "openmp";
+ case OFK_HIP:
+return "hip";
// TODO: Add other programming models here.
}
Modified: cfe/trunk/lib/Driver/Compilation.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Compilation.cpp?rev=331811&r1=331810&r2=331811&view=diff
==
--- cfe/trunk/lib/Driver/Compilation.cpp (original)
+++ cfe/trunk/lib/Driver/Compilation.cpp Tue May 8 14:02:12 2018
@@ -196,10 +196,10 @@ static bool ActionFailed(const Action *A
if (FailingCommands.empty())
return false;
- // CUDA can have the same input source code compiled multiple times so do not
- // compiled again if there are already failures. It is OK to abort the CUDA
- // pipeline on errors.
- if (A->isOffloading(Action::OFK_Cuda))
+ // CUDA/HIP can have the same input source code compiled multiple times so do
+ // not compiled again if there are already failures. It is OK to abort the
+ // CUDA pipeline on errors.
+ if (A->isOffloading(Action::OFK_Cuda) || A->isOffloading(Action::OFK_HIP))
return true;
for (const auto &CI : FailingCommands)
Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=331811&r1=331810&r2=331811&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Tue May 8 14:02:12 2018
@@ -131,6 +131,10 @@ forAllAssociatedToolChains(Compilation &
Work(*C.getSingleOffloadToolChain());
else if (JA.isDeviceOffloading(Action::OFK_Cuda))
Work(*C.getSingleOffloadToolChain());
+ else if (JA.isHostOffloading(Action::OFK_HIP))
+Work(*C.getSingleOffloadToolChain());
+ else if (JA.isDeviceOffloading(Action::OFK_HIP))
+Work(*C.getSingleOffloadToolChain());
if (JA.isHostOffloading(Action::OFK_OpenMP)) {
auto TCs = C.getOffloadToolChains();
@@ -3105,13 +3109,14 @@ void Clang::ConstructJob(Compilation &C,
// Check number of inputs for sanity. We need at least one input.
assert(Inputs.size() >= 1 && "Must have at least one input.");
const InputInfo &Input = Inputs[0];
- // CUDA compilation may have multiple inputs (source file + results of
+ // CUDA/HIP compilation may have multiple inputs (source file + results of
// device-side compilations). OpenMP device jobs also take the host IR as a
// second input. All other jobs are expected to have exactly one
// input.
bool IsCuda = JA.isOffloading(Action::OFK_Cuda);
+ bool IsHIP = JA.isOffloading(Action::OFK_HIP);
bool IsOpenMPDevice = JA.isDeviceOffloading(Action::O
[PATCH] D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers
jyknight added a comment.
In https://reviews.llvm.org/D42933#1091817, @jfb wrote:
> In https://reviews.llvm.org/D42933#1091809, @jyknight wrote:
>
> > I also think that special casing these two specifiers doesn't make sense.
> > The problem is a general issue -- and one I've often found irritating. This
> > exact same situation comes up all the time in non-Darwin contexts too.
>
>
> I don't think that's true. In this very specific case the platform guarantees
> that `(sizeof(size_t) == sizeof(NSInteger)) && (sizeof(ssize_t) ==
> sizeof(NSUInteger))` for all architectures this platform supports. This exact
> same situation does not come up all the time in other contexts because the
> `int` / `long` / `long long` distinction isn't backed by a portability
> guarantee. The warning is there to say "this code isn't portable!", but in
> the very specific case of `NSInteger` and `NSUInteger` it is and users rely
> on it so it cannot be broken. The warning is therefore spurious, users
> therefore rightly complain.
The printf format specifier warning is not primarily a cross-platform
portability checker. And, although in a few limited cases it can act somewhat
like one, in general it does not. Take my previous example -- you get no
warning on a platform that has int64_t as a typedef for long -- if this feature
is to be useful as a portability checker, it should require that you used the
PRId64 macro. Or, given `ssize_t x = 0; printf("%ld", x);`, it doesn't tell you
to use "%zd" instead if ssize_t is a typedef for long -- although to be
portable you ought to.
No, the major usefulness of the printf warning is to tell you that your code is
incorrect for the _current_ platform. And //most// importantly when you chose
the wrong size for your argument.
Those types which have matched size and alignment are still different types,
and so it's technically appropriate to warn about using the wrong specifier
there, too. But it's also entirely reasonable to not want to be bothered by
such useless trivia, so skipping the warning when there's only a technical and
not actual mismatch seems entirely sensible. (I might suggest that it should
even be the default behavior for the warning, and if you want the stricter
checks you'd ask for them...but I bet I'll get more pushback on that...)
Repository:
rC Clang
https://reviews.llvm.org/D42933
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46471: [HIP] Add hip offload kind
This revision was automatically updated to reflect the committed changes.
yaxunl marked an inline comment as done.
Closed by commit rL331811: [HIP] Add hip offload kind (authored by yaxunl,
committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D46471?vs=145472&id=145780#toc
Repository:
rL LLVM
https://reviews.llvm.org/D46471
Files:
cfe/trunk/include/clang/Driver/Action.h
cfe/trunk/lib/Driver/Action.cpp
cfe/trunk/lib/Driver/Compilation.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp
@@ -131,6 +131,10 @@
Work(*C.getSingleOffloadToolChain());
else if (JA.isDeviceOffloading(Action::OFK_Cuda))
Work(*C.getSingleOffloadToolChain());
+ else if (JA.isHostOffloading(Action::OFK_HIP))
+Work(*C.getSingleOffloadToolChain());
+ else if (JA.isDeviceOffloading(Action::OFK_HIP))
+Work(*C.getSingleOffloadToolChain());
if (JA.isHostOffloading(Action::OFK_OpenMP)) {
auto TCs = C.getOffloadToolChains();
@@ -3105,13 +3109,14 @@
// Check number of inputs for sanity. We need at least one input.
assert(Inputs.size() >= 1 && "Must have at least one input.");
const InputInfo &Input = Inputs[0];
- // CUDA compilation may have multiple inputs (source file + results of
+ // CUDA/HIP compilation may have multiple inputs (source file + results of
// device-side compilations). OpenMP device jobs also take the host IR as a
// second input. All other jobs are expected to have exactly one
// input.
bool IsCuda = JA.isOffloading(Action::OFK_Cuda);
+ bool IsHIP = JA.isOffloading(Action::OFK_HIP);
bool IsOpenMPDevice = JA.isDeviceOffloading(Action::OFK_OpenMP);
- assert((IsCuda || (IsOpenMPDevice && Inputs.size() == 2) ||
+ assert((IsCuda || IsHIP || (IsOpenMPDevice && Inputs.size() == 2) ||
Inputs.size() == 1) &&
"Unable to handle multiple inputs.");
@@ -3123,10 +3128,10 @@
bool IsWindowsMSVC = RawTriple.isWindowsMSVCEnvironment();
bool IsIAMCU = RawTriple.isOSIAMCU();
- // Adjust IsWindowsXYZ for CUDA compilations. Even when compiling in device
- // mode (i.e., getToolchain().getTriple() is NVPTX, not Windows), we need to
- // pass Windows-specific flags to cc1.
- if (IsCuda) {
+ // Adjust IsWindowsXYZ for CUDA/HIP compilations. Even when compiling in
+ // device mode (i.e., getToolchain().getTriple() is NVPTX/AMDGCN, not
+ // Windows), we need to pass Windows-specific flags to cc1.
+ if (IsCuda || IsHIP) {
IsWindowsMSVC |= AuxTriple && AuxTriple->isWindowsMSVCEnvironment();
IsWindowsGNU |= AuxTriple && AuxTriple->isWindowsGNUEnvironment();
IsWindowsCygnus |= AuxTriple && AuxTriple->isWindowsCygwinEnvironment();
@@ -3150,18 +3155,21 @@
Args.ClaimAllArgs(options::OPT_MJ);
}
- if (IsCuda) {
-// We have to pass the triple of the host if compiling for a CUDA device and
-// vice-versa.
+ if (IsCuda || IsHIP) {
+// We have to pass the triple of the host if compiling for a CUDA/HIP device
+// and vice-versa.
std::string NormalizedTriple;
-if (JA.isDeviceOffloading(Action::OFK_Cuda))
+if (JA.isDeviceOffloading(Action::OFK_Cuda) ||
+JA.isDeviceOffloading(Action::OFK_HIP))
NormalizedTriple = C.getSingleOffloadToolChain()
->getTriple()
.normalize();
else
- NormalizedTriple = C.getSingleOffloadToolChain()
- ->getTriple()
- .normalize();
+ NormalizedTriple =
+ (IsCuda ? C.getSingleOffloadToolChain()
+ : C.getSingleOffloadToolChain())
+ ->getTriple()
+ .normalize();
CmdArgs.push_back("-aux-triple");
CmdArgs.push_back(Args.MakeArgString(NormalizedTriple));
Index: cfe/trunk/lib/Driver/Compilation.cpp
===
--- cfe/trunk/lib/Driver/Compilation.cpp
+++ cfe/trunk/lib/Driver/Compilation.cpp
@@ -196,10 +196,10 @@
if (FailingCommands.empty())
return false;
- // CUDA can have the same input source code compiled multiple times so do not
- // compiled again if there are already failures. It is OK to abort the CUDA
- // pipeline on errors.
- if (A->isOffloading(Action::OFK_Cuda))
+ // CUDA/HIP can have the same input source code compiled multiple times so do
+ // not compiled again if there are already failures. It is OK to abort the
+ // CUDA pipeline on errors.
+ if (A->isOffloading(Action::OFK_Cuda) || A->isOffloading(Action::OFK_HIP))
return true;
for (const auto &CI : FailingCommands)
Index: cfe/trunk/lib/Driver/Action.cpp
===
--- cfe/trunk/lib/Driver/Action.cpp
+++ cfe/trunk/l
[PATCH] D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers
smeenai added a subscriber: rsmith.
smeenai added a comment.
In https://reviews.llvm.org/D42933#1091943, @jyknight wrote:
> In https://reviews.llvm.org/D42933#1091817, @jfb wrote:
>
> > In https://reviews.llvm.org/D42933#1091809, @jyknight wrote:
> >
> > > I also think that special casing these two specifiers doesn't make sense.
> > > The problem is a general issue -- and one I've often found irritating.
> > > This exact same situation comes up all the time in non-Darwin contexts
> > > too.
> >
> >
> > I don't think that's true. In this very specific case the platform
> > guarantees that `(sizeof(size_t) == sizeof(NSInteger)) && (sizeof(ssize_t)
> > == sizeof(NSUInteger))` for all architectures this platform supports. This
> > exact same situation does not come up all the time in other contexts
> > because the `int` / `long` / `long long` distinction isn't backed by a
> > portability guarantee. The warning is there to say "this code isn't
> > portable!", but in the very specific case of `NSInteger` and `NSUInteger`
> > it is and users rely on it so it cannot be broken. The warning is therefore
> > spurious, users therefore rightly complain.
>
>
> The printf format specifier warning is not primarily a cross-platform
> portability checker. And, although in a few limited cases it can act somewhat
> like one, in general it does not. Take my previous example -- you get no
> warning on a platform that has int64_t as a typedef for long -- if this
> feature is to be useful as a portability checker, it should require that you
> used the PRId64 macro. Or, given `ssize_t x = 0; printf("%ld", x);`, it
> doesn't tell you to use "%zd" instead if ssize_t is a typedef for long --
> although to be portable you ought to.
>
> No, the major usefulness of the printf warning is to tell you that your code
> is incorrect for the _current_ platform. And //most// importantly when you
> chose the wrong size for your argument.
>
> Those types which have matched size and alignment are still different types,
> and so it's technically appropriate to warn about using the wrong specifier
> there, too. But it's also entirely reasonable to not want to be bothered by
> such useless trivia, so skipping the warning when there's only a technical
> and not actual mismatch seems entirely sensible. (I might suggest that it
> should even be the default behavior for the warning, and if you want the
> stricter checks you'd ask for them...but I bet I'll get more pushback on
> that...)
+1 to everything you said; what are other people's opinions on this? @rsmith
perhaps?
Repository:
rC Clang
https://reviews.llvm.org/D42933
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43778: [clang-tidy] Adding RestrictIncludes check to Fuchsia module
juliehockett updated this revision to Diff 145788.
juliehockett marked 10 inline comments as done.
juliehockett added a comment.
Made the check for system headers more comprehensive & fixed newline issues
https://reviews.llvm.org/D43778
Files:
clang-tidy/fuchsia/CMakeLists.txt
clang-tidy/fuchsia/FuchsiaTidyModule.cpp
clang-tidy/fuchsia/RestrictSystemIncludesCheck.cpp
clang-tidy/fuchsia/RestrictSystemIncludesCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/fuchsia-restrict-system-includes.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/Inputs/fuchsia-restrict-system-includes/a.h
test/clang-tidy/Inputs/fuchsia-restrict-system-includes/system/j.h
test/clang-tidy/Inputs/fuchsia-restrict-system-includes/system/r.h
test/clang-tidy/Inputs/fuchsia-restrict-system-includes/system/s.h
test/clang-tidy/Inputs/fuchsia-restrict-system-includes/system/t.h
test/clang-tidy/Inputs/fuchsia-restrict-system-includes/system/transitive.h
test/clang-tidy/Inputs/fuchsia-restrict-system-includes/transitive2.h
test/clang-tidy/fuchsia-restrict-system-includes-headers.cpp
test/clang-tidy/fuchsia-restrict-system-includes.cpp
Index: test/clang-tidy/fuchsia-restrict-system-includes.cpp
===
--- /dev/null
+++ test/clang-tidy/fuchsia-restrict-system-includes.cpp
@@ -0,0 +1,25 @@
+// RUN: %check_clang_tidy %s fuchsia-restrict-system-includes %t \
+// RUN: -- -config="{CheckOptions: [{key: fuchsia-restrict-system-includes.Includes, value: 's.h'}]}" \
+// RUN: -- -std=c++11 -I %S/Inputs/fuchsia-restrict-system-includes -isystem %S/Inputs/fuchsia-restrict-system-includes/system
+
+#include "a.h"
+
+#include
+#include
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: system include t.h not allowed
+// CHECK-FIXES-NOT: #include
+
+#include "s.h"
+#include "t.h"
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: system include t.h not allowed
+// CHECK-FIXES-NOT: #include "t.h"
+
+#define foo
+
+#include foo
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: system include j.h not allowed
+// CHECK-FIXES-NOT: #include foo
+
+#/* comment */ include /* comment */ foo
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: system include j.h not allowed
+// CHECK-FIXES-NOT: # /* comment */ include /* comment */ foo
Index: test/clang-tidy/fuchsia-restrict-system-includes-headers.cpp
===
--- /dev/null
+++ test/clang-tidy/fuchsia-restrict-system-includes-headers.cpp
@@ -0,0 +1,20 @@
+// RUN: cp -r %S/Inputs/fuchsia-restrict-system-includes %T/Inputs
+// RUN: %check_clang_tidy %s fuchsia-restrict-system-includes %t \
+// RUN: -- -config="{CheckOptions: [{key: fuchsia-restrict-system-includes.Includes, value: 'transitive.h;s.h'}]}" \
+// RUN: -system-headers -header-filter=.* \
+// RUN: -- -std=c++11 -I %T/Inputs/fuchsia-restrict-system-includes -isystem %T/Inputs/fuchsia-restrict-system-includes/system
+// RUN: FileCheck -input-file=%T/Inputs/transitive2.h %s -check-prefix=CHECK-HEADER-FIXES
+
+// transitive.h includes and
+#include
+// CHECK-MESSAGES: :1:1: warning: system include r.h not allowed, transitively included from {{(.*\/)*}}Inputs/fuchsia-restrict-system-includes/system/transitive.h
+// CHECK-MESSAGES: :2:1: warning: system include t.h not allowed, transitively included from {{(.*\/)*}}Inputs/fuchsia-restrict-system-includes/system/transitive.h
+
+// transitive.h includes and
+#include "transitive2.h"
+// CHECK-MESSAGES: :2:1: warning: system include t.h not allowed, transitively included from {{(.*\/)*}}Inputs/fuchsia-restrict-system-includes/transitive2.h
+// CHECK-HEADER-FIXES-NOT: #include
+
+int main() {
+ // f() is declared in r.h
+}
Index: test/clang-tidy/Inputs/fuchsia-restrict-system-includes/transitive2.h
===
--- /dev/null
+++ test/clang-tidy/Inputs/fuchsia-restrict-system-includes/transitive2.h
@@ -0,0 +1,2 @@
+#include
+#include
Index: test/clang-tidy/Inputs/fuchsia-restrict-system-includes/system/transitive.h
===
--- /dev/null
+++ test/clang-tidy/Inputs/fuchsia-restrict-system-includes/system/transitive.h
@@ -0,0 +1,3 @@
+#include
+#include
+#include
Index: test/clang-tidy/Inputs/fuchsia-restrict-system-includes/system/r.h
===
--- /dev/null
+++ test/clang-tidy/Inputs/fuchsia-restrict-system-includes/system/r.h
@@ -0,0 +1 @@
+void f() {}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -95,6 +95,7 @@
fuchsia-default-arguments
fuchsia-multiple-inheritance
fuchsia-overloaded-operator
+ fuchsia-restrict-system-includes
fuchsia-statically-constructed-objects
fuchsia-trailing-return
fuchsia-virtual-inheritance
Index: docs/cla
[PATCH] D46112: Allow _Atomic to be specified on incomplete types
efriedma added a comment.
I think the request was that we check that a type is trivially copyable when we
perform an atomic operation? I don't see the code for that anywhere.
Also needs some test coverage for atomic operations which aren't calls, like
"typedef struct S S; void f(_Atomic S *s, _Atomic S *s2) { *s = *s2; };".
https://reviews.llvm.org/D46112
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r331812 - Fix float->int conversion warnings when near barriers.
Author: erichkeane
Date: Tue May 8 14:26:21 2018
New Revision: 331812
URL: http://llvm.org/viewvc/llvm-project?rev=331812&view=rev
Log:
Fix float->int conversion warnings when near barriers.
As Eli brought up here: https://reviews.llvm.org/D46535
I'd previously messed up this fix by missing conversions
that are just slightly outside the range. This patch fixes
this by no longer ignoring the return value of
convertToInteger. Additionally, one of the error messages
wasn't very sensical (mentioning out of range value, when it
really was not), so it was cleaned up as well.
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/SemaCXX/warn-float-conversion.cpp
cfe/trunk/test/SemaCXX/warn-literal-conversion.cpp
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=331812&r1=331811&r2=331812&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue May 8 14:26:21
2018
@@ -3148,8 +3148,7 @@ def warn_impcast_float_integer : Warning
InGroup, DefaultIgnore;
def warn_impcast_float_to_integer : Warning<
- "implicit conversion of out of range value from %0 to %1 changes value "
- "from %2 to %3">,
+ "implicit conversion from %0 to %1 changes value from %2 to %3">,
InGroup, DefaultIgnore;
def warn_impcast_float_to_integer_out_of_range : Warning<
"implicit conversion of out of range value from %0 to %1 is undefined">,
Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=331812&r1=331811&r2=331812&view=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Tue May 8 14:26:21 2018
@@ -9419,26 +9419,25 @@ static void DiagnoseFloatingImpCast(Sema
llvm::APSInt IntegerValue(S.Context.getIntWidth(T),
T->hasUnsignedIntegerRepresentation());
- if (Value.convertToInteger(IntegerValue, llvm::APFloat::rmTowardZero,
- &isExact) == llvm::APFloat::opOK &&
- isExact) {
+ llvm::APFloat::opStatus Result = Value.convertToInteger(
+ IntegerValue, llvm::APFloat::rmTowardZero, &isExact);
+
+ if (Result == llvm::APFloat::opOK && isExact) {
if (IsLiteral) return;
return DiagnoseImpCast(S, E, T, CContext, diag::warn_impcast_float_integer,
PruneWarnings);
}
+ // Conversion of a floating-point value to a non-bool integer where the
+ // integral part cannot be represented by the integer type is undefined.
+ if (!IsBool && Result == llvm::APFloat::opInvalidOp)
+return DiagnoseImpCast(
+S, E, T, CContext,
+IsLiteral ? diag::warn_impcast_literal_float_to_integer_out_of_range
+ : diag::warn_impcast_float_to_integer_out_of_range);
+
unsigned DiagID = 0;
if (IsLiteral) {
-// Conversion of a floating-point value to a non-bool integer where the
-// integral part cannot be represented by the integer type is undefined.
-if (!IsBool &&
-((IntegerValue.isSigned() && (IntegerValue.isMaxSignedValue() ||
- IntegerValue.isMinSignedValue())) ||
- (IntegerValue.isUnsigned() &&
- (IntegerValue.isMaxValue() || IntegerValue.isMinValue()
- return DiagnoseImpCast(
- S, E, T, CContext,
- diag::warn_impcast_literal_float_to_integer_out_of_range);
// Warn on floating point literal to integer.
DiagID = diag::warn_impcast_literal_float_to_integer;
} else if (IntegerValue == 0) {
@@ -9454,19 +9453,12 @@ static void DiagnoseFloatingImpCast(Sema
return DiagnoseImpCast(S, E, T, CContext,
diag::warn_impcast_float_integer,
PruneWarnings);
}
- if (!IsBool && (IntegerValue.isMaxValue() || IntegerValue.isMinValue()))
-return DiagnoseImpCast(S, E, T, CContext,
-
diag::warn_impcast_float_to_integer_out_of_range,
- PruneWarnings);
} else { // IntegerValue.isSigned()
if (!IntegerValue.isMaxSignedValue() &&
!IntegerValue.isMinSignedValue()) {
return DiagnoseImpCast(S, E, T, CContext,
diag::warn_impcast_float_integer,
PruneWarnings);
}
- return DiagnoseImpCast(S, E, T, CContext,
- diag::warn_impcast_float_to_integer_out_of_range,
- PruneWarnings);
}
// Warn on evaluatable floating point expression to integer conversion.
DiagID = diag::warn_impcast_float_to_integer;
Modified: cfe/trunk/test/Sema
[PATCH] D43778: [clang-tidy] Adding RestrictIncludes check to Fuchsia module
Eugene.Zelenko added inline comments. Comment at: docs/ReleaseNotes.rst:116 + + Checks for allowed system includes and suggests removal of any others. If no + includes are specified, the check will exit without issuing any warnings. Is it necessary to highlight that warnings will not be emitted in case of disallowed headers are not found? Same in documentation. https://reviews.llvm.org/D43778 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46602: [clang-tidy] Store checks profiling info as CSV files
lebedev.ri created this revision.
lebedev.ri added reviewers: alexfh, sbenza.
Herald added subscribers: mgrang, xazax.hun.
Continuation of https://reviews.llvm.org/D46504.
Example output:
$ clang-tidy -enable-check-profile -store-check-profile=.
-store-check-profile-elide-prefix=. -checks=-*,readability-function-size
source.cpp 2>&1 >/dev/null
$ cat .csv
"User Time","System Time","User+System","Wall Time","Name"
8.842049973945e-01,1.20267757e-01,1.004472997370e+00,1.0031676292419434e+00,"readability-function-size"
8.842049973945e-01,1.20267757e-01,1.004472997370e+00,1.0031676292419434e+00,"Total"
There are two arguments that control profile storage:
- `-store-check-profile=` This option controls the prefix where these
per-TU profiles are stored as CSV. If the prefix is not an absolute path, it is
considered to be relative to the directory from where you have run
`clang-tidy`. All `.` and `..` patterns in the path are collapsed, and symlinks
are resolved.
**Example**: Let's suppose you have a source file named `example.cpp`,
located in `/source` directory.
- If you specify `-store-check-profile=/tmp`, then the profile will be saved
to `/tmp/source/example.cpp.csv`
- If you run `clang-tidy` from within `/foo` directory, and specify
`-store-check-profile=.`, then the profile will still be saved to
`/foo/source/example.cpp.csv`
- `-store-check-profile-elide-prefix=` When specified, this prefix will
be elided from the source file name, before prepending it with the prefix
specified by `-store-check-profile`. If the prefix is not an absolute path, it
is considered to be relative to the directory from where you have run
`clang-tidy`. All `.` and `..` patterns in the path are collapsed, and symlinks
are resolved.
**Example**: Let's suppose you have a source file named `example.cpp`,
located in `/source` directory.
- If you specify `-store-check-profile=/tmp
-store-check-profile-elide-prefix=/source` , then the profile will be saved to
`/tmp/example.cpp.csv`
- If you run `clang-tidy` from within `/source` directory, and specify
`-store-check-profile=/foo -store-check-profile-elide-prefix=.`, then the
profile will be saved to `/foo/example.cpp.csv`
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D46602
Files:
clang-tidy/ClangTidy.cpp
clang-tidy/ClangTidy.h
clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tidy/ClangTidyDiagnosticConsumer.h
clang-tidy/ClangTidyProfiling.cpp
clang-tidy/ClangTidyProfiling.h
clang-tidy/tool/ClangTidyMain.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/index.rst
test/clang-tidy/clang-tidy-store-check-profile-one-tu.cpp
Index: test/clang-tidy/clang-tidy-store-check-profile-one-tu.cpp
===
--- /dev/null
+++ test/clang-tidy/clang-tidy-store-check-profile-one-tu.cpp
@@ -0,0 +1,17 @@
+// RUN: clang-tidy -enable-check-profile -checks='-*,readability-function-size' -store-check-profile=%T -store-check-profile-elide-prefix=%s %s 2>&1 | FileCheck --match-full-lines -implicit-check-not='{{warning:|error:}}' -check-prefix=CHECK-CONSOLE %s
+// RUN: FileCheck --match-full-lines -implicit-check-not='{{warning:|error:}}' -input-file=%T/.csv -check-prefix=CHECK-FILE %s
+
+// CHECK-CONSOLE: ===-===
+// CHECK-CONSOLE-NEXT: {{.*}} --- Name ---
+// CHECK-CONSOLE-NEXT: {{.*}} readability-function-size
+// CHECK-CONSOLE-NEXT: {{.*}} Total
+// CHECK-CONSOLE-NEXT: ===-===
+
+// CHECK-FILE: {{.*}}"Wall Time","Name"
+// CHECK-FILE-NEXT: {{.*}}{{[0-9]}}.{{[0-9]+}}e{{[-+]}}{{[0-9]}}{{[0-9]}},"readability-function-size"
+// CHECK-FILE-NEXT: {{.*}}{{[0-9]}}.{{[0-9]+}}e{{[-+]}}{{[0-9]}}{{[0-9]}},"Total"
+
+class A {
+ A() {}
+ ~A() {}
+};
Index: docs/clang-tidy/index.rst
===
--- docs/clang-tidy/index.rst
+++ docs/clang-tidy/index.rst
@@ -99,114 +99,127 @@
.. code-block:: console
- $ clang-tidy -help
+ $ clang-tidy --help
USAGE: clang-tidy [options] [... ]
OPTIONS:
Generic Options:
--help- Display available options (-help-hidden for more)
--help-list - Display list of available options (-help-list-hidden for more)
--version - Display the version of this program
+-help - Display available options (-help-hidden for more)
+-help-list - Display list of available options (-help-list-hidden for more)
+-version - Display the version of this program
clang-tidy options:
--checks= -
- Comma-separated list of globs with optional '-'
- prefix. Globs
[PATCH] D46603: [Support] Print TimeRecord as CSV
lebedev.ri created this revision.
lebedev.ri added reviewers: alexfh, sbenza, bkramer, george.karpenkov.
This is needed for the continuation of https://reviews.llvm.org/D46504,
to be able to store the timings as CSV.
The floating-point values are dumped with no precision loss.
See dependent differential https://reviews.llvm.org/D46602 for details/use-case.
Repository:
rL LLVM
https://reviews.llvm.org/D46603
Files:
include/llvm/Support/Timer.h
lib/Support/Timer.cpp
Index: lib/Support/Timer.cpp
===
--- lib/Support/Timer.cpp
+++ lib/Support/Timer.cpp
@@ -22,6 +22,8 @@
#include "llvm/Support/Process.h"
#include "llvm/Support/YAMLTraits.h"
#include "llvm/Support/raw_ostream.h"
+#include
+
using namespace llvm;
// This ugly hack is brought to you courtesy of constructor/destructor ordering
@@ -169,6 +171,35 @@
OS << format("%9" PRId64 " ", (int64_t)getMemUsed());
}
+static void dumpVal(double Val, raw_ostream &OS) {
+ constexpr auto max_digits10 = std::numeric_limits::max_digits10;
+ // FIXME: can ',' be a decimal separator?
+ OS << format("%.*e", max_digits10 - 1, Val);
+}
+
+void TimeRecord::printCSV(const TimeRecord &Total, raw_ostream &OS) const {
+ int Column = 0;
+ auto comma = [&Column, &OS]() {
+if (Column)
+ OS << ','; // FIXME: can ',' be a decimal separator?
+++Column;
+ };
+ auto printColumn = [comma, &OS](bool Print, double Val) {
+if (!Print)
+ return;
+comma();
+dumpVal(Val, OS);
+ };
+
+ printColumn(Total.getUserTime(), getUserTime());
+ printColumn(Total.getSystemTime(), getSystemTime());
+ printColumn(Total.getProcessTime(), getProcessTime());
+ printColumn(true, getWallTime());
+ if (Total.getMemUsed()) {
+comma();
+OS << format("%9" PRId64, (int64_t)getMemUsed());
+ }
+}
//===--===//
// NamedRegionTimer Implementation
Index: include/llvm/Support/Timer.h
===
--- include/llvm/Support/Timer.h
+++ include/llvm/Support/Timer.h
@@ -64,6 +64,10 @@
/// Print the current time record to \p OS, with a breakdown showing
/// contributions to the \p Total time record.
void print(const TimeRecord &Total, raw_ostream &OS) const;
+
+ /// Print the current time record to \p OS as CSV, with full precision.
+ /// Only the values in this timer are printed, no percentages.
+ void printCSV(const TimeRecord &Total, raw_ostream &OS) const;
};
/// This class is used to track the amount of time spent between invocations of
Index: lib/Support/Timer.cpp
===
--- lib/Support/Timer.cpp
+++ lib/Support/Timer.cpp
@@ -22,6 +22,8 @@
#include "llvm/Support/Process.h"
#include "llvm/Support/YAMLTraits.h"
#include "llvm/Support/raw_ostream.h"
+#include
+
using namespace llvm;
// This ugly hack is brought to you courtesy of constructor/destructor ordering
@@ -169,6 +171,35 @@
OS << format("%9" PRId64 " ", (int64_t)getMemUsed());
}
+static void dumpVal(double Val, raw_ostream &OS) {
+ constexpr auto max_digits10 = std::numeric_limits::max_digits10;
+ // FIXME: can ',' be a decimal separator?
+ OS << format("%.*e", max_digits10 - 1, Val);
+}
+
+void TimeRecord::printCSV(const TimeRecord &Total, raw_ostream &OS) const {
+ int Column = 0;
+ auto comma = [&Column, &OS]() {
+if (Column)
+ OS << ','; // FIXME: can ',' be a decimal separator?
+++Column;
+ };
+ auto printColumn = [comma, &OS](bool Print, double Val) {
+if (!Print)
+ return;
+comma();
+dumpVal(Val, OS);
+ };
+
+ printColumn(Total.getUserTime(), getUserTime());
+ printColumn(Total.getSystemTime(), getSystemTime());
+ printColumn(Total.getProcessTime(), getProcessTime());
+ printColumn(true, getWallTime());
+ if (Total.getMemUsed()) {
+comma();
+OS << format("%9" PRId64, (int64_t)getMemUsed());
+ }
+}
//===--===//
// NamedRegionTimer Implementation
Index: include/llvm/Support/Timer.h
===
--- include/llvm/Support/Timer.h
+++ include/llvm/Support/Timer.h
@@ -64,6 +64,10 @@
/// Print the current time record to \p OS, with a breakdown showing
/// contributions to the \p Total time record.
void print(const TimeRecord &Total, raw_ostream &OS) const;
+
+ /// Print the current time record to \p OS as CSV, with full precision.
+ /// Only the values in this timer are printed, no percentages.
+ void printCSV(const TimeRecord &Total, raw_ostream &OS) const;
};
/// This class is used to track the amount of time spent between invocations of
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers
rjmccall added a comment. I agree that the format-specifier checker is not intended to be a portability checker. Any attempt to check portability problems has to account for two things: - Not all code is intended to be portable. If you're going to warn about portability problems, you need some way to not annoy programmers who might have good reason to say that they only care about their code running on Linux / macOS / 64-bit / 32-bit / whatever. Generally this means splitting the portability warning out as a separate warning group. - There are always established idioms for solving portability problems. In this case, a certain set of important typedefs from the C standard have been blessed with dedicated format modifiers like "z", but every other typedef in the world has to find some other solution, either by asserting that some existing format is good enough (as NSInteger does) or by introducing a macro that expands to an appropriate format (as some things in POSIX do). A proper format-portability warning would have to know about all of these conventions (in order to check that e.g. part of the format string came from the right macro), which just seems wildly out-of-scope for a compiler warning. Repository: rC Clang https://reviews.llvm.org/D42933 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[Diffusion] rL331456: [clang-tidy] Remove AnalyzeTemporaryDtors option.
lebedev.ri added subscribers: cfe-commits, alexfh, lebedev.ri. lebedev.ri raised a concern with this commit. lebedev.ri added a comment. Every single `.clang-tidy` config file that still happens to contain `AnalyzeTemporaryDtors: true/false` param specified is now **silently** (!) ignored, and a default config is used. I have found that out the hard way. Users: alexfh (Author) lebedev.ri (Auditor) https://reviews.llvm.org/rL331456 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43778: [clang-tidy] Adding RestrictIncludes check to Fuchsia module
aaron.ballman added inline comments.
Comment at: clang-tidy/fuchsia/RestrictSystemIncludesCheck.cpp:67
+ llvm::SmallDenseMap IncludeDirectives;
+ llvm::StringMap IsSystem;
+
Rather than use this `StringMap`, can you change `InclusionDirective()` to
filter out includes you don't care about? Something along the lines of:
```
FileID FID = SM.translateFile(File);
assert(FID != FileID() && "Source Manager does not know about this header
file");
bool Invalid;
const SLocEntry &Entry = SM.getSLocEntry(FID, &Invalid);
if (!Invalid && SrcMgr::isSystem(Entry.getFile().getFileCharacteristic())) {
// It's a system file.
}
```
This way you don't have to store all the path information and test against
paths in `EndOfMainFile()`, unless I'm missing something. Note, this is
untested code and perhaps that assert will fire (maybe SourceManager is unaware
of this file by the time InclusionDirective() is called).
Alternatively, you could alter InclusionDirective() to also pass in the
`CharacteristicKind` calculated by `Preprocessor::HandleIncludeDirective()` as
that seems like generally useful information for the callback to have on hand.
Comment at: clang-tidy/fuchsia/RestrictSystemIncludesCheck.h:1-2
+//===--- RestrictSystemIncludesCheck.h - clang-tidy-*-
+//C++-*-===//
+//
This should only be on a single line -- you can remove some `-` characters.
Comment at: docs/clang-tidy/checks/fuchsia-restrict-system-includes.rst:32
+ A string containing a semi-colon separated list of allowed include
filenames.
+ The default is an empty string, which allows all includes.
This default seems a bit odd to me, but perhaps it's fine. What's novel is that
the check is a no-op by default, so how do Fuchsia developers get the correct
list? Or is there no canonical list and developers are expected to populate
their own manually?
https://reviews.llvm.org/D43778
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r331802 - Add missing newlines to cl::extrahelp uses
Author: sas
Date: Tue May 8 12:46:29 2018
New Revision: 331802
URL: http://llvm.org/viewvc/llvm-project?rev=331802&view=rev
Log:
Add missing newlines to cl::extrahelp uses
Modified:
cfe/trunk/docs/LibASTMatchersTutorial.rst
cfe/trunk/docs/LibTooling.rst
cfe/trunk/include/clang/Tooling/CommonOptionsParser.h
Modified: cfe/trunk/docs/LibASTMatchersTutorial.rst
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LibASTMatchersTutorial.rst?rev=331802&r1=331801&r2=331802&view=diff
==
--- cfe/trunk/docs/LibASTMatchersTutorial.rst (original)
+++ cfe/trunk/docs/LibASTMatchersTutorial.rst Tue May 8 12:46:29 2018
@@ -146,7 +146,7 @@ documentation `_.
static cl::extrahelp CommonHelp(CommonOptionsParser::HelpMessage);
// A help message for this specific tool can be added afterwards.
- static cl::extrahelp MoreHelp("\nMore help text...");
+ static cl::extrahelp MoreHelp("\nMore help text...\n");
int main(int argc, const char **argv) {
CommonOptionsParser OptionsParser(argc, argv, MyToolCategory);
Modified: cfe/trunk/docs/LibTooling.rst
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LibTooling.rst?rev=331802&r1=331801&r2=331802&view=diff
==
--- cfe/trunk/docs/LibTooling.rst (original)
+++ cfe/trunk/docs/LibTooling.rst Tue May 8 12:46:29 2018
@@ -130,7 +130,7 @@ version of this example tool is also che
static cl::extrahelp CommonHelp(CommonOptionsParser::HelpMessage);
// A help message for this specific tool can be added afterwards.
- static cl::extrahelp MoreHelp("\nMore help text...");
+ static cl::extrahelp MoreHelp("\nMore help text...\n");
int main(int argc, const char **argv) {
CommonOptionsParser OptionsParser(argc, argv, MyToolCategory);
Modified: cfe/trunk/include/clang/Tooling/CommonOptionsParser.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/CommonOptionsParser.h?rev=331802&r1=331801&r2=331802&view=diff
==
--- cfe/trunk/include/clang/Tooling/CommonOptionsParser.h (original)
+++ cfe/trunk/include/clang/Tooling/CommonOptionsParser.h Tue May 8 12:46:29
2018
@@ -52,7 +52,7 @@ namespace tooling {
///
/// static cl::OptionCategory MyToolCategory("My tool options");
/// static cl::extrahelp CommonHelp(CommonOptionsParser::HelpMessage);
-/// static cl::extrahelp MoreHelp("\nMore help text...");
+/// static cl::extrahelp MoreHelp("\nMore help text...\n");
/// static cl::opt YourOwnOption(...);
/// ...
///
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers
smeenai added a comment. In https://reviews.llvm.org/D42933#1092048, @rjmccall wrote: > I agree that the format-specifier checker is not intended to be a portability > checker. > > Any attempt to check portability problems has to account for two things: > > - Not all code is intended to be portable. If you're going to warn about > portability problems, you need some way to not annoy programmers who might > have good reason to say that they only care about their code running on Linux > / macOS / 64-bit / 32-bit / whatever. Generally this means splitting the > portability warning out as a separate warning group. > - There are always established idioms for solving portability problems. In > this case, a certain set of important typedefs from the C standard have been > blessed with dedicated format modifiers like "z", but every other typedef in > the world has to find some other solution, either by asserting that some > existing format is good enough (as NSInteger does) or by introducing a macro > that expands to an appropriate format (as some things in POSIX do). A proper > format-portability warning would have to know about all of these conventions > (in order to check that e.g. part of the format string came from the right > macro), which just seems wildly out-of-scope for a compiler warning. Apple's current recommendation for using printf with the NSInteger types is casting to a long, per https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/Strings/Articles/formatSpecifiers.html. Are you suggesting that recommendation would change to using `%zd` instead? Repository: rC Clang https://reviews.llvm.org/D42933 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46602: [clang-tidy] Store checks profiling info as CSV files
Eugene.Zelenko added a comment. I think will be good idea to store data in JSON format too. Comment at: docs/ReleaseNotes.rst:60 +- clang-tidy learned to store checks profiling info as CSV files. + May be //Profile information could be stored in SSV format.// sounds better? Comment at: docs/clang-tidy/index.rst:762 + +To enable profiling info collection, use ``-enable-check-profile`` argument. +The timings will be outputted to the ``stderr`` as a table. Example output: Please use ` for command line options and other non-C/C++ constructs. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46602 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46602: [clang-tidy] Store checks profiling info as CSV files
lebedev.ri added a comment. In https://reviews.llvm.org/D46602#1092084, @Eugene.Zelenko wrote: > I think will be good idea to store data in JSON format too. Yeah, i have thought about it, and i'm not sure. The output is so dumb so there isn't even much point in using anything more advanced than CSV. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46602 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46602: [clang-tidy] Store checks profiling info as CSV files
rja added a comment. +1 for JSON Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46602 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libcxx] r331818 - Revert "Emit an error when mixing and "
Author: vsapsai
Date: Tue May 8 15:50:35 2018
New Revision: 331818
URL: http://llvm.org/viewvc/llvm-project?rev=331818&view=rev
Log:
Revert "Emit an error when mixing and "
It reverts commit r331379 because turned out `__ALLOW_STDC_ATOMICS_IN_CXX__`
doesn't work well in practice.
Removed:
libcxx/trunk/test/libcxx/atomics/c_compatibility.fail.cpp
Modified:
libcxx/trunk/include/atomic
Modified: libcxx/trunk/include/atomic
URL:
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/atomic?rev=331818&r1=331817&r2=331818&view=diff
==
--- libcxx/trunk/include/atomic (original)
+++ libcxx/trunk/include/atomic Tue May 8 15:50:35 2018
@@ -555,9 +555,6 @@ void atomic_signal_fence(memory_order m)
#if !defined(_LIBCPP_HAS_C_ATOMIC_IMP) && !defined(_LIBCPP_HAS_GCC_ATOMIC_IMP)
#error is not implemented
#endif
-#ifdef __ALLOW_STDC_ATOMICS_IN_CXX__
-#error is incompatible with the C++ standard library
-#endif
#if _LIBCPP_STD_VER > 14
# define __cpp_lib_atomic_is_always_lock_free 201603L
Removed: libcxx/trunk/test/libcxx/atomics/c_compatibility.fail.cpp
URL:
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/libcxx/atomics/c_compatibility.fail.cpp?rev=331817&view=auto
==
--- libcxx/trunk/test/libcxx/atomics/c_compatibility.fail.cpp (original)
+++ libcxx/trunk/test/libcxx/atomics/c_compatibility.fail.cpp (removed)
@@ -1,28 +0,0 @@
-//===--===//
-//
-// 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.
-//
-//===--===//
-//
-// UNSUPPORTED: libcpp-has-no-threads
-//
-//
-
-// Test that including fails to compile when we want to use C atomics
-// in C++ and have corresponding macro defined.
-
-// MODULES_DEFINES: __ALLOW_STDC_ATOMICS_IN_CXX__
-#ifndef __ALLOW_STDC_ATOMICS_IN_CXX__
-#define __ALLOW_STDC_ATOMICS_IN_CXX__
-#endif
-
-#include
-// expected-error@atomic:* {{ is incompatible with the C++
standard library}}
-
-int main()
-{
-}
-
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46332: [X86] Only enable the __ud2 and __int2c builtins if intrin.h has been included.
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm https://reviews.llvm.org/D46332 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46603: [Support] Print TimeRecord as CSV
george.karpenkov added a comment. @lebedev.ri LLVM already has facilities for outputting statistics in JSON, it seems that would be sufficient for your purposes? I did something similar for the static analyzer in https://reviews.llvm.org/D43131 Repository: rL LLVM https://reviews.llvm.org/D46603 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r331822 - Partially revert r331456: [clang-tidy] Remove AnalyzeTemporaryDtors option.
Author: lebedevri
Date: Tue May 8 16:15:58 2018
New Revision: 331822
URL: http://llvm.org/viewvc/llvm-project?rev=331822&view=rev
Log:
Partially revert r331456: [clang-tidy] Remove AnalyzeTemporaryDtors option.
That broke every single .clang-tidy config out there
which happened to specify AnalyzeTemporaryDtors option:
YAML:5:24: error: unknown key 'AnalyzeTemporaryDtors'
AnalyzeTemporaryDtors: false
^
Error parsing <...>/.clang-tidy: Invalid argument
More so, that error isn't actually a error, the
clang-tidy does not exit with $? != 0, it continues
with the default config.
Surely this breakage isn't the intended behavior.
But if it is, feel free to revert this commit.
Modified:
clang-tools-extra/trunk/clang-tidy/ClangTidyOptions.cpp
clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyOptionsTest.cpp
Modified: clang-tools-extra/trunk/clang-tidy/ClangTidyOptions.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidyOptions.cpp?rev=331822&r1=331821&r2=331822&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/ClangTidyOptions.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidyOptions.cpp Tue May 8 16:15:58
2018
@@ -83,9 +83,11 @@ template <> struct MappingTraits NOpts(
IO, Options.CheckOptions);
+bool Ignored = false;
IO.mapOptional("Checks", Options.Checks);
IO.mapOptional("WarningsAsErrors", Options.WarningsAsErrors);
IO.mapOptional("HeaderFilterRegex", Options.HeaderFilterRegex);
+IO.mapOptional("AnalyzeTemporaryDtors", Ignored); // legacy compatibility
IO.mapOptional("FormatStyle", Options.FormatStyle);
IO.mapOptional("User", Options.User);
IO.mapOptional("CheckOptions", NOpts->Options);
Modified: clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyOptionsTest.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyOptionsTest.cpp?rev=331822&r1=331821&r2=331822&view=diff
==
--- clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyOptionsTest.cpp
(original)
+++ clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyOptionsTest.cpp Tue
May 8 16:15:58 2018
@@ -58,6 +58,7 @@ TEST(ParseConfiguration, ValidConfigurat
llvm::ErrorOr Options =
parseConfiguration("Checks: \"-*,misc-*\"\n"
"HeaderFilterRegex: \".*\"\n"
+ "AnalyzeTemporaryDtors: true\n"
"User: some.user");
EXPECT_TRUE(!!Options);
EXPECT_EQ("-*,misc-*", *Options->Checks);
@@ -69,6 +70,7 @@ TEST(ParseConfiguration, MergeConfigurat
llvm::ErrorOr Options1 = parseConfiguration(R"(
Checks: "check1,check2"
HeaderFilterRegex: "filter1"
+ AnalyzeTemporaryDtors: true
User: user1
ExtraArgs: ['arg1', 'arg2']
ExtraArgsBefore: ['arg-before1', 'arg-before2']
@@ -77,6 +79,7 @@ TEST(ParseConfiguration, MergeConfigurat
llvm::ErrorOr Options2 = parseConfiguration(R"(
Checks: "check3,check4"
HeaderFilterRegex: "filter2"
+ AnalyzeTemporaryDtors: false
User: user2
ExtraArgs: ['arg3', 'arg4']
ExtraArgsBefore: ['arg-before3', 'arg-before4']
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46603: [Support] Print TimeRecord as CSV
lebedev.ri added a comment. In https://reviews.llvm.org/D46603#1092135, @george.karpenkov wrote: > @lebedev.ri LLVM already has facilities for outputting statistics in JSON, it > seems that would be sufficient for your purposes? > I did something similar for the static analyzer in > https://reviews.llvm.org/D43131 YAML != JSON. I did see all that `TimerGroup` stuff. I will take a second look, but the `dumpVal()` change will still be needed either way. Repository: rL LLVM https://reviews.llvm.org/D46603 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers
jfb added a comment. In https://reviews.llvm.org/D42933#1092077, @smeenai wrote: > In https://reviews.llvm.org/D42933#1092048, @rjmccall wrote: > > > I agree that the format-specifier checker is not intended to be a > > portability checker. > I don't disagree with the original intent, but AFAICT that's exactly the intent of the warning I'd like to get rid of. I'm making a very narrow point: there is no portability problem with the warning I'm specifically trying to silence (using `%z` with `NS[U]Integer` on Darwin). If we decide that `-Wformat` shouldn't emit portability warnings then my point is moot, so let's see if people agree to that. If not, then my point still stands. >> Any attempt to check portability problems has to account for two things: >> >> - Not all code is intended to be portable. If you're going to warn about >> portability problems, you need some way to not annoy programmers who might >> have good reason to say that they only care about their code running on >> Linux / macOS / 64-bit / 32-bit / whatever. Generally this means splitting >> the portability warning out as a separate warning group. Right, it seems like recently `-Wformat` went against this, and users aren't thrilled. >> - There are always established idioms for solving portability problems. In >> this case, a certain set of important typedefs from the C standard have been >> blessed with dedicated format modifiers like "z", but every other typedef in >> the world has to find some other solution, either by asserting that some >> existing format is good enough (as NSInteger does) or by introducing a macro >> that expands to an appropriate format (as some things in POSIX do). A >> proper format-portability warning would have to know about all of these >> conventions (in order to check that e.g. part of the format string came from >> the right macro), which just seems wildly out-of-scope for a compiler >> warning. We could provide a macro for `NS[U]Integer`, but it's been long enough and there's enough existing code with `%z`. Not sure the code churn on user is worth it, if we can instead say "`%z` works". > Apple's current recommendation for using printf with the NSInteger types is > casting to a long, per > https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/Strings/Articles/formatSpecifiers.html. > Are you suggesting that recommendation would change to using `%zd` instead? Indeed, I believe that came from https://github.com/llvm-mirror/clang/commit/ec08735e1b6a51c11533311b774bf6336c0a5d63 and I intend to update documentation once the compiler is updated. It's not *wrong*, it's kinda more portable in that it's what you probably want to do if it's not `NS[U]Integer`, but it's more typing and users clearly haven't been following the recommendation (because let's be honest, `%z` is fine). Repository: rC Clang https://reviews.llvm.org/D42933 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46610: [CMake] Set CMAKE_BUILD_WITH_INSTALL_RPATH for Fuchsia runtimes
phosek created this revision.
phosek added a reviewer: mcgrathr.
Herald added subscribers: cfe-commits, mgorny.
This doesn't make any difference since we don't use RPATH/RUNPATH
on Fuchsia but it avoids the CMake error when re-linking libraries
while building with Ninja.
Repository:
rC Clang
https://reviews.llvm.org/D46610
Files:
clang/cmake/caches/Fuchsia-stage2.cmake
Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -67,6 +67,7 @@
foreach(target x86_64;aarch64)
set(RUNTIMES_${target}-fuchsia_CMAKE_SYSTEM_NAME Fuchsia CACHE STRING "")
set(RUNTIMES_${target}-fuchsia_CMAKE_BUILD_TYPE
${FUCHSIA_RUNTIMES_BUILD_TYPE} CACHE STRING "")
+ set(RUNTIMES_${target}-fuchsia_CMAKE_BUILD_WITH_INSTALL_RPATH ON CACHE
STRING "")
set(RUNTIMES_${target}-fuchsia_CMAKE_ASM_FLAGS ${FUCHSIA_${target}_C_FLAGS}
CACHE PATH "")
set(RUNTIMES_${target}-fuchsia_CMAKE_C_FLAGS ${FUCHSIA_${target}_C_FLAGS}
CACHE PATH "")
set(RUNTIMES_${target}-fuchsia_CMAKE_CXX_FLAGS
${FUCHSIA_${target}_CXX_FLAGS} CACHE PATH "")
Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -67,6 +67,7 @@
foreach(target x86_64;aarch64)
set(RUNTIMES_${target}-fuchsia_CMAKE_SYSTEM_NAME Fuchsia CACHE STRING "")
set(RUNTIMES_${target}-fuchsia_CMAKE_BUILD_TYPE ${FUCHSIA_RUNTIMES_BUILD_TYPE} CACHE STRING "")
+ set(RUNTIMES_${target}-fuchsia_CMAKE_BUILD_WITH_INSTALL_RPATH ON CACHE STRING "")
set(RUNTIMES_${target}-fuchsia_CMAKE_ASM_FLAGS ${FUCHSIA_${target}_C_FLAGS} CACHE PATH "")
set(RUNTIMES_${target}-fuchsia_CMAKE_C_FLAGS ${FUCHSIA_${target}_C_FLAGS} CACHE PATH "")
set(RUNTIMES_${target}-fuchsia_CMAKE_CXX_FLAGS ${FUCHSIA_${target}_CXX_FLAGS} CACHE PATH "")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46610: [CMake] Set CMAKE_BUILD_WITH_INSTALL_RPATH for Fuchsia runtimes
phosek added a comment. Actually after testing this I believe this is really what we want, without this option CMake sets RPATH for build libraries and then removes it when installing these. With this option it doesn't even set the RPATH for build libraries. Repository: rC Clang https://reviews.llvm.org/D46610 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46612: [CMake] Include llvm-strip in Fuchsia toolchain distribution
phosek created this revision. phosek added a reviewer: jakehehrlich. Herald added subscribers: cfe-commits, mgorny. Now that llvm-strip is available, include it in the Fuchsia toolchain. Repository: rC Clang https://reviews.llvm.org/D46612 Files: clang/cmake/caches/Fuchsia-stage2.cmake Index: clang/cmake/caches/Fuchsia-stage2.cmake === --- clang/cmake/caches/Fuchsia-stage2.cmake +++ clang/cmake/caches/Fuchsia-stage2.cmake @@ -108,6 +108,7 @@ llvm-readelf llvm-readobj llvm-size + llvm-strip llvm-symbolizer opt sancov Index: clang/cmake/caches/Fuchsia-stage2.cmake === --- clang/cmake/caches/Fuchsia-stage2.cmake +++ clang/cmake/caches/Fuchsia-stage2.cmake @@ -108,6 +108,7 @@ llvm-readelf llvm-readobj llvm-size + llvm-strip llvm-symbolizer opt sancov ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45470: Emit an error when mixing and
vsapsai reopened this revision. vsapsai added a comment. This revision is now accepted and ready to land. `__ALLOW_STDC_ATOMICS_IN_CXX__` approach didn't work in practice, I've reverted all changes. Repository: rC Clang https://reviews.llvm.org/D45470 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45470: Emit an error when mixing and
vsapsai updated this revision to Diff 145817. vsapsai added a comment. Here is another approach that should emit an error only when mixing headers causes compilation problems. Have no ideas how to test the change. `-verify` doesn't work with fatal errors and libcxx doesn't use FileCheck. Performed only manual testing. https://reviews.llvm.org/D45470 Files: libcxx/include/atomic Index: libcxx/include/atomic === --- libcxx/include/atomic +++ libcxx/include/atomic @@ -555,6 +555,9 @@ #if !defined(_LIBCPP_HAS_C_ATOMIC_IMP) && !defined(_LIBCPP_HAS_GCC_ATOMIC_IMP) #error is not implemented #endif +#ifdef kill_dependency +#error C++ standard library is incompatible with +#endif #if _LIBCPP_STD_VER > 14 # define __cpp_lib_atomic_is_always_lock_free 201603L Index: libcxx/include/atomic === --- libcxx/include/atomic +++ libcxx/include/atomic @@ -555,6 +555,9 @@ #if !defined(_LIBCPP_HAS_C_ATOMIC_IMP) && !defined(_LIBCPP_HAS_GCC_ATOMIC_IMP) #error is not implemented #endif +#ifdef kill_dependency +#error C++ standard library is incompatible with +#endif #if _LIBCPP_STD_VER > 14 # define __cpp_lib_atomic_is_always_lock_free 201603L ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46612: [CMake] Include llvm-strip in Fuchsia toolchain distribution
This revision was automatically updated to reflect the committed changes. Closed by commit rC331826: [CMake] Include llvm-strip in Fuchsia toolchain distribution (authored by phosek, committed by ). Changed prior to commit: https://reviews.llvm.org/D46612?vs=145815&id=145818#toc Repository: rC Clang https://reviews.llvm.org/D46612 Files: cmake/caches/Fuchsia-stage2.cmake Index: cmake/caches/Fuchsia-stage2.cmake === --- cmake/caches/Fuchsia-stage2.cmake +++ cmake/caches/Fuchsia-stage2.cmake @@ -108,6 +108,7 @@ llvm-readelf llvm-readobj llvm-size + llvm-strip llvm-symbolizer opt sancov Index: cmake/caches/Fuchsia-stage2.cmake === --- cmake/caches/Fuchsia-stage2.cmake +++ cmake/caches/Fuchsia-stage2.cmake @@ -108,6 +108,7 @@ llvm-readelf llvm-readobj llvm-size + llvm-strip llvm-symbolizer opt sancov ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r331826 - [CMake] Include llvm-strip in Fuchsia toolchain distribution
Author: phosek Date: Tue May 8 17:05:28 2018 New Revision: 331826 URL: http://llvm.org/viewvc/llvm-project?rev=331826&view=rev Log: [CMake] Include llvm-strip in Fuchsia toolchain distribution Now that llvm-strip is available, include it in the Fuchsia toolchain. Differential Revision: https://reviews.llvm.org/D46612 Modified: cfe/trunk/cmake/caches/Fuchsia-stage2.cmake Modified: cfe/trunk/cmake/caches/Fuchsia-stage2.cmake URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/cmake/caches/Fuchsia-stage2.cmake?rev=331826&r1=331825&r2=331826&view=diff == --- cfe/trunk/cmake/caches/Fuchsia-stage2.cmake (original) +++ cfe/trunk/cmake/caches/Fuchsia-stage2.cmake Tue May 8 17:05:28 2018 @@ -108,6 +108,7 @@ set(LLVM_TOOLCHAIN_TOOLS llvm-readelf llvm-readobj llvm-size + llvm-strip llvm-symbolizer opt sancov ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46610: [CMake] Set CMAKE_BUILD_WITH_INSTALL_RPATH for Fuchsia runtimes
jakehehrlich added a comment. Need some clarification on this option 1. The rpath is the path from which libs mentioned in .dynamic are relative to? 2. Normally cmake first links binaries without setting the RPATH and then sets it later. 3. This makes RPATH always equal to the empty string and avoids the "relinking" step that resets RPATH If so this LGTM Repository: rC Clang https://reviews.llvm.org/D46610 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46300: [Clang] Implement function attribute no_stack_protector.
rnk added inline comments.
Comment at: include/clang/Basic/Attr.td:1494
+def NoStackProtector : InheritableAttr {
+ let Spellings = [GCC<"no_stack_protector">];
+ let Subjects = SubjectList<[Function]>;
aaron.ballman wrote:
> manojgupta wrote:
> > aaron.ballman wrote:
> > > This is not a GCC attribute, so this should use the Clang spelling.
> > >
> > > However, why spell the attribute this way rather than use the GCC
> > > spelling (`optimize("no-stack-protector")`?
> > Thanks, I have changed it to use Clang spelling.
> >
> > Regarding __attribute__((optimize("..."))), it is a generic facility in GCC
> > that works for many optimizer flags.
> > Clang currently does not support this syntax currently instead preferring
> > its own version for some options e.g. -O0.
> > e.g.
> > ```
> > __attribute__((optimize("O0"))) // clang version is
> > __attribute__((optnone))
> > ```
> > If we want to support the GCC syntax, future expectation would be support
> > more flags under this syntax. Is that the path we want to take (I do not
> > know the history related to previous syntax decisions but better GCC
> > compatibility will be a nice thing to have)
> The history of `optnone` predates my involvement with Clang and I've not been
> able to find the original review thread (I did find the one where I gave my
> LGTM on the original patch, but that was a resubmission after the original
> design was signed off).
>
> I'm not keen on attributes that have the same semantics but differ in
> spelling from attributes supported by GCC unless there's a good reason to
> deviate. Given that we've already deviated, I'd like to understand why better
> -- I don't want to push you to implement something we've already decided was
> a poor design, but I also don't want to accept code if we can instead use
> syntax that is compatible with GCC.
I do not think we want to pursue supporting generic optimization flags with
`__attribute__((optimize("...")))`.
Part of the reason we only support `optnone` is that LLVM's pass pipeline is
global to the entire module. It's not trivial to enable optimizations for a
single function, but it is much easier to have optimization passes skip
functions marked `optnone`.
Repository:
rC Clang
https://reviews.llvm.org/D46300
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46613: _Atomic of empty struct shouldn't assert
jfb created this revision.
jfb added reviewers: arphaman, rjmccall.
Herald added subscribers: cfe-commits, aheejin.
An _Atomic of an empty struct is pretty silly. In general we just widen empty
structs to hold a byte's worth of storage, and we represent size and alignment
as 0 internally and let LLVM figure out what to do. For _Atomic it's a bit
different: the memory model mandates concrete effects occur when atomic
operations occur, so in most cases actual instructions need to get emitted.
It's really not worth trying to optimize empty struct atomics by figuring out
e.g. that a fence would do, even though sane compilers should do optimize
atomics. Further, wg21.link/p0528 will fix C++20 atomics with padding bits so
that cmpxchg on them works, which means that we'll likely need to do the
zero-init song and dance for empty atomic structs anyways (and I think we
shouldn't special-case this behavior to C++20 because prior standards are just
broken).
This patch therefore makes a minor change to r176658 "Promote atomic type sizes
up to a power of two": if the width of the atomic's value type is 0, just use 1
byte for width and alignment.
This fixes an assertion:
(NumBits >= MIN_INT_BITS && "bitwidth too small"), function get, file
../lib/IR/Type.cpp, line 241.
It seems like this has run into other assertions before (namely the unreachable
Kind check in ImpCastExprToType), but I haven't reproduced that issue with
tip-of-tree.
rdar://problem/39678063
Repository:
rC Clang
https://reviews.llvm.org/D46613
Files:
lib/AST/ASTContext.cpp
test/CodeGen/c11atomics-ios.c
test/CodeGen/c11atomics.c
Index: test/CodeGen/c11atomics.c
===
--- test/CodeGen/c11atomics.c
+++ test/CodeGen/c11atomics.c
@@ -474,3 +474,17 @@
// CHECK: ret i1 [[RES]]
return __c11_atomic_compare_exchange_strong(addr, desired, *new, 5, 5);
}
+
+struct Empty {};
+
+struct Empty test_empty_struct_load(_Atomic(struct Empty)* empty) {
+ // CHECK-LABEL: @test_empty_struct_load(
+ // CHECK: call arm_aapcscc zeroext i8 @__atomic_load_1(i8* %{{.*}}, i32 5)
+ return __c11_atomic_load(empty, 5);
+}
+
+void test_empty_struct_store(_Atomic(struct Empty)* empty, struct Empty value)
{
+ // CHECK-LABEL: @test_empty_struct_store(
+ // CHECK: call arm_aapcscc void @__atomic_store_1(i8* %{{.*}}, i8 zeroext
%{{.*}}, i32 5)
+ __c11_atomic_store(empty, value, 5);
+}
Index: test/CodeGen/c11atomics-ios.c
===
--- test/CodeGen/c11atomics-ios.c
+++ test/CodeGen/c11atomics-ios.c
@@ -319,3 +319,19 @@
return __c11_atomic_compare_exchange_strong(addr, desired, *new, 5, 5);
}
+
+struct Empty {};
+
+struct Empty testEmptyStructLoad(_Atomic(struct Empty)* empty) {
+ // CHECK-LABEL: @testEmptyStructLoad(
+ // CHECK-NOT: @__atomic_load
+ // CHECK: load atomic i8, i8* %{{.*}} seq_cst, align 1
+ return *empty;
+}
+
+void testEmptyStructStore(_Atomic(struct Empty)* empty, struct Empty value) {
+ // CHECK-LABEL: @testEmptyStructStore(
+ // CHECK-NOT: @__atomic_store
+ // CHECK: store atomic i8 %{{.*}}, i8* %{{.*}} seq_cst, align 1
+ *empty = value;
+}
Index: lib/AST/ASTContext.cpp
===
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -1958,10 +1958,16 @@
Width = Info.Width;
Align = Info.Align;
-// If the size of the type doesn't exceed the platform's max
-// atomic promotion width, make the size and alignment more
-// favorable to atomic operations:
-if (Width != 0 && Width <= Target->getMaxAtomicPromoteWidth()) {
+if (!Width) {
+ // An otherwise zero-sized type should still generate an
+ // atomic operation.
+ Width = Target->getCharWidth();
+ Align = Target->getCharWidth();
+} else if (Width <= Target->getMaxAtomicPromoteWidth()) {
+ // If the size of the type doesn't exceed the platform's max
+ // atomic promotion width, make the size and alignment more
+ // favorable to atomic operations:
+
// Round the size up to a power of 2.
if (!llvm::isPowerOf2_64(Width))
Width = llvm::NextPowerOf2(Width);
Index: test/CodeGen/c11atomics.c
===
--- test/CodeGen/c11atomics.c
+++ test/CodeGen/c11atomics.c
@@ -474,3 +474,17 @@
// CHECK: ret i1 [[RES]]
return __c11_atomic_compare_exchange_strong(addr, desired, *new, 5, 5);
}
+
+struct Empty {};
+
+struct Empty test_empty_struct_load(_Atomic(struct Empty)* empty) {
+ // CHECK-LABEL: @test_empty_struct_load(
+ // CHECK: call arm_aapcscc zeroext i8 @__atomic_load_1(i8* %{{.*}}, i32 5)
+ return __c11_atomic_load(empty, 5);
+}
+
+void test_empty_struct_store(_Atomic(struct Empty)* empty, struct Empty value) {
+ // CHECK-LABEL: @test_empty_struct_store(
+ // CHECK: call arm_aapcscc void @__atomic_store_1(i8* %{{.*}}, i8 zeroext %
[PATCH] D46520: [Driver] Use -fuse-line-directives by default in MSVC mode
rnk reopened this revision. rnk added a comment. This revision is now accepted and ready to land. Please don't do this, this is actually really problematic, since `#line` directives lose information about what's a system header. That means the result of -E usually won't compile, since Windows headers are typically full of warnings and default-error warnings. Repository: rL LLVM https://reviews.llvm.org/D46520 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46614: [clang] Adding CharacteristicKind to PPCallbacks::InclusionDirective
juliehockett created this revision.
juliehockett added a reviewer: aaron.ballman.
juliehockett added a project: clang.
Herald added subscribers: kbarton, nemanjai.
Adding a SrcMgr::CharacteristicKind parameter to the InclusionDirective in
PPCallbacks, and updating calls to that function. This will be useful in
https://reviews.llvm.org/D43778 to determine which includes are system headers.
https://reviews.llvm.org/D46614
Files:
include/clang/Lex/PPCallbacks.h
include/clang/Lex/PreprocessingRecord.h
lib/CodeGen/MacroPPCallbacks.cpp
lib/CodeGen/MacroPPCallbacks.h
lib/Frontend/DependencyFile.cpp
lib/Frontend/DependencyGraph.cpp
lib/Frontend/ModuleDependencyCollector.cpp
lib/Frontend/PrintPreprocessedOutput.cpp
lib/Frontend/Rewrite/InclusionRewriter.cpp
lib/Lex/PPDirectives.cpp
lib/Lex/PreprocessingRecord.cpp
tools/libclang/Indexing.cpp
unittests/Lex/PPCallbacksTest.cpp
Index: unittests/Lex/PPCallbacksTest.cpp
===
--- unittests/Lex/PPCallbacksTest.cpp
+++ unittests/Lex/PPCallbacksTest.cpp
@@ -39,16 +39,18 @@
StringRef FileName, bool IsAngled,
CharSourceRange FilenameRange, const FileEntry *File,
StringRef SearchPath, StringRef RelativePath,
- const Module *Imported) override {
- this->HashLoc = HashLoc;
- this->IncludeTok = IncludeTok;
- this->FileName = FileName.str();
- this->IsAngled = IsAngled;
- this->FilenameRange = FilenameRange;
- this->File = File;
- this->SearchPath = SearchPath.str();
- this->RelativePath = RelativePath.str();
- this->Imported = Imported;
+ const Module *Imported,
+ SrcMgr::CharacteristicKind FileType) override {
+this->HashLoc = HashLoc;
+this->IncludeTok = IncludeTok;
+this->FileName = FileName.str();
+this->IsAngled = IsAngled;
+this->FilenameRange = FilenameRange;
+this->File = File;
+this->SearchPath = SearchPath.str();
+this->RelativePath = RelativePath.str();
+this->Imported = Imported;
+this->FileType = FileType;
}
SourceLocation HashLoc;
@@ -60,6 +62,7 @@
SmallString<16> SearchPath;
SmallString<16> RelativePath;
const Module* Imported;
+ SrcMgr::CharacteristicKind FileType;
};
// Stub to collect data from PragmaOpenCLExtension callbacks.
Index: tools/libclang/Indexing.cpp
===
--- tools/libclang/Indexing.cpp
+++ tools/libclang/Indexing.cpp
@@ -249,7 +249,8 @@
StringRef FileName, bool IsAngled,
CharSourceRange FilenameRange, const FileEntry *File,
StringRef SearchPath, StringRef RelativePath,
- const Module *Imported) override {
+ const Module *Imported,
+ SrcMgr::CharacteristicKind FileType) override {
bool isImport = (IncludeTok.is(tok::identifier) &&
IncludeTok.getIdentifierInfo()->getPPKeywordID() == tok::pp_import);
DataConsumer.ppIncludedFile(HashLoc, FileName, File, isImport, IsAngled,
Index: lib/Lex/PreprocessingRecord.cpp
===
--- lib/Lex/PreprocessingRecord.cpp
+++ lib/Lex/PreprocessingRecord.cpp
@@ -471,7 +471,8 @@
const FileEntry *File,
StringRef SearchPath,
StringRef RelativePath,
-const Module *Imported) {
+const Module *Imported,
+SrcMgr::CharacteristicKind FileType) {
InclusionDirective::InclusionKind Kind = InclusionDirective::Include;
switch (IncludeTok.getIdentifierInfo()->getPPKeywordID()) {
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1968,7 +1968,7 @@
HashLoc, IncludeTok,
LangOpts.MSVCCompat ? NormalizedPath.c_str() : Filename, isAngled,
FilenameRange, File, SearchPath, RelativePath,
-ShouldEnter ? nullptr : SuggestedModule.getModule());
+ShouldEnter ? nullptr : SuggestedModule.getModule(), FileCharacter);
if (SkipHeader && !SuggestedModule.getModule())
Callbacks->FileSkipped(*File, FilenameTok, FileCharacter);
}
Index: lib/Frontend/Rewrite/InclusionRewriter.cpp
===
--- lib/Frontend/Rewrite/InclusionRewriter.cpp
+++ lib/Frontend/Rewrite/InclusionRewriter.cpp
@@ -77,7 +77,8 @@
StringRef FileName, bool IsAngled,
CharSourceRange FilenameRange, const FileEntry *File,
StringRef SearchPath, StringRef RelativePath,
- const Module *Imported) override;
+ const Module *Imported,
+ SrcMgr
[PATCH] D46615: [tools] Updating PPCallbacks::InclusionDirective calls
juliehockett created this revision.
juliehockett added a reviewer: aaron.ballman.
juliehockett added a project: clang-tools-extra.
Herald added subscribers: jkorous, kbarton, ioeric, nemanjai.
[[ https://reviews.llvm.org/D46614 | [https://reviews.llvm.org/D46614] ]] adds
SrcMgr::CharacteristicKind to the InclusionDirective callback, this patch
updates instances of it in clang-tools-extra.
https://reviews.llvm.org/D46615
Files:
clang-move/ClangMove.cpp
clang-tidy/llvm/IncludeOrderCheck.cpp
clang-tidy/modernize/DeprecatedHeadersCheck.cpp
clang-tidy/utils/IncludeInserter.cpp
clangd/ClangdUnit.cpp
clangd/Headers.cpp
modularize/CoverageChecker.cpp
modularize/PreprocessorTracker.cpp
pp-trace/PPCallbacksTracker.cpp
pp-trace/PPCallbacksTracker.h
Index: pp-trace/PPCallbacksTracker.h
===
--- pp-trace/PPCallbacksTracker.h
+++ pp-trace/PPCallbacksTracker.h
@@ -102,7 +102,8 @@
const clang::FileEntry *File,
llvm::StringRef SearchPath,
llvm::StringRef RelativePath,
- const clang::Module *Imported) override;
+ const clang::Module *Imported,
+ SrcMgr::CharacteristicKind FileType) override;
void moduleImport(clang::SourceLocation ImportLoc, clang::ModuleIdPath Path,
const clang::Module *Imported) override;
void EndOfMainFile() override;
Index: pp-trace/PPCallbacksTracker.cpp
===
--- pp-trace/PPCallbacksTracker.cpp
+++ pp-trace/PPCallbacksTracker.cpp
@@ -139,7 +139,7 @@
llvm::StringRef FileName, bool IsAngled,
clang::CharSourceRange FilenameRange, const clang::FileEntry *File,
llvm::StringRef SearchPath, llvm::StringRef RelativePath,
-const clang::Module *Imported) {
+const clang::Module *Imported, SrcMgr::CharacteristicKind FileType) {
beginCallback("InclusionDirective");
appendArgument("IncludeTok", IncludeTok);
appendFilePathArgument("FileName", FileName);
@@ -149,6 +149,7 @@
appendFilePathArgument("SearchPath", SearchPath);
appendFilePathArgument("RelativePath", RelativePath);
appendArgument("Imported", Imported);
+ appendArgument("FileType", FileType);
}
// Callback invoked whenever there was an explicit module-import
Index: modularize/PreprocessorTracker.cpp
===
--- modularize/PreprocessorTracker.cpp
+++ modularize/PreprocessorTracker.cpp
@@ -750,7 +750,8 @@
const clang::FileEntry *File,
llvm::StringRef SearchPath,
llvm::StringRef RelativePath,
- const clang::Module *Imported) override;
+ const clang::Module *Imported,
+ SrcMgr::CharacteristicKind FileType) override;
void FileChanged(clang::SourceLocation Loc,
clang::PPCallbacks::FileChangeReason Reason,
clang::SrcMgr::CharacteristicKind FileType,
@@ -1289,7 +1290,7 @@
llvm::StringRef FileName, bool IsAngled,
clang::CharSourceRange FilenameRange, const clang::FileEntry *File,
llvm::StringRef SearchPath, llvm::StringRef RelativePath,
-const clang::Module *Imported) {
+const clang::Module *Imported, SrcMgr::CharacteristicKind FileType) {
int DirectiveLine, DirectiveColumn;
std::string HeaderPath = getSourceLocationFile(PP, HashLoc);
getSourceLocationLineAndColumn(PP, HashLoc, DirectiveLine, DirectiveColumn);
Index: modularize/CoverageChecker.cpp
===
--- modularize/CoverageChecker.cpp
+++ modularize/CoverageChecker.cpp
@@ -90,7 +90,8 @@
StringRef FileName, bool IsAngled,
CharSourceRange FilenameRange, const FileEntry *File,
StringRef SearchPath, StringRef RelativePath,
- const Module *Imported) override {
+ const Module *Imported,
+ SrcMgr::CharacteristicKind FileType) override {
Checker.collectUmbrellaHeaderHeader(File->getName());
}
Index: clangd/Headers.cpp
===
--- clangd/Headers.cpp
+++ clangd/Headers.cpp
@@ -34,7 +34,8 @@
CharSourceRange /*FilenameRange*/,
const FileEntry *File, llvm::StringRef /*SearchPath*/,
llvm::StringRef /*RelativePath*/,
- const Module * /*Imported*/) override {
+ const Module * /*Imported*/,
+ SrcMgr::CharacteristicKind FileType) override {
WrittenHeaders.insert(
(IsAngled ? "<" + FileName + ">" : "\"" + FileName + "\"").
[PATCH] D45470: Emit an error when include after
jfb added a comment. In https://reviews.llvm.org/D45470#1092212, @vsapsai wrote: > Here is another approach that should emit an error only when mixing headers > causes compilation problems. > > Have no ideas how to test the change. `-verify` doesn't work with fatal errors > and libcxx doesn't use FileCheck. Performed only manual testing. This worked with `` before `` as well as with the order reversed? LGTM pending testing answer. This seems simpler than implementing http://wg21.link/p0943 outright, though that would still be the best solution IMO. Comment at: libcxx/include/atomic:558 #endif +#ifdef kill_dependency +#error C++ standard library is incompatible with I checked and C guarantees that this is a macro :) https://reviews.llvm.org/D45470 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46610: [CMake] Set CMAKE_BUILD_WITH_INSTALL_RPATH for Fuchsia runtimes
jakehehrlich accepted this revision. jakehehrlich added a comment. This revision is now accepted and ready to land. Spoke offline, LGTM Repository: rC Clang https://reviews.llvm.org/D46610 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r331833 - Set CMAKE_BUILD_WITH_INSTALL_RPATH for Fuchsia runtimes
Author: phosek
Date: Tue May 8 17:58:12 2018
New Revision: 331833
URL: http://llvm.org/viewvc/llvm-project?rev=331833&view=rev
Log:
Set CMAKE_BUILD_WITH_INSTALL_RPATH for Fuchsia runtimes
This doesn't make any difference since we don't use RPATH/RUNPATH
on Fuchsia but it avoids the CMake error when re-linking libraries
while building with Ninja.
Differntial Revision: https://reviews.llvm.org/D46610
Modified:
cfe/trunk/cmake/caches/Fuchsia-stage2.cmake
Modified: cfe/trunk/cmake/caches/Fuchsia-stage2.cmake
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/cmake/caches/Fuchsia-stage2.cmake?rev=331833&r1=331832&r2=331833&view=diff
==
--- cfe/trunk/cmake/caches/Fuchsia-stage2.cmake (original)
+++ cfe/trunk/cmake/caches/Fuchsia-stage2.cmake Tue May 8 17:58:12 2018
@@ -67,6 +67,7 @@ endif()
foreach(target x86_64;aarch64)
set(RUNTIMES_${target}-fuchsia_CMAKE_SYSTEM_NAME Fuchsia CACHE STRING "")
set(RUNTIMES_${target}-fuchsia_CMAKE_BUILD_TYPE
${FUCHSIA_RUNTIMES_BUILD_TYPE} CACHE STRING "")
+ set(RUNTIMES_${target}-fuchsia_CMAKE_BUILD_WITH_INSTALL_RPATH ON CACHE
STRING "")
set(RUNTIMES_${target}-fuchsia_CMAKE_ASM_FLAGS ${FUCHSIA_${target}_C_FLAGS}
CACHE PATH "")
set(RUNTIMES_${target}-fuchsia_CMAKE_C_FLAGS ${FUCHSIA_${target}_C_FLAGS}
CACHE PATH "")
set(RUNTIMES_${target}-fuchsia_CMAKE_CXX_FLAGS
${FUCHSIA_${target}_CXX_FLAGS} CACHE PATH "")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46320: Remove \brief commands from doxygen comments.
This revision was automatically updated to reflect the committed changes. Closed by commit rC331834: Remove \brief commands from doxygen comments. (authored by adrian, committed by ). Changed prior to commit: https://reviews.llvm.org/D46320?vs=144736&id=145831#toc Repository: rC Clang https://reviews.llvm.org/D46320 Files: docs/LibFormat.rst docs/doxygen.cfg.in include/clang-c/BuildSystem.h include/clang-c/CXCompilationDatabase.h include/clang-c/CXErrorCode.h include/clang-c/CXString.h include/clang-c/Documentation.h include/clang-c/Index.h include/clang/ARCMigrate/ARCMT.h include/clang/ARCMigrate/ARCMTActions.h include/clang/AST/APValue.h include/clang/AST/ASTConsumer.h include/clang/AST/ASTContext.h include/clang/AST/ASTDiagnostic.h include/clang/AST/ASTFwd.h include/clang/AST/ASTImporter.h include/clang/AST/ASTLambda.h include/clang/AST/ASTMutationListener.h include/clang/AST/ASTTypeTraits.h include/clang/AST/ASTUnresolvedSet.h include/clang/AST/Attr.h include/clang/AST/Availability.h include/clang/AST/CXXInheritance.h include/clang/AST/CanonicalType.h include/clang/AST/CommentBriefParser.h include/clang/AST/CommentCommandTraits.h include/clang/AST/CommentLexer.h include/clang/AST/CommentSema.h include/clang/AST/ComparisonCategories.h include/clang/AST/DataCollection.h include/clang/AST/Decl.h include/clang/AST/DeclBase.h include/clang/AST/DeclCXX.h include/clang/AST/DeclContextInternals.h include/clang/AST/DeclObjC.h include/clang/AST/DeclOpenMP.h include/clang/AST/DeclTemplate.h include/clang/AST/DeclVisitor.h include/clang/AST/DeclarationName.h include/clang/AST/EvaluatedExprVisitor.h include/clang/AST/Expr.h include/clang/AST/ExprCXX.h include/clang/AST/ExprObjC.h include/clang/AST/ExprOpenMP.h include/clang/AST/ExternalASTSource.h include/clang/AST/LambdaCapture.h include/clang/AST/LocInfoType.h include/clang/AST/Mangle.h include/clang/AST/MangleNumberingContext.h include/clang/AST/NSAPI.h include/clang/AST/NestedNameSpecifier.h include/clang/AST/OpenMPClause.h include/clang/AST/OperationKinds.def include/clang/AST/OperationKinds.h include/clang/AST/ParentMap.h include/clang/AST/PrettyPrinter.h include/clang/AST/QualTypeNames.h include/clang/AST/RawCommentList.h include/clang/AST/RecordLayout.h include/clang/AST/RecursiveASTVisitor.h include/clang/AST/Redeclarable.h include/clang/AST/SelectorLocationsKind.h include/clang/AST/Stmt.h include/clang/AST/StmtCXX.h include/clang/AST/StmtObjC.h include/clang/AST/StmtOpenMP.h include/clang/AST/StmtVisitor.h include/clang/AST/TemplateBase.h include/clang/AST/TemplateName.h include/clang/AST/Type.h include/clang/AST/TypeLoc.h include/clang/AST/TypeOrdering.h include/clang/AST/TypeVisitor.h include/clang/AST/UnresolvedSet.h include/clang/AST/VTTBuilder.h include/clang/AST/VTableBuilder.h include/clang/ASTMatchers/ASTMatchFinder.h include/clang/ASTMatchers/ASTMatchers.h include/clang/ASTMatchers/ASTMatchersInternal.h include/clang/ASTMatchers/ASTMatchersMacros.h include/clang/ASTMatchers/Dynamic/Diagnostics.h include/clang/ASTMatchers/Dynamic/Parser.h include/clang/ASTMatchers/Dynamic/Registry.h include/clang/ASTMatchers/Dynamic/VariantValue.h include/clang/Analysis/Analyses/Consumed.h include/clang/Analysis/Analyses/Dominators.h include/clang/Analysis/Analyses/FormatString.h include/clang/Analysis/Analyses/PostOrderCFGView.h include/clang/Analysis/Analyses/ThreadSafety.h include/clang/Analysis/Analyses/ThreadSafetyCommon.h include/clang/Analysis/Analyses/ThreadSafetyLogical.h include/clang/Analysis/AnalysisDeclContext.h include/clang/Analysis/CFG.h include/clang/Analysis/CallGraph.h include/clang/Analysis/CodeInjector.h include/clang/Analysis/ProgramPoint.h include/clang/Basic/ABI.h include/clang/Basic/AddressSpaces.h include/clang/Basic/AlignedAllocation.h include/clang/Basic/AllDiagnostics.h include/clang/Basic/AttrKinds.h include/clang/Basic/AttrSubjectMatchRules.h include/clang/Basic/Attributes.h include/clang/Basic/Builtins.h include/clang/Basic/BuiltinsWebAssembly.def include/clang/Basic/CapturedStmt.h include/clang/Basic/CommentOptions.h include/clang/Basic/Diagnostic.h include/clang/Basic/DiagnosticError.h include/clang/Basic/DiagnosticIDs.h include/clang/Basic/DiagnosticOptions.h include/clang/Basic/ExceptionSpecificationType.h include/clang/Basic/ExpressionTraits.h include/clang/Basic/FileManager.h include/clang/Basic/FileSystemOptions.h include/clang/Basic/FileSystemStatCache.h include/clang/Basic/IdentifierTable.h include/clang/Basic/LLVM.h include/clang/Basic/Lambda.h include/clang/Basic/LangOptions.def include/clang/Basic/LangOptions.h include/clang/Basic/Linkage.h include/clang/Basic/MacroBuilder.h include/clang/Basic/Module.h include/clang/Basic/ObjCRuntime.h include/clang/Basic/OpenCLOp
[PATCH] D46320: Remove \brief commands from doxygen comments.
This revision was automatically updated to reflect the committed changes. Closed by commit rL331834: Remove \brief commands from doxygen comments. (authored by adrian, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D46320?vs=144736&id=145832#toc Repository: rL LLVM https://reviews.llvm.org/D46320 Files: cfe/trunk/docs/LibFormat.rst cfe/trunk/docs/doxygen.cfg.in cfe/trunk/include/clang-c/BuildSystem.h cfe/trunk/include/clang-c/CXCompilationDatabase.h cfe/trunk/include/clang-c/CXErrorCode.h cfe/trunk/include/clang-c/CXString.h cfe/trunk/include/clang-c/Documentation.h cfe/trunk/include/clang-c/Index.h cfe/trunk/include/clang/ARCMigrate/ARCMT.h cfe/trunk/include/clang/ARCMigrate/ARCMTActions.h cfe/trunk/include/clang/AST/APValue.h cfe/trunk/include/clang/AST/ASTConsumer.h cfe/trunk/include/clang/AST/ASTContext.h cfe/trunk/include/clang/AST/ASTDiagnostic.h cfe/trunk/include/clang/AST/ASTFwd.h cfe/trunk/include/clang/AST/ASTImporter.h cfe/trunk/include/clang/AST/ASTLambda.h cfe/trunk/include/clang/AST/ASTMutationListener.h cfe/trunk/include/clang/AST/ASTTypeTraits.h cfe/trunk/include/clang/AST/ASTUnresolvedSet.h cfe/trunk/include/clang/AST/Attr.h cfe/trunk/include/clang/AST/Availability.h cfe/trunk/include/clang/AST/CXXInheritance.h cfe/trunk/include/clang/AST/CanonicalType.h cfe/trunk/include/clang/AST/CommentBriefParser.h cfe/trunk/include/clang/AST/CommentCommandTraits.h cfe/trunk/include/clang/AST/CommentLexer.h cfe/trunk/include/clang/AST/CommentSema.h cfe/trunk/include/clang/AST/ComparisonCategories.h cfe/trunk/include/clang/AST/DataCollection.h cfe/trunk/include/clang/AST/Decl.h cfe/trunk/include/clang/AST/DeclBase.h cfe/trunk/include/clang/AST/DeclCXX.h cfe/trunk/include/clang/AST/DeclContextInternals.h cfe/trunk/include/clang/AST/DeclObjC.h cfe/trunk/include/clang/AST/DeclOpenMP.h cfe/trunk/include/clang/AST/DeclTemplate.h cfe/trunk/include/clang/AST/DeclVisitor.h cfe/trunk/include/clang/AST/DeclarationName.h cfe/trunk/include/clang/AST/EvaluatedExprVisitor.h cfe/trunk/include/clang/AST/Expr.h cfe/trunk/include/clang/AST/ExprCXX.h cfe/trunk/include/clang/AST/ExprObjC.h cfe/trunk/include/clang/AST/ExprOpenMP.h cfe/trunk/include/clang/AST/ExternalASTSource.h cfe/trunk/include/clang/AST/LambdaCapture.h cfe/trunk/include/clang/AST/LocInfoType.h cfe/trunk/include/clang/AST/Mangle.h cfe/trunk/include/clang/AST/MangleNumberingContext.h cfe/trunk/include/clang/AST/NSAPI.h cfe/trunk/include/clang/AST/NestedNameSpecifier.h cfe/trunk/include/clang/AST/OpenMPClause.h cfe/trunk/include/clang/AST/OperationKinds.def cfe/trunk/include/clang/AST/OperationKinds.h cfe/trunk/include/clang/AST/ParentMap.h cfe/trunk/include/clang/AST/PrettyPrinter.h cfe/trunk/include/clang/AST/QualTypeNames.h cfe/trunk/include/clang/AST/RawCommentList.h cfe/trunk/include/clang/AST/RecordLayout.h cfe/trunk/include/clang/AST/RecursiveASTVisitor.h cfe/trunk/include/clang/AST/Redeclarable.h cfe/trunk/include/clang/AST/SelectorLocationsKind.h cfe/trunk/include/clang/AST/Stmt.h cfe/trunk/include/clang/AST/StmtCXX.h cfe/trunk/include/clang/AST/StmtObjC.h cfe/trunk/include/clang/AST/StmtOpenMP.h cfe/trunk/include/clang/AST/StmtVisitor.h cfe/trunk/include/clang/AST/TemplateBase.h cfe/trunk/include/clang/AST/TemplateName.h cfe/trunk/include/clang/AST/Type.h cfe/trunk/include/clang/AST/TypeLoc.h cfe/trunk/include/clang/AST/TypeOrdering.h cfe/trunk/include/clang/AST/TypeVisitor.h cfe/trunk/include/clang/AST/UnresolvedSet.h cfe/trunk/include/clang/AST/VTTBuilder.h cfe/trunk/include/clang/AST/VTableBuilder.h cfe/trunk/include/clang/ASTMatchers/ASTMatchFinder.h cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h cfe/trunk/include/clang/ASTMatchers/ASTMatchersMacros.h cfe/trunk/include/clang/ASTMatchers/Dynamic/Diagnostics.h cfe/trunk/include/clang/ASTMatchers/Dynamic/Parser.h cfe/trunk/include/clang/ASTMatchers/Dynamic/Registry.h cfe/trunk/include/clang/ASTMatchers/Dynamic/VariantValue.h cfe/trunk/include/clang/Analysis/Analyses/Consumed.h cfe/trunk/include/clang/Analysis/Analyses/Dominators.h cfe/trunk/include/clang/Analysis/Analyses/FormatString.h cfe/trunk/include/clang/Analysis/Analyses/PostOrderCFGView.h cfe/trunk/include/clang/Analysis/Analyses/ThreadSafety.h cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyCommon.h cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyLogical.h cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h cfe/trunk/include/clang/Analysis/CFG.h cfe/trunk/include/clang/Analysis/CallGraph.h cfe/trunk/include/clang/Analysis/CodeInjector.h cfe/trunk/include/clang/Analysis/ProgramPoint.h cfe/trunk/include/clang/Basic/ABI.h cfe/trunk/include/clang/Basic/AddressSpaces.h cfe/trunk/include/clang/Basic/AlignedA
[PATCH] D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers
rjmccall added a comment. In https://reviews.llvm.org/D42933#1092077, @smeenai wrote: > Apple's current recommendation for using printf with the NSInteger types is > casting to a long, per > https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/Strings/Articles/formatSpecifiers.html. > Are you suggesting that recommendation would change to using `%zd` instead? I'm not saying that the recommendation would change, but `long` being pointer-sized is a pretty universal assumption on Darwin, I'm afraid. In https://reviews.llvm.org/D42933#1092154, @jfb wrote: > I don't disagree with the original intent, but AFAICT that's exactly the > intent of the warning I'd like to get rid of. I'm making a very narrow point: > there is no portability problem with the warning I'm specifically trying to > silence (using `%z` with `NS[U]Integer` on Darwin). If we decide that > `-Wformat` shouldn't emit portability warnings then my point is moot, so > let's see if people agree to that. If not, then my point still stands. Agreed. >>> - There are always established idioms for solving portability problems. In >>> this case, a certain set of important typedefs from the C standard have >>> been blessed with dedicated format modifiers like "z", but every other >>> typedef in the world has to find some other solution, either by asserting >>> that some existing format is good enough (as NSInteger does) or by >>> introducing a macro that expands to an appropriate format (as some things >>> in POSIX do). A proper format-portability warning would have to know about >>> all of these conventions (in order to check that e.g. part of the format >>> string came from the right macro), which just seems wildly out-of-scope for >>> a compiler warning. > > We could provide a macro for `NS[U]Integer`, but it's been long enough and > there's enough existing code with `%z`. Not sure the code churn on user is > worth it, if we can instead say "`%z` works". Right. I'm not trying to argue that we should add a macro for NSInteger, I'm saying that that's the sort of thing that a portability warning would have to consider. Portability warnings are... always tempting, but they're very, very difficult in C. Since we're not going to do that sort of work, we should back off from doing a portability warning. John. Repository: rC Clang https://reviews.llvm.org/D42933 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46613: _Atomic of empty struct shouldn't assert
rjmccall added inline comments.
Comment at: lib/AST/ASTContext.cpp:1965
+ Width = Target->getCharWidth();
+ Align = Target->getCharWidth();
+} else if (Width <= Target->getMaxAtomicPromoteWidth()) {
Alignment, unlike size, is definitely never 0. I think you should leave the
original alignment in place; it's a weird case, but we honor over-aligned empty
types in a bunch of other places.
Repository:
rC Clang
https://reviews.llvm.org/D46613
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43322: Diagnose cases of "return x" that should be "return std::move(x)" for efficiency
Quuxplusone added a comment. > Can you post the diagnostic exactly as it appears in the compiler output? I > am surprised that it would appear here. It should appear here only if the > standard library's implicit conversion from std::map<...>::iterator to > std::map<...>::const_iterator were implemented as a conversion operator > instead of as a converting constructor. I have verified that both libstdc++ > trunk and libc++ trunk implement it "correctly" as a converting constructor, > and I have verified on Wandbox/Godbolt that no warning is emitted on your > sample code when using either of those standard libraries. > > Is it possible that you are using a standard library that is neither libc++ > or libstdc++? > > Is it possible that that standard library implements the > iterator-to-const_iterator conversion as a conversion operator instead of a > converting constructor? @thakis ping — did you ever resolve this issue to your satisfaction? Repository: rC Clang https://reviews.llvm.org/D43322 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45045: [DebugInfo] Generate debug information for labels.
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 rL331843: [DebugInfo] Generate debug information for labels.
(authored by shiva, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D45045?vs=143243&id=145849#toc
Repository:
rL LLVM
https://reviews.llvm.org/D45045
Files:
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/lib/CodeGen/CGDebugInfo.h
cfe/trunk/lib/CodeGen/CGStmt.cpp
cfe/trunk/test/CodeGen/backend-unsupported-error.ll
cfe/trunk/test/CodeGen/debug-label-inline.c
cfe/trunk/test/CodeGen/debug-label.c
Index: cfe/trunk/lib/CodeGen/CGDebugInfo.h
===
--- cfe/trunk/lib/CodeGen/CGDebugInfo.h
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.h
@@ -396,6 +396,9 @@
llvm::Value *AI,
CGBuilderTy &Builder);
+ /// Emit call to \c llvm.dbg.label for an label.
+ void EmitLabel(const LabelDecl *D, CGBuilderTy &Builder);
+
/// Emit call to \c llvm.dbg.declare for an imported variable
/// declaration in a block.
void EmitDeclareOfBlockDeclRefVariable(const VarDecl *variable,
Index: cfe/trunk/lib/CodeGen/CGStmt.cpp
===
--- cfe/trunk/lib/CodeGen/CGStmt.cpp
+++ cfe/trunk/lib/CodeGen/CGStmt.cpp
@@ -531,6 +531,16 @@
}
EmitBlock(Dest.getBlock());
+
+ // Emit debug info for labels.
+ if (CGDebugInfo *DI = getDebugInfo()) {
+if (CGM.getCodeGenOpts().getDebugInfo() >=
+codegenoptions::LimitedDebugInfo) {
+ DI->setLocation(D->getLocation());
+ DI->EmitLabel(D, Builder);
+}
+ }
+
incrementProfileCounter(D->getStmt());
}
Index: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
===
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
@@ -3647,6 +3647,32 @@
return EmitDeclare(VD, Storage, llvm::None, Builder);
}
+void CGDebugInfo::EmitLabel(const LabelDecl *D, CGBuilderTy &Builder) {
+ assert(DebugKind >= codegenoptions::LimitedDebugInfo);
+ assert(!LexicalBlockStack.empty() && "Region stack mismatch, stack empty!");
+
+ if (D->hasAttr())
+return;
+
+ auto *Scope = cast(LexicalBlockStack.back());
+ llvm::DIFile *Unit = getOrCreateFile(D->getLocation());
+
+ // Get location information.
+ unsigned Line = getLineNumber(D->getLocation());
+ unsigned Column = getColumnNumber(D->getLocation());
+
+ StringRef Name = D->getName();
+
+ // Create the descriptor for the label.
+ auto *L =
+ DBuilder.createLabel(Scope, Name, Unit, Line, CGM.getLangOpts().Optimize);
+
+ // Insert an llvm.dbg.label into the current block.
+ DBuilder.insertLabel(L,
+ llvm::DebugLoc::get(Line, Column, Scope, CurInlinedAt),
+ Builder.GetInsertBlock());
+}
+
llvm::DIType *CGDebugInfo::CreateSelfType(const QualType &QualTy,
llvm::DIType *Ty) {
llvm::DIType *CachedTy = getTypeOrNull(QualTy);
Index: cfe/trunk/test/CodeGen/debug-label.c
===
--- cfe/trunk/test/CodeGen/debug-label.c
+++ cfe/trunk/test/CodeGen/debug-label.c
@@ -0,0 +1,16 @@
+// This test will test the correstness of generating DILabel and
+// llvm.dbg.label for labels.
+//
+// RUN: %clang_cc1 -emit-llvm %s -o - -emit-llvm -debug-info-kind=limited | FileCheck %s
+
+int f1(int a, int b) {
+ int sum;
+
+top:
+ // CHECK: call void @llvm.dbg.label(metadata [[LABEL_METADATA:!.*]]), !dbg [[LABEL_LOCATION:!.*]]
+ sum = a + b;
+ return sum;
+}
+
+// CHECK: [[LABEL_METADATA]] = !DILabel({{.*}}, name: "top", {{.*}}, line: 9)
+// CHECK: [[LABEL_LOCATION]] = !DILocation(line: 9,
Index: cfe/trunk/test/CodeGen/backend-unsupported-error.ll
===
--- cfe/trunk/test/CodeGen/backend-unsupported-error.ll
+++ cfe/trunk/test/CodeGen/backend-unsupported-error.ll
@@ -30,11 +30,11 @@
!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 3.9.0", isOptimized: false, runtimeVersion: 0, emissionKind: 1, enums: !2)
!1 = !DIFile(filename: "test.c", directory: "")
!2 = !{}
-!4 = distinct !DISubprogram(name: "bar", scope: !1, file: !1, line: 2, type: !5, isLocal: false, isDefinition: true, scopeLine: 2, isOptimized: false, unit: !0, variables: !2)
+!4 = distinct !DISubprogram(name: "bar", scope: !1, file: !1, line: 2, type: !5, isLocal: false, isDefinition: true, scopeLine: 2, isOptimized: false, unit: !0, retainedNodes: !2)
!5 = !DISubroutineType(types: !6)
!6 = !{!7}
!7 = !DIBasicType(name: "int", size: 32, align: 32, encoding: DW_ATE_signed)
-!8 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 3, type: !5, isLocal: false, isDe
[PATCH] D46613: _Atomic of empty struct shouldn't assert
jfb updated this revision to Diff 145858.
jfb added a comment.
- Assert on zero alignment, instead of making it always byte-aligned.
Repository:
rC Clang
https://reviews.llvm.org/D46613
Files:
lib/AST/ASTContext.cpp
test/CodeGen/c11atomics-ios.c
test/CodeGen/c11atomics.c
Index: test/CodeGen/c11atomics.c
===
--- test/CodeGen/c11atomics.c
+++ test/CodeGen/c11atomics.c
@@ -474,3 +474,17 @@
// CHECK: ret i1 [[RES]]
return __c11_atomic_compare_exchange_strong(addr, desired, *new, 5, 5);
}
+
+struct Empty {};
+
+struct Empty test_empty_struct_load(_Atomic(struct Empty)* empty) {
+ // CHECK-LABEL: @test_empty_struct_load(
+ // CHECK: call arm_aapcscc zeroext i8 @__atomic_load_1(i8* %{{.*}}, i32 5)
+ return __c11_atomic_load(empty, 5);
+}
+
+void test_empty_struct_store(_Atomic(struct Empty)* empty, struct Empty value)
{
+ // CHECK-LABEL: @test_empty_struct_store(
+ // CHECK: call arm_aapcscc void @__atomic_store_1(i8* %{{.*}}, i8 zeroext
%{{.*}}, i32 5)
+ __c11_atomic_store(empty, value, 5);
+}
Index: test/CodeGen/c11atomics-ios.c
===
--- test/CodeGen/c11atomics-ios.c
+++ test/CodeGen/c11atomics-ios.c
@@ -319,3 +319,19 @@
return __c11_atomic_compare_exchange_strong(addr, desired, *new, 5, 5);
}
+
+struct Empty {};
+
+struct Empty testEmptyStructLoad(_Atomic(struct Empty)* empty) {
+ // CHECK-LABEL: @testEmptyStructLoad(
+ // CHECK-NOT: @__atomic_load
+ // CHECK: load atomic i8, i8* %{{.*}} seq_cst, align 1
+ return *empty;
+}
+
+void testEmptyStructStore(_Atomic(struct Empty)* empty, struct Empty value) {
+ // CHECK-LABEL: @testEmptyStructStore(
+ // CHECK-NOT: @__atomic_store
+ // CHECK: store atomic i8 %{{.*}}, i8* %{{.*}} seq_cst, align 1
+ *empty = value;
+}
Index: lib/AST/ASTContext.cpp
===
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -1958,10 +1958,16 @@
Width = Info.Width;
Align = Info.Align;
-// If the size of the type doesn't exceed the platform's max
-// atomic promotion width, make the size and alignment more
-// favorable to atomic operations:
-if (Width != 0 && Width <= Target->getMaxAtomicPromoteWidth()) {
+if (!Width) {
+ // An otherwise zero-sized type should still generate an
+ // atomic operation.
+ Width = Target->getCharWidth();
+ assert(Align);
+} else if (Width <= Target->getMaxAtomicPromoteWidth()) {
+ // If the size of the type doesn't exceed the platform's max
+ // atomic promotion width, make the size and alignment more
+ // favorable to atomic operations:
+
// Round the size up to a power of 2.
if (!llvm::isPowerOf2_64(Width))
Width = llvm::NextPowerOf2(Width);
Index: test/CodeGen/c11atomics.c
===
--- test/CodeGen/c11atomics.c
+++ test/CodeGen/c11atomics.c
@@ -474,3 +474,17 @@
// CHECK: ret i1 [[RES]]
return __c11_atomic_compare_exchange_strong(addr, desired, *new, 5, 5);
}
+
+struct Empty {};
+
+struct Empty test_empty_struct_load(_Atomic(struct Empty)* empty) {
+ // CHECK-LABEL: @test_empty_struct_load(
+ // CHECK: call arm_aapcscc zeroext i8 @__atomic_load_1(i8* %{{.*}}, i32 5)
+ return __c11_atomic_load(empty, 5);
+}
+
+void test_empty_struct_store(_Atomic(struct Empty)* empty, struct Empty value) {
+ // CHECK-LABEL: @test_empty_struct_store(
+ // CHECK: call arm_aapcscc void @__atomic_store_1(i8* %{{.*}}, i8 zeroext %{{.*}}, i32 5)
+ __c11_atomic_store(empty, value, 5);
+}
Index: test/CodeGen/c11atomics-ios.c
===
--- test/CodeGen/c11atomics-ios.c
+++ test/CodeGen/c11atomics-ios.c
@@ -319,3 +319,19 @@
return __c11_atomic_compare_exchange_strong(addr, desired, *new, 5, 5);
}
+
+struct Empty {};
+
+struct Empty testEmptyStructLoad(_Atomic(struct Empty)* empty) {
+ // CHECK-LABEL: @testEmptyStructLoad(
+ // CHECK-NOT: @__atomic_load
+ // CHECK: load atomic i8, i8* %{{.*}} seq_cst, align 1
+ return *empty;
+}
+
+void testEmptyStructStore(_Atomic(struct Empty)* empty, struct Empty value) {
+ // CHECK-LABEL: @testEmptyStructStore(
+ // CHECK-NOT: @__atomic_store
+ // CHECK: store atomic i8 %{{.*}}, i8* %{{.*}} seq_cst, align 1
+ *empty = value;
+}
Index: lib/AST/ASTContext.cpp
===
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -1958,10 +1958,16 @@
Width = Info.Width;
Align = Info.Align;
-// If the size of the type doesn't exceed the platform's max
-// atomic promotion width, make the size and alignment more
-// favorable to atomic operations:
-if (Width != 0 && Width <= Target->getMaxAtomicPromoteWidth()) {
+if (!Width) {
+ // An otherwise zero-sized type should still generate an
+
[PATCH] D46613: _Atomic of empty struct shouldn't assert
jfb marked an inline comment as done.
jfb added inline comments.
Comment at: lib/AST/ASTContext.cpp:1965
+ Width = Target->getCharWidth();
+ Align = Target->getCharWidth();
+} else if (Width <= Target->getMaxAtomicPromoteWidth()) {
rjmccall wrote:
> Alignment, unlike size, is definitely never 0. I think you should leave the
> original alignment in place; it's a weird case, but we honor over-aligned
> empty types in a bunch of other places.
Yeah that makes total sense. I turned it to an assert.
Repository:
rC Clang
https://reviews.llvm.org/D46613
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46613: _Atomic of empty struct shouldn't assert
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. LGTM. Repository: rC Clang https://reviews.llvm.org/D46613 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r331845 - _Atomic of empty struct shouldn't assert
Author: jfb
Date: Tue May 8 20:51:12 2018
New Revision: 331845
URL: http://llvm.org/viewvc/llvm-project?rev=331845&view=rev
Log:
_Atomic of empty struct shouldn't assert
Summary:
An _Atomic of an empty struct is pretty silly. In general we just widen empty
structs to hold a byte's worth of storage, and we represent size and alignment
as 0 internally and let LLVM figure out what to do. For _Atomic it's a bit
different: the memory model mandates concrete effects occur when atomic
operations occur, so in most cases actual instructions need to get emitted. It's
really not worth trying to optimize empty struct atomics by figuring out e.g.
that a fence would do, even though sane compilers should do optimize atomics.
Further, wg21.link/p0528 will fix C++20 atomics with padding bits so that
cmpxchg on them works, which means that we'll likely need to do the zero-init
song and dance for empty atomic structs anyways (and I think we shouldn't
special-case this behavior to C++20 because prior standards are just broken).
This patch therefore makes a minor change to r176658 "Promote atomic type sizes
up to a power of two": if the width of the atomic's value type is 0, just use 1
byte for width and leave alignment as-is (since it should never be zero, and
over-aligned zero-width structs are weird but fine).
This fixes an assertion:
(NumBits >= MIN_INT_BITS && "bitwidth too small"), function get, file
../lib/IR/Type.cpp, line 241.
It seems like this has run into other assertions before (namely the unreachable
Kind check in ImpCastExprToType), but I haven't reproduced that issue with
tip-of-tree.
Reviewers: arphaman, rjmccall
Subscribers: aheejin, cfe-commits
Differential Revision: https://reviews.llvm.org/D46613
Modified:
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/test/CodeGen/c11atomics-ios.c
cfe/trunk/test/CodeGen/c11atomics.c
Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=331845&r1=331844&r2=331845&view=diff
==
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Tue May 8 20:51:12 2018
@@ -1958,10 +1958,16 @@ TypeInfo ASTContext::getTypeInfoImpl(con
Width = Info.Width;
Align = Info.Align;
-// If the size of the type doesn't exceed the platform's max
-// atomic promotion width, make the size and alignment more
-// favorable to atomic operations:
-if (Width != 0 && Width <= Target->getMaxAtomicPromoteWidth()) {
+if (!Width) {
+ // An otherwise zero-sized type should still generate an
+ // atomic operation.
+ Width = Target->getCharWidth();
+ assert(Align);
+} else if (Width <= Target->getMaxAtomicPromoteWidth()) {
+ // If the size of the type doesn't exceed the platform's max
+ // atomic promotion width, make the size and alignment more
+ // favorable to atomic operations:
+
// Round the size up to a power of 2.
if (!llvm::isPowerOf2_64(Width))
Width = llvm::NextPowerOf2(Width);
Modified: cfe/trunk/test/CodeGen/c11atomics-ios.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/c11atomics-ios.c?rev=331845&r1=331844&r2=331845&view=diff
==
--- cfe/trunk/test/CodeGen/c11atomics-ios.c (original)
+++ cfe/trunk/test/CodeGen/c11atomics-ios.c Tue May 8 20:51:12 2018
@@ -319,3 +319,19 @@ _Bool test_promoted_cmpxchg(_Atomic(PS)
return __c11_atomic_compare_exchange_strong(addr, desired, *new, 5, 5);
}
+
+struct Empty {};
+
+struct Empty testEmptyStructLoad(_Atomic(struct Empty)* empty) {
+ // CHECK-LABEL: @testEmptyStructLoad(
+ // CHECK-NOT: @__atomic_load
+ // CHECK: load atomic i8, i8* %{{.*}} seq_cst, align 1
+ return *empty;
+}
+
+void testEmptyStructStore(_Atomic(struct Empty)* empty, struct Empty value) {
+ // CHECK-LABEL: @testEmptyStructStore(
+ // CHECK-NOT: @__atomic_store
+ // CHECK: store atomic i8 %{{.*}}, i8* %{{.*}} seq_cst, align 1
+ *empty = value;
+}
Modified: cfe/trunk/test/CodeGen/c11atomics.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/c11atomics.c?rev=331845&r1=331844&r2=331845&view=diff
==
--- cfe/trunk/test/CodeGen/c11atomics.c (original)
+++ cfe/trunk/test/CodeGen/c11atomics.c Tue May 8 20:51:12 2018
@@ -474,3 +474,17 @@ _Bool test_promoted_cmpxchg(_Atomic(PS)
// CHECK: ret i1 [[RES]]
return __c11_atomic_compare_exchange_strong(addr, desired, *new, 5, 5);
}
+
+struct Empty {};
+
+struct Empty test_empty_struct_load(_Atomic(struct Empty)* empty) {
+ // CHECK-LABEL: @test_empty_struct_load(
+ // CHECK: call arm_aapcscc zeroext i8 @__atomic_load_1(i8* %{{.*}}, i32 5)
+ return __c11_atomic_load(empty, 5);
+}
+
+void test_empty_struct_store(_Atomic(struct Empty)* empty, struct Empty value)
{
+
[PATCH] D46613: _Atomic of empty struct shouldn't assert
This revision was automatically updated to reflect the committed changes.
jfb marked an inline comment as done.
Closed by commit rC331845: _Atomic of empty struct shouldn't assert
(authored by jfb, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D46613?vs=145858&id=145859#toc
Repository:
rC Clang
https://reviews.llvm.org/D46613
Files:
lib/AST/ASTContext.cpp
test/CodeGen/c11atomics-ios.c
test/CodeGen/c11atomics.c
Index: lib/AST/ASTContext.cpp
===
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -1958,10 +1958,16 @@
Width = Info.Width;
Align = Info.Align;
-// If the size of the type doesn't exceed the platform's max
-// atomic promotion width, make the size and alignment more
-// favorable to atomic operations:
-if (Width != 0 && Width <= Target->getMaxAtomicPromoteWidth()) {
+if (!Width) {
+ // An otherwise zero-sized type should still generate an
+ // atomic operation.
+ Width = Target->getCharWidth();
+ assert(Align);
+} else if (Width <= Target->getMaxAtomicPromoteWidth()) {
+ // If the size of the type doesn't exceed the platform's max
+ // atomic promotion width, make the size and alignment more
+ // favorable to atomic operations:
+
// Round the size up to a power of 2.
if (!llvm::isPowerOf2_64(Width))
Width = llvm::NextPowerOf2(Width);
Index: test/CodeGen/c11atomics-ios.c
===
--- test/CodeGen/c11atomics-ios.c
+++ test/CodeGen/c11atomics-ios.c
@@ -319,3 +319,19 @@
return __c11_atomic_compare_exchange_strong(addr, desired, *new, 5, 5);
}
+
+struct Empty {};
+
+struct Empty testEmptyStructLoad(_Atomic(struct Empty)* empty) {
+ // CHECK-LABEL: @testEmptyStructLoad(
+ // CHECK-NOT: @__atomic_load
+ // CHECK: load atomic i8, i8* %{{.*}} seq_cst, align 1
+ return *empty;
+}
+
+void testEmptyStructStore(_Atomic(struct Empty)* empty, struct Empty value) {
+ // CHECK-LABEL: @testEmptyStructStore(
+ // CHECK-NOT: @__atomic_store
+ // CHECK: store atomic i8 %{{.*}}, i8* %{{.*}} seq_cst, align 1
+ *empty = value;
+}
Index: test/CodeGen/c11atomics.c
===
--- test/CodeGen/c11atomics.c
+++ test/CodeGen/c11atomics.c
@@ -474,3 +474,17 @@
// CHECK: ret i1 [[RES]]
return __c11_atomic_compare_exchange_strong(addr, desired, *new, 5, 5);
}
+
+struct Empty {};
+
+struct Empty test_empty_struct_load(_Atomic(struct Empty)* empty) {
+ // CHECK-LABEL: @test_empty_struct_load(
+ // CHECK: call arm_aapcscc zeroext i8 @__atomic_load_1(i8* %{{.*}}, i32 5)
+ return __c11_atomic_load(empty, 5);
+}
+
+void test_empty_struct_store(_Atomic(struct Empty)* empty, struct Empty value)
{
+ // CHECK-LABEL: @test_empty_struct_store(
+ // CHECK: call arm_aapcscc void @__atomic_store_1(i8* %{{.*}}, i8 zeroext
%{{.*}}, i32 5)
+ __c11_atomic_store(empty, value, 5);
+}
Index: lib/AST/ASTContext.cpp
===
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -1958,10 +1958,16 @@
Width = Info.Width;
Align = Info.Align;
-// If the size of the type doesn't exceed the platform's max
-// atomic promotion width, make the size and alignment more
-// favorable to atomic operations:
-if (Width != 0 && Width <= Target->getMaxAtomicPromoteWidth()) {
+if (!Width) {
+ // An otherwise zero-sized type should still generate an
+ // atomic operation.
+ Width = Target->getCharWidth();
+ assert(Align);
+} else if (Width <= Target->getMaxAtomicPromoteWidth()) {
+ // If the size of the type doesn't exceed the platform's max
+ // atomic promotion width, make the size and alignment more
+ // favorable to atomic operations:
+
// Round the size up to a power of 2.
if (!llvm::isPowerOf2_64(Width))
Width = llvm::NextPowerOf2(Width);
Index: test/CodeGen/c11atomics-ios.c
===
--- test/CodeGen/c11atomics-ios.c
+++ test/CodeGen/c11atomics-ios.c
@@ -319,3 +319,19 @@
return __c11_atomic_compare_exchange_strong(addr, desired, *new, 5, 5);
}
+
+struct Empty {};
+
+struct Empty testEmptyStructLoad(_Atomic(struct Empty)* empty) {
+ // CHECK-LABEL: @testEmptyStructLoad(
+ // CHECK-NOT: @__atomic_load
+ // CHECK: load atomic i8, i8* %{{.*}} seq_cst, align 1
+ return *empty;
+}
+
+void testEmptyStructStore(_Atomic(struct Empty)* empty, struct Empty value) {
+ // CHECK-LABEL: @testEmptyStructStore(
+ // CHECK-NOT: @__atomic_store
+ // CHECK: store atomic i8 %{{.*}}, i8* %{{.*}} seq_cst, align 1
+ *empty = value;
+}
Index: test/CodeGen/c11atomics.c
===
--- test/CodeGen/c11atomics.c
+++ test/CodeGen/c11atomics.c
@@ -474,3 +474,17 @@
// CHEC
[PATCH] D46520: [Driver] Use -fuse-line-directives by default in MSVC mode
mstorsjo added a comment. In https://reviews.llvm.org/D46520#1092233, @rnk wrote: > Please don't do this, this is actually really problematic, since `#line` > directives lose information about what's a system header. That means the > result of -E usually won't compile, since Windows headers are typically full > of warnings and default-error warnings. Oh, ok - I can revert it then; I didn't have any specific need for this, I just noticed that clang in msvc mode and cl.exe differed in this aspect. Repository: rL LLVM https://reviews.llvm.org/D46520 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46383: implementing Cursor.get_included_file in python bindings
jlaz added a comment. @jbcoe could you commit this change? I don't have repository access for commit. Thank you. Repository: rC Clang https://reviews.llvm.org/D46383 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r331743 - [x86] Introduce the encl[u|s|v] intrinsics
Author: gbuella
Date: Tue May 8 00:12:34 2018
New Revision: 331743
URL: http://llvm.org/viewvc/llvm-project?rev=331743&view=rev
Log:
[x86] Introduce the encl[u|s|v] intrinsics
Reviewers: craig.topper, zvi
Reviewed By: craig.topper
Differential Revision: https://reviews.llvm.org/D46435
Added:
cfe/trunk/lib/Headers/sgxintrin.h (with props)
cfe/trunk/test/Headers/sgxintrin.c (with props)
Modified:
cfe/trunk/lib/Headers/CMakeLists.txt
cfe/trunk/lib/Headers/module.modulemap
cfe/trunk/lib/Headers/x86intrin.h
Modified: cfe/trunk/lib/Headers/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/CMakeLists.txt?rev=331743&r1=331742&r2=331743&view=diff
==
--- cfe/trunk/lib/Headers/CMakeLists.txt (original)
+++ cfe/trunk/lib/Headers/CMakeLists.txt Tue May 8 00:12:34 2018
@@ -78,6 +78,7 @@ set(files
prfchwintrin.h
rdseedintrin.h
rtmintrin.h
+ sgxintrin.h
s390intrin.h
shaintrin.h
smmintrin.h
Modified: cfe/trunk/lib/Headers/module.modulemap
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/module.modulemap?rev=331743&r1=331742&r2=331743&view=diff
==
--- cfe/trunk/lib/Headers/module.modulemap (original)
+++ cfe/trunk/lib/Headers/module.modulemap Tue May 8 00:12:34 2018
@@ -68,6 +68,7 @@ module _Builtin_intrinsics [system] [ext
textual header "waitpkgintrin.h"
textual header "movdirintrin.h"
textual header "pconfigintrin.h"
+textual header "sgxintrin.h"
explicit module mm_malloc {
requires !freestanding
Added: cfe/trunk/lib/Headers/sgxintrin.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/sgxintrin.h?rev=331743&view=auto
==
--- cfe/trunk/lib/Headers/sgxintrin.h (added)
+++ cfe/trunk/lib/Headers/sgxintrin.h Tue May 8 00:12:34 2018
@@ -0,0 +1,70 @@
+/*=== sgxintrin.h - X86 SGX intrinsics configuration ---===
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ *===---===
+ */
+
+#ifndef __X86INTRIN_H
+#error "Never use directly; include instead."
+#endif
+
+#ifndef __SGXINTRIN_H
+#define __SGXINTRIN_H
+
+/* Define the default attributes for the functions in this file. */
+#define __DEFAULT_FN_ATTRS \
+ __attribute__((__always_inline__, __nodebug__, __target__("sgx")))
+
+static __inline unsigned int __DEFAULT_FN_ATTRS
+_enclu_u32(unsigned int __leaf, __SIZE_TYPE__ __d[])
+{
+ unsigned int __result;
+ __asm__ ("enclu"
+ : "=a" (__result), "=b" (__d[0]), "=c" (__d[1]), "=d" (__d[2])
+ : "a" (__leaf), "b" (__d[0]), "c" (__d[1]), "d" (__d[2])
+ : "cc");
+ return __result;
+}
+
+static __inline unsigned int __DEFAULT_FN_ATTRS
+_encls_u32(unsigned int __leaf, __SIZE_TYPE__ __d[])
+{
+ unsigned int __result;
+ __asm__ ("encls"
+ : "=a" (__result), "=b" (__d[0]), "=c" (__d[1]), "=d" (__d[2])
+ : "a" (__leaf), "b" (__d[0]), "c" (__d[1]), "d" (__d[2])
+ : "cc");
+ return __result;
+}
+
+static __inline unsigned int __DEFAULT_FN_ATTRS
+_enclv_u32(unsigned int __leaf, __SIZE_TYPE__ __d[])
+{
+ unsigned int __result;
+ __asm__ ("enclv"
+ : "=a" (__result), "=b" (__d[0]), "=c" (__d[1]), "=d" (__d[2])
+ : "a" (__leaf), "b" (__d[0]), "c" (__d[1]), "d" (__d[2])
+ : "cc");
+ return __result;
+}
+
+#undef __DEFAULT_FN_ATTRS
+
+#endif
Propchange: cfe/trunk/lib/Headers/sgxintrin.h
--
svn:eol-style = native
Propchange: cfe/trunk/lib/Headers/sgxintrin.h
--
svn:keywords = Author Date Id Rev URL
Propchange: c
[PATCH] D46435: [x86] Introduce the encl[u|s|v] intrinsics
This revision was automatically updated to reflect the committed changes.
Closed by commit rC331743: [x86] Introduce the encl[u|s|v] intrinsics (authored
by GBuella, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D46435?vs=145205&id=145638#toc
Repository:
rC Clang
https://reviews.llvm.org/D46435
Files:
lib/Headers/CMakeLists.txt
lib/Headers/module.modulemap
lib/Headers/sgxintrin.h
lib/Headers/x86intrin.h
test/Headers/sgxintrin.c
Index: test/Headers/sgxintrin.c
===
--- test/Headers/sgxintrin.c
+++ test/Headers/sgxintrin.c
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 %s -ffreestanding -triple x86_64-unknown-unknown -target-feature +sgx -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-64
+// RUN: %clang_cc1 %s -ffreestanding -triple i386 -target-feature +sgx -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-32
+
+#include
+#include
+#include
+
+uint32_t test_encls(uint32_t leaf, size_t data[3]) {
+// CHECK-64: call { i32, i64, i64, i64 } asm "encls", "={ax},={bx},={cx},={dx},{ax},{bx},{cx},{dx},~{cc},~{dirflag},~{fpsr},~{flags}"(i32 %{{.*}}, i64 %{{.*}}, i64 %{{.*}}, i64 %{{.*}})
+// CHECK-32: call { i32, i32, i32, i32 } asm "encls", "={ax},={bx},={cx},={dx},{ax},{bx},{cx},{dx},~{cc},~{dirflag},~{fpsr},~{flags}"(i32 %{{.*}}, i32 %{{.*}}, i32 %{{.*}}, i32 %{{.*}})
+
+ return _encls_u32(leaf, data);
+}
+
+uint32_t test_enclu(uint32_t leaf, size_t data[3]) {
+// CHECK-64: call { i32, i64, i64, i64 } asm "enclu", "={ax},={bx},={cx},={dx},{ax},{bx},{cx},{dx},~{cc},~{dirflag},~{fpsr},~{flags}"(i32 %{{.*}}, i64 %{{.*}}, i64 %{{.*}}, i64 %{{.*}})
+// CHECK-32: call { i32, i32, i32, i32 } asm "enclu", "={ax},={bx},={cx},={dx},{ax},{bx},{cx},{dx},~{cc},~{dirflag},~{fpsr},~{flags}"(i32 %{{.*}}, i32 %{{.*}}, i32 %{{.*}}, i32 %{{.*}})
+
+ return _enclu_u32(leaf, data);
+}
+
+uint32_t test_enclv(uint32_t leaf, size_t data[3]) {
+// CHECK-64: call { i32, i64, i64, i64 } asm "enclv", "={ax},={bx},={cx},={dx},{ax},{bx},{cx},{dx},~{cc},~{dirflag},~{fpsr},~{flags}"(i32 %{{.*}}, i64 %{{.*}}, i64 %{{.*}}, i64 %{{.*}})
+// CHECK-32: call { i32, i32, i32, i32 } asm "enclv", "={ax},={bx},={cx},={dx},{ax},{bx},{cx},{dx},~{cc},~{dirflag},~{fpsr},~{flags}"(i32 %{{.*}}, i32 %{{.*}}, i32 %{{.*}}, i32 %{{.*}})
+
+ return _enclv_u32(leaf, data);
+}
Index: lib/Headers/sgxintrin.h
===
--- lib/Headers/sgxintrin.h
+++ lib/Headers/sgxintrin.h
@@ -0,0 +1,70 @@
+/*=== sgxintrin.h - X86 SGX intrinsics configuration ---===
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ *===---===
+ */
+
+#ifndef __X86INTRIN_H
+#error "Never use directly; include instead."
+#endif
+
+#ifndef __SGXINTRIN_H
+#define __SGXINTRIN_H
+
+/* Define the default attributes for the functions in this file. */
+#define __DEFAULT_FN_ATTRS \
+ __attribute__((__always_inline__, __nodebug__, __target__("sgx")))
+
+static __inline unsigned int __DEFAULT_FN_ATTRS
+_enclu_u32(unsigned int __leaf, __SIZE_TYPE__ __d[])
+{
+ unsigned int __result;
+ __asm__ ("enclu"
+ : "=a" (__result), "=b" (__d[0]), "=c" (__d[1]), "=d" (__d[2])
+ : "a" (__leaf), "b" (__d[0]), "c" (__d[1]), "d" (__d[2])
+ : "cc");
+ return __result;
+}
+
+static __inline unsigned int __DEFAULT_FN_ATTRS
+_encls_u32(unsigned int __leaf, __SIZE_TYPE__ __d[])
+{
+ unsigned int __result;
+ __asm__ ("encls"
+ : "=a" (__result), "=b" (__d[0]), "=c" (__d[1]), "=d" (__d[2])
+ : "a" (__leaf), "b" (__d[0]), "c" (__d[1]), "d" (__d[2])
+ : "cc");
+ return __result;
+}
+
+static __inline unsigned int __DEFAULT_FN_ATTRS
+_enclv_u32(unsigned int __leaf, __SIZE_TYPE__ __d[])
+{
+ unsigned int __result;
+ __asm__ ("enclv"
+ : "=a" (__result), "=b" (__d[0]), "=c" (__d[1]), "
[PATCH] D46115: [ASTImporter] properly import SrcLoc of Attr
r.stahl updated this revision to Diff 145641.
r.stahl marked 6 inline comments as done.
https://reviews.llvm.org/D46115
Files:
include/clang/AST/ASTImporter.h
lib/AST/ASTImporter.cpp
test/Import/attr/Inputs/S.cpp
test/Import/attr/test.cpp
Index: test/Import/attr/Inputs/S.cpp
===
--- test/Import/attr/Inputs/S.cpp
+++ test/Import/attr/Inputs/S.cpp
@@ -0,0 +1,13 @@
+extern void f() __attribute__((const));
+
+struct S {
+ struct {
+int a __attribute__((packed));
+ };
+};
+
+void stmt() {
+#pragma unroll
+ for (;;)
+;
+}
Index: test/Import/attr/test.cpp
===
--- test/Import/attr/test.cpp
+++ test/Import/attr/test.cpp
@@ -0,0 +1,26 @@
+// RUN: clang-import-test -dump-ast -import %S/Inputs/S.cpp -expression %s | FileCheck %s
+// CHECK: FunctionDecl
+// CHECK-SAME: S.cpp:1:1, col:13
+// CHECK-NEXT: ConstAttr
+// CHECK-SAME: col:32
+
+// CHECK: IndirectFieldDecl
+// CHECK-NEXT: Field
+// CHECK-NEXT: Field
+// CHECK-NEXT: PackedAttr
+// CHECK-SAME: col:26
+
+// CHECK: AttributedStmt
+// CHECK-NEXT: LoopHintAttr
+// CHECK-SAME: line:10:9
+
+extern void f() __attribute__((const));
+
+struct S;
+
+void stmt();
+
+void expr() {
+ f();
+ struct S s;
+}
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -2646,8 +2646,8 @@
Importer.getToContext(), DC, Loc, Name.getAsIdentifierInfo(), T,
{NamedChain, D->getChainingSize()});
- for (const auto *Attr : D->attrs())
-ToIndirectField->addAttr(Attr->clone(Importer.getToContext()));
+ for (const auto *A : D->attrs())
+ToIndirectField->addAttr(Importer.Import(A));
ToIndirectField->setAccess(D->getAccess());
ToIndirectField->setLexicalDeclContext(LexicalDC);
@@ -4721,15 +4721,8 @@
SourceLocation ToAttrLoc = Importer.Import(S->getAttrLoc());
ArrayRef FromAttrs(S->getAttrs());
SmallVector ToAttrs(FromAttrs.size());
- ASTContext &_ToContext = Importer.getToContext();
- std::transform(FromAttrs.begin(), FromAttrs.end(), ToAttrs.begin(),
-[&_ToContext](const Attr *A) -> const Attr * {
- return A->clone(_ToContext);
-});
- for (const auto *ToA : ToAttrs) {
-if (!ToA)
- return nullptr;
- }
+ if (ImportArrayChecked(FromAttrs.begin(), FromAttrs.end(), ToAttrs.begin()))
+return nullptr;
Stmt *ToSubStmt = Importer.Import(S->getSubStmt());
if (!ToSubStmt && S->getSubStmt())
return nullptr;
@@ -6544,6 +6537,12 @@
Import(FromTSI->getTypeLoc().getLocStart()));
}
+Attr *ASTImporter::Import(const Attr *FromAttr) {
+ Attr *ToAttr = FromAttr->clone(ToContext);
+ ToAttr->setRange(Import(FromAttr->getRange()));
+ return ToAttr;
+}
+
Decl *ASTImporter::GetAlreadyImportedOrNull(Decl *FromD) {
llvm::DenseMap::iterator Pos = ImportedDecls.find(FromD);
if (Pos != ImportedDecls.end()) {
@@ -7199,8 +7198,8 @@
Decl *ASTImporter::Imported(Decl *From, Decl *To) {
if (From->hasAttrs()) {
-for (auto *FromAttr : From->getAttrs())
- To->addAttr(FromAttr->clone(To->getASTContext()));
+for (const auto *FromAttr : From->getAttrs())
+ To->addAttr(Import(FromAttr));
}
if (From->isUsed()) {
To->setIsUsed();
Index: include/clang/AST/ASTImporter.h
===
--- include/clang/AST/ASTImporter.h
+++ include/clang/AST/ASTImporter.h
@@ -41,6 +41,7 @@
class Stmt;
class TagDecl;
class TypeSourceInfo;
+class Attr;
/// \brief Imports selected nodes from one AST context into another context,
/// merging AST nodes where appropriate.
@@ -130,6 +131,12 @@
/// context, or NULL if an error occurred.
TypeSourceInfo *Import(TypeSourceInfo *FromTSI);
+/// \brief Import the given attribute from the "from" context into the
+/// "to" context.
+///
+/// \returns the equivalent attribute in the "to" context.
+Attr *Import(const Attr *FromAttr);
+
/// \brief Import the given declaration from the "from" context into the
/// "to" context.
///
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46115: [ASTImporter] properly import SrcLoc of Attr
r.stahl added inline comments.
Comment at: include/clang/AST/ASTImporter.h:137
+///
+/// \returns the equivalent attribute in the "to" context, or NULL if an
+/// error occurred.
a.sidorin wrote:
> nullptr
I tried to stay consistent with the other descriptions. Will be removed anyway
(see below).
Comment at: lib/AST/ASTImporter.cpp:2650
+ for (const auto *A : D->attrs())
+ToIndirectField->addAttr(Importer.Import(const_cast(A)));
a.sidorin wrote:
> Could we just remove 'const' qualifier from `A` to avoid `const_cast`? (Same
> below)
I made the interface const instead.
Comment at: lib/AST/ASTImporter.cpp:6547
+Attr *ASTImporter::Import(Attr *FromAttr) {
+ if (!FromAttr)
+return nullptr;
a.sidorin wrote:
> Is it possible to get into a situation where a nullptr attribute is imported?
I looked at the Decl variant and thought it should act similarly, but as far as
I can tell, null attributes should not exist.
https://reviews.llvm.org/D46115
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
