[PATCH] D28047: Remove the '-disable-llvm-passes' flag (which I didn't even know existed, and I suspect many others aren't aware of either) and strength '-disable-llvm-optzns' to do the same thing.
chandlerc created this revision. chandlerc added reviewers: rsmith, mehdi_amini. chandlerc added a subscriber: cfe-commits. Herald added a subscriber: mcrosier. Much to my surprise, '-disable-llvm-optzns' which I thought was the magical flag I wanted to get at the raw LLVM IR coming out of Clang deosn't do that. It still runs some passes over the IR. I don't want that, I really want the *raw* IR coming out of Clang and I strongly suspect everyone else using it is in the same camp. I've not talked to anyone debugging Clang and LLVM interactions that needed these different flags so I'd like to remove the less widely known one (I think) and consolidate on a simple, strong model for the one flag left. This is part of simplifying how Clang drives LLVM to make it cleaner to wire up to the new pass manager. https://reviews.llvm.org/D28047 Files: include/clang/Driver/CC1Options.td include/clang/Frontend/CodeGenOptions.def lib/CodeGen/BackendUtil.cpp lib/Driver/Tools.cpp lib/Frontend/CompilerInvocation.cpp test/CodeGen/debug-info-scope.c test/CodeGen/sanitize-address-field-padding.cpp test/CodeGenCUDA/convergent.cu test/CodeGenCUDA/fp-contract.cu test/CodeGenCUDA/link-device-bitcode.cu test/CodeGenCUDA/nothrow.cu test/CodeGenCXX/alias-available-externally.cpp test/CodeGenCoroutines/coro-alloc.cpp test/CodeGenCoroutines/coro-builtins.c test/CodeGenCoroutines/microsoft-abi-operator-coawait.cpp test/CodeGenObjC/arc-linetable.m test/Driver/save-temps.c Index: test/Driver/save-temps.c === --- test/Driver/save-temps.c +++ test/Driver/save-temps.c @@ -2,7 +2,7 @@ // RUN: | FileCheck %s // CHECK: "-o" "save-temps.i" // CHECK: "-emit-llvm-uselists" -// CHECK: "-disable-llvm-passes" +// CHECK: "-disable-llvm-optzns" // CHECK: "-o" "save-temps.bc" // CHECK: "-o" "save-temps.s" // CHECK: "-o" "save-temps.o" @@ -14,7 +14,7 @@ // RUN: | FileCheck %s -check-prefix=CWD // CWD: "-o" "save-temps.i" // CWD: "-emit-llvm-uselists" -// CWD: "-disable-llvm-passes" +// CWD: "-disable-llvm-optzns" // CWD: "-o" "save-temps.bc" // CWD: "-o" "save-temps.s" // CWD: "-o" "save-temps.o" @@ -63,16 +63,16 @@ // RUN: %clang -target x86_64-apple-darwin -save-temps=obj -o obj/dir/a.out -arch x86_64 %s -### 2>&1 \ // RUN: | FileCheck %s -check-prefix=CHECK-OBJ // CHECK-OBJ: "-o" "obj/dir{{/|}}save-temps.i" -// CHECK-OBJ: "-disable-llvm-passes" +// CHECK-OBJ: "-disable-llvm-optzns" // CHECK-OBJ: "-o" "obj/dir{{/|}}save-temps.bc" // CHECK-OBJ: "-o" "obj/dir{{/|}}save-temps.s" // CHECK-OBJ: "-o" "obj/dir{{/|}}save-temps.o" // CHECK-OBJ: "-o" "obj/dir{{/|}}a.out" // // RUN: %clang -target x86_64-apple-darwin -save-temps=obj -arch x86_64 %s -### 2>&1 \ // RUN: | FileCheck %s -check-prefix=CHECK-OBJ-NOO // CHECK-OBJ-NOO: "-o" "save-temps.i" -// CHECK-OBJ-NOO: "-disable-llvm-passes" +// CHECK-OBJ-NOO: "-disable-llvm-optzns" // CHECK-OBJ-NOO: "-o" "save-temps.bc" // CHECK-OBJ-NOO: "-o" "save-temps.s" // CHECK-OBJ-NOO: "-o" "save-temps.o" Index: test/CodeGenObjC/arc-linetable.m === --- test/CodeGenObjC/arc-linetable.m +++ test/CodeGenObjC/arc-linetable.m @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -emit-llvm -fblocks -fobjc-arc -debug-info-kind=standalone -dwarf-version=4 -disable-llvm-passes -triple x86_64-apple-darwin10 %s -o - | FileCheck %s +// RUN: %clang_cc1 -emit-llvm -fblocks -fobjc-arc -debug-info-kind=standalone -dwarf-version=4 -disable-llvm-optzns -triple x86_64-apple-darwin10 %s -o - | FileCheck %s // Legend: EXP = Return expression, RET = ret instruction Index: test/CodeGenCoroutines/microsoft-abi-operator-coawait.cpp === --- test/CodeGenCoroutines/microsoft-abi-operator-coawait.cpp +++ test/CodeGenCoroutines/microsoft-abi-operator-coawait.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc18.0.0 -fcoroutines-ts -emit-llvm %s -o - -std=c++14 -disable-llvm-passes | FileCheck %s +// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc18.0.0 -fcoroutines-ts -emit-llvm %s -o - -std=c++14 -disable-llvm-optzns | FileCheck %s struct no_suspend { bool await_ready() { return true; } template void await_suspend(F) {} Index: test/CodeGenCoroutines/coro-builtins.c === --- test/CodeGenCoroutines/coro-builtins.c +++ test/CodeGenCoroutines/coro-builtins.c @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc18.0.0 -fcoroutines-ts -emit-llvm %s -o - -disable-llvm-passes | FileCheck %s +// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc18.0.0 -fcoroutines-ts -emit-llvm %s -o - -disable-llvm-optzns | FileCheck %s void *myAlloc(long long); Index: test/CodeGenCoroutines/coro-alloc.cpp === --- test/CodeGenCoroutines/coro-allo
[PATCH] D28034: [ASTMatchers] Add hasInClassInitializer traversal matcher for FieldDecl.
Prazek added inline comments. Comment at: docs/LibASTMatchersReference.html:2442 }; -fieldDecl(isBitField()) +fieldDecl(hasBitWidth(2)) matches 'int a;' and 'int c;' but not 'int b;'. Fix not connected to patch? Comment at: docs/LibASTMatchersReference.html:4763 +fieldDecl(hasInClassInitializer(integerLiteral(equals(2 + matches 'int a;' but not 'int b;'. + It would be good to add int c; without initializer and show that fieldDecl(hasInClassInitializer(anything())) will match to a and b Comment at: include/clang/ASTMatchers/ASTMatchers.h:547 /// \endcode -/// fieldDecl(isBitField()) +/// fieldDecl(hasBitWidth(2)) /// matches 'int a;' and 'int c;' but not 'int b;'. Samw https://reviews.llvm.org/D28034 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28048: [OpenCL] Align fake address space map with the SPIR target maps.
echuraev created this revision. echuraev added a reviewer: Anastasia. echuraev added subscribers: cfe-commits, bader, yaxunl. We compile user opencl kernel code with spir triple. But built-ins are written in OpenCL and we compile it with triple x86_64 to be able to use x86 intrinsics. And we need address spaces to match in both cases. So, we change fake address space map in OpenCL for matching with spir. On CPU address spaces are not really important but we'd like to preserve address space information in order to perform optimizations relying on this info like enhanced alias analysis. https://reviews.llvm.org/D28048 Files: lib/AST/ASTContext.cpp test/CodeGen/blocks-opencl.cl test/CodeGenOpenCL/address-space-constant-initializers.cl test/CodeGenOpenCL/address-spaces-mangling.cl test/CodeGenOpenCL/address-spaces.cl test/CodeGenOpenCL/cl20-device-side-enqueue.cl test/CodeGenOpenCL/const-str-array-decay.cl test/CodeGenOpenCL/constant-addr-space-globals.cl test/CodeGenOpenCL/local-initializer-undef.cl test/CodeGenOpenCL/local.cl test/CodeGenOpenCL/memcpy.cl test/CodeGenOpenCL/str_literals.cl test/SemaOpenCL/extern.cl Index: test/SemaOpenCL/extern.cl === --- test/SemaOpenCL/extern.cl +++ test/SemaOpenCL/extern.cl @@ -1,7 +1,7 @@ // RUN: %clang_cc1 -x cl -cl-opt-disable -cl-std=CL1.2 -emit-llvm -ffake-address-space-map %s -o - -verify | FileCheck %s // expected-no-diagnostics -// CHECK: @foo = external addrspace(3) constant float +// CHECK: @foo = external addrspace(2) constant float extern constant float foo; kernel void test(global float* buf) { Index: test/CodeGenOpenCL/str_literals.cl === --- test/CodeGenOpenCL/str_literals.cl +++ test/CodeGenOpenCL/str_literals.cl @@ -3,7 +3,7 @@ __constant char * __constant x = "hello world"; __constant char * __constant y = "hello world"; -// CHECK: unnamed_addr addrspace(3) constant -// CHECK-NOT: addrspace(3) unnamed_addr constant -// CHECK: @x = addrspace(3) constant i8 addrspace(3)* -// CHECK: @y = addrspace(3) constant i8 addrspace(3)* +// CHECK: unnamed_addr addrspace(2) constant +// CHECK-NOT: addrspace(2) unnamed_addr constant +// CHECK: @x = addrspace(2) constant i8 addrspace(2)* +// CHECK: @y = addrspace(2) constant i8 addrspace(2)* Index: test/CodeGenOpenCL/memcpy.cl === --- test/CodeGenOpenCL/memcpy.cl +++ test/CodeGenOpenCL/memcpy.cl @@ -2,7 +2,7 @@ // CHECK-LABEL: @test // CHECK-NOT: addrspacecast -// CHECK: call void @llvm.memcpy.p1i8.p3i8 +// CHECK: call void @llvm.memcpy.p1i8.p2i8 kernel void test(global float *g, constant float *c) { __builtin_memcpy(g, c, 32); } Index: test/CodeGenOpenCL/local.cl === --- test/CodeGenOpenCL/local.cl +++ test/CodeGenOpenCL/local.cl @@ -3,7 +3,7 @@ void func(local int*); __kernel void foo(void) { - // CHECK: @foo.i = internal addrspace(2) global i32 undef + // CHECK: @foo.i = internal addrspace(3) global i32 undef __local int i; func(&i); } Index: test/CodeGenOpenCL/local-initializer-undef.cl === --- test/CodeGenOpenCL/local-initializer-undef.cl +++ test/CodeGenOpenCL/local-initializer-undef.cl @@ -6,10 +6,10 @@ float z; } Foo; -// CHECK-DAG: @test.lds_int = internal addrspace(2) global i32 undef -// CHECK-DAG: @test.lds_int_arr = internal addrspace(2) global [128 x i32] undef -// CHECK-DAG: @test.lds_struct = internal addrspace(2) global %struct.Foo undef -// CHECK-DAG: @test.lds_struct_arr = internal addrspace(2) global [64 x %struct.Foo] undef +// CHECK-DAG: @test.lds_int = internal addrspace(3) global i32 undef +// CHECK-DAG: @test.lds_int_arr = internal addrspace(3) global [128 x i32] undef +// CHECK-DAG: @test.lds_struct = internal addrspace(3) global %struct.Foo undef +// CHECK-DAG: @test.lds_struct_arr = internal addrspace(3) global [64 x %struct.Foo] undef __kernel void test() { __local int lds_int; Index: test/CodeGenOpenCL/constant-addr-space-globals.cl === --- test/CodeGenOpenCL/constant-addr-space-globals.cl +++ test/CodeGenOpenCL/constant-addr-space-globals.cl @@ -12,9 +12,9 @@ // in the constant address space). void foo(constant const int *p1, const int *p2, const int *p3); -// CHECK: @k.arr1 = internal addrspace(3) constant [3 x i32] [i32 1, i32 2, i32 3] -// CHECK: @k.arr2 = private unnamed_addr addrspace(3) constant [3 x i32] [i32 4, i32 5, i32 6] -// CHECK: @k.arr3 = private unnamed_addr addrspace(3) constant [3 x i32] [i32 7, i32 8, i32 9] +// CHECK: @k.arr1 = internal addrspace(2) constant [3 x i32] [i32 1, i32 2, i32 3] +// CHECK: @k.arr2 = private unnamed_addr addrspace(2) constant [3 x i32] [i32 4, i32 5, i32 6] +// CHECK: @k.arr3 = pr
[PATCH] D28034: [ASTMatchers] Add hasInClassInitializer traversal matcher for FieldDecl.
malcolm.parsons added inline comments. Comment at: docs/LibASTMatchersReference.html:2442 }; -fieldDecl(isBitField()) +fieldDecl(hasBitWidth(2)) matches 'int a;' and 'int c;' but not 'int b;'. Prazek wrote: > Fix not connected to patch? Yes. The documentation is generated from the comments. Comment at: include/clang/ASTMatchers/ASTMatchers.h:547 /// \endcode -/// fieldDecl(isBitField()) +/// fieldDecl(hasBitWidth(2)) /// matches 'int a;' and 'int c;' but not 'int b;'. Prazek wrote: > Samw Yes. https://reviews.llvm.org/D28034 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28052: [change-namespace] consider namespace aliases to shorten qualified names.
ioeric created this revision. ioeric added a reviewer: hokein. ioeric added a subscriber: cfe-commits. https://reviews.llvm.org/D28052 Files: change-namespace/ChangeNamespace.cpp change-namespace/ChangeNamespace.h unittests/change-namespace/ChangeNamespaceTests.cpp Index: unittests/change-namespace/ChangeNamespaceTests.cpp === --- unittests/change-namespace/ChangeNamespaceTests.cpp +++ unittests/change-namespace/ChangeNamespaceTests.cpp @@ -825,6 +825,109 @@ EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code)); } +TEST_F(ChangeNamespaceTest, NamespaceAliasInGlobal) { + std::string Code = "namespace glob {\n" + "class Glob {};\n" + "}\n" + "namespace gl = glob;\n" + "namespace na {\n" + "namespace nb {\n" + "void f() { gl::Glob g; }\n" + "} // namespace nb\n" + "} // namespace na\n"; + + std::string Expected = "namespace glob {\n" + "class Glob {};\n" + "}\n" + "namespace gl = glob;\n" + "\n" + "namespace x {\n" + "namespace y {\n" + "void f() { gl::Glob g; }\n" + "} // namespace y\n" + "} // namespace x\n"; + EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code)); +} + +TEST_F(ChangeNamespaceTest, NamespaceAliasInNamespace) { + std::string Code = "namespace glob {\n" + "class Glob {};\n" + "}\n" + "namespace na {\n" + "namespace nb {\n" + "namespace gl = glob;\n" + "void f() { gl::Glob g; }\n" + "} // namespace nb\n" + "} // namespace na\n"; + + std::string Expected = "namespace glob {\n" + "class Glob {};\n" + "}\n" + "\n" + "namespace x {\n" + "namespace y {\n" + "namespace gl = glob;\n" + "void f() { gl::Glob g; }\n" + "} // namespace y\n" + "} // namespace x\n"; + EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code)); +} + +TEST_F(ChangeNamespaceTest, NamespaceAliasInAncestorNamespace) { + NewNamespace = "na::nx"; + std::string Code = "namespace glob {\n" + "class Glob {};\n" + "}\n" + "namespace other { namespace gl = glob; }\n" + "namespace na {\n" + "namespace ga = glob;\n" + "namespace nb {\n" + "void f() { ga::Glob g; }\n" + "} // namespace nb\n" + "} // namespace na\n"; + + std::string Expected = "namespace glob {\n" + "class Glob {};\n" + "}\n" + "namespace other { namespace gl = glob; }\n" + "namespace na {\n" + "namespace ga = glob;\n" + "\n" + "namespace nx {\n" + "void f() { ga::Glob g; }\n" + "} // namespace nx\n" + "} // namespace na\n"; + EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code)); +} + +TEST_F(ChangeNamespaceTest, NamespaceAliasInOtherNamespace) { + std::string Code = "namespace glob {\n" + "class Glob {};\n" + "}\n" + "namespace other { namespace gl = glob; }\n" + "namespace na {\n" + "namespace ga = glob;\n" + "namespace nb {\n" + "void f() { glob::Glob g; }\n" + "} // namespace nb\n" + "} // namespace na\n"; + + std::string Expected = "namespace glob {\n" + "class Glob {};\n" + "}\n" + "namespace other { namespace gl = glob; }\n" + "namespace na {\n" + "namespace ga = glob;\n" + "\n" + "} // namespace na\n" + "namespace x {\n" + "namespace y {\n" + "void f() { glob::Glo g; }\n" + "} // namespace y\n" + "} // namespace x\n"; + EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code)); +} + TEST_F(ChangeNamespaceTest, UsingDeclAfterReference) { std::string Code = "namespace glob {\n" "class Glob {};\n" Index: change-namespace/
[PATCH] D26465: [Diag] Optimize DiagnosticIDs::getDiagnosticSeverity
ogoffart marked an inline comment as done. ogoffart added a comment. In https://reviews.llvm.org/D26465#607860, @arphaman wrote: > What did you test the parsing on? Will this patch get similar improvements > for code that compiles without errors and warnings? It was benchamerked with https://github.com/woboq/woboq_codebrowser generating itself. The code does not contain warnings. (unless you really consider the warn_cxx98_compat_* to be warnings) Comment at: lib/Basic/DiagnosticIDs.cpp:423 +Mapping = &Pos->State->getOrAddMapping((diag::kind)DiagID); + } arphaman wrote: > I think it would be better if you wrap this piece of code in a static > function that returns `DiagnosticMapping &`, as it should allow you to get > rid of all these `.` to `->` changes below. The problem is that most things are private in DiagnosticsEngine, so i made it a privte member of DiagnosticIds (which is a friend) https://reviews.llvm.org/D26465 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28047: Remove the '-disable-llvm-passes' flag (which I didn't even know existed, and I suspect many others aren't aware of either) and strength '-disable-llvm-optzns' to do the same thing.
joerg added a comment. There are two issues I have with the flags right now. The first one you are addressing with this patch. The second issue is they remain CC1-only flags. Can we introduce a proper "-emit-raw-llvm" flag to work like "-emit-llvm" but without the additional IR processing. We can make "-disable-llvm-optzns" an alias on the CC1-level for backwards compatibility. It is fine as a second step, but it would be nice to consider the big picture before shuffling things around. https://reviews.llvm.org/D28047 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26465: [Diag] Optimize DiagnosticIDs::getDiagnosticSeverity
ogoffart updated this revision to Diff 82325. ogoffart marked an inline comment as done. https://reviews.llvm.org/D26465 Files: include/clang/Basic/Diagnostic.h include/clang/Basic/DiagnosticIDs.h lib/Basic/Diagnostic.cpp lib/Basic/DiagnosticIDs.cpp lib/Serialization/ASTReader.cpp Index: lib/Serialization/ASTReader.cpp === --- lib/Serialization/ASTReader.cpp +++ lib/Serialization/ASTReader.cpp @@ -5328,6 +5328,10 @@ diag::Severity Map = (diag::Severity)F.PragmaDiagMappings[Idx++]; DiagnosticMapping Mapping = Diag.makeUserMapping(Map, Loc); Diag.GetCurDiagState()->setMapping(DiagID, Mapping); + +// We are overriding a default behavior +Diag.DiagStates.front().getOrAddMapping(DiagID).setMightBeOverriden( +true); } } } Index: lib/Basic/DiagnosticIDs.cpp === --- lib/Basic/DiagnosticIDs.cpp +++ lib/Basic/DiagnosticIDs.cpp @@ -396,6 +396,23 @@ return toLevel(getDiagnosticSeverity(DiagID, Loc, Diag)); } +DiagnosticMapping & +DiagnosticIDs::getDiagnosticMapping(unsigned DiagID, SourceLocation Loc, +const DiagnosticsEngine &Diag) const { + // Calling GetDiagStatePointForLoc might be costly as it needs to create + // FullSourceLoc and compare them to the ones in the states. So first query + // the base state and check if it might be overridden. + DiagnosticsEngine::DiagState &BaseState = Diag.DiagStates.front(); + assert(&BaseState == Diag.DiagStatePoints.front().State); + DiagnosticMapping &Mapping = BaseState.getOrAddMapping((diag::kind)DiagID); + if (!Mapping.mightBeOverriden()) +return Mapping; + + DiagnosticsEngine::DiagStatePointsTy::iterator Pos = + Diag.GetDiagStatePointForLoc(Loc); + return Pos->State->getOrAddMapping((diag::kind)DiagID); +} + /// \brief Based on the way the client configured the Diagnostic /// object, classify the specified diagnostic ID into a Level, consumable by /// the DiagnosticClient. @@ -406,17 +423,11 @@ DiagnosticIDs::getDiagnosticSeverity(unsigned DiagID, SourceLocation Loc, const DiagnosticsEngine &Diag) const { assert(getBuiltinDiagClass(DiagID) != CLASS_NOTE); - // Specific non-error diagnostics may be mapped to various levels from ignored // to error. Errors can only be mapped to fatal. diag::Severity Result = diag::Severity::Fatal; - DiagnosticsEngine::DiagStatePointsTy::iterator -Pos = Diag.GetDiagStatePointForLoc(Loc); - DiagnosticsEngine::DiagState *State = Pos->State; - - // Get the mapping information, or compute it lazily. - DiagnosticMapping &Mapping = State->getOrAddMapping((diag::kind)DiagID); + DiagnosticMapping &Mapping = getDiagnosticMapping(DiagID, Loc, Diag); // TODO: Can a null severity really get here? if (Mapping.getSeverity() != diag::Severity()) Index: lib/Basic/Diagnostic.cpp === --- lib/Basic/Diagnostic.cpp +++ lib/Basic/Diagnostic.cpp @@ -201,6 +201,11 @@ } DiagnosticMapping Mapping = makeUserMapping(Map, L); + if (Loc.isValid()) { +// We are potentially overriding a default behavior +DiagStates.front().getOrAddMapping(Diag).setMightBeOverriden(true); + } + // Common case; setting all the diagnostics of a group in one place. if (Loc.isInvalid() || Loc == LastStateChangePos) { GetCurDiagState()->setMapping(Diag, Mapping); Index: include/clang/Basic/DiagnosticIDs.h === --- include/clang/Basic/DiagnosticIDs.h +++ include/clang/Basic/DiagnosticIDs.h @@ -84,6 +84,7 @@ unsigned IsPragma : 1; unsigned HasNoWarningAsError : 1; unsigned HasNoErrorAsFatal : 1; + unsigned MightBeOverriden : 1; public: static DiagnosticMapping Make(diag::Severity Severity, bool IsUser, @@ -94,6 +95,8 @@ Result.IsPragma = IsPragma; Result.HasNoWarningAsError = 0; Result.HasNoErrorAsFatal = 0; +Result.MightBeOverriden = 0; + return Result; } @@ -108,6 +111,11 @@ bool hasNoErrorAsFatal() const { return HasNoErrorAsFatal; } void setNoErrorAsFatal(bool Value) { HasNoErrorAsFatal = Value; } + + /// Only makes sense for the base state: specify if the diagnostic might be + /// changed for other source location + bool mightBeOverriden() const { return MightBeOverriden; } + void setMightBeOverriden(bool Value) { MightBeOverriden = Value; } }; /// \brief Used for handling and querying diagnostic IDs. @@ -261,6 +269,10 @@ getDiagnosticLevel(unsigned DiagID, SourceLocation Loc, const DiagnosticsEngine &Diag) const LLVM_READONLY; + DiagnosticMapping & + getDiagnosticMapping(unsigned DiagID, SourceLocation Loc, + const DiagnosticsEngine &Diag) const LLVM_READONLY; + diag::Severity getDiag
[PATCH] D26350: Keep invalid Switch in the AST
ogoffart added a comment. ping2 https://reviews.llvm.org/D26350 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28048: [OpenCL] Align fake address space map with the SPIR target maps.
Anastasia added a subscriber: pekka.jaaskelainen. Anastasia added a comment. I believe it's not that important what those numbers actually are, but having the consistency with SPIR might be better indeed at least even for documentation purposes! @pekka.jaaskelainen, I think you are using x86 target in the POCL toolchain, do you envision any issues with this change? https://reviews.llvm.org/D28048 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27981: Fix problems in "[OpenCL] Enabling the usage of CLK_NULL_QUEUE as compare operand."
Anastasia accepted this revision. Anastasia added a comment. This revision is now accepted and ready to land. LGTM! If you checked there are no warnings any more, it seems fine to commit! :) https://reviews.llvm.org/D27981 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r290337 - clang-format: Less eagerly try to keep label-value pairs on a line.
Author: djasper Date: Thu Dec 22 06:37:06 2016 New Revision: 290337 URL: http://llvm.org/viewvc/llvm-project?rev=290337&view=rev Log: clang-format: Less eagerly try to keep label-value pairs on a line. Before: string v = StrCat("aaa: ", SomeFunction(, aaa), bbb); After: string v = StrCat("aaa: ", SomeFunction(, aaa), bbb); Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp cfe/trunk/unittests/Format/FormatTest.cpp Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=290337&r1=290336&r2=290337&view=diff == --- cfe/trunk/lib/Format/TokenAnnotator.cpp (original) +++ cfe/trunk/lib/Format/TokenAnnotator.cpp Thu Dec 22 06:37:06 2016 @@ -2001,7 +2001,7 @@ unsigned TokenAnnotator::splitPenalty(co if (Left.isOneOf(tok::plus, tok::comma) && Left.Previous && Left.Previous->isLabelString() && (Left.NextOperator || Left.OperatorIndex != 0)) -return 100; +return 45; if (Right.is(tok::plus) && Left.isLabelString() && (Right.NextOperator || Right.OperatorIndex != 0)) return 25; Modified: cfe/trunk/unittests/Format/FormatTest.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=290337&r1=290336&r2=290337&view=diff == --- cfe/trunk/unittests/Format/FormatTest.cpp (original) +++ cfe/trunk/unittests/Format/FormatTest.cpp Thu Dec 22 06:37:06 2016 @@ -5220,6 +5220,10 @@ TEST_F(FormatTest, KeepStringLabelValueP verifyFormat("string v = StrCat(\": \" +\n" " (aaa + a));", getLLVMStyleWithColumns(40)); + verifyFormat( + "string v = StrCat(\"aaa: \",\n" + " SomeFunction(, .aaa),\n" + " bbb);"); } TEST_F(FormatTest, UnderstandsEquals) { ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28053: Cleanup the handling of noinline function attributes, -fno-inline, -fno-inline-functions, -O0, and optnone.
chandlerc created this revision. chandlerc added reviewers: rsmith, rnk, mehdi_amini. chandlerc added a subscriber: cfe-commits. chandlerc added a dependency: D28047: Remove the '-disable-llvm-passes' flag (which I didn't even know existed, and I suspect many others aren't aware of either) and strength '-disable-llvm-optzns' to do the same thing.. Herald added subscribers: nhaehnle, nemanjai, mcrosier, jholewinski. Herald added a reviewer: tstellarAMD. These were really, really tangled together: - We used the noinline LLVM attribute for -fno-inline - But not for -fno-inline-functions (breaking LTO) - But we did use it for -finline-hint-functions (yay, LTO is happy!) - But we didn't for -O0 (LTO is sad yet again...) - We had weird structuring of CodeGenOpts with both an inlining enumeration and a boolean. They interacted in weird ways and needlessly. - A *lot* of set smashing went on with setting these, and then got worse when we considered optnone and other inlining-effecting attributes. - A bunch of inline affecting attributes were managed in a completely different place from -fno-inline. - Even with -fno-inline we failed to put the LLVM noinline attribute onto many generated function definitions because they didn't show up as AST-level functions. - If you passed -O0 but -finline-functions we would run the normal inliner pass in LLVM despite it being in the O0 pipeline, which really doesn't make much sense. Sadly, this causes a bunch of churn in tests because we don't run the optimizer in the tests and check the contents of attribute sets. It would be awesome if attribute sets were a bit more FileCheck friendly, but oh well. I think this is a significant improvement and should remove the semantic need to change what inliner pass we run in order to comply with the requested inlining semantics by relying completely on attributes. It also cleans up tho optnone and related handling a bit. One unfortunate aspect of this is that for generating alwaysinline routines like those in OpenMP we end up removing noinline and then adding alwaysinline. I tried a bunch of other approaches, but because we recompute function attributes from scratch and don't have a declaration here I couldn't find anything substantially cleaner than this. Depends on https://reviews.llvm.org/D28047 https://reviews.llvm.org/D28053 Files: include/clang/Frontend/CodeGenOptions.def include/clang/Frontend/CodeGenOptions.h lib/CodeGen/BackendUtil.cpp lib/CodeGen/CGOpenMPRuntime.cpp lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp lib/CodeGen/CodeGenFunction.cpp lib/CodeGen/CodeGenModule.cpp lib/Frontend/CompilerInvocation.cpp test/CXX/special/class.dtor/p3-0x.cpp test/CodeGen/2008-04-08-NoExceptions.c test/CodeGen/address-safety-attr-kasan.cpp test/CodeGen/address-safety-attr.cpp test/CodeGen/address-space-field1.c test/CodeGen/alias.c test/CodeGen/attr-minsize.cpp test/CodeGen/attributes.c test/CodeGen/incomplete-function-type-2.c test/CodeGen/inline-optim.c test/CodeGen/mips16-attr.c test/CodeGen/mrtd.c test/CodeGen/ms-declspecs.c test/CodeGen/ppc64-complex-parms.c test/CodeGen/ppc64-complex-return.c test/CodeGen/ppc64-extend.c test/CodeGen/sanitize-thread-attr.cpp test/CodeGen/sanitize-thread-no-checking-at-run-time.m test/CodeGen/unwind-attr.c test/CodeGenCXX/attr.cpp test/CodeGenCXX/cxx11-exception-spec.cpp test/CodeGenCXX/cxx11-noreturn.cpp test/CodeGenCXX/derived-to-base.cpp test/CodeGenCXX/global-dtor-no-atexit.cpp test/CodeGenCXX/global-init.cpp test/CodeGenCXX/inline-hint.cpp test/CodeGenCXX/main-norecurse.cpp test/CodeGenCXX/microsoft-abi-array-cookies.cpp test/CodeGenCXX/no-exceptions.cpp test/CodeGenCXX/optnone-class-members.cpp test/CodeGenCXX/optnone-def-decl.cpp test/CodeGenCXX/reference-cast.cpp test/CodeGenCXX/threadsafe-statics.cpp test/CodeGenCXX/thunks.cpp test/CodeGenCXX/virtual-base-cast.cpp test/CodeGenObjC/gnu-exceptions.m test/CodeGenObjC/objc-literal-tests.m test/CodeGenObjCXX/lambda-expressions.mm test/CodeGenOpenCL/amdgpu-attrs.cl test/Driver/darwin-iphone-defaults.m test/PCH/objc_container.m Index: test/PCH/objc_container.m === --- test/PCH/objc_container.m +++ test/PCH/objc_container.m @@ -21,5 +21,5 @@ // CHECK-IR: {{call.*objc_msgSend}} // CHECK-IR: ret void -// CHECK-IR: attributes #0 = { nounwind {{.*}} } +// CHECK-IR: attributes #0 = { noinline nounwind {{.*}} } // CHECK-IR: attributes #1 = { nonlazybind } Index: test/Driver/darwin-iphone-defaults.m === --- test/Driver/darwin-iphone-defaults.m +++ test/Driver/darwin-iphone-defaults.m @@ -26,4 +26,4 @@ [I1 alloc]; } -// CHECK: attributes [[F0]] = { ssp{{.*}} } +// CHECK: attributes [[F0]] = { noinline ssp{{.*}} } Index: test/CodeGenOpenCL/amdgpu-attrs.cl
[PATCH] D27753: [analyzer] alpha.security.DirtyScalar Checker
gerazo updated this revision to Diff 82336. https://reviews.llvm.org/D27753 Files: include/clang/StaticAnalyzer/Checkers/Checkers.td lib/StaticAnalyzer/Checkers/CMakeLists.txt lib/StaticAnalyzer/Checkers/DirtyScalarChecker.cpp lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp test/Analysis/dirty-scalar-unbounded.cpp Index: test/Analysis/dirty-scalar-unbounded.cpp === --- /dev/null +++ test/Analysis/dirty-scalar-unbounded.cpp @@ -0,0 +1,92 @@ +// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.security.taint,alpha.security.DirtyScalar -verify -analyzer-config alpha.security.DirtyScalar:criticalOnly=true %s +// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.security.taint,alpha.security.DirtyScalar -verify -analyzer-config alpha.security.DirtyScalar:criticalOnly=false -DDIRTYSCALARSTRICT=1 %s + +#include "Inputs/system-header-simulator.h" + +typedef long ssize_t; + +ssize_t recv(int s, void *buf, size_t len, int flags); + +void gets_tainted_ival(int val) { + (void)val; +} + +void gets_tainted_uval(unsigned int val) { + (void)val; +} + +int tainted_usage() { + int size; + scanf("%d", &size); + char *buff = new char[size]; // expected-warning{{Tainted variable is used without proper bound checking}} + for (int i = 0; i < size; ++i) { +#if DIRTYSCALARSTRICT +// expected-warning@-2{{Tainted variable is used without proper bound checking}} +#endif +scanf("%d", &buff[i]); + } + buff[size - 1] = 0; // expected-warning{{Tainted variable is used without proper bound checking}} + *(buff + size - 2) = 0; // expected-warning{{Tainted variable is used without proper bound checking}} + gets_tainted_ival(size); +#if DIRTYSCALARSTRICT +// expected-warning@-2{{Tainted variable is used without proper bound checking}} +#endif + + return 0; +} + +int tainted_usage_checked() { + int size; + scanf("%d", &size); + if (size < 0 || size > 255) +return -1; + char *buff = new char[size]; // no warning + for (int i = 0; i < size; ++i) { // no warning +scanf("%d", &buff[i]); // no warning + } + buff[size - 1] = 0; // no warning + *(buff + size - 2) = 0; // no warning + gets_tainted_ival(size); // no warning + + unsigned int idx; + scanf("%d", &idx); + if (idx > 255) +return -1; + gets_tainted_uval(idx); // no warning + + return 0; +} + +int detect_tainted(char const **messages) { + int sock, index; + scanf("%d", &sock); + if (recv(sock, &index, sizeof(index), 0) != sizeof(index)) { +#if DIRTYSCALARSTRICT +// expected-warning@-2{{Tainted variable is used without proper bound checking}} +#endif +return -1; + } + int index2 = index; + printf("%s\n", messages[index]); // expected-warning{{Tainted variable is used without proper bound checking}} + printf("%s\n", messages[index2]); // expected-warning{{Tainted variable is used without proper bound checking}} + + return 0; +} + +int skip_sizes_likely_used_for_table_access(char const **messages) { + int sock; + char byte; + + scanf("%d", &sock); + if (recv(sock, &byte, sizeof(byte), 0) != sizeof(byte)) { +#if DIRTYSCALARSTRICT +// expected-warning@-2{{Tainted variable is used without proper bound checking}} +#endif +return -1; + } + char byte2 = byte; + printf("%s\n", messages[byte]); // no warning + printf("%s\n", messages[byte2]); // no warning + + return 0; +} Index: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp === --- lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp +++ lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp @@ -222,6 +222,7 @@ .Case("getline", TaintPropagationRule(2, 0)) .Case("getdelim", TaintPropagationRule(3, 0)) .Case("fgetln", TaintPropagationRule(0, ReturnValueIndex)) +.Case("recv", TaintPropagationRule(InvalidArgIndex, 1, true)) .Default(TaintPropagationRule()); if (!Rule.isNull()) Index: lib/StaticAnalyzer/Checkers/DirtyScalarChecker.cpp === --- /dev/null +++ lib/StaticAnalyzer/Checkers/DirtyScalarChecker.cpp @@ -0,0 +1,231 @@ +//===-- DirtyScalarChecker.cpp *- C++ -*--// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// +// +// Reports the usage of dirty integers in code. +// A dirty value is tainted and wasn't bound checked properly by the programmer. +// By default (criticalOnly == true) reports dirty usage in +// - memcpy, malloc, calloc, strcpy, strncpy, memmove functions +// - array indexing +// - memory allocation with new +// - pointer arithmetic +// otherwise (criticalOnly == false) it also reports usage as +// - function argument +// - loop condition +// +//===--
[PATCH] D27753: [analyzer] alpha.security.DirtyScalar Checker
gerazo marked 6 inline comments as done. gerazo added a comment. Thank you very much for your help. I've added all suggested modifications including tests covering all checker option settings. https://reviews.llvm.org/D27753 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27753: [analyzer] alpha.security.DirtyScalar Checker
gerazo marked an inline comment as done. gerazo added a comment. So thank you again for the valuable questions. In this checker, I give warnings for values which are both tainted and were also not checked by the programmer. So unlike GenericTaintChecker, I do implement the boundedness check here for certain, interesting constructs (which is controlled by the critical option). GenericTaintChecker focuses purely on taintedness, almost like a service for other checkers. I've added a new rule to it, improving the taintedness logic, but I felt mixing the bound checking logic into it would make the two ideas inseparable. I've also checked others using tainted values. ArrayBoundCheckerV2 also works with array indexing and modifies its behaviour on finding tainted values. However, the main difference is that ArrayBoundCheckerV2 uses region information to check bounds which may or may not be present, while this checker can give a warning whenever any information from the constraint manager does not prove that any bound checking were performed on the value before (and potentially works on many other constructs where region information shouldn't be there at all). Not having correct region information with tainted values is typical when reading data from unknown sources. This dirty approach is better in this regard. ArrayBoundCheckerV2 on the other hand can give warnings solely based on region information. Yes, it can happen that both will give a warning as well for the same construct. Do you think it is distracting? Should I remove the array indexing checks from this checker (still it gives warning for pointer arithmetic as well)? Comment at: test/Analysis/dirty-scalar-unbounded.cpp:1 +// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.security.taint,alpha.security.DirtyScalar -verify -analyzer-config alpha.security.DirtyScalar:criticalOnly=false %s + a.sidorin wrote: > 1. It will be good to have tests for option set to true. > 2. Is there any test that makes usage of 'RecurseLimit` variable? Now both settings are covered. For RecurseLimit, I've added a named constant and better explanation. This is only a practical limit to not let the analysis dive too deep into complex loop expressions. The limit currently set should be adequate, so it would not make sense to set it programmatically. https://reviews.llvm.org/D27753 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r290340 - [clang-tidy] cppcoreguidelines-slicing: display discarded state size in bytes
Author: courbet Date: Thu Dec 22 08:12:31 2016 New Revision: 290340 URL: http://llvm.org/viewvc/llvm-project?rev=290340&view=rev Log: [clang-tidy] cppcoreguidelines-slicing: display discarded state size in bytes https://reviews.llvm.org/D27212 Modified: clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SlicingCheck.cpp clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-slicing.cpp Modified: clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SlicingCheck.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SlicingCheck.cpp?rev=290340&r1=290339&r2=290340&view=diff == --- clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SlicingCheck.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SlicingCheck.cpp Thu Dec 22 08:12:31 2016 @@ -122,10 +122,11 @@ void SlicingCheck::check(const MatchFind BaseDecl->getASTContext().getASTRecordLayout(BaseDecl); const auto &DerivedLayout = DerivedDecl->getASTContext().getASTRecordLayout(DerivedDecl); - const auto StateSize = DerivedLayout.getDataSize() - BaseLayout.getDataSize(); + const CharUnits StateSize = + DerivedLayout.getDataSize() - BaseLayout.getDataSize(); if (StateSize.isPositive()) { diag(Call->getExprLoc(), "slicing object from type %0 to %1 discards " - "%2*sizeof(char) bytes of state") + "%2 bytes of state") << DerivedDecl << BaseDecl << static_cast(StateSize.getQuantity()); } } Modified: clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-slicing.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-slicing.cpp?rev=290340&r1=290339&r2=290340&view=diff == --- clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-slicing.cpp (original) +++ clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-slicing.cpp Thu Dec 22 08:12:31 2016 @@ -31,18 +31,18 @@ DerivedWithMemberVariables ReturnsDerive void positivesWithMemberVariables() { DerivedWithMemberVariables b; Base a{b}; - // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: slicing object from type 'DerivedWithMemberVariables' to 'Base' discards {{[0-9]*}}*sizeof(char) bytes of state [cppcoreguidelines-slicing] + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: slicing object from type 'DerivedWithMemberVariables' to 'Base' discards {{[0-9]*}} bytes of state [cppcoreguidelines-slicing] a = b; - // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: slicing object from type 'DerivedWithMemberVariables' to 'Base' discards {{[0-9]*}}*sizeof(char) bytes of state + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: slicing object from type 'DerivedWithMemberVariables' to 'Base' discards {{[0-9]*}} bytes of state TakesBaseByValue(b); - // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: slicing object from type 'DerivedWithMemberVariables' to 'Base' discards {{[0-9]*}}*sizeof(char) bytes of state + // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: slicing object from type 'DerivedWithMemberVariables' to 'Base' discards {{[0-9]*}} bytes of state TwiceDerivedWithNoMemberVariables c; a = c; - // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: slicing object from type 'TwiceDerivedWithNoMemberVariables' to 'Base' discards {{[0-9]*}}*sizeof(char) bytes of state + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: slicing object from type 'TwiceDerivedWithNoMemberVariables' to 'Base' discards {{[0-9]*}} bytes of state a = ReturnsDerived(); - // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: slicing object from type 'DerivedWithMemberVariables' to 'Base' discards {{[0-9]*}}*sizeof(char) bytes of state + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: slicing object from type 'DerivedWithMemberVariables' to 'Base' discards {{[0-9]*}} bytes of state } void positivesWithOverride() { ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [clang-tools-extra] r290202 - [clang-tidy] Add modernize-use-default-member-init check
On Wed, Dec 21, 2016 at 6:10 PM, NAKAMURA Takumi wrote: > It'd be the issue if the test depended on installed headers. The builder > doesn't have MS headers installed. Those type names do not require any headers to be installed. They're keywords in C++11 and later, but MSVC did not support them until MSVC 2015. ~Aaron > > On Thu, Dec 22, 2016 at 1:27 AM Aaron Ballman > wrote: >> >> On Tue, Dec 20, 2016 at 5:58 PM, Malcolm Parsons >> wrote: >> > On 20 December 2016 at 22:32, Aaron Ballman >> > wrote: >> >> On Tue, Dec 20, 2016 at 4:26 PM, Malcolm Parsons via cfe-commits >> >> wrote: >> >>> Author: malcolm.parsons >> >>> Date: Tue Dec 20 15:26:07 2016 >> >>> New Revision: 290202 >> >>> >> >>> URL: http://llvm.org/viewvc/llvm-project?rev=290202&view=rev >> >>> Log: >> >>> [clang-tidy] Add modernize-use-default-member-init check >> >>> >> >>> Summary: Fixes PR18858 >> >> >> >> This appears to have broken one of the bots: >> >> >> >> http://bb.pgr.jp/builders/ninja-x64-msvc-RA-centos6/builds/33046 >> > >> > error: unknown type name 'char16_t' >> > error: unknown type name 'char32_t' >> > >> > I see commented tests in other clang-tidy tests: >> > >> > // TODO: enable these tests once all supported compilers >> > // support char16_t and char32_t (VS2013 does not) >> > >> > // disabled for now until it is clear >> > // how to enable them in the test >> > //} catch (const char16_t*) { >> >> We no longer support MSVC 2013, so that should not be an issue. >> Takumi, I'm not certain what to make of this builder (MSVC centos?). >> What version of Visual Studio is it compiling with? Or is this an >> instance where we need to set the -fms-compatibility-version=19 when >> running the test with clang-cl? >> >> ~Aaron >> >> > >> > -- >> > Malcolm Parsons ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28022: [clang-tidy] Handle constructors in performance-unnecessary-value-param
alexfh added inline comments. Comment at: clang-tidy/performance/UnnecessaryValueParamCheck.cpp:96 - // Do not trigger on non-const value parameters when: - // 1. they are in a constructor definition since they can likely trigger - //modernize-pass-by-value which will suggest to move the argument. - if (!IsConstQualified && (llvm::isa(Function) || -!Function->doesThisDeclarationHaveABody())) -return; + const FunctionDecl &Definition = *Function->getDefinition(); aaron.ballman wrote: > malcolm.parsons wrote: > > aaron.ballman wrote: > > > malcolm.parsons wrote: > > > > malcolm.parsons wrote: > > > > > aaron.ballman wrote: > > > > > > Instead of using `hasBody()` and `getDefinition()`, you should use > > > > > > `FunctionDecl::getBody()` and pass in an argument to receive the > > > > > > function's definition. > > > > > I don't want the body - the `CXXCtorInitializer`s are not in it. > > > > I should use `hasBody()` and pass in an argument. > > > You are calling `Function-hasBody()` followed by > > > `Function->getDefinition()`, which is what `FunctionDecl::getBody()` > > > does. The difference is that `FunctionDecl::getBody()` will also load > > > serialized bodies from the AST (if the body happens to be in a PCH, for > > > instance). > > Do I want to load serialized bodies? > I would imagine you would want to deserialize, since you care about the > contents of the body, not just the presence of one. This may or may not be > important with modules (Richard Smith would be able to give a more definitive > answer there). I would suggest writing minimal sane code that works until we have good reasons to write more complex code to support module-enabled builds (and appropriate tests). Comment at: clang-tidy/utils/DeclRefExprUtils.h:51 + +/// Returns true if DeclRefExpr is the argument of a copy-constructor call +/// expression within Decl. Please enclose `true`, `DeclRefExpr` and other inline code snippets in double backquotes. https://reviews.llvm.org/D28022 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r290341 - [analyzer] Improve suppress-on-sink behavior in incomplete analyses.
Author: dergachev Date: Thu Dec 22 08:48:52 2016 New Revision: 290341 URL: http://llvm.org/viewvc/llvm-project?rev=290341&view=rev Log: [analyzer] Improve suppress-on-sink behavior in incomplete analyses. Warnings with suppress-on-sink are discarded during FlushReports when BugReporter notices that all paths in ExplodedGraph that pass through the warning eventually run into a sink node. However, suppress-on-sink fails to filter out false positives when the analysis terminates too early - by running into analyzer limits, such as block count limits or graph size limits - and the interruption hits the narrow window between throwing the leak report and reaching the no-return function call. In such case the report is there, however suppression-on-sink doesn't work, because the sink node was never constructed in the incomplete ExplodedGraph. This patch implements a very partial solution: also suppress reports thrown against a statement-node that corresponds to a statement that belongs to a no-return block of the CFG. rdar://problem/28832541 Differential Revision: https://reviews.llvm.org/D28023 Added: cfe/trunk/test/Analysis/max-nodes-suppress-on-sink.c Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp?rev=290341&r1=290340&r2=290341&view=diff == --- cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp Thu Dec 22 08:48:52 2016 @@ -21,6 +21,7 @@ #include "clang/AST/StmtCXX.h" #include "clang/AST/StmtObjC.h" #include "clang/Analysis/CFG.h" +#include "clang/Analysis/CFGStmtMap.h" #include "clang/Analysis/ProgramPoint.h" #include "clang/Basic/SourceManager.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" @@ -3285,6 +3286,19 @@ struct FRIEC_WLItem { }; } +static const CFGBlock *findBlockForNode(const ExplodedNode *N) { + ProgramPoint P = N->getLocation(); + if (auto BEP = P.getAs()) +return BEP->getBlock(); + + // Find the node's current statement in the CFG. + if (const Stmt *S = PathDiagnosticLocation::getStmt(N)) +return N->getLocationContext()->getAnalysisDeclContext() + ->getCFGStmtMap()->getBlock(S); + + return nullptr; +} + static BugReport * FindReportInEquivalenceClass(BugReportEquivClass& EQ, SmallVectorImpl &bugReports) { @@ -,6 +3347,18 @@ FindReportInEquivalenceClass(BugReportEq continue; } +// See if we are in a no-return CFG block. If so, treat this similarly +// to being post-dominated by a sink. This works better when the analysis +// is incomplete and we have never reached a no-return function +// we're post-dominated by. +// This is not quite enough to handle the incomplete analysis case. +// We may be post-dominated in subsequent blocks, or even +// inter-procedurally. However, it is not clear if more complicated +// cases are generally worth suppressing. +if (const CFGBlock *B = findBlockForNode(errorNode)) + if (B->hasNoReturnElement()) +continue; + // At this point we know that 'N' is not a sink and it has at least one // successor. Use a DFS worklist to find a non-sink end-of-path node. typedef FRIEC_WLItem WLItem; Added: cfe/trunk/test/Analysis/max-nodes-suppress-on-sink.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/max-nodes-suppress-on-sink.c?rev=290341&view=auto == --- cfe/trunk/test/Analysis/max-nodes-suppress-on-sink.c (added) +++ cfe/trunk/test/Analysis/max-nodes-suppress-on-sink.c Thu Dec 22 08:48:52 2016 @@ -0,0 +1,31 @@ +// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-config max-nodes=12 -verify %s + +// Here we test how "suppress on sink" feature of certain bugtypes interacts +// with reaching analysis limits. + +// If we report a warning of a bug-type with "suppress on sink" attribute set +// (such as MallocChecker's memory leak warning), then failing to reach the +// reason for the sink (eg. no-return function such as "exit()") due to analysis +// limits (eg. max-nodes option), we may produce a false positive. + +typedef __typeof(sizeof(int)) size_t; +void *malloc(size_t); + +extern void exit(int) __attribute__ ((__noreturn__)); + +void clang_analyzer_warnIfReached(void); + +void test_single_cfg_block_sink() { + void *p = malloc(1); // no-warning (wherever the leak warning may occur here) + + // Due to max-nodes option in the run line, we should reach the first call + // but bail out before the second call. + // If the test on these two lines starts failing, see if modifying + // the max-nodes run-line helps. + clang_analyzer_warnIfReached
[PATCH] D28023: [analyzer] Fix leak false positives before no-return functions caused by incomplete analyses.
This revision was automatically updated to reflect the committed changes. Closed by commit rL290341: [analyzer] Improve suppress-on-sink behavior in incomplete analyses. (authored by dergachev). Changed prior to commit: https://reviews.llvm.org/D28023?vs=82229&id=82339#toc Repository: rL LLVM https://reviews.llvm.org/D28023 Files: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp cfe/trunk/test/Analysis/max-nodes-suppress-on-sink.c Index: cfe/trunk/test/Analysis/max-nodes-suppress-on-sink.c === --- cfe/trunk/test/Analysis/max-nodes-suppress-on-sink.c +++ cfe/trunk/test/Analysis/max-nodes-suppress-on-sink.c @@ -0,0 +1,31 @@ +// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-config max-nodes=12 -verify %s + +// Here we test how "suppress on sink" feature of certain bugtypes interacts +// with reaching analysis limits. + +// If we report a warning of a bug-type with "suppress on sink" attribute set +// (such as MallocChecker's memory leak warning), then failing to reach the +// reason for the sink (eg. no-return function such as "exit()") due to analysis +// limits (eg. max-nodes option), we may produce a false positive. + +typedef __typeof(sizeof(int)) size_t; +void *malloc(size_t); + +extern void exit(int) __attribute__ ((__noreturn__)); + +void clang_analyzer_warnIfReached(void); + +void test_single_cfg_block_sink() { + void *p = malloc(1); // no-warning (wherever the leak warning may occur here) + + // Due to max-nodes option in the run line, we should reach the first call + // but bail out before the second call. + // If the test on these two lines starts failing, see if modifying + // the max-nodes run-line helps. + clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}} + clang_analyzer_warnIfReached(); // no-warning + + // Even though we do not reach this line, we should still suppress + // the leak report. + exit(0); +} Index: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp === --- cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp +++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp @@ -21,6 +21,7 @@ #include "clang/AST/StmtCXX.h" #include "clang/AST/StmtObjC.h" #include "clang/Analysis/CFG.h" +#include "clang/Analysis/CFGStmtMap.h" #include "clang/Analysis/ProgramPoint.h" #include "clang/Basic/SourceManager.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" @@ -3285,6 +3286,19 @@ }; } +static const CFGBlock *findBlockForNode(const ExplodedNode *N) { + ProgramPoint P = N->getLocation(); + if (auto BEP = P.getAs()) +return BEP->getBlock(); + + // Find the node's current statement in the CFG. + if (const Stmt *S = PathDiagnosticLocation::getStmt(N)) +return N->getLocationContext()->getAnalysisDeclContext() + ->getCFGStmtMap()->getBlock(S); + + return nullptr; +} + static BugReport * FindReportInEquivalenceClass(BugReportEquivClass& EQ, SmallVectorImpl &bugReports) { @@ -,6 +3347,18 @@ continue; } +// See if we are in a no-return CFG block. If so, treat this similarly +// to being post-dominated by a sink. This works better when the analysis +// is incomplete and we have never reached a no-return function +// we're post-dominated by. +// This is not quite enough to handle the incomplete analysis case. +// We may be post-dominated in subsequent blocks, or even +// inter-procedurally. However, it is not clear if more complicated +// cases are generally worth suppressing. +if (const CFGBlock *B = findBlockForNode(errorNode)) + if (B->hasNoReturnElement()) +continue; + // At this point we know that 'N' is not a sink and it has at least one // successor. Use a DFS worklist to find a non-sink end-of-path node. typedef FRIEC_WLItem WLItem; Index: cfe/trunk/test/Analysis/max-nodes-suppress-on-sink.c === --- cfe/trunk/test/Analysis/max-nodes-suppress-on-sink.c +++ cfe/trunk/test/Analysis/max-nodes-suppress-on-sink.c @@ -0,0 +1,31 @@ +// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-config max-nodes=12 -verify %s + +// Here we test how "suppress on sink" feature of certain bugtypes interacts +// with reaching analysis limits. + +// If we report a warning of a bug-type with "suppress on sink" attribute set +// (such as MallocChecker's memory leak warning), then failing to reach the +// reason for the sink (eg. no-return function such as "exit()") due to analysis +// limits (eg. max-nodes option), we may produce a false positive. + +typedef __typeof(sizeof(int)) size_t; +void *malloc(size_t); + +extern void exit(int) __attribute__ ((__noreturn__)); + +void clang_analyzer_warnIfReached(void); + +void test_single_cfg_bloc
[PATCH] D28034: [ASTMatchers] Add hasInClassInitializer traversal matcher for FieldDecl.
aaron.ballman accepted this revision. aaron.ballman added a reviewer: aaron.ballman. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, modulo the unrelated documentation changes. https://reviews.llvm.org/D28034 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28022: [clang-tidy] Handle constructors in performance-unnecessary-value-param
aaron.ballman added inline comments. Comment at: clang-tidy/performance/UnnecessaryValueParamCheck.cpp:96 - // Do not trigger on non-const value parameters when: - // 1. they are in a constructor definition since they can likely trigger - //modernize-pass-by-value which will suggest to move the argument. - if (!IsConstQualified && (llvm::isa(Function) || -!Function->doesThisDeclarationHaveABody())) -return; + const FunctionDecl &Definition = *Function->getDefinition(); alexfh wrote: > aaron.ballman wrote: > > malcolm.parsons wrote: > > > aaron.ballman wrote: > > > > malcolm.parsons wrote: > > > > > malcolm.parsons wrote: > > > > > > aaron.ballman wrote: > > > > > > > Instead of using `hasBody()` and `getDefinition()`, you should > > > > > > > use `FunctionDecl::getBody()` and pass in an argument to receive > > > > > > > the function's definition. > > > > > > I don't want the body - the `CXXCtorInitializer`s are not in it. > > > > > I should use `hasBody()` and pass in an argument. > > > > You are calling `Function-hasBody()` followed by > > > > `Function->getDefinition()`, which is what `FunctionDecl::getBody()` > > > > does. The difference is that `FunctionDecl::getBody()` will also load > > > > serialized bodies from the AST (if the body happens to be in a PCH, for > > > > instance). > > > Do I want to load serialized bodies? > > I would imagine you would want to deserialize, since you care about the > > contents of the body, not just the presence of one. This may or may not be > > important with modules (Richard Smith would be able to give a more > > definitive answer there). > I would suggest writing minimal sane code that works until we have good > reasons to write more complex code to support module-enabled builds (and > appropriate tests). I think either approach is minimal, sane code and we should understand why we shouldn't use the interface provided by `FunctionDecl::getBody()` for getting the definition before deciding to not use it. I *think* we're fine to not use `getBody()` because the matcher should match over all of the function declarations with a definition, and so eventually it will find the definition with the actual body. In the case that the function definition is serialized, I think this already does the right thing and deserializes it. (I think the only case where we need the deserialization may be when we have a function declaration and we need to traverse from that to the function body, which may be in a different `FunctionDecl`.) So the use of `hasBody()` in the matcher above seems like the correct solution to me. https://reviews.llvm.org/D28022 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28052: [change-namespace] consider namespace aliases to shorten qualified names.
hokein added inline comments. Comment at: change-namespace/ChangeNamespace.cpp:719 +// The alias is defined in some namespace. +assert(AliasQualifiedName.size() >= AliasName.size() + 2); +llvm::StringRef AliasNs = maybe `assert(StringRef(AliasQualifiedName).end_with("::"+AliasName))`? Comment at: change-namespace/ChangeNamespace.cpp:741 if (TargetDecl == FromDecl) { ReplaceName = FromDecl->getNameAsString(); Matched = true; Do we also need to check `FromDecl->getNameAsString().size() < ReplaceName.size()` before assigning the name to `ReplaceName`? Comment at: unittests/change-namespace/ChangeNamespaceTests.cpp:832 + "}\n" + "namespace gl = glob;\n" + "namespace na {\n" Add a testcase for the namespace alias decl with preceding `::`, e.g. `namespace gl = ::glob`? https://reviews.llvm.org/D28052 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r290297 - Add the alloc_size attribute to clang, attempt 2.
Hi George, I'm guessing this may have caused PR31453. Simple reproducer included in bug report. Chad On 2016-12-21 21:50, George Burgess IV via cfe-commits wrote: Author: gbiv Date: Wed Dec 21 20:50:20 2016 New Revision: 290297 URL: http://llvm.org/viewvc/llvm-project?rev=290297&view=rev Log: Add the alloc_size attribute to clang, attempt 2. This is a recommit of r290149, which was reverted in r290169 due to msan failures. msan was failing because we were calling `isMostDerivedAnUnsizedArray` on an invalid designator, which caused us to read uninitialized memory. To fix this, the logic of the caller of said function was simplified, and we now have a `!Invalid` assert in `isMostDerivedAnUnsizedArray`, so we can catch this particular bug more easily in the future. Fingers crossed that this patch sticks this time. :) Original commit message: This patch does three things: - Gives us the alloc_size attribute in clang, which lets us infer the number of bytes handed back to us by malloc/realloc/calloc/any user functions that act in a similar manner. - Teaches our constexpr evaluator that evaluating some `const` variables is OK sometimes. This is why we have a change in test/SemaCXX/constant-expression-cxx11.cpp and other seemingly unrelated tests. Richard Smith okay'ed this idea some time ago in person. - Uniques some Blocks in CodeGen, which was reviewed separately at D26410. Lack of uniquing only really shows up as a problem when combined with our new eagerness in the face of const. Added: cfe/trunk/test/CodeGen/alloc-size.c cfe/trunk/test/CodeGenCXX/alloc-size.cpp cfe/trunk/test/Sema/alloc-size.c Modified: cfe/trunk/include/clang/Basic/Attr.td cfe/trunk/include/clang/Basic/AttrDocs.td cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/lib/AST/ExprConstant.cpp cfe/trunk/lib/CodeGen/CGBlocks.cpp cfe/trunk/lib/CodeGen/CGCall.cpp cfe/trunk/lib/CodeGen/CodeGenFunction.h cfe/trunk/lib/CodeGen/CodeGenModule.h cfe/trunk/lib/Sema/SemaDeclAttr.cpp cfe/trunk/test/CodeGenCXX/block-in-ctor-dtor.cpp cfe/trunk/test/CodeGenCXX/global-init.cpp cfe/trunk/test/CodeGenOpenCL/cl20-device-side-enqueue.cl cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp Modified: cfe/trunk/include/clang/Basic/Attr.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=290297&r1=290296&r2=290297&view=diff == --- cfe/trunk/include/clang/Basic/Attr.td (original) +++ cfe/trunk/include/clang/Basic/Attr.td Wed Dec 21 20:50:20 2016 @@ -780,6 +780,15 @@ def EmptyBases : InheritableAttr, Target let Documentation = [EmptyBasesDocs]; } +def AllocSize : InheritableAttr { + let Spellings = [GCC<"alloc_size">]; + let Subjects = SubjectList<[HasFunctionProto], WarnDiag, + "ExpectedFunctionWithProtoType">; + let Args = [IntArgument<"ElemSizeParam">, IntArgument<"NumElemsParam", 1>]; + let TemplateDependent = 1; + let Documentation = [AllocSizeDocs]; +} + def EnableIf : InheritableAttr { let Spellings = [GNU<"enable_if">]; let Subjects = SubjectList<[Function]>; Modified: cfe/trunk/include/clang/Basic/AttrDocs.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=290297&r1=290296&r2=290297&view=diff == --- cfe/trunk/include/clang/Basic/AttrDocs.td (original) +++ cfe/trunk/include/clang/Basic/AttrDocs.td Wed Dec 21 20:50:20 2016 @@ -206,6 +206,44 @@ to enforce the provided alignment assump }]; } +def AllocSizeDocs : Documentation { + let Category = DocCatFunction; + let Content = [{ +The ``alloc_size`` attribute can be placed on functions that return pointers in +order to hint to the compiler how many bytes of memory will be available at the +returned poiner. ``alloc_size`` takes one or two arguments. + +- ``alloc_size(N)`` implies that argument number N equals the number of + available bytes at the returned pointer. +- ``alloc_size(N, M)`` implies that the product of argument number N and + argument number M equals the number of available bytes at the returned + pointer. + +Argument numbers are 1-based. + +An example of how to use ``alloc_size`` + +.. code-block:: c + + void *my_malloc(int a) __attribute__((alloc_size(1))); + void *my_calloc(int a, int b) __attribute__((alloc_size(1, 2))); + + int main() { +void *const p = my_malloc(100); +assert(__builtin_object_size(p, 0) == 100); +void *const a = my_calloc(20, 5); +assert(__builtin_object_size(a, 0) == 100); + } + +.. Note:: This attribute works differently in clang than it does in GCC. + Specifically, clang will only trace ``const`` pointers (as above); we give up + on pointers that are not marked as ``const``. In the vast majority of cases, + this is unimportant, because LLVM has support for
[PATCH] D27424: Add the diagnose_if attribute to clang.
aaron.ballman added a comment. I really like this attribute, thank you for working on it! Comment at: include/clang/Basic/Attr.td:1584 +def DiagnoseIf : InheritableAttr { + let Spellings = [GNU<"diagnose_if">]; + let Subjects = SubjectList<[Function]>; I think this should also have a C++ spelling under the clang namespace. Comment at: include/clang/Basic/AttrDocs.td:359 + int val3 = abs(val); + int val4 = must_abs(val); // because run-time checks are not emitted for +// diagnose_if attributes, this executes without Capitalize the B in because. Comment at: include/clang/Basic/DiagnosticCommonKinds.td:164 InGroup; +def ext_clang_diagnose_if : Extension<"'diagnose_if' is a clang extension">, +InGroup; If we do add the clang namespaced spelling, we should have a test to ensure that this diagnostic is not triggered when the attribute is spelled `[[clang::diagnose_if(...)]]`. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2145 +def err_diagnose_if_invalid_diagnostic_type : Error< + "invalid diagnostic type for diagnose_if. Try 'error' or 'warning'.">; def err_constexpr_body_no_return : Error< Diagnostics don't have full stops, so I think a better way to write this is: `invalid diagnostic type for 'diagnose_if'; use \"error\" or \"warning\" instead`. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:3361 "pass_object_size attribute">; -def note_ovl_candidate_disabled_by_enable_if_attr : Note< +def err_diagnose_if_succeeded: Error<"%0">; +def warn_diagnose_if_succeeded : Warning<"%0">, InGroup; Space before the colon. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:4380 InGroup; +def note_from_diagnose_if : Note<"from diagnose_if attribute on %0:">; def warn_property_method_deprecated : Please single quote the use of diagnose_if. Comment at: include/clang/Sema/Overload.h:664 + +/// Basically an TinyPtrVector that doesn't own the +/// vector: If NumTriggeredDiagnoseIfs is 0 or 1, this is a DiagnoseIfAttr an -> a Comment at: include/clang/Sema/Sema.h:2616 + DiagnoseIfAttr * + checkArgDependentDiagnoseIf(FunctionDecl *Function, ArrayRef Args, + SmallVectorImpl &Nonfatal, Can this (and the other new functions) accept the `FunctionDecl` as a `const FunctionDecl *` instead? Comment at: lib/Sema/SemaDeclAttr.cpp:857 +public: + ParmVarDeclChecker(FunctionDecl *FD) { +Parms.insert(FD->param_begin(), FD->param_end()); This should accept `FD` as a const pointer. Comment at: lib/Sema/SemaExpr.cpp:368 + +if (DiagnoseIfAttr *A = +checkArgIndependentDiagnoseIf(FD, DiagnoseIfWarnings)) { `const DiagnoseIfAttr *`? Comment at: lib/Sema/SemaExpr.cpp:388 + + for (DiagnoseIfAttr *W : DiagnoseIfWarnings) +emitDiagnoseIfDiagnostic(Loc, W); `const auto *`? Comment at: lib/Sema/SemaExpr.cpp:5186 + + for (DiagnoseIfAttr *Attr : Nonfatal) +S.emitDiagnoseIfDiagnostic(Fn->getLocStart(), Attr); `const auto *`? Comment at: lib/Sema/SemaOverload.cpp:827 destroyCandidates(); - ConversionSequenceAllocator.Reset(); - NumInlineSequences = 0; + // DiagnoseIfAttrs are just pointers, so we don't need to destroy them. + SlabAllocator.Reset(); DiagnoseIfAttrs aren't the only thing allocated with the slab allocator though, right? Since this is being generalized, I wonder if asserting somewhere that the slab allocator only deals with pointers would make sense? Comment at: lib/Sema/SemaOverload.cpp:6151 + bool MissingImplicitThis) { + auto EnableIfAttrs = getOrderedEnableIfAttrs(Function); + if (EnableIfAttrs.empty()) Please spell out the type instead of using `auto`, since the type isn't spelled in the initialization. Comment at: lib/Sema/SemaOverload.cpp:6178-6179 + SmallVectorImpl &Nonfatal) { + for (Attr *A : Function->attrs()) +if (auto *DIA = dyn_cast(A)) + if (ArgDependent == DIA->getArgDependent()) { Instead of doing this, you can use `specific_attrs()` Comment at: lib/Sema/SemaOverload.cpp:9009 +static bool isCandidateUnavailableDueToDiagnoseIf(const OverloadCandidate &OC) { + return OC.NumTriggeredDiagnoseIfs == 1 && + OC.DiagnoseIfInfo.get()->isError(); Can you run into a situation with stacked diagnose_if errors, so that there's > 1? Comment at: lib/Sema/SemaOverload.cp
[PATCH] D28058: [OpenCL] Correct ndrange_t implementation
dmitry created this revision. dmitry added a reviewer: yaxunl. dmitry added subscribers: Anastasia, cfe-commits. Since we don't have an ideal option for ndrange_t implementation (which was discussed there: https://reviews.llvm.org/D23086), I propose to stick with identification by name approach suggested by @Anastasia (solution 2 from the last comment of the discussion - http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20160815/168540.html). We take implementation of ndrange_t from lib/Headers/opencl-c.h and we perform identification of the type by its name. https://reviews.llvm.org/D28058 Files: include/clang/AST/ASTContext.h include/clang/AST/BuiltinTypes.def include/clang/AST/Type.h include/clang/Serialization/ASTBitCodes.h lib/AST/ASTContext.cpp lib/AST/ExprConstant.cpp lib/AST/ItaniumMangle.cpp lib/AST/MicrosoftMangle.cpp lib/AST/NSAPI.cpp lib/AST/Type.cpp lib/AST/TypeLoc.cpp lib/CodeGen/CGBuiltin.cpp lib/CodeGen/CGDebugInfo.cpp lib/CodeGen/CGOpenCLRuntime.cpp lib/CodeGen/CodeGenTypes.cpp lib/CodeGen/ItaniumCXXABI.cpp lib/Headers/opencl-c.h lib/Index/USRGeneration.cpp lib/Sema/Sema.cpp lib/Sema/SemaChecking.cpp lib/Serialization/ASTCommon.cpp lib/Serialization/ASTReader.cpp test/CodeGenOpenCL/cl20-device-side-enqueue.cl test/Headers/opencl-c-header.cl test/PCH/ocl_types.h test/SemaOpenCL/cl20-device-side-enqueue.cl tools/libclang/CIndex.cpp Index: tools/libclang/CIndex.cpp === --- tools/libclang/CIndex.cpp +++ tools/libclang/CIndex.cpp @@ -1491,7 +1491,6 @@ case BuiltinType::OCLEvent: case BuiltinType::OCLClkEvent: case BuiltinType::OCLQueue: - case BuiltinType::OCLNDRange: case BuiltinType::OCLReserveID: #define BUILTIN_TYPE(Id, SingletonId) #define SIGNED_TYPE(Id, SingletonId) case BuiltinType::Id: Index: test/SemaOpenCL/cl20-device-side-enqueue.cl === --- test/SemaOpenCL/cl20-device-side-enqueue.cl +++ test/SemaOpenCL/cl20-device-side-enqueue.cl @@ -1,6 +1,7 @@ // RUN: %clang_cc1 %s -cl-std=CL2.0 -triple "spir-unknown-unknown" -verify -pedantic -fsyntax-only -DB32 // RUN: %clang_cc1 %s -cl-std=CL2.0 -triple "spir64-unknown-unknown" -verify -pedantic -fsyntax-only -Wconversion -DWCONV +typedef struct {int a;} ndrange_t; // Diagnostic tests for different overloads of enqueue_kernel from Table 6.13.17.1 of OpenCL 2.0 Spec. kernel void enqueue_kernel_tests() { queue_t default_queue; Index: test/PCH/ocl_types.h === --- test/PCH/ocl_types.h +++ test/PCH/ocl_types.h @@ -32,9 +32,6 @@ // queue_t typedef queue_t q_t; -// ndrange_t -typedef ndrange_t range_t; - // reserve_id_t typedef reserve_id_t reserveid_t; Index: test/Headers/opencl-c-header.cl === --- test/Headers/opencl-c-header.cl +++ test/Headers/opencl-c-header.cl @@ -1,6 +1,8 @@ -// RUN: %clang_cc1 -triple spir-unknown-unknown -internal-isystem ../../lib/Headers -include opencl-c.h -emit-llvm -o - %s | FileCheck %s -// RUN: %clang_cc1 -triple spir-unknown-unknown -internal-isystem ../../lib/Headers -include opencl-c.h -emit-llvm -o - %s -cl-std=CL1.1| FileCheck %s +// RUN: %clang_cc1 -O0 -triple spir-unknown-unknown -internal-isystem ../../lib/Headers -include opencl-c.h -emit-llvm -o - %s| FileCheck %s +// RUN: %clang_cc1 -O0 -triple spir-unknown-unknown -internal-isystem ../../lib/Headers -include opencl-c.h -emit-llvm -o - %s -cl-std=CL1.1| FileCheck %s +// CHECK-NOT: ndrange_t +// CHECK20: ndrange_t // CHECK: _Z16convert_char_rtec // CHECK-NOT: _Z3ctzc // CHECK20: _Z3ctzc @@ -30,32 +32,30 @@ // === // Compile for OpenCL 2.0 for the first time. The module should change. -// RUN: %clang_cc1 -triple spir-unknown-unknown -emit-llvm -o - -cl-std=CL2.0 -finclude-default-header -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -fdisable-module-hash -ftime-report %s 2>&1 | FileCheck --check-prefix=CHECK20 --check-prefix=CHECK-MOD %s +// RUN: %clang_cc1 -triple spir-unknown-unknown -O0 -emit-llvm -o - -cl-std=CL2.0 -finclude-default-header -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -fdisable-module-hash -ftime-report %s 2>&1 | FileCheck --check-prefix=CHECK20 --check-prefix=CHECK-MOD %s // RUN: not diff %t/1_0.pcm %t/opencl_c.pcm // RUN: chmod u-w %t/opencl_c.pcm // === // Compile for OpenCL 2.0 for the second time. The module should not change. -// RUN: %clang_cc1 -triple spir-unknown-unknown -emit-llvm -o - -cl-std=CL2.0 -finclude-default-header -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -fdisable-module-hash -ftime-report %s 2>&1 | FileCheck --check-prefix=CHECK20 --check-prefix=CHECK-MOD %s +// RUN: %clang_cc1 -triple spir-unknown-unknown -O0 -emit-llvm -o - -cl-std=CL2.0 -finclude-default-header -fmodules -fimplicit-module-maps -fmodules-cache-pa
Re: [clang-tools-extra] r290202 - [clang-tidy] Add modernize-use-default-member-init check
Does the test invoke cl.exe? I supposed it were clang. On Thu, Dec 22, 2016 at 11:24 PM Aaron Ballman wrote: > On Wed, Dec 21, 2016 at 6:10 PM, NAKAMURA Takumi > wrote: > > It'd be the issue if the test depended on installed headers. The builder > > doesn't have MS headers installed. > > Those type names do not require any headers to be installed. They're > keywords in C++11 and later, but MSVC did not support them until MSVC > 2015. > > ~Aaron > > > > > On Thu, Dec 22, 2016 at 1:27 AM Aaron Ballman > > wrote: > >> > >> On Tue, Dec 20, 2016 at 5:58 PM, Malcolm Parsons > >> wrote: > >> > On 20 December 2016 at 22:32, Aaron Ballman > >> > wrote: > >> >> On Tue, Dec 20, 2016 at 4:26 PM, Malcolm Parsons via cfe-commits > >> >> wrote: > >> >>> Author: malcolm.parsons > >> >>> Date: Tue Dec 20 15:26:07 2016 > >> >>> New Revision: 290202 > >> >>> > >> >>> URL: http://llvm.org/viewvc/llvm-project?rev=290202&view=rev > >> >>> Log: > >> >>> [clang-tidy] Add modernize-use-default-member-init check > >> >>> > >> >>> Summary: Fixes PR18858 > >> >> > >> >> This appears to have broken one of the bots: > >> >> > >> >> http://bb.pgr.jp/builders/ninja-x64-msvc-RA-centos6/builds/33046 > >> > > >> > error: unknown type name 'char16_t' > >> > error: unknown type name 'char32_t' > >> > > >> > I see commented tests in other clang-tidy tests: > >> > > >> > // TODO: enable these tests once all supported compilers > >> > // support char16_t and char32_t (VS2013 does not) > >> > > >> > // disabled for now until it is clear > >> > // how to enable them in the test > >> > //} catch (const char16_t*) { > >> > >> We no longer support MSVC 2013, so that should not be an issue. > >> Takumi, I'm not certain what to make of this builder (MSVC centos?). > >> What version of Visual Studio is it compiling with? Or is this an > >> instance where we need to set the -fms-compatibility-version=19 when > >> running the test with clang-cl? > >> > >> ~Aaron > >> > >> > > >> > -- > >> > Malcolm Parsons > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [clang-tools-extra] r290202 - [clang-tidy] Add modernize-use-default-member-init check
On Thu, Dec 22, 2016 at 11:43 AM, NAKAMURA Takumi wrote: > Does the test invoke cl.exe? I supposed it were clang. It shouldn't, but the test also does not specify a target triple, so the behavior depends on what version of MSVC you built clang (and clang-tidy) with. What version of MSVC is used to build that configuration? ~Aaron > > On Thu, Dec 22, 2016 at 11:24 PM Aaron Ballman > wrote: >> >> On Wed, Dec 21, 2016 at 6:10 PM, NAKAMURA Takumi >> wrote: >> > It'd be the issue if the test depended on installed headers. The builder >> > doesn't have MS headers installed. >> >> Those type names do not require any headers to be installed. They're >> keywords in C++11 and later, but MSVC did not support them until MSVC >> 2015. >> >> ~Aaron >> >> > >> > On Thu, Dec 22, 2016 at 1:27 AM Aaron Ballman >> > wrote: >> >> >> >> On Tue, Dec 20, 2016 at 5:58 PM, Malcolm Parsons >> >> wrote: >> >> > On 20 December 2016 at 22:32, Aaron Ballman >> >> > wrote: >> >> >> On Tue, Dec 20, 2016 at 4:26 PM, Malcolm Parsons via cfe-commits >> >> >> wrote: >> >> >>> Author: malcolm.parsons >> >> >>> Date: Tue Dec 20 15:26:07 2016 >> >> >>> New Revision: 290202 >> >> >>> >> >> >>> URL: http://llvm.org/viewvc/llvm-project?rev=290202&view=rev >> >> >>> Log: >> >> >>> [clang-tidy] Add modernize-use-default-member-init check >> >> >>> >> >> >>> Summary: Fixes PR18858 >> >> >> >> >> >> This appears to have broken one of the bots: >> >> >> >> >> >> http://bb.pgr.jp/builders/ninja-x64-msvc-RA-centos6/builds/33046 >> >> > >> >> > error: unknown type name 'char16_t' >> >> > error: unknown type name 'char32_t' >> >> > >> >> > I see commented tests in other clang-tidy tests: >> >> > >> >> > // TODO: enable these tests once all supported compilers >> >> > // support char16_t and char32_t (VS2013 does not) >> >> > >> >> > // disabled for now until it is clear >> >> > // how to enable them in the test >> >> > //} catch (const char16_t*) { >> >> >> >> We no longer support MSVC 2013, so that should not be an issue. >> >> Takumi, I'm not certain what to make of this builder (MSVC centos?). >> >> What version of Visual Studio is it compiling with? Or is this an >> >> instance where we need to set the -fms-compatibility-version=19 when >> >> running the test with clang-cl? >> >> >> >> ~Aaron >> >> >> >> > >> >> > -- >> >> > Malcolm Parsons ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28047: Remove the '-disable-llvm-passes' flag (which I didn't even know existed, and I suspect many others aren't aware of either) and strength '-disable-llvm-optzns' to do the same thing.
rnk added a comment. How about standardizing on -disable-llvm-passes instead of -disable-llvm-optzns? I never liked "optzns" and can't remember how to spell it. Also, this flag disables LLVM instrumentation passes (ASan) as well as optimization passes, so I think -disable-llvm-passes is the better name. https://reviews.llvm.org/D28047 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28052: [change-namespace] consider namespace aliases to shorten qualified names.
ioeric updated this revision to Diff 82345. ioeric marked 3 inline comments as done. ioeric added a comment. - addressed review comments. https://reviews.llvm.org/D28052 Files: change-namespace/ChangeNamespace.cpp change-namespace/ChangeNamespace.h unittests/change-namespace/ChangeNamespaceTests.cpp Index: unittests/change-namespace/ChangeNamespaceTests.cpp === --- unittests/change-namespace/ChangeNamespaceTests.cpp +++ unittests/change-namespace/ChangeNamespaceTests.cpp @@ -825,6 +825,114 @@ EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code)); } +TEST_F(ChangeNamespaceTest, NamespaceAliasInGlobal) { + std::string Code = "namespace glob {\n" + "class Glob {};\n" + "}\n" + "namespace glob2 { class Glob2 {}; }\n" + "namespace gl = glob;\n" + "namespace gl2 = ::glob2;\n" + "namespace na {\n" + "namespace nb {\n" + "void f() { gl::Glob g; gl2::Glob2 g2; }\n" + "} // namespace nb\n" + "} // namespace na\n"; + + std::string Expected = + "namespace glob {\n" + "class Glob {};\n" + "}\n" + "namespace glob2 { class Glob2 {}; }\n" + "namespace gl = glob;\n" + "namespace gl2 = ::glob2;\n" + "\n" + "namespace x {\n" + "namespace y {\n" + "void f() { gl::Glob g; gl2::Glob2 g2; }\n" + "} // namespace y\n" + "} // namespace x\n"; + EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code)); +} + +TEST_F(ChangeNamespaceTest, NamespaceAliasInNamespace) { + std::string Code = "namespace glob {\n" + "class Glob {};\n" + "}\n" + "namespace na {\n" + "namespace nb {\n" + "namespace gl = glob;\n" + "void f() { gl::Glob g; }\n" + "} // namespace nb\n" + "} // namespace na\n"; + + std::string Expected = "namespace glob {\n" + "class Glob {};\n" + "}\n" + "\n" + "namespace x {\n" + "namespace y {\n" + "namespace gl = glob;\n" + "void f() { gl::Glob g; }\n" + "} // namespace y\n" + "} // namespace x\n"; + EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code)); +} + +TEST_F(ChangeNamespaceTest, NamespaceAliasInAncestorNamespace) { + NewNamespace = "na::nx"; + std::string Code = "namespace glob {\n" + "class Glob {};\n" + "}\n" + "namespace other { namespace gl = glob; }\n" + "namespace na {\n" + "namespace ga = glob;\n" + "namespace nb {\n" + "void f() { ga::Glob g; }\n" + "} // namespace nb\n" + "} // namespace na\n"; + + std::string Expected = "namespace glob {\n" + "class Glob {};\n" + "}\n" + "namespace other { namespace gl = glob; }\n" + "namespace na {\n" + "namespace ga = glob;\n" + "\n" + "namespace nx {\n" + "void f() { ga::Glob g; }\n" + "} // namespace nx\n" + "} // namespace na\n"; + EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code)); +} + +TEST_F(ChangeNamespaceTest, NamespaceAliasInOtherNamespace) { + std::string Code = "namespace glob {\n" + "class Glob {};\n" + "}\n" + "namespace other { namespace gl = glob; }\n" + "namespace na {\n" + "namespace ga = glob;\n" + "namespace nb {\n" + "void f() { glob::Glob g; }\n" + "} // namespace nb\n" + "} // namespace na\n"; + + std::string Expected = "namespace glob {\n" + "class Glob {};\n" + "}\n" + "namespace other { namespace gl = glob; }\n" + "namespace na {\n" + "namespace ga = glob;\n" + "\n" + "} // namespace na\n" + "namespace x {\n" + "namespace y {\n" + "void f() { glob::Glob g; }\n" + "} // namespace y\n" + "} // namespace x\n"; + EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code)); +} + TEST_F(ChangeNamespaceTest, UsingDeclAfterReference) { std::
[PATCH] D28053: Cleanup the handling of noinline function attributes, -fno-inline, -fno-inline-functions, -O0, and optnone.
rnk added a comment. The big change here is that `clang -O0` now applies the noinline attribute everywhere. I can see why someone might expect things to work that way, but it seems surprising to me at first glance. Before this change I can imagine someone distributing a static archive of bitcode which could be used to produce debug binaries and release binaries. There are some issues with that (lack of lifetime markers), but it should mostly work. Before this change, this command line would do inlining: `clang -S -emit-llvm foo.c -o - | opt -O2 -S` Now it won't, even though I didn't explicitly pass -O0. Are we happy with that? https://reviews.llvm.org/D28053 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27898: [compiler-rt] [builtins] Implement __floattitf() & __floatuntitf()
rengolin accepted this revision. rengolin added a comment. This revision is now accepted and ready to land. My floating-point-fu is lacking, so I'll trust you guys that this makes sense. :) I didn't see anything egregious, so LGTM with the nit inline. cheers, --renato Comment at: lib/builtins/floatuntitf.c:73 +long_double_bits fb; +fb.u.high.all = (du_int)(e + 16383) << 48/* exponent */ + | ((a >> 64) & 0xLL); /* mantissa */ nit: an hexadecimal pattern here would be clearer. same above. https://reviews.llvm.org/D27898 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28047: Remove the '-disable-llvm-passes' flag (which I didn't even know existed, and I suspect many others aren't aware of either) and strength '-disable-llvm-optzns' to do the same thing.
mehdi_amini added a comment. In https://reviews.llvm.org/D28047#629746, @rnk wrote: > How about standardizing on -disable-llvm-passes instead of > -disable-llvm-optzns? I never liked "optzns" and can't remember how to spell > it. Also, this flag disables LLVM instrumentation passes (ASan) as well as > optimization passes, so I think -disable-llvm-passes is the better name. I agree with that. To be friendly with developers/users we could print an error mentioning to use `-disable-llvm-passes` when they try to use `-disable-llvm-optzns` (which we accept as a `-mllvm -disable-llvm-optzns` hardcoded in the driver by the way). https://reviews.llvm.org/D28047 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28053: Cleanup the handling of noinline function attributes, -fno-inline, -fno-inline-functions, -O0, and optnone.
mehdi_amini added a comment. > It would be awesome if attribute sets were a bit more FileCheck friendly, but > oh well. I've been wondering about that, what's the point of attribute sets in the textual IR? I understand the idea for saving space in the Bitcode, but the IR does not have to use the same representation. https://reviews.llvm.org/D28053 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r290352 - [analyzer] Update GTestChecker to tighten API detection
Author: dcoughlin Date: Thu Dec 22 11:52:57 2016 New Revision: 290352 URL: http://llvm.org/viewvc/llvm-project?rev=290352&view=rev Log: [analyzer] Update GTestChecker to tighten API detection Update the GTestChecker to tighten up the API detection and make it cleaner in response to post-commit feedback. Also add tests for when temporary destructors are enabled to make sure we get the expected behavior when inlining constructors for temporaries. Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/GTestChecker.cpp cfe/trunk/test/Analysis/gtest.cpp Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/GTestChecker.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/GTestChecker.cpp?rev=290352&r1=290351&r2=290352&view=diff == --- cfe/trunk/lib/StaticAnalyzer/Checkers/GTestChecker.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Checkers/GTestChecker.cpp Thu Dec 22 11:52:57 2016 @@ -7,7 +7,7 @@ // //===--===// // -// This checker models the behavior of un-inlined APIs from the the gtest +// This checker models the behavior of un-inlined APIs from the gtest // unit-testing library to avoid false positives when using assertions from // that library. // @@ -85,7 +85,7 @@ using namespace ento; // does not inline these since it does not yet reliably call temporary // destructors). // -// This checker compensates for the missing inlining by propgagating the +// This checker compensates for the missing inlining by propagating the // _success value across the bool and copy constructors so the assertion behaves // as expected. @@ -102,7 +102,7 @@ public: private: void modelAssertionResultBoolConstructor(const CXXConstructorCall *Call, - CheckerContext &C) const; + bool IsRef, CheckerContext &C) const; void modelAssertionResultCopyConstructor(const CXXConstructorCall *Call, CheckerContext &C) const; @@ -122,35 +122,25 @@ private: GTestChecker::GTestChecker() : AssertionResultII(nullptr), SuccessII(nullptr) {} -/// Model a call to an un-inlined AssertionResult(boolean expression). +/// Model a call to an un-inlined AssertionResult(bool) or +/// AssertionResult(bool &, ...). /// To do so, constrain the value of the newly-constructed instance's 'success_' /// field to be equal to the passed-in boolean value. +/// +/// \param IsRef Whether the boolean parameter is a reference or not. void GTestChecker::modelAssertionResultBoolConstructor( -const CXXConstructorCall *Call, CheckerContext &C) const { - assert(Call->getNumArgs() > 0); - - // Depending on the version of gtest the constructor can be either of: - // - // v1.7 and earlier: - // AssertionResult(bool success) - // - // v1.8 and greater: - // template - // AssertionResult(const T& success, - // typename internal::EnableIf< - // !internal::ImplicitlyConvertible::value>::type*) +const CXXConstructorCall *Call, bool IsRef, CheckerContext &C) const { + assert(Call->getNumArgs() >= 1 && Call->getNumArgs() <= 2); + ProgramStateRef State = C.getState(); SVal BooleanArgVal = Call->getArgSVal(0); - if (Call->getDecl()->getParamDecl(0)->getType()->getAs()) { -// We have v1.8+, so load the value from the reference. + if (IsRef) { +// The argument is a reference, so load from it to get the boolean value. if (!BooleanArgVal.getAs()) return; BooleanArgVal = C.getState()->getSVal(BooleanArgVal.castAs()); } - ProgramStateRef State = C.getState(); - SVal ThisVal = Call->getCXXThisVal(); SVal ThisSuccess = getAssertionResultSuccessFieldValue( @@ -169,7 +159,7 @@ void GTestChecker::modelAssertionResultB /// 'success_' field. void GTestChecker::modelAssertionResultCopyConstructor( const CXXConstructorCall *Call, CheckerContext &C) const { - assert(Call->getNumArgs() > 0); + assert(Call->getNumArgs() == 1); // The first parameter of the the copy constructor must be the other // instance to initialize this instances fields from. @@ -206,22 +196,44 @@ void GTestChecker::checkPostCall(const C if (CtorParent->getIdentifier() != AssertionResultII) return; - if (CtorDecl->getNumParams() == 0) -return; - + unsigned ParamCount = CtorDecl->getNumParams(); - // Call the appropriate modeling method based on the type of the first - // constructor parameter. - const ParmVarDecl *ParamDecl = CtorDecl->getParamDecl(0); - QualType ParamType = ParamDecl->getType(); - if (CtorDecl->getNumParams() <= 2 && - ParamType.getNonReferenceType()->getCanonicalTypeUnqualified() == - C.getASTContext().BoolTy) { -// The first parameter is either a boolean or reference to a boolean -modelAssertionRes
[PATCH] D27773: [analyzer] Add checker modeling gtest APIs.
dcoughlin added a comment. I cleaned up the parameter detection and added more tests in r290352. https://reviews.llvm.org/D27773 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28053: Cleanup the handling of noinline function attributes, -fno-inline, -fno-inline-functions, -O0, and optnone.
mehdi_amini added a comment. In https://reviews.llvm.org/D28053#629768, @rnk wrote: > The big change here is that `clang -O0` now applies the noinline attribute > everywhere. I can see why someone might expect things to work that way, but > it seems surprising to me at first glance. > > Before this change I can imagine someone distributing a static archive of > bitcode which could be used to produce debug binaries and release binaries. > There are some issues with that (lack of lifetime markers), but it should > mostly work. Note that LTO runs a bunch of optimizations during the compile phase that you wouldn't get with that. Steven had issues in the past when mixing O0 and https://reviews.llvm.org/owners/package/2/ in LTO, I don't remember the details unfortunately (and Steven is OOO). Right now we don't add the `optnone` attribute when building with O0, maybe we should as well if we go this route. https://reviews.llvm.org/D28053 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27898: [compiler-rt] [builtins] Implement __floattitf() & __floatuntitf()
mgorny added inline comments. Comment at: lib/builtins/floatuntitf.c:73 +long_double_bits fb; +fb.u.high.all = (du_int)(e + 16383) << 48/* exponent */ + | ((a >> 64) & 0xLL); /* mantissa */ rengolin wrote: > nit: an hexadecimal pattern here would be clearer. same above. Do you mean something like: `(foo << 48) & 0x`? https://reviews.llvm.org/D27898 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28053: Cleanup the handling of noinline function attributes, -fno-inline, -fno-inline-functions, -O0, and optnone.
mehdi_amini added inline comments. Comment at: lib/Frontend/CompilerInvocation.cpp:453 +// There is no effect at O0 when we won't inline anyways. +if (Opts.OptimizationLevel > 1) { + const Option &InlineOpt = InlineArg->getOption(); I'd switch the two if https://reviews.llvm.org/D28053 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27898: [compiler-rt] [builtins] Implement __floattitf() & __floatuntitf()
rengolin added inline comments. Comment at: lib/builtins/floatuntitf.c:73 +long_double_bits fb; +fb.u.high.all = (du_int)(e + 16383) << 48/* exponent */ + | ((a >> 64) & 0xLL); /* mantissa */ mgorny wrote: > rengolin wrote: > > nit: an hexadecimal pattern here would be clearer. same above. > Do you mean something like: `(foo << 48) & 0x`? No, just the `16383` to `0x3FFF` https://reviews.llvm.org/D27898 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27898: [compiler-rt] [builtins] Implement __floattitf() & __floatuntitf()
rengolin added inline comments. Comment at: lib/builtins/floatuntitf.c:73 +long_double_bits fb; +fb.u.high.all = (du_int)(e + 16383) << 48/* exponent */ + | ((a >> 64) & 0xLL); /* mantissa */ rengolin wrote: > mgorny wrote: > > rengolin wrote: > > > nit: an hexadecimal pattern here would be clearer. same above. > > Do you mean something like: `(foo << 48) & 0x`? > No, just the `16383` to `0x3FFF` Though, now I think your proposal may be better. :) Whatever works, I'm not too concerned about that. https://reviews.llvm.org/D27898 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27898: [compiler-rt] [builtins] Implement __floattitf() & __floatuntitf()
scanon added a comment. Please s/mantissa/significand/. I know that "mantissa" is used in a number of places in llvm sources, but it's incorrect terminology. Significand is the IEEE-754 nomenclature, which any new work should follow. https://reviews.llvm.org/D27898 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28053: Cleanup the handling of noinline function attributes, -fno-inline, -fno-inline-functions, -O0, and optnone.
mehdi_amini added inline comments. Comment at: lib/Frontend/CompilerInvocation.cpp:453 +// There is no effect at O0 when we won't inline anyways. +if (Opts.OptimizationLevel > 1) { + const Option &InlineOpt = InlineArg->getOption(); mehdi_amini wrote: > I'd switch the two if The test `> 1` and the comment about `-O0` are confusing to me as well. What about the following (IIUC): ``` if (Opts.OptimizationLevel <= 1) { // "always-inline" required for correctness at O0 and O1. Opts.setInlining(CodeGenOptions::OnlyAlwaysInlining); else if (Opts.OptimizationLevel > 1) { // Normal inlining at O2 and above Opts.setInlining((CodeGenOptions::NormalInlining); // -fno-inline-functions overrides if (Arg *InlineArg = Args.getLastArg( options::OPT_finline_functions, options::OPT_finline_hint_functions, options::OPT_fno_inline_functions, options::OPT_fno_inline)) { if (InlineOpt.matches(options::OPT_finline_functions)) Opts.setInlining(CodeGenOptions::NormalInlining); else if (InlineOpt.matches(options::OPT_finline_hint_functions)) Opts.setInlining(CodeGenOptions::OnlyHintInlining); else Opts.setInlining(CodeGenOptions::OnlyAlwaysInlining); } } ``` https://reviews.llvm.org/D28053 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27898: [compiler-rt] [builtins] Implement __floattitf() & __floatuntitf()
scanon added inline comments. Comment at: lib/builtins/floattitf.c:65 +if (a & ((tu_int)1 << LDBL_MANT_DIG)) { +a >>= 1; +++e; Strictly speaking there's no need to adjust `a` here. If we rounded up into a new binade, then `a` is necessarily `0b1000...0`, and the leading 1 bit will get killed by the mask when we assemble `fb.u.high.all` regardless of its position. Same comment applies to floatuntitf. https://reviews.llvm.org/D27898 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27898: [compiler-rt] [builtins] Implement __floattitf() & __floatuntitf()
mgorny added inline comments. Comment at: lib/builtins/floatuntitf.c:73 +long_double_bits fb; +fb.u.high.all = (du_int)(e + 16383) << 48/* exponent */ + | ((a >> 64) & 0xLL); /* mantissa */ rengolin wrote: > rengolin wrote: > > mgorny wrote: > > > rengolin wrote: > > > > nit: an hexadecimal pattern here would be clearer. same above. > > > Do you mean something like: `(foo << 48) & 0x`? > > No, just the `16383` to `0x3FFF` > Though, now I think your proposal may be better. :) > > Whatever works, I'm not too concerned about that. Well, I used the decimal form because that's how the IEEE 754 standard specifies it, so I guessed the correlation would be clearer that way. https://reviews.llvm.org/D27898 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27898: [compiler-rt] [builtins] Implement __floattitf() & __floatuntitf()
scanon added inline comments. Comment at: lib/builtins/floatuntitf.c:73 +long_double_bits fb; +fb.u.high.all = (du_int)(e + 16383) << 48/* exponent */ + | ((a >> 64) & 0xLL); /* mantissa */ mgorny wrote: > rengolin wrote: > > rengolin wrote: > > > mgorny wrote: > > > > rengolin wrote: > > > > > nit: an hexadecimal pattern here would be clearer. same above. > > > > Do you mean something like: `(foo << 48) & 0x`? > > > No, just the `16383` to `0x3FFF` > > Though, now I think your proposal may be better. :) > > > > Whatever works, I'm not too concerned about that. > Well, I used the decimal form because that's how the IEEE 754 standard > specifies it, so I guessed the correlation would be clearer that way. FWIW, I think both are pretty obvious, but if you want to be totally explicit: const int bias = 0x3fff; ... (e + bias) ... https://reviews.llvm.org/D27898 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r290353 - Make alloc_size only applicable to Functions.
Author: gbiv Date: Thu Dec 22 12:48:34 2016 New Revision: 290353 URL: http://llvm.org/viewvc/llvm-project?rev=290353&view=rev Log: Make alloc_size only applicable to Functions. I don't remember why I didn't make alloc_size only applicable to Functions a year ago, but I can't see any compelling reason not to do so now. Fixes PR31453. Modified: cfe/trunk/include/clang/Basic/Attr.td cfe/trunk/test/Sema/alloc-size.c Modified: cfe/trunk/include/clang/Basic/Attr.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=290353&r1=290352&r2=290353&view=diff == --- cfe/trunk/include/clang/Basic/Attr.td (original) +++ cfe/trunk/include/clang/Basic/Attr.td Thu Dec 22 12:48:34 2016 @@ -782,8 +782,7 @@ def EmptyBases : InheritableAttr, Target def AllocSize : InheritableAttr { let Spellings = [GCC<"alloc_size">]; - let Subjects = SubjectList<[HasFunctionProto], WarnDiag, - "ExpectedFunctionWithProtoType">; + let Subjects = SubjectList<[Function]>; let Args = [IntArgument<"ElemSizeParam">, IntArgument<"NumElemsParam", 1>]; let TemplateDependent = 1; let Documentation = [AllocSizeDocs]; Modified: cfe/trunk/test/Sema/alloc-size.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/alloc-size.c?rev=290353&r1=290352&r2=290353&view=diff == --- cfe/trunk/test/Sema/alloc-size.c (original) +++ cfe/trunk/test/Sema/alloc-size.c Thu Dec 22 12:48:34 2016 @@ -14,10 +14,12 @@ void *fail8(int a, int b) __attribute__( int fail9(int a) __attribute__((alloc_size(1))); //expected-warning{{'alloc_size' attribute only applies to return values that are pointers}} -int fail10 __attribute__((alloc_size(1))); //expected-warning{{'alloc_size' attribute only applies to non-K&R-style functions}} +int fail10 __attribute__((alloc_size(1))); //expected-warning{{'alloc_size' attribute only applies to functions}} void *fail11(void *a) __attribute__((alloc_size(1))); //expected-error{{'alloc_size' attribute argument may only refer to a function parameter of integer type}} void *fail12(int a) __attribute__((alloc_size("abc"))); //expected-error{{'alloc_size' attribute requires parameter 1 to be an integer constant}} void *fail12(int a) __attribute__((alloc_size(1, "abc"))); //expected-error{{'alloc_size' attribute requires parameter 2 to be an integer constant}} void *fail13(int a) __attribute__((alloc_size(1U<<31))); //expected-error{{integer constant expression evaluates to value 2147483648 that cannot be represented in a 32-bit signed integer type}} + +int (*PR31453)(int) __attribute__((alloc_size(1))); //expected-warning{{'alloc_size' attribute only applies to functions}} ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r290356 - Fix warning introduced by r290297.
Author: gbiv Date: Thu Dec 22 13:00:31 2016 New Revision: 290356 URL: http://llvm.org/viewvc/llvm-project?rev=290356&view=rev Log: Fix warning introduced by r290297. Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=290356&r1=290355&r2=290356&view=diff == --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original) +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Thu Dec 22 13:00:31 2016 @@ -256,7 +256,7 @@ static bool checkPositiveIntArgument(Sem if (!checkUInt32Argument(S, Attr, Expr, UVal, Idx)) return false; - if (UVal > std::numeric_limits::max()) { + if (UVal > (uint32_t)std::numeric_limits::max()) { llvm::APSInt I(32); // for toString I = UVal; S.Diag(Expr->getExprLoc(), diag::err_ice_too_large) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r290169 - Revert r290149: Add the alloc_size attribute to clang.
On 20 Dec 2016, at 09:28, Chandler Carruth via cfe-commits wrote: > Author: chandlerc > Date: Tue Dec 20 02:28:19 2016 > New Revision: 290169 > > URL: http://llvm.org/viewvc/llvm-project?rev=290169&view=rev > Log: > Revert r290149: Add the alloc_size attribute to clang. > > This commit fails MSan when running test/CodeGen/object-size.c in > a confusing way. After some discussion with George, it isn't really > clear what is going on here. We can make the MSan failure go away by > testing for the invalid bit, but *why* things are invalid isn't clear. > And yet, other code in the surrounding area is doing precisely this and > testing for invalid. > > George is going to take a closer look at this to better understand the > nature of the failure and recommit it, for now backing it out to clean > up MSan builds. Hmm, was this reapplied later on? I'm still getting the following AddressSanitizer failures on FreeBSD, and bisecting has pointed to r290149 as the cause: FAIL: AddressSanitizer-Unit :: Asan-i386-inline-Test/AddressSanitizer.ReallocFreedPointerTest (2124 of 30204) TEST 'AddressSanitizer-Unit :: Asan-i386-inline-Test/AddressSanitizer.ReallocFreedPointerTest' FAILED Note: Google Test filter = AddressSanitizer.ReallocFreedPointerTest [==] Running 1 test from 1 test case. [--] Global test environment set-up. [--] 1 test from AddressSanitizer [ RUN ] AddressSanitizer.ReallocFreedPointerTest /share/dim/src/llvm/trunk/projects/compiler-rt/lib/asan/tests/asan_test.cc:377: Failure Death test: ptr = realloc(ptr, 77) Result: failed to die. Error msg: [ DEATH ] [ FAILED ] AddressSanitizer.ReallocFreedPointerTest (30 ms) [--] 1 test from AddressSanitizer (30 ms total) [--] Global test environment tear-down [==] 1 test from 1 test case ran. (31 ms total) [ PASSED ] 0 tests. [ FAILED ] 1 test, listed below: [ FAILED ] AddressSanitizer.ReallocFreedPointerTest 1 FAILED TEST YOU HAVE 24 DISABLED TESTS Testing: 0 . FAIL: AddressSanitizer-Unit :: Asan-i386-with-calls-Test/AddressSanitizer.ReallocFreedPointerTest (2233 of 30204) TEST 'AddressSanitizer-Unit :: Asan-i386-with-calls-Test/AddressSanitizer.ReallocFreedPointerTest' FAILED Note: Google Test filter = AddressSanitizer.ReallocFreedPointerTest [==] Running 1 test from 1 test case. [--] Global test environment set-up. [--] 1 test from AddressSanitizer [ RUN ] AddressSanitizer.ReallocFreedPointerTest /share/dim/src/llvm/trunk/projects/compiler-rt/lib/asan/tests/asan_test.cc:377: Failure Death test: ptr = realloc(ptr, 77) Result: failed to die. Error msg: [ DEATH ] [ FAILED ] AddressSanitizer.ReallocFreedPointerTest (24 ms) [--] 1 test from AddressSanitizer (24 ms total) [--] Global test environment tear-down [==] 1 test from 1 test case ran. (25 ms total) [ PASSED ] 0 tests. [ FAILED ] 1 test, listed below: [ FAILED ] AddressSanitizer.ReallocFreedPointerTest 1 FAILED TEST YOU HAVE 24 DISABLED TESTS Testing: 0 . FAIL: AddressSanitizer-i386-freebsd :: TestCases/Posix/free_hook_realloc.cc (2399 of 30204) TEST 'AddressSanitizer-i386-freebsd :: TestCases/Posix/free_hook_realloc.cc' FAILED Script: -- /home/dim/obj/llvm-290338-trunk-freebsd12-i386-ninja-rel-1/./bin/clang --driver-mode=g++ -fsanitize=address -mno-omit-leaf-frame-pointer -fno-omit-frame-pointer -fno-optimize-sibling-calls -gline-tables-only -m32 -O2 /share/dim/src/llvm/trunk/projects/compiler-rt/test/asan/TestCases/Posix/free_hook_realloc.cc -o /home/dim/obj/llvm-290338-trunk-freebsd12-i386-ninja-rel-1/projects/compiler-rt/test/asan/I386FreeBSDConfig/TestCases/Posix/Output/free_hook_realloc.cc.tmp /home/dim/obj/llvm-290338-trunk-freebsd12-i386-ninja-rel-1/projects/compiler-rt/test/asan/I386FreeBSDConfig/TestCases/Posix/Output/free_hook_realloc.cc.tmp 2>&1 | FileCheck /share/dim/src/llvm/trunk/projects/compiler-rt/test/asan/TestCases/Posix/free_hook_realloc.cc -- Exit Code: 2 Command Output (stderr): -- FileCheck error: '-' is empty. FileCheck command line: FileCheck /share/dim/src/llvm/trunk/projects/compiler-rt/test/asan/TestCases/Posix/free_hook_realloc.cc -- Testing: 0 .. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. Testing Time: 880.55s Failing Tests (3): AddressSanitizer-Unit :: Asan-i386-inline-Test/AddressSanitizer.ReallocFreedPointerTest AddressSanitizer-Unit :: Asan-i386-with-calls-Test/AddressSanitizer.ReallocFreedPointerTest AddressSanitizer-i386-freebsd :: TestCases/Posix/free_hook_realloc.cc Expected Passes: 26881 Expected Failures : 144 Unsupported Tests : 3176 Unexpected Failures: 3 1 warning(s) in tests. -Dimitry signature.asc Description: Message signed with OpenPGP using GPG
[PATCH] D28047: Remove the '-disable-llvm-passes' flag (which I didn't even know existed, and I suspect many others aren't aware of either) and strength '-disable-llvm-optzns' to do the same thing.
chandlerc added a comment. In https://reviews.llvm.org/D28047#629824, @mehdi_amini wrote: > In https://reviews.llvm.org/D28047#629746, @rnk wrote: > > > How about standardizing on -disable-llvm-passes instead of > > -disable-llvm-optzns? I never liked "optzns" and can't remember how to > > spell it. Also, this flag disables LLVM instrumentation passes (ASan) as > > well as optimization passes, so I think -disable-llvm-passes is the better > > name. > > > I agree with that. > To be friendly with developers/users we could print an error mentioning to > use `-disable-llvm-passes` when they try to use `-disable-llvm-optzns` (which > we accept as a `-mllvm -disable-llvm-optzns` hardcoded in the driver by the > way). I only have a mild preference for how we spell the name based on the fact that i've told people to use -disable-llvm-optzns for years. If we want to go with ...-passes, fine with me. I'll probably just add an alias so that you can use either spelling, but change the tests to consolidate on whichever spelling we end up with. SG? I can regenerate the patch if desired, but it'll just be a lot of perl... https://reviews.llvm.org/D28047 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28053: Cleanup the handling of noinline function attributes, -fno-inline, -fno-inline-functions, -O0, and optnone.
chandlerc added a comment. In https://reviews.llvm.org/D28053#629830, @mehdi_amini wrote: > > It would be awesome if attribute sets were a bit more FileCheck friendly, > > but oh well. > > I've been wondering about that, what's the point of attribute sets in the > textual IR? > I understand the idea for saving space in the Bitcode, but the IR does not > have to use the same representation. Will return to the actual patch review, but as an aside, I would *strongly* support going back to printing attributes in the canonical location in the function declaration / definition. It has huge advantages: - Can FileCheck individual function's attributes rather than a set at a time - Can place those checks after a CHECK-LABEL for the function name - Can have a CHECK-LABEL for the function name, CHECKs for attributes *and* CHECKs for function body stuff, which today is extremely hard to combine into a single test. Happy to help with this if you want to pursue it. https://reviews.llvm.org/D28053 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28047: Remove the '-disable-llvm-passes' flag (which I didn't even know existed, and I suspect many others aren't aware of either) and strength '-disable-llvm-optzns' to do the same thing.
mehdi_amini added a comment. LGTM, but please wait for @rnk to confirm :) Also any opinion about a driver option -emit-raw-llvm as @joerg suggested? https://reviews.llvm.org/D28047 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28047: Remove the '-disable-llvm-passes' flag (which I didn't even know existed, and I suspect many others aren't aware of either) and strength '-disable-llvm-optzns' to do the same thing.
rnk added a comment. In https://reviews.llvm.org/D28047#629889, @chandlerc wrote: > I only have a mild preference for how we spell the name based on the fact > that i've told people to use -disable-llvm-optzns for years. If we want to go > with ...-passes, fine with me. We have some freedom to change the -cc1 interface. I liked being able to pass -g to -cc1, but then we went to -debug-info-kind=(limited|full|linetables) or whatever. =? > I'll probably just add an alias so that you can use either spelling, but > change the tests to consolidate on whichever spelling we end up with. > > SG? I can regenerate the patch if desired, but it'll just be a lot of perl... Sounds good! We definitely don't need both -disable-llvm-optzns and -disable-llvm-passes. https://reviews.llvm.org/D28047 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r290169 - Revert r290149: Add the alloc_size attribute to clang.
Yes, this was reapplied in r290297 with fixes for the msan issue we caught; these asan unit test failures are news to me. Can you give me the command that you're using to run these tests, please? On Thu, Dec 22, 2016 at 11:10 AM, Dimitry Andric wrote: > On 20 Dec 2016, at 09:28, Chandler Carruth via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > > Author: chandlerc > > Date: Tue Dec 20 02:28:19 2016 > > New Revision: 290169 > > > > URL: http://llvm.org/viewvc/llvm-project?rev=290169&view=rev > > Log: > > Revert r290149: Add the alloc_size attribute to clang. > > > > This commit fails MSan when running test/CodeGen/object-size.c in > > a confusing way. After some discussion with George, it isn't really > > clear what is going on here. We can make the MSan failure go away by > > testing for the invalid bit, but *why* things are invalid isn't clear. > > And yet, other code in the surrounding area is doing precisely this and > > testing for invalid. > > > > George is going to take a closer look at this to better understand the > > nature of the failure and recommit it, for now backing it out to clean > > up MSan builds. > > Hmm, was this reapplied later on? I'm still getting the following > AddressSanitizer failures on FreeBSD, and bisecting has pointed to r290149 > as the cause: > > FAIL: AddressSanitizer-Unit :: Asan-i386-inline-Test/AddressS > anitizer.ReallocFreedPointerTest (2124 of 30204) > TEST 'AddressSanitizer-Unit :: > Asan-i386-inline-Test/AddressSanitizer.ReallocFreedPointerTest' FAILED > > Note: Google Test filter = AddressSanitizer.ReallocFreedPointerTest > [==] Running 1 test from 1 test case. > [--] Global test environment set-up. > [--] 1 test from AddressSanitizer > [ RUN ] AddressSanitizer.ReallocFreedPointerTest > /share/dim/src/llvm/trunk/projects/compiler-rt/lib/asan/tests/asan_test.cc:377: > Failure > Death test: ptr = realloc(ptr, 77) > Result: failed to die. > Error msg: > [ DEATH ] > [ FAILED ] AddressSanitizer.ReallocFreedPointerTest (30 ms) > [--] 1 test from AddressSanitizer (30 ms total) > > [--] Global test environment tear-down > [==] 1 test from 1 test case ran. (31 ms total) > [ PASSED ] 0 tests. > [ FAILED ] 1 test, listed below: > [ FAILED ] AddressSanitizer.ReallocFreedPointerTest > > 1 FAILED TEST > YOU HAVE 24 DISABLED TESTS > > > > Testing: 0 . > FAIL: AddressSanitizer-Unit :: Asan-i386-with-calls-Test/Addr > essSanitizer.ReallocFreedPointerTest (2233 of 30204) > TEST 'AddressSanitizer-Unit :: > Asan-i386-with-calls-Test/AddressSanitizer.ReallocFreedPointerTest' > FAILED > Note: Google Test filter = AddressSanitizer.ReallocFreedPointerTest > [==] Running 1 test from 1 test case. > [--] Global test environment set-up. > [--] 1 test from AddressSanitizer > [ RUN ] AddressSanitizer.ReallocFreedPointerTest > /share/dim/src/llvm/trunk/projects/compiler-rt/lib/asan/tests/asan_test.cc:377: > Failure > Death test: ptr = realloc(ptr, 77) > Result: failed to die. > Error msg: > [ DEATH ] > [ FAILED ] AddressSanitizer.ReallocFreedPointerTest (24 ms) > [--] 1 test from AddressSanitizer (24 ms total) > > [--] Global test environment tear-down > [==] 1 test from 1 test case ran. (25 ms total) > [ PASSED ] 0 tests. > [ FAILED ] 1 test, listed below: > [ FAILED ] AddressSanitizer.ReallocFreedPointerTest > > 1 FAILED TEST > YOU HAVE 24 DISABLED TESTS > > > > Testing: 0 . > FAIL: AddressSanitizer-i386-freebsd :: TestCases/Posix/free_hook_realloc.cc > (2399 of 30204) > TEST 'AddressSanitizer-i386-freebsd :: > TestCases/Posix/free_hook_realloc.cc' FAILED > Script: > -- > /home/dim/obj/llvm-290338-trunk-freebsd12-i386-ninja-rel-1/./bin/clang > --driver-mode=g++ -fsanitize=address -mno-omit-leaf-frame-pointer > -fno-omit-frame-pointer -fno-optimize-sibling-calls -gline-tables-only -m32 > -O2 /share/dim/src/llvm/trunk/projects/compiler-rt/test/asan/Tes > tCases/Posix/free_hook_realloc.cc -o /home/dim/obj/llvm-290338-trun > k-freebsd12-i386-ninja-rel-1/projects/compiler-rt/test/asan/ > I386FreeBSDConfig/TestCases/Posix/Output/free_hook_realloc.cc.tmp > /home/dim/obj/llvm-290338-trunk-freebsd12-i386-ninja-rel-1/p > rojects/compiler-rt/test/asan/I386FreeBSDConfig/TestCases/ > Posix/Output/free_hook_realloc.cc.tmp 2>&1 | FileCheck > /share/dim/src/llvm/trunk/projects/compiler-rt/test/asan/Tes > tCases/Posix/free_hook_realloc.cc > -- > Exit Code: 2 > > Command Output (stderr): > -- > FileCheck error: '-' is empty. > FileCheck command line: FileCheck /share/dim/src/llvm/trunk/proj > ects/compiler-rt/test/asan/TestCases/Posix/free_hook_realloc.cc > > -- > > > Testing: 0 .. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. > Testing Time: 880.55s >
[PATCH] D28053: Cleanup the handling of noinline function attributes, -fno-inline, -fno-inline-functions, -O0, and optnone.
chandlerc updated this revision to Diff 82356. chandlerc added a comment. Herald added a subscriber: wdng. Fix an thinko found in review, clean up comments, and clean up pass pipeline selection as that actually needs to happen in this patch as well. Also update one test using -O1 and observing this, including making it use FileCheck more effectively. https://reviews.llvm.org/D28053 Files: include/clang/Frontend/CodeGenOptions.def include/clang/Frontend/CodeGenOptions.h lib/CodeGen/BackendUtil.cpp lib/CodeGen/CGOpenMPRuntime.cpp lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp lib/CodeGen/CodeGenFunction.cpp lib/CodeGen/CodeGenModule.cpp lib/Frontend/CompilerInvocation.cpp test/CXX/special/class.dtor/p3-0x.cpp test/CodeGen/2008-04-08-NoExceptions.c test/CodeGen/address-safety-attr-kasan.cpp test/CodeGen/address-safety-attr.cpp test/CodeGen/address-space-field1.c test/CodeGen/alias.c test/CodeGen/attr-minsize.cpp test/CodeGen/attributes.c test/CodeGen/incomplete-function-type-2.c test/CodeGen/inline-optim.c test/CodeGen/mips16-attr.c test/CodeGen/mrtd.c test/CodeGen/ms-declspecs.c test/CodeGen/ppc64-complex-parms.c test/CodeGen/ppc64-complex-return.c test/CodeGen/ppc64-extend.c test/CodeGen/sanitize-thread-attr.cpp test/CodeGen/sanitize-thread-no-checking-at-run-time.m test/CodeGen/unwind-attr.c test/CodeGenCXX/attr.cpp test/CodeGenCXX/cxx11-exception-spec.cpp test/CodeGenCXX/cxx11-noreturn.cpp test/CodeGenCXX/derived-to-base.cpp test/CodeGenCXX/global-dtor-no-atexit.cpp test/CodeGenCXX/global-init.cpp test/CodeGenCXX/inline-hint.cpp test/CodeGenCXX/main-norecurse.cpp test/CodeGenCXX/microsoft-abi-array-cookies.cpp test/CodeGenCXX/no-exceptions.cpp test/CodeGenCXX/optnone-class-members.cpp test/CodeGenCXX/optnone-def-decl.cpp test/CodeGenCXX/reference-cast.cpp test/CodeGenCXX/threadsafe-statics.cpp test/CodeGenCXX/thunks.cpp test/CodeGenCXX/virtual-base-cast.cpp test/CodeGenObjC/gnu-exceptions.m test/CodeGenObjC/objc-literal-tests.m test/CodeGenObjCXX/lambda-expressions.mm test/CodeGenOpenCL/amdgpu-attrs.cl test/Driver/darwin-iphone-defaults.m test/PCH/objc_container.m Index: test/PCH/objc_container.m === --- test/PCH/objc_container.m +++ test/PCH/objc_container.m @@ -21,5 +21,5 @@ // CHECK-IR: {{call.*objc_msgSend}} // CHECK-IR: ret void -// CHECK-IR: attributes #0 = { nounwind {{.*}} } +// CHECK-IR: attributes #0 = { noinline nounwind {{.*}} } // CHECK-IR: attributes #1 = { nonlazybind } Index: test/Driver/darwin-iphone-defaults.m === --- test/Driver/darwin-iphone-defaults.m +++ test/Driver/darwin-iphone-defaults.m @@ -26,4 +26,4 @@ [I1 alloc]; } -// CHECK: attributes [[F0]] = { ssp{{.*}} } +// CHECK: attributes [[F0]] = { noinline ssp{{.*}} } Index: test/CodeGenOpenCL/amdgpu-attrs.cl === --- test/CodeGenOpenCL/amdgpu-attrs.cl +++ test/CodeGenOpenCL/amdgpu-attrs.cl @@ -141,26 +141,26 @@ // CHECK-NOT: "amdgpu-num-sgpr"="0" // CHECK-NOT: "amdgpu-num-vgpr"="0" -// CHECK-DAG: attributes [[FLAT_WORK_GROUP_SIZE_32_64]] = { nounwind "amdgpu-flat-work-group-size"="32,64" -// CHECK-DAG: attributes [[WAVES_PER_EU_2]] = { nounwind "amdgpu-waves-per-eu"="2" -// CHECK-DAG: attributes [[WAVES_PER_EU_2_4]] = { nounwind "amdgpu-waves-per-eu"="2,4" -// CHECK-DAG: attributes [[NUM_SGPR_32]] = { nounwind "amdgpu-num-sgpr"="32" -// CHECK-DAG: attributes [[NUM_VGPR_64]] = { nounwind "amdgpu-num-vgpr"="64" +// CHECK-DAG: attributes [[FLAT_WORK_GROUP_SIZE_32_64]] = { noinline nounwind "amdgpu-flat-work-group-size"="32,64" +// CHECK-DAG: attributes [[WAVES_PER_EU_2]] = { noinline nounwind "amdgpu-waves-per-eu"="2" +// CHECK-DAG: attributes [[WAVES_PER_EU_2_4]] = { noinline nounwind "amdgpu-waves-per-eu"="2,4" +// CHECK-DAG: attributes [[NUM_SGPR_32]] = { noinline nounwind "amdgpu-num-sgpr"="32" +// CHECK-DAG: attributes [[NUM_VGPR_64]] = { noinline nounwind "amdgpu-num-vgpr"="64" -// CHECK-DAG: attributes [[FLAT_WORK_GROUP_SIZE_32_64_WAVES_PER_EU_2]] = { nounwind "amdgpu-flat-work-group-size"="32,64" "amdgpu-waves-per-eu"="2" -// CHECK-DAG: attributes [[FLAT_WORK_GROUP_SIZE_32_64_WAVES_PER_EU_2_4]] = { nounwind "amdgpu-flat-work-group-size"="32,64" "amdgpu-waves-per-eu"="2,4" -// CHECK-DAG: attributes [[FLAT_WORK_GROUP_SIZE_32_64_NUM_SGPR_32]] = { nounwind "amdgpu-flat-work-group-size"="32,64" "amdgpu-num-sgpr"="32" -// CHECK-DAG: attributes [[FLAT_WORK_GROUP_SIZE_32_64_NUM_VGPR_64]] = { nounwind "amdgpu-flat-work-group-size"="32,64" "amdgpu-num-vgpr"="64" -// CHECK-DAG: attributes [[WAVES_PER_EU_2_NUM_SGPR_32]] = { nounwind "amdgpu-num-sgpr"="32" "amdgpu-waves-per-eu"="2" -// CHECK-DAG: attributes [[WAVES_PER_EU_2_NUM_VGPR_64]] = { nounwind "amdgpu-num-vgpr"="64" "amdgpu-waves-per-eu"="2" -// CHECK-DAG: attributes [[WAVES_PER_EU_2_4_
[PATCH] D28053: Cleanup the handling of noinline function attributes, -fno-inline, -fno-inline-functions, -O0, and optnone.
chandlerc added a comment. In https://reviews.llvm.org/D28053#629834, @mehdi_amini wrote: > In https://reviews.llvm.org/D28053#629768, @rnk wrote: > > > The big change here is that `clang -O0` now applies the noinline attribute > > everywhere. I can see why someone might expect things to work that way, but > > it seems surprising to me at first glance. > > > > Before this change I can imagine someone distributing a static archive of > > bitcode which could be used to produce debug binaries and release binaries. > > There are some issues with that (lack of lifetime markers), but it should > > mostly work. > > > Note that LTO runs a bunch of optimizations during the compile phase that you > wouldn't get with that. There are other issues with this workflow as well. For example, Clang at -O0 will intentionally reuse local allocas to save stack space without the backend having to track lifetime. This destroys aliasing information left and right though, so you really wouldn't want this for a mixed-use bitcode archive. I think a much more effective technique (if folks actually want to do this) would be (with whatever flag spelling you want) % clang -O -emit-llvm -mllvm -disable-llvm-passes This will generate a frontend-optimized but backend pristine bitcode file that can be processed more or less depending on the desire of the user... But I also don't think we really need to do a ton of work to support this hypothetical. If someone really wants to make this work, they should specifically request some mode and we should surface a clear flag documented to provide it. Joerg's idea about '-emit-raw-llvm' would be a nice step in this direction IMO. > Steven had issues in the past when mixing O0 and > https://reviews.llvm.org/owners/package/2/ in LTO, I don't remember the > details unfortunately (and Steven is OOO). > Right now we don't add the `optnone` attribute when building with O0, maybe > we should as well if we go this route. Fundamentally, yes. This patch is trying to follow the path we've been going down where we try to propagate optimization restrictions to LTO. We do this for -Os and -Oz, but not -O0. This propagates one aspect (inlining), but I would be interested in using optnone to more effectively propagate O0. That said, I agree with Reid that it is a bit surprising. After thinking about it a lot, the reason I personally find it surprising is actually that I find -O0 being the default surprising. But I'm OK with not changing that here, and maybe never changing that at the CC1 layer. Thoughts? Generally happy with this direction? There is another semantic change here that I intended but Mehdi highlighted I failed to fully implement: I'm also *not* using 'noinline' at -O1, and instead just building a simpler pass pipeline. I think this better fits with the intent: -O1 vs -O2 vs -O3 don't really change the *target* of optimization but instead *how much* optimization to do. As such, they feel like they don't need to be in attributes where -O0, -Os, and -Oz do. Comment at: lib/Frontend/CompilerInvocation.cpp:453 +// There is no effect at O0 when we won't inline anyways. +if (Opts.OptimizationLevel > 1) { + const Option &InlineOpt = InlineArg->getOption(); mehdi_amini wrote: > mehdi_amini wrote: > > I'd switch the two if > The test `> 1` and the comment about `-O0` are confusing to me as well. What > about the following (IIUC): > > ``` > if (Opts.OptimizationLevel <= 1) { > // "always-inline" required for correctness at O0 and O1. > Opts.setInlining(CodeGenOptions::OnlyAlwaysInlining); > else if (Opts.OptimizationLevel > 1) { > // Normal inlining at O2 and above > Opts.setInlining((CodeGenOptions::NormalInlining); > // -fno-inline-functions overrides > if (Arg *InlineArg = Args.getLastArg( > options::OPT_finline_functions, options::OPT_finline_hint_functions, > options::OPT_fno_inline_functions, options::OPT_fno_inline)) { > if (InlineOpt.matches(options::OPT_finline_functions)) > Opts.setInlining(CodeGenOptions::NormalInlining); > else if (InlineOpt.matches(options::OPT_finline_hint_functions)) > Opts.setInlining(CodeGenOptions::OnlyHintInlining); > else > Opts.setInlining(CodeGenOptions::OnlyAlwaysInlining); > } > } > ``` > Yea, this was confusing, and I hadn't actually implemented my intent here either. My goal was to make the `Inlining` setting correspond to whether we *forcibly* disable some aspects of inlining or not, rather than anything to do with what inliner pass we run. This means we should only be setting `OnlyAlwaysInlining` in O0, not at O1, and the O1 behavior should be up to the backend's construction of a pass pipeline appropriate to that optimization level. (I actually think using the always inliner is a mistake here, but that's a separate discussion.) To make all of this work requires me to merge in the next pat
[PATCH] D28047: Remove the '-disable-llvm-passes' flag (which I didn't even know existed, and I suspect many others aren't aware of either) and strength '-disable-llvm-optzns' to do the same thing.
chandlerc added a comment. In https://reviews.llvm.org/D28047#629892, @mehdi_amini wrote: > LGTM, but please wait for @rnk to confirm :) > > Also any opinion about a driver option -emit-raw-llvm as @joerg suggested? I like the idea generally, but I don't have an immediate use case and would defer that to a subsequent patch authored by someone who wants this? I really only wanted to remove the divergence between two flags here. https://reviews.llvm.org/D28047 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r290364 - [OPENMP] Fix for PR31417: assert failure when compiling trivial openmp
Author: abataev Date: Thu Dec 22 13:44:05 2016 New Revision: 290364 URL: http://llvm.org/viewvc/llvm-project?rev=290364&view=rev Log: [OPENMP] Fix for PR31417: assert failure when compiling trivial openmp program Offload related code is not quite ready yet, but some simple examples must not crash the compiler. Patch fixes the problem in offloading code with exceptions. Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.h cfe/trunk/test/OpenMP/nvptx_target_codegen.cpp Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp?rev=290364&r1=290363&r2=290364&view=diff == --- cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp (original) +++ cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp Thu Dec 22 13:44:05 2016 @@ -244,6 +244,9 @@ void CGOpenMPRuntimeNVPTX::emitEntryHead void CGOpenMPRuntimeNVPTX::emitEntryFooter(CodeGenFunction &CGF, EntryFunctionState &EST) { + if (!EST.ExitBB) +EST.ExitBB = CGF.createBasicBlock(".exit"); + CGBuilderTy &Bld = CGF.Builder; llvm::BasicBlock *TerminateBB = CGF.createBasicBlock(".termination.notifier"); CGF.EmitBranch(TerminateBB); @@ -259,6 +262,7 @@ void CGOpenMPRuntimeNVPTX::emitEntryFoot CGF.EmitBranch(EST.ExitBB); CGF.EmitBlock(EST.ExitBB); + EST.ExitBB = nullptr; } /// \brief Returns specified OpenMP runtime function for the current OpenMP Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.h?rev=290364&r1=290363&r2=290364&view=diff == --- cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.h (original) +++ cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.h Thu Dec 22 13:44:05 2016 @@ -25,11 +25,8 @@ namespace CodeGen { class CGOpenMPRuntimeNVPTX : public CGOpenMPRuntime { public: - class EntryFunctionState { - public: -llvm::BasicBlock *ExitBB; - -EntryFunctionState() : ExitBB(nullptr){}; + struct EntryFunctionState { +llvm::BasicBlock *ExitBB = nullptr; }; class WorkerFunctionState { Modified: cfe/trunk/test/OpenMP/nvptx_target_codegen.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/nvptx_target_codegen.cpp?rev=290364&r1=290363&r2=290364&view=diff == --- cfe/trunk/test/OpenMP/nvptx_target_codegen.cpp (original) +++ cfe/trunk/test/OpenMP/nvptx_target_codegen.cpp Thu Dec 22 13:44:05 2016 @@ -3,6 +3,7 @@ // RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s --check-prefix CHECK --check-prefix CHECK-64 // RUN: %clang_cc1 -verify -fopenmp -x c++ -triple i386-unknown-unknown -fopenmp-targets=nvptx-nvidia-cuda -emit-llvm-bc %s -o %t-x86-host.bc // RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx-unknown-unknown -fopenmp-targets=nvptx-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-x86-host.bc -o - | FileCheck %s --check-prefix CHECK --check-prefix CHECK-32 +// RUN: %clang_cc1 -verify -fopenmp -fexceptions -fcxx-exceptions -x c++ -triple nvptx-unknown-unknown -fopenmp-targets=nvptx-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-x86-host.bc -o - | FileCheck %s --check-prefix CHECK --check-prefix CHECK-32 // expected-no-diagnostics #ifndef HEADER #define HEADER @@ -25,7 +26,7 @@ int foo(int n) { double cn[5][n]; TT d; - // CHECK-LABEL: define {{.*}}void {{@__omp_offloading_.+foo.+l86}}_worker() + // CHECK-LABEL: define {{.*}}void {{@__omp_offloading_.+foo.+l87}}_worker() // CHECK: br label {{%?}}[[AWAIT_WORK:.+]] // // CHECK: [[AWAIT_WORK]] @@ -53,7 +54,7 @@ int foo(int n) { // CHECK: [[EXIT]] // CHECK: ret void - // CHECK: define {{.*}}void [[T1:@__omp_offloading_.+foo.+l86]]() + // CHECK: define {{.*}}void [[T1:@__omp_offloading_.+foo.+l87]]() // CHECK: [[NTID:%.+]] = call i32 @llvm.nvvm.read.ptx.sreg.ntid.x() // CHECK: [[WS:%.+]] = call i32 @llvm.nvvm.read.ptx.sreg.warpsize() // CHECK: [[A:%.+]] = sub i32 [[WS]], 1 @@ -68,7 +69,7 @@ int foo(int n) { // CHECK: br i1 [[IS_WORKER]], label {{%?}}[[WORKER:.+]], label {{%?}}[[MASTER:.+]] // // CHECK: [[WORKER]] - // CHECK: call void [[T1]]_worker() + // CHECK: {{call|invoke}} void [[T1]]_worker() // CHECK: br label {{%?}}[[EXIT]] // // CHECK: [[MASTER]] @@ -92,7 +93,7 @@ int foo(int n) { { } - // CHECK-LABEL: define {{.*}}void {{@__omp_offloading_.+foo.+l157}}_worker() + // CHECK-LABEL: define {{.*}}void {{@__omp_offloading_.+foo.+l158}}_worker() // CHECK: br label {{%?}}[[AWA
[PATCH] D28053: Cleanup the handling of noinline function attributes, -fno-inline, -fno-inline-functions, -O0, and optnone.
mehdi_amini added inline comments. Comment at: lib/Frontend/CompilerInvocation.cpp:453 +// There is no effect at O0 when we won't inline anyways. +if (Opts.OptimizationLevel > 1) { + const Option &InlineOpt = InlineArg->getOption(); chandlerc wrote: > mehdi_amini wrote: > > mehdi_amini wrote: > > > I'd switch the two if > > The test `> 1` and the comment about `-O0` are confusing to me as well. > > What about the following (IIUC): > > > > ``` > > if (Opts.OptimizationLevel <= 1) { > > // "always-inline" required for correctness at O0 and O1. > > Opts.setInlining(CodeGenOptions::OnlyAlwaysInlining); > > else if (Opts.OptimizationLevel > 1) { > > // Normal inlining at O2 and above > > Opts.setInlining((CodeGenOptions::NormalInlining); > > // -fno-inline-functions overrides > > if (Arg *InlineArg = Args.getLastArg( > > options::OPT_finline_functions, > > options::OPT_finline_hint_functions, > > options::OPT_fno_inline_functions, options::OPT_fno_inline)) { > > if (InlineOpt.matches(options::OPT_finline_functions)) > > Opts.setInlining(CodeGenOptions::NormalInlining); > > else if (InlineOpt.matches(options::OPT_finline_hint_functions)) > > Opts.setInlining(CodeGenOptions::OnlyHintInlining); > > else > > Opts.setInlining(CodeGenOptions::OnlyAlwaysInlining); > > } > > } > > ``` > > > Yea, this was confusing, and I hadn't actually implemented my intent here > either. > > My goal was to make the `Inlining` setting correspond to whether we > *forcibly* disable some aspects of inlining or not, rather than anything to > do with what inliner pass we run. > > This means we should only be setting `OnlyAlwaysInlining` in O0, not at O1, > and the O1 behavior should be up to the backend's construction of a pass > pipeline appropriate to that optimization level. (I actually think using the > always inliner is a mistake here, but that's a separate discussion.) > > To make all of this work requires me to merge in the next patch I was working > on which I've done. It doesn't really scope creep much. > > I've updated the formulation of the conditions and the comments to hopefully > be more clear. > > I'm still doing the weird order of testing because i want to always run > `getLastArg` to mark these flags as used. > always run getLastArg to mark these flags as used. Oh I missed this side effect in the test, that's nasty. Is it covered by a test? (i.e. if someone refactor this code in the future will we warn for unused and break a test?). https://reviews.llvm.org/D28053 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r290169 - Revert r290149: Add the alloc_size attribute to clang.
This is when running "ninja check-all", in a tree with llvm, clang and compiler-rt checked out. The first program that shows a failure is projects/compiler-rt/lib/asan/tests/default/Asan-i386-inline-Test: [==] Running 92 tests from 3 test cases. [--] Global test environment set-up. [--] 14 tests from AddressSanitizerInterface ... [ RUN ] AddressSanitizer.ReallocFreedPointerTest /share/dim/src/llvm/trunk/projects/compiler-rt/lib/asan/tests/asan_test.cc:377: Failure Death test: ptr = realloc(ptr, 77) Result: failed to die. Error msg: [ DEATH ] [ FAILED ] AddressSanitizer.ReallocFreedPointerTest (48 ms) A similar failure shows when running projects/compiler-rt/lib/asan/tests/default/Asan-i386-with-calls-Test: [==] Running 92 tests from 3 test cases. [--] Global test environment set-up. [--] 14 tests from AddressSanitizerInterface ... [ RUN ] AddressSanitizer.ReallocFreedPointerTest /share/dim/src/llvm/trunk/projects/compiler-rt/lib/asan/tests/asan_test.cc:377: Failure Death test: ptr = realloc(ptr, 77) Result: failed to die. Error msg: [ DEATH ] [ FAILED ] AddressSanitizer.ReallocFreedPointerTest (55 ms) Interestingly, the Asan-i386-inline-Noinst-Test and Asan-i386-with-calls-Noinst-Test do not show this particular failure. The other test that fails is projects/compiler-rt/test/asan/I386FreeBSDConfig/TestCases/Posix/Output/free_hook_realloc.cc.tmp, which simply returns 1 without printing any output. Debugging the program shows that it seems to be skipping completely over the realloc() call, and jumping directly to the _exit(1), but this may be due to optimization. -Dimitry > On 22 Dec 2016, at 20:27, George Burgess IV > wrote: > > Yes, this was reapplied in r290297 with fixes for the msan issue we caught; > these asan unit test failures are news to me. Can you give me the command > that you're using to run these tests, please? > > On Thu, Dec 22, 2016 at 11:10 AM, Dimitry Andric wrote: > On 20 Dec 2016, at 09:28, Chandler Carruth via cfe-commits > wrote: > > Author: chandlerc > > Date: Tue Dec 20 02:28:19 2016 > > New Revision: 290169 > > > > URL: http://llvm.org/viewvc/llvm-project?rev=290169&view=rev > > Log: > > Revert r290149: Add the alloc_size attribute to clang. > > > > This commit fails MSan when running test/CodeGen/object-size.c in > > a confusing way. After some discussion with George, it isn't really > > clear what is going on here. We can make the MSan failure go away by > > testing for the invalid bit, but *why* things are invalid isn't clear. > > And yet, other code in the surrounding area is doing precisely this and > > testing for invalid. > > > > George is going to take a closer look at this to better understand the > > nature of the failure and recommit it, for now backing it out to clean > > up MSan builds. > > Hmm, was this reapplied later on? I'm still getting the following > AddressSanitizer failures on FreeBSD, and bisecting has pointed to r290149 as > the cause: > > FAIL: AddressSanitizer-Unit :: > Asan-i386-inline-Test/AddressSanitizer.ReallocFreedPointerTest (2124 of 30204) > TEST 'AddressSanitizer-Unit :: > Asan-i386-inline-Test/AddressSanitizer.ReallocFreedPointerTest' FAILED > > Note: Google Test filter = AddressSanitizer.ReallocFreedPointerTest > [==] Running 1 test from 1 test case. > [--] Global test environment set-up. > [--] 1 test from AddressSanitizer > [ RUN ] AddressSanitizer.ReallocFreedPointerTest > /share/dim/src/llvm/trunk/projects/compiler-rt/lib/asan/tests/asan_test.cc:377: > Failure > Death test: ptr = realloc(ptr, 77) > Result: failed to die. > Error msg: > [ DEATH ] > [ FAILED ] AddressSanitizer.ReallocFreedPointerTest (30 ms) > [--] 1 test from AddressSanitizer (30 ms total) > > [--] Global test environment tear-down > [==] 1 test from 1 test case ran. (31 ms total) > [ PASSED ] 0 tests. > [ FAILED ] 1 test, listed below: > [ FAILED ] AddressSanitizer.ReallocFreedPointerTest > > 1 FAILED TEST > YOU HAVE 24 DISABLED TESTS > > > > Testing: 0 . > FAIL: AddressSanitizer-Unit :: > Asan-i386-with-calls-Test/AddressSanitizer.ReallocFreedPointerTest (2233 of > 30204) > TEST 'AddressSanitizer-Unit :: > Asan-i386-with-calls-Test/AddressSanitizer.ReallocFreedPointerTest' FAILED > > Note: Google Test filter = AddressSanitizer.ReallocFreedPointerTest > [==] Running 1 test from 1 test case. > [--] Global test environment set-up. > [--] 1 test from AddressSanitizer > [ RUN ] AddressSanitizer.ReallocFreedPointerTest > /share/dim/src/llvm/trunk/projects/compiler-rt/lib/asan/tests/asan_test.cc:377: > Failure > Death test: ptr = realloc(ptr, 77) > Result: failed to die. > Error msg: > [ DEATH ] > [ FAILED ] AddressSanitizer.ReallocFreedPointerTest
[PATCH] D28053: Cleanup the handling of noinline function attributes, -fno-inline, -fno-inline-functions, -O0, and optnone.
mehdi_amini added a comment. > That said, I agree with Reid that it is a bit surprising. After thinking > about it a lot, the reason I personally find it surprising is actually that I > find -O0 being the default surprising. But I'm OK with not changing that > here, and maybe never changing that at the CC1 layer. We could reduce the "surprising effect" for LTO behavior by requiring an explicit -O when -flto is passed to the driver. > Thoughts? Generally happy with this direction? LGTM. https://reviews.llvm.org/D28053 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r290367 - Testbed and skeleton of a new expression parser
Author: spyffe Date: Thu Dec 22 14:03:14 2016 New Revision: 290367 URL: http://llvm.org/viewvc/llvm-project?rev=290367&view=rev Log: Testbed and skeleton of a new expression parser Recommitted after formal approval. LLVM's JIT is now the foundation of dynamic-compilation features for many languages. Clang also has low-level support for dynamic compilation (ASTImporter and ExternalASTSource, notably). How the compiler is set up for dynamic parsing is generally left up to individual clients, for example LLDB's C/C++/Objective-C expression parser and the ROOT project. Although this arrangement offers external clients the flexibility to implement dynamic features as they see fit, the lack of an in-tree client means that subtle bugs can be introduced that cause regressions in the external clients but aren't caught by tests (or users) until much later. LLDB for example regularly encounters complicated ODR violation scenarios where it is not immediately clear who is at fault. Other external clients (notably, Cling) rely on similar functionality, and another goal is to break this functionality up into composable parts so that any client can be built easily on top of Clang without requiring extensive additional code. I propose that the parts required to build a simple expression parser be added to Clang. Initially, I aim to have the following features: A piece that looks up external declarations from a variety of sources (e.g., from previous dynamic compilations, from modules, or from DWARF) and uses clear conflict resolution rules to reconcile differences, with easily understood errors. This functionality will be supported by in-tree tests. A piece that works hand in hand with the LLVM JIT to resolve the locations of external declarations so that e.g. variables can be redeclared and (for high-performance applications like DTrace) external variables can be accessed directly from the registers where they reside. This commit adds a tester that parses a sequence of source files and then uses them as source data for an expression. External references are resolved using an ExternalASTSource that responds to name queries using an ASTImporter. This is the setup that LLDB uses, and the motivating reason for MinimalImport in ASTImporter. When complete, this tester will implement the first of the above goals. Differential Revision: https://reviews.llvm.org/D27180 Added: cfe/trunk/test/Import/ cfe/trunk/test/Import/clang-flags/ cfe/trunk/test/Import/clang-flags/Inputs/ cfe/trunk/test/Import/clang-flags/Inputs/S.c cfe/trunk/test/Import/clang-flags/test.c cfe/trunk/test/Import/empty-struct/ cfe/trunk/test/Import/empty-struct/Inputs/ cfe/trunk/test/Import/empty-struct/Inputs/S.c cfe/trunk/test/Import/empty-struct/test.c cfe/trunk/test/Import/error-in-expression/ cfe/trunk/test/Import/error-in-expression/Inputs/ cfe/trunk/test/Import/error-in-expression/Inputs/S.c cfe/trunk/test/Import/error-in-expression/test.c cfe/trunk/test/Import/error-in-import/ cfe/trunk/test/Import/error-in-import/Inputs/ cfe/trunk/test/Import/error-in-import/Inputs/S.c cfe/trunk/test/Import/error-in-import/test.c cfe/trunk/test/Import/missing-import/ cfe/trunk/test/Import/missing-import/test.c cfe/trunk/tools/clang-import-test/ cfe/trunk/tools/clang-import-test/CMakeLists.txt cfe/trunk/tools/clang-import-test/clang-import-test.cpp Modified: cfe/trunk/test/CMakeLists.txt cfe/trunk/tools/CMakeLists.txt Modified: cfe/trunk/test/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CMakeLists.txt?rev=290367&r1=290366&r2=290367&view=diff == --- cfe/trunk/test/CMakeLists.txt (original) +++ cfe/trunk/test/CMakeLists.txt Thu Dec 22 14:03:14 2016 @@ -39,6 +39,7 @@ list(APPEND CLANG_TEST_DEPS c-index-test diagtool clang-tblgen clang-offload-bundler + clang-import-test ) if(CLANG_ENABLE_STATIC_ANALYZER) Added: cfe/trunk/test/Import/clang-flags/Inputs/S.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Import/clang-flags/Inputs/S.c?rev=290367&view=auto == --- cfe/trunk/test/Import/clang-flags/Inputs/S.c (added) +++ cfe/trunk/test/Import/clang-flags/Inputs/S.c Thu Dec 22 14:03:14 2016 @@ -0,0 +1,2 @@ +STRUCT S { +}; Added: cfe/trunk/test/Import/clang-flags/test.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Import/clang-flags/test.c?rev=290367&view=auto == --- cfe/trunk/test/Import/clang-flags/test.c (added) +++ cfe/trunk/test/Import/clang-flags/test.c Thu Dec 22 14:03:14 2016 @@ -0,0 +1,5 @@ +// RUN: clang-import-test -import %S/Inputs/S.c -expression %s -Xcc -DSTRUCT=struct +void expr() { + STRUCT S MyS; + void *MyPtr = &MyS; +} Added: cfe/trunk/test/Im
[PATCH] D27180: Testbed and skeleton of a new expression parser
This revision was automatically updated to reflect the committed changes. Closed by commit rL290367: Testbed and skeleton of a new expression parser (authored by spyffe). Changed prior to commit: https://reviews.llvm.org/D27180?vs=81991&id=82360#toc Repository: rL LLVM https://reviews.llvm.org/D27180 Files: cfe/trunk/test/CMakeLists.txt cfe/trunk/test/Import/clang-flags/Inputs/S.c cfe/trunk/test/Import/clang-flags/test.c cfe/trunk/test/Import/empty-struct/Inputs/S.c cfe/trunk/test/Import/empty-struct/test.c cfe/trunk/test/Import/error-in-expression/Inputs/S.c cfe/trunk/test/Import/error-in-expression/test.c cfe/trunk/test/Import/error-in-import/Inputs/S.c cfe/trunk/test/Import/error-in-import/test.c cfe/trunk/test/Import/missing-import/test.c cfe/trunk/tools/CMakeLists.txt cfe/trunk/tools/clang-import-test/CMakeLists.txt cfe/trunk/tools/clang-import-test/clang-import-test.cpp Index: cfe/trunk/test/CMakeLists.txt === --- cfe/trunk/test/CMakeLists.txt +++ cfe/trunk/test/CMakeLists.txt @@ -39,6 +39,7 @@ c-index-test diagtool clang-tblgen clang-offload-bundler + clang-import-test ) if(CLANG_ENABLE_STATIC_ANALYZER) Index: cfe/trunk/test/Import/empty-struct/test.c === --- cfe/trunk/test/Import/empty-struct/test.c +++ cfe/trunk/test/Import/empty-struct/test.c @@ -0,0 +1,5 @@ +// RUN: clang-import-test -import %S/Inputs/S.c -expression %s +void expr() { + struct S MyS; + void *MyPtr = &MyS; +} Index: cfe/trunk/test/Import/empty-struct/Inputs/S.c === --- cfe/trunk/test/Import/empty-struct/Inputs/S.c +++ cfe/trunk/test/Import/empty-struct/Inputs/S.c @@ -0,0 +1,2 @@ +struct S { +}; Index: cfe/trunk/test/Import/clang-flags/Inputs/S.c === --- cfe/trunk/test/Import/clang-flags/Inputs/S.c +++ cfe/trunk/test/Import/clang-flags/Inputs/S.c @@ -0,0 +1,2 @@ +STRUCT S { +}; Index: cfe/trunk/test/Import/clang-flags/test.c === --- cfe/trunk/test/Import/clang-flags/test.c +++ cfe/trunk/test/Import/clang-flags/test.c @@ -0,0 +1,5 @@ +// RUN: clang-import-test -import %S/Inputs/S.c -expression %s -Xcc -DSTRUCT=struct +void expr() { + STRUCT S MyS; + void *MyPtr = &MyS; +} Index: cfe/trunk/test/Import/error-in-expression/Inputs/S.c === --- cfe/trunk/test/Import/error-in-expression/Inputs/S.c +++ cfe/trunk/test/Import/error-in-expression/Inputs/S.c @@ -0,0 +1,2 @@ +struct S { +}; Index: cfe/trunk/test/Import/error-in-expression/test.c === --- cfe/trunk/test/Import/error-in-expression/test.c +++ cfe/trunk/test/Import/error-in-expression/test.c @@ -0,0 +1,6 @@ +// RUN: not clang-import-test -import %S/Inputs/S.c -expression %s 2>&1 | FileCheck %s +// CHECK: {{.*}}no viable conversion{{.*}} +void expr() { + struct S MyS; + void *MyPtr = MyS; +} Index: cfe/trunk/test/Import/error-in-import/test.c === --- cfe/trunk/test/Import/error-in-import/test.c +++ cfe/trunk/test/Import/error-in-import/test.c @@ -0,0 +1,6 @@ +// RUN: not clang-import-test -import %S/Inputs/S.c -expression %s 2>&1 | FileCheck %s +// CHECK: {{.*}}expected unqualified-id{{.*}} +void expr() { + struct S MyS; + void *MyPtr = &MyS; +} Index: cfe/trunk/test/Import/error-in-import/Inputs/S.c === --- cfe/trunk/test/Import/error-in-import/Inputs/S.c +++ cfe/trunk/test/Import/error-in-import/Inputs/S.c @@ -0,0 +1,2 @@ +struct S [ +]; Index: cfe/trunk/test/Import/missing-import/test.c === --- cfe/trunk/test/Import/missing-import/test.c +++ cfe/trunk/test/Import/missing-import/test.c @@ -0,0 +1,6 @@ +// RUN: not clang-import-test -import %S/Inputs/S.c -expression %s 2>&1 | FileCheck %s +// CHECK: {{.*}}Couldn't open{{.*}}Inputs/S.c{{.*}} +void expr() { + struct S MyS; + void *MyPtr = &MyS; +} Index: cfe/trunk/tools/CMakeLists.txt === --- cfe/trunk/tools/CMakeLists.txt +++ cfe/trunk/tools/CMakeLists.txt @@ -5,6 +5,7 @@ add_clang_subdirectory(clang-format) add_clang_subdirectory(clang-format-vs) add_clang_subdirectory(clang-fuzzer) +add_clang_subdirectory(clang-import-test) add_clang_subdirectory(clang-offload-bundler) add_clang_subdirectory(c-index-test) Index: cfe/trunk/tools/clang-import-test/CMakeLists.txt === --- cfe/trunk/tools/clang-import-test/CMakeLists.txt +++ cfe/trunk/tools/clang-import-test/CMakeLists.txt @@ -0,0 +1,27 @@ +set(LLVM_LINK_COMPONENT
Re: r290169 - Revert r290149: Add the alloc_size attribute to clang.
Okay, I'm seeing this failure now if I tag my system's `realloc` declaration with `alloc_size`. (Which FreeBSD seems to do in their headers). Because all that clang does with `alloc_size` is use it to answer `__builtin_object_size` queries and lower it to LLVM's `allocsize` attribute, this is presumably a latent bug in LLVM's `allocsize` attribute. Let me mess around for a bit and see what I can dig up. :) On Thu, Dec 22, 2016 at 11:59 AM, Dimitry Andric wrote: > This is when running "ninja check-all", in a tree with llvm, clang and > compiler-rt checked out. The first program that shows a failure is > projects/compiler-rt/lib/asan/tests/default/Asan-i386-inline-Test: > > [==] Running 92 tests from 3 test cases. > [--] Global test environment set-up. > [--] 14 tests from AddressSanitizerInterface > ... > [ RUN ] AddressSanitizer.ReallocFreedPointerTest > /share/dim/src/llvm/trunk/projects/compiler-rt/lib/asan/tests/asan_test.cc:377: > Failure > Death test: ptr = realloc(ptr, 77) > Result: failed to die. > Error msg: > [ DEATH ] > [ FAILED ] AddressSanitizer.ReallocFreedPointerTest (48 ms) > > A similar failure shows when running projects/compiler-rt/lib/asan/ > tests/default/Asan-i386-with-calls-Test: > > [==] Running 92 tests from 3 test cases. > [--] Global test environment set-up. > [--] 14 tests from AddressSanitizerInterface > ... > [ RUN ] AddressSanitizer.ReallocFreedPointerTest > /share/dim/src/llvm/trunk/projects/compiler-rt/lib/asan/tests/asan_test.cc:377: > Failure > Death test: ptr = realloc(ptr, 77) > Result: failed to die. > Error msg: > [ DEATH ] > [ FAILED ] AddressSanitizer.ReallocFreedPointerTest (55 ms) > > Interestingly, the Asan-i386-inline-Noinst-Test and > Asan-i386-with-calls-Noinst-Test do not show this particular failure. > > The other test that fails is projects/compiler-rt/test/asan > /I386FreeBSDConfig/TestCases/Posix/Output/free_hook_realloc.cc.tmp, which > simply returns 1 without printing any output. Debugging the program shows > that it seems to be skipping completely over the realloc() call, and > jumping directly to the _exit(1), but this may be due to optimization. > > -Dimitry > > > On 22 Dec 2016, at 20:27, George Burgess IV > wrote: > > > > Yes, this was reapplied in r290297 with fixes for the msan issue we > caught; these asan unit test failures are news to me. Can you give me the > command that you're using to run these tests, please? > > > > On Thu, Dec 22, 2016 at 11:10 AM, Dimitry Andric > wrote: > > On 20 Dec 2016, at 09:28, Chandler Carruth via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > > > Author: chandlerc > > > Date: Tue Dec 20 02:28:19 2016 > > > New Revision: 290169 > > > > > > URL: http://llvm.org/viewvc/llvm-project?rev=290169&view=rev > > > Log: > > > Revert r290149: Add the alloc_size attribute to clang. > > > > > > This commit fails MSan when running test/CodeGen/object-size.c in > > > a confusing way. After some discussion with George, it isn't really > > > clear what is going on here. We can make the MSan failure go away by > > > testing for the invalid bit, but *why* things are invalid isn't clear. > > > And yet, other code in the surrounding area is doing precisely this and > > > testing for invalid. > > > > > > George is going to take a closer look at this to better understand the > > > nature of the failure and recommit it, for now backing it out to clean > > > up MSan builds. > > > > Hmm, was this reapplied later on? I'm still getting the following > AddressSanitizer failures on FreeBSD, and bisecting has pointed to r290149 > as the cause: > > > > FAIL: AddressSanitizer-Unit :: Asan-i386-inline-Test/AddressS > anitizer.ReallocFreedPointerTest (2124 of 30204) > > TEST 'AddressSanitizer-Unit :: > Asan-i386-inline-Test/AddressSanitizer.ReallocFreedPointerTest' FAILED > > > Note: Google Test filter = AddressSanitizer.ReallocFreedPointerTest > > [==] Running 1 test from 1 test case. > > [--] Global test environment set-up. > > [--] 1 test from AddressSanitizer > > [ RUN ] AddressSanitizer.ReallocFreedPointerTest > > /share/dim/src/llvm/trunk/projects/compiler-rt/lib/asan/tests/asan_test.cc:377: > Failure > > Death test: ptr = realloc(ptr, 77) > > Result: failed to die. > > Error msg: > > [ DEATH ] > > [ FAILED ] AddressSanitizer.ReallocFreedPointerTest (30 ms) > > [--] 1 test from AddressSanitizer (30 ms total) > > > > [--] Global test environment tear-down > > [==] 1 test from 1 test case ran. (31 ms total) > > [ PASSED ] 0 tests. > > [ FAILED ] 1 test, listed below: > > [ FAILED ] AddressSanitizer.ReallocFreedPointerTest > > > > 1 FAILED TEST > > YOU HAVE 24 DISABLED TESTS > > > > > > > > Testing: 0 . > > FAIL: AddressSanitizer-Unit :: Asan-i386-with-calls-Test/Addr > essSanitizer.ReallocFreedPointerTest (2233 of 30204)
[PATCH] D28048: [OpenCL] Align fake address space map with the SPIR target maps.
pekka.jaaskelainen accepted this revision. pekka.jaaskelainen added a reviewer: pekka.jaaskelainen. pekka.jaaskelainen added a comment. This revision is now accepted and ready to land. In https://reviews.llvm.org/D28048#629569, @Anastasia wrote: > @pekka.jaaskelainen, I think you are using x86 target in the POCL toolchain, > do you envision any issues with this change? No, this should simplify things actually. https://reviews.llvm.org/D28048 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28047: Remove the '-disable-llvm-passes' flag (which I didn't even know existed, and I suspect many others aren't aware of either) and strength '-disable-llvm-optzns' to do the same thing.
mehdi_amini accepted this revision. mehdi_amini added a comment. This revision is now accepted and ready to land. In https://reviews.llvm.org/D28047#629922, @chandlerc wrote: > In https://reviews.llvm.org/D28047#629892, @mehdi_amini wrote: > > > LGTM, but please wait for @rnk to confirm :) > > Also any opinion about a driver option -emit-raw-llvm as @joerg suggested? > > > I like the idea generally, but I don't have an immediate use case and would > defer that to a subsequent patch authored by someone who wants this? I really > only wanted to remove the divergence between two flags here. Sure, even @joerg mentioned it was fine to leave for later in his suggestion, I just didn't want it to go unnoticed. https://reviews.llvm.org/D28047 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r290390 - Fix example: byref struct's init was incorrect, and the block literal's holder should point to it.
Author: jroelofs Date: Thu Dec 22 17:48:23 2016 New Revision: 290390 URL: http://llvm.org/viewvc/llvm-project?rev=290390&view=rev Log: Fix example: byref struct's init was incorrect, and the block literal's holder should point to it. Modified: cfe/trunk/docs/Block-ABI-Apple.rst Modified: cfe/trunk/docs/Block-ABI-Apple.rst URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/Block-ABI-Apple.rst?rev=290390&r1=290389&r2=290390&view=diff == --- cfe/trunk/docs/Block-ABI-Apple.rst (original) +++ cfe/trunk/docs/Block-ABI-Apple.rst Thu Dec 22 17:48:23 2016 @@ -530,13 +530,13 @@ and: .. code-block:: c -struct _block_byref_i i = {( .forwarding=&i, .flags=0, .size=sizeof(struct _block_byref_i) )}; +struct _block_byref_i i = {( .isa=NULL, .forwarding=&i, .flags=0, .size=sizeof(struct _block_byref_i), .captured_i=2 )}; struct __block_literal_5 _block_literal = { &_NSConcreteStackBlock, (1<<25)|(1<<29), , __block_invoke_5, &__block_descriptor_5, - 2, + &i, }; Importing ``__attribute__((NSObject))`` ``__block`` variables ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r290392 - Make '-disable-llvm-optzns' an alias for '-disable-llvm-passes'.
Author: chandlerc Date: Thu Dec 22 18:23:01 2016 New Revision: 290392 URL: http://llvm.org/viewvc/llvm-project?rev=290392&view=rev Log: Make '-disable-llvm-optzns' an alias for '-disable-llvm-passes'. Much to my surprise, '-disable-llvm-optzns' which I thought was the magical flag I wanted to get at the raw LLVM IR coming out of Clang deosn't do that. It still runs some passes over the IR. I don't want that, I really want the *raw* IR coming out of Clang and I strongly suspect everyone else using it is in the same camp. There is actually a flag that does what I want that I didn't know about called '-disable-llvm-passes'. I suspect many others don't know about it either. It both does what I want and is much simpler. This removes the confusing version and makes that spelling of the flag an alias for '-disable-llvm-passes'. I've also moved everything in Clang to use the 'passes' spelling as it seems both more accurate (*all* LLVM passes are disabled, not just optimizations) and much easier to remember and spell correctly. This is part of simplifying how Clang drives LLVM to make it cleaner to wire up to the new pass manager. Differential Revision: https://reviews.llvm.org/D28047 Modified: cfe/trunk/include/clang/Driver/CC1Options.td cfe/trunk/include/clang/Frontend/CodeGenOptions.def cfe/trunk/lib/CodeGen/BackendUtil.cpp cfe/trunk/lib/Driver/Tools.cpp cfe/trunk/lib/Frontend/CompilerInvocation.cpp cfe/trunk/test/CXX/drs/dr158.cpp cfe/trunk/test/CXX/temp/temp.spec/temp.explicit/p9-linkage.cpp cfe/trunk/test/CodeGen/always_inline.c cfe/trunk/test/CodeGen/attr-minsize.cpp cfe/trunk/test/CodeGen/bool_test.c cfe/trunk/test/CodeGen/builtin-expect.c cfe/trunk/test/CodeGen/builtin-unpredictable.c cfe/trunk/test/CodeGen/fixup-depth-overflow.c cfe/trunk/test/CodeGen/function-attributes.c cfe/trunk/test/CodeGen/inline.c cfe/trunk/test/CodeGen/may-alias.c cfe/trunk/test/CodeGen/tbaa-class.cpp cfe/trunk/test/CodeGen/tbaa-ms-abi.cpp cfe/trunk/test/CodeGen/tbaa.cpp cfe/trunk/test/CodeGenCXX/PR26569.cpp cfe/trunk/test/CodeGenCXX/ctor-dtor-alias.cpp cfe/trunk/test/CodeGenCXX/cxx1y-variable-template-linkage.cpp cfe/trunk/test/CodeGenCXX/destructors.cpp cfe/trunk/test/CodeGenCXX/dllexport.cpp cfe/trunk/test/CodeGenCXX/dllimport-rtti.cpp cfe/trunk/test/CodeGenCXX/dllimport.cpp cfe/trunk/test/CodeGenCXX/exceptions-seh.cpp cfe/trunk/test/CodeGenCXX/explicit-instantiation.cpp cfe/trunk/test/CodeGenCXX/inline-hint.cpp cfe/trunk/test/CodeGenCXX/invariant.group-for-vptrs.cpp cfe/trunk/test/CodeGenCXX/linkage.cpp cfe/trunk/test/CodeGenCXX/microsoft-abi-eh-catch.cpp cfe/trunk/test/CodeGenCXX/microsoft-abi-eh-cleanups.cpp cfe/trunk/test/CodeGenCXX/microsoft-abi-extern-template.cpp cfe/trunk/test/CodeGenCXX/microsoft-abi-structors-alias.cpp cfe/trunk/test/CodeGenCXX/microsoft-abi-vftables.cpp cfe/trunk/test/CodeGenCXX/pr24097.cpp cfe/trunk/test/CodeGenCXX/sanitize-dtor-bit-field.cpp cfe/trunk/test/CodeGenCXX/sanitize-dtor-derived-class.cpp cfe/trunk/test/CodeGenCXX/sanitize-dtor-tail-call.cpp cfe/trunk/test/CodeGenCXX/sanitize-dtor-trivial.cpp cfe/trunk/test/CodeGenCXX/sanitize-dtor-vtable.cpp cfe/trunk/test/CodeGenCXX/stack-reuse-miscompile.cpp cfe/trunk/test/CodeGenCXX/strict-vtable-pointers.cpp cfe/trunk/test/CodeGenCXX/template-instantiation.cpp cfe/trunk/test/CodeGenCXX/thunks.cpp cfe/trunk/test/CodeGenCXX/virtual-destructor-calls.cpp cfe/trunk/test/CodeGenCXX/visibility-inlines-hidden.cpp cfe/trunk/test/CodeGenCXX/vtable-assume-load.cpp cfe/trunk/test/CodeGenCXX/vtable-available-externally.cpp cfe/trunk/test/CodeGenCXX/vtable-linkage.cpp cfe/trunk/test/CodeGenObjC/arc-blocks.m cfe/trunk/test/CodeGenObjC/arc-bridged-cast.m cfe/trunk/test/CodeGenObjC/arc-literals.m cfe/trunk/test/CodeGenObjC/arc-no-arc-exceptions.m cfe/trunk/test/CodeGenObjC/arc-precise-lifetime.m cfe/trunk/test/CodeGenObjC/arc-ternary-op.m cfe/trunk/test/CodeGenObjC/arc-unsafeclaim.m cfe/trunk/test/CodeGenObjC/arc.m cfe/trunk/test/CodeGenObjC/nsvalue-objc-boxable-ios-arc.m cfe/trunk/test/CodeGenObjC/nsvalue-objc-boxable-ios.m cfe/trunk/test/CodeGenObjC/nsvalue-objc-boxable-mac-arc.m cfe/trunk/test/CodeGenObjC/nsvalue-objc-boxable-mac.m cfe/trunk/test/CodeGenObjCXX/arc-globals.mm cfe/trunk/test/CodeGenObjCXX/arc-move.mm cfe/trunk/test/CodeGenObjCXX/arc-new-delete.mm cfe/trunk/test/CodeGenObjCXX/arc-references.mm cfe/trunk/test/CodeGenObjCXX/arc.mm cfe/trunk/test/CodeGenObjCXX/destroy.mm cfe/trunk/test/CodeGenObjCXX/literals.mm cfe/trunk/test/Driver/cc1-response-files.c cfe/trunk/test/Driver/cl-options.c cfe/trunk/test/Modules/cxx-irgen.cpp cfe/trunk/test/OpenMP/declare_reduction_codegen.c cfe/trunk/test/OpenMP/declare_reduction_codegen.cpp cfe/trunk/test/Profile/f
[PATCH] D28047: Remove the '-disable-llvm-passes' flag (which I didn't even know existed, and I suspect many others aren't aware of either) and strength '-disable-llvm-optzns' to do the same thing.
This revision was automatically updated to reflect the committed changes. Closed by commit rL290392: Make '-disable-llvm-optzns' an alias for '-disable-llvm-passes'. (authored by chandlerc). Changed prior to commit: https://reviews.llvm.org/D28047?vs=82313&id=82383#toc Repository: rL LLVM https://reviews.llvm.org/D28047 Files: cfe/trunk/include/clang/Driver/CC1Options.td cfe/trunk/include/clang/Frontend/CodeGenOptions.def cfe/trunk/lib/CodeGen/BackendUtil.cpp cfe/trunk/lib/Driver/Tools.cpp cfe/trunk/lib/Frontend/CompilerInvocation.cpp cfe/trunk/test/CXX/drs/dr158.cpp cfe/trunk/test/CXX/temp/temp.spec/temp.explicit/p9-linkage.cpp cfe/trunk/test/CodeGen/always_inline.c cfe/trunk/test/CodeGen/attr-minsize.cpp cfe/trunk/test/CodeGen/bool_test.c cfe/trunk/test/CodeGen/builtin-expect.c cfe/trunk/test/CodeGen/builtin-unpredictable.c cfe/trunk/test/CodeGen/fixup-depth-overflow.c cfe/trunk/test/CodeGen/function-attributes.c cfe/trunk/test/CodeGen/inline.c cfe/trunk/test/CodeGen/may-alias.c cfe/trunk/test/CodeGen/tbaa-class.cpp cfe/trunk/test/CodeGen/tbaa-ms-abi.cpp cfe/trunk/test/CodeGen/tbaa.cpp cfe/trunk/test/CodeGenCXX/PR26569.cpp cfe/trunk/test/CodeGenCXX/ctor-dtor-alias.cpp cfe/trunk/test/CodeGenCXX/cxx1y-variable-template-linkage.cpp cfe/trunk/test/CodeGenCXX/destructors.cpp cfe/trunk/test/CodeGenCXX/dllexport.cpp cfe/trunk/test/CodeGenCXX/dllimport-rtti.cpp cfe/trunk/test/CodeGenCXX/dllimport.cpp cfe/trunk/test/CodeGenCXX/exceptions-seh.cpp cfe/trunk/test/CodeGenCXX/explicit-instantiation.cpp cfe/trunk/test/CodeGenCXX/inline-hint.cpp cfe/trunk/test/CodeGenCXX/invariant.group-for-vptrs.cpp cfe/trunk/test/CodeGenCXX/linkage.cpp cfe/trunk/test/CodeGenCXX/microsoft-abi-eh-catch.cpp cfe/trunk/test/CodeGenCXX/microsoft-abi-eh-cleanups.cpp cfe/trunk/test/CodeGenCXX/microsoft-abi-extern-template.cpp cfe/trunk/test/CodeGenCXX/microsoft-abi-structors-alias.cpp cfe/trunk/test/CodeGenCXX/microsoft-abi-vftables.cpp cfe/trunk/test/CodeGenCXX/pr24097.cpp cfe/trunk/test/CodeGenCXX/sanitize-dtor-bit-field.cpp cfe/trunk/test/CodeGenCXX/sanitize-dtor-derived-class.cpp cfe/trunk/test/CodeGenCXX/sanitize-dtor-tail-call.cpp cfe/trunk/test/CodeGenCXX/sanitize-dtor-trivial.cpp cfe/trunk/test/CodeGenCXX/sanitize-dtor-vtable.cpp cfe/trunk/test/CodeGenCXX/stack-reuse-miscompile.cpp cfe/trunk/test/CodeGenCXX/strict-vtable-pointers.cpp cfe/trunk/test/CodeGenCXX/template-instantiation.cpp cfe/trunk/test/CodeGenCXX/thunks.cpp cfe/trunk/test/CodeGenCXX/virtual-destructor-calls.cpp cfe/trunk/test/CodeGenCXX/visibility-inlines-hidden.cpp cfe/trunk/test/CodeGenCXX/vtable-assume-load.cpp cfe/trunk/test/CodeGenCXX/vtable-available-externally.cpp cfe/trunk/test/CodeGenCXX/vtable-linkage.cpp cfe/trunk/test/CodeGenObjC/arc-blocks.m cfe/trunk/test/CodeGenObjC/arc-bridged-cast.m cfe/trunk/test/CodeGenObjC/arc-literals.m cfe/trunk/test/CodeGenObjC/arc-no-arc-exceptions.m cfe/trunk/test/CodeGenObjC/arc-precise-lifetime.m cfe/trunk/test/CodeGenObjC/arc-ternary-op.m cfe/trunk/test/CodeGenObjC/arc-unsafeclaim.m cfe/trunk/test/CodeGenObjC/arc.m cfe/trunk/test/CodeGenObjC/nsvalue-objc-boxable-ios-arc.m cfe/trunk/test/CodeGenObjC/nsvalue-objc-boxable-ios.m cfe/trunk/test/CodeGenObjC/nsvalue-objc-boxable-mac-arc.m cfe/trunk/test/CodeGenObjC/nsvalue-objc-boxable-mac.m cfe/trunk/test/CodeGenObjCXX/arc-globals.mm cfe/trunk/test/CodeGenObjCXX/arc-move.mm cfe/trunk/test/CodeGenObjCXX/arc-new-delete.mm cfe/trunk/test/CodeGenObjCXX/arc-references.mm cfe/trunk/test/CodeGenObjCXX/arc.mm cfe/trunk/test/CodeGenObjCXX/destroy.mm cfe/trunk/test/CodeGenObjCXX/literals.mm cfe/trunk/test/Driver/cc1-response-files.c cfe/trunk/test/Driver/cl-options.c cfe/trunk/test/Modules/cxx-irgen.cpp cfe/trunk/test/OpenMP/declare_reduction_codegen.c cfe/trunk/test/OpenMP/declare_reduction_codegen.cpp cfe/trunk/test/Profile/func-entry.c cfe/trunk/test/Profile/gcc-flag-compatibility.c cfe/trunk/test/Profile/profile-summary.c Index: cfe/trunk/include/clang/Driver/CC1Options.td === --- cfe/trunk/include/clang/Driver/CC1Options.td +++ cfe/trunk/include/clang/Driver/CC1Options.td @@ -160,13 +160,13 @@ HelpText<"Disable implicit builtin knowledge of math functions">; } -def disable_llvm_optzns : Flag<["-"], "disable-llvm-optzns">, - HelpText<"Don't run LLVM optimization passes">; def disable_llvm_verifier : Flag<["-"], "disable-llvm-verifier">, HelpText<"Don't run the LLVM IR verifier pass">; def disable_llvm_passes : Flag<["-"], "disable-llvm-passes">, HelpText<"Use together with -emit-llvm to get pristine LLVM IR from the " "frontend by not running any LLVM passes at all">; +def disable_llvm_optzns : Flag<["-"], "disable-llvm-optzns">, + Alias; def disable_red_zone : Flag<["-"], "disable-red-zone">, HelpText<"Do not emit code that uses
Re: r290169 - Revert r290149: Add the alloc_size attribute to clang.
It looks like the root of this is that we're treating calls to `allocsize` functions as AllocLike (e.g. any allocation function type except realloc) functions, which caused us to perform invalid optimizations. For example, in ReallocFreedPointerTest, EarlyCSE DCE'd the realloc because llvm::isInstructionTriviallyDead calls llvm::isAllocLikeFn, and isAllocLikeFn would return true if it saw the allocsize attribute. It really shouldn't do that. r290397 should fix this behavior by making allocsize alone insufficient to consider a function an allocation function. Thanks for your help! On Thu, Dec 22, 2016 at 1:10 PM, George Burgess IV < george.burgess...@gmail.com> wrote: > Okay, I'm seeing this failure now if I tag my system's `realloc` > declaration with `alloc_size`. (Which FreeBSD seems to do in their > headers). Because all that clang does with `alloc_size` is use it to answer > `__builtin_object_size` queries and lower it to LLVM's `allocsize` > attribute, this is presumably a latent bug in LLVM's `allocsize` attribute. > > Let me mess around for a bit and see what I can dig up. :) > > On Thu, Dec 22, 2016 at 11:59 AM, Dimitry Andric > wrote: > >> This is when running "ninja check-all", in a tree with llvm, clang and >> compiler-rt checked out. The first program that shows a failure is >> projects/compiler-rt/lib/asan/tests/default/Asan-i386-inline-Test: >> >> [==] Running 92 tests from 3 test cases. >> [--] Global test environment set-up. >> [--] 14 tests from AddressSanitizerInterface >> ... >> [ RUN ] AddressSanitizer.ReallocFreedPointerTest >> /share/dim/src/llvm/trunk/projects/compiler-rt/lib/asan/tests/asan_test.cc:377: >> Failure >> Death test: ptr = realloc(ptr, 77) >> Result: failed to die. >> Error msg: >> [ DEATH ] >> [ FAILED ] AddressSanitizer.ReallocFreedPointerTest (48 ms) >> >> A similar failure shows when running projects/compiler-rt/lib/asan/ >> tests/default/Asan-i386-with-calls-Test: >> >> [==] Running 92 tests from 3 test cases. >> [--] Global test environment set-up. >> [--] 14 tests from AddressSanitizerInterface >> ... >> [ RUN ] AddressSanitizer.ReallocFreedPointerTest >> /share/dim/src/llvm/trunk/projects/compiler-rt/lib/asan/tests/asan_test.cc:377: >> Failure >> Death test: ptr = realloc(ptr, 77) >> Result: failed to die. >> Error msg: >> [ DEATH ] >> [ FAILED ] AddressSanitizer.ReallocFreedPointerTest (55 ms) >> >> Interestingly, the Asan-i386-inline-Noinst-Test and >> Asan-i386-with-calls-Noinst-Test do not show this particular failure. >> >> The other test that fails is projects/compiler-rt/test/asan >> /I386FreeBSDConfig/TestCases/Posix/Output/free_hook_realloc.cc.tmp, >> which simply returns 1 without printing any output. Debugging the program >> shows that it seems to be skipping completely over the realloc() call, and >> jumping directly to the _exit(1), but this may be due to optimization. >> >> -Dimitry >> >> > On 22 Dec 2016, at 20:27, George Burgess IV < >> george.burgess...@gmail.com> wrote: >> > >> > Yes, this was reapplied in r290297 with fixes for the msan issue we >> caught; these asan unit test failures are news to me. Can you give me the >> command that you're using to run these tests, please? >> > >> > On Thu, Dec 22, 2016 at 11:10 AM, Dimitry Andric >> wrote: >> > On 20 Dec 2016, at 09:28, Chandler Carruth via cfe-commits < >> cfe-commits@lists.llvm.org> wrote: >> > > Author: chandlerc >> > > Date: Tue Dec 20 02:28:19 2016 >> > > New Revision: 290169 >> > > >> > > URL: http://llvm.org/viewvc/llvm-project?rev=290169&view=rev >> > > Log: >> > > Revert r290149: Add the alloc_size attribute to clang. >> > > >> > > This commit fails MSan when running test/CodeGen/object-size.c in >> > > a confusing way. After some discussion with George, it isn't really >> > > clear what is going on here. We can make the MSan failure go away by >> > > testing for the invalid bit, but *why* things are invalid isn't clear. >> > > And yet, other code in the surrounding area is doing precisely this >> and >> > > testing for invalid. >> > > >> > > George is going to take a closer look at this to better understand the >> > > nature of the failure and recommit it, for now backing it out to clean >> > > up MSan builds. >> > >> > Hmm, was this reapplied later on? I'm still getting the following >> AddressSanitizer failures on FreeBSD, and bisecting has pointed to r290149 >> as the cause: >> > >> > FAIL: AddressSanitizer-Unit :: Asan-i386-inline-Test/AddressS >> anitizer.ReallocFreedPointerTest (2124 of 30204) >> > TEST 'AddressSanitizer-Unit :: >> Asan-i386-inline-Test/AddressSanitizer.ReallocFreedPointerTest' FAILED >> >> > Note: Google Test filter = AddressSanitizer.ReallocFreedPointerTest >> > [==] Running 1 test from 1 test case. >> > [--] Global test environment set-up. >> > [--] 1 test from AddressSanitizer >> > [ RUN ] AddressSa
r290398 - Cleanup the handling of noinline function attributes, -fno-inline,
Author: chandlerc Date: Thu Dec 22 19:24:49 2016 New Revision: 290398 URL: http://llvm.org/viewvc/llvm-project?rev=290398&view=rev Log: Cleanup the handling of noinline function attributes, -fno-inline, -fno-inline-functions, -O0, and optnone. These were really, really tangled together: - We used the noinline LLVM attribute for -fno-inline - But not for -fno-inline-functions (breaking LTO) - But we did use it for -finline-hint-functions (yay, LTO is happy!) - But we didn't for -O0 (LTO is sad yet again...) - We had weird structuring of CodeGenOpts with both an inlining enumeration and a boolean. They interacted in weird ways and needlessly. - A *lot* of set smashing went on with setting these, and then got worse when we considered optnone and other inlining-effecting attributes. - A bunch of inline affecting attributes were managed in a completely different place from -fno-inline. - Even with -fno-inline we failed to put the LLVM noinline attribute onto many generated function definitions because they didn't show up as AST-level functions. - If you passed -O0 but -finline-functions we would run the normal inliner pass in LLVM despite it being in the O0 pipeline, which really doesn't make much sense. - Lastly, we used things like '-fno-inline' to manipulate the pass pipeline which forced the pass pipeline to be much more parameterizable than it really needs to be. Instead we can *just* use the optimization level to select a pipeline and control the rest via attributes. Sadly, this causes a bunch of churn in tests because we don't run the optimizer in the tests and check the contents of attribute sets. It would be awesome if attribute sets were a bit more FileCheck friendly, but oh well. I think this is a significant improvement and should remove the semantic need to change what inliner pass we run in order to comply with the requested inlining semantics by relying completely on attributes. It also cleans up tho optnone and related handling a bit. One unfortunate aspect of this is that for generating alwaysinline routines like those in OpenMP we end up removing noinline and then adding alwaysinline. I tried a bunch of other approaches, but because we recompute function attributes from scratch and don't have a declaration here I couldn't find anything substantially cleaner than this. Differential Revision: https://reviews.llvm.org/D28053 Modified: cfe/trunk/include/clang/Frontend/CodeGenOptions.def cfe/trunk/include/clang/Frontend/CodeGenOptions.h cfe/trunk/lib/CodeGen/BackendUtil.cpp cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp cfe/trunk/lib/CodeGen/CodeGenFunction.cpp cfe/trunk/lib/CodeGen/CodeGenModule.cpp cfe/trunk/lib/Frontend/CompilerInvocation.cpp cfe/trunk/test/CXX/special/class.dtor/p3-0x.cpp cfe/trunk/test/CodeGen/2008-04-08-NoExceptions.c cfe/trunk/test/CodeGen/address-safety-attr-kasan.cpp cfe/trunk/test/CodeGen/address-safety-attr.cpp cfe/trunk/test/CodeGen/address-space-field1.c cfe/trunk/test/CodeGen/alias.c cfe/trunk/test/CodeGen/attr-minsize.cpp cfe/trunk/test/CodeGen/attributes.c cfe/trunk/test/CodeGen/incomplete-function-type-2.c cfe/trunk/test/CodeGen/inline-optim.c cfe/trunk/test/CodeGen/mips16-attr.c cfe/trunk/test/CodeGen/mrtd.c cfe/trunk/test/CodeGen/ms-declspecs.c cfe/trunk/test/CodeGen/ppc64-complex-parms.c cfe/trunk/test/CodeGen/ppc64-complex-return.c cfe/trunk/test/CodeGen/ppc64-extend.c cfe/trunk/test/CodeGen/sanitize-thread-attr.cpp cfe/trunk/test/CodeGen/sanitize-thread-no-checking-at-run-time.m cfe/trunk/test/CodeGen/unwind-attr.c cfe/trunk/test/CodeGenCXX/attr.cpp cfe/trunk/test/CodeGenCXX/cxx11-exception-spec.cpp cfe/trunk/test/CodeGenCXX/cxx11-noreturn.cpp cfe/trunk/test/CodeGenCXX/derived-to-base.cpp cfe/trunk/test/CodeGenCXX/global-dtor-no-atexit.cpp cfe/trunk/test/CodeGenCXX/global-init.cpp cfe/trunk/test/CodeGenCXX/inline-hint.cpp cfe/trunk/test/CodeGenCXX/main-norecurse.cpp cfe/trunk/test/CodeGenCXX/microsoft-abi-array-cookies.cpp cfe/trunk/test/CodeGenCXX/no-exceptions.cpp cfe/trunk/test/CodeGenCXX/optnone-class-members.cpp cfe/trunk/test/CodeGenCXX/optnone-def-decl.cpp cfe/trunk/test/CodeGenCXX/reference-cast.cpp cfe/trunk/test/CodeGenCXX/threadsafe-statics.cpp cfe/trunk/test/CodeGenCXX/thunks.cpp cfe/trunk/test/CodeGenCXX/virtual-base-cast.cpp cfe/trunk/test/CodeGenObjC/gnu-exceptions.m cfe/trunk/test/CodeGenObjC/objc-literal-tests.m cfe/trunk/test/CodeGenObjCXX/lambda-expressions.mm cfe/trunk/test/CodeGenOpenCL/amdgpu-attrs.cl cfe/trunk/test/Driver/darwin-iphone-defaults.m cfe/trunk/test/PCH/objc_container.m Modified: cfe/trunk/include/clang/Frontend/CodeGenOptions.def URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CodeGenOptions.def?rev=290398&r1=290397&r2=290398&v
[PATCH] D28053: Cleanup the handling of noinline function attributes, -fno-inline, -fno-inline-functions, -O0, and optnone.
This revision was automatically updated to reflect the committed changes. Closed by commit rL290398: Cleanup the handling of noinline function attributes, -fno-inline, (authored by chandlerc). Changed prior to commit: https://reviews.llvm.org/D28053?vs=82356&id=82390#toc Repository: rL LLVM https://reviews.llvm.org/D28053 Files: cfe/trunk/include/clang/Frontend/CodeGenOptions.def cfe/trunk/include/clang/Frontend/CodeGenOptions.h cfe/trunk/lib/CodeGen/BackendUtil.cpp cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp cfe/trunk/lib/CodeGen/CodeGenFunction.cpp cfe/trunk/lib/CodeGen/CodeGenModule.cpp cfe/trunk/lib/Frontend/CompilerInvocation.cpp cfe/trunk/test/CXX/special/class.dtor/p3-0x.cpp cfe/trunk/test/CodeGen/2008-04-08-NoExceptions.c cfe/trunk/test/CodeGen/address-safety-attr-kasan.cpp cfe/trunk/test/CodeGen/address-safety-attr.cpp cfe/trunk/test/CodeGen/address-space-field1.c cfe/trunk/test/CodeGen/alias.c cfe/trunk/test/CodeGen/attr-minsize.cpp cfe/trunk/test/CodeGen/attributes.c cfe/trunk/test/CodeGen/incomplete-function-type-2.c cfe/trunk/test/CodeGen/inline-optim.c cfe/trunk/test/CodeGen/mips16-attr.c cfe/trunk/test/CodeGen/mrtd.c cfe/trunk/test/CodeGen/ms-declspecs.c cfe/trunk/test/CodeGen/ppc64-complex-parms.c cfe/trunk/test/CodeGen/ppc64-complex-return.c cfe/trunk/test/CodeGen/ppc64-extend.c cfe/trunk/test/CodeGen/sanitize-thread-attr.cpp cfe/trunk/test/CodeGen/sanitize-thread-no-checking-at-run-time.m cfe/trunk/test/CodeGen/unwind-attr.c cfe/trunk/test/CodeGenCXX/attr.cpp cfe/trunk/test/CodeGenCXX/cxx11-exception-spec.cpp cfe/trunk/test/CodeGenCXX/cxx11-noreturn.cpp cfe/trunk/test/CodeGenCXX/derived-to-base.cpp cfe/trunk/test/CodeGenCXX/global-dtor-no-atexit.cpp cfe/trunk/test/CodeGenCXX/global-init.cpp cfe/trunk/test/CodeGenCXX/inline-hint.cpp cfe/trunk/test/CodeGenCXX/main-norecurse.cpp cfe/trunk/test/CodeGenCXX/microsoft-abi-array-cookies.cpp cfe/trunk/test/CodeGenCXX/no-exceptions.cpp cfe/trunk/test/CodeGenCXX/optnone-class-members.cpp cfe/trunk/test/CodeGenCXX/optnone-def-decl.cpp cfe/trunk/test/CodeGenCXX/reference-cast.cpp cfe/trunk/test/CodeGenCXX/threadsafe-statics.cpp cfe/trunk/test/CodeGenCXX/thunks.cpp cfe/trunk/test/CodeGenCXX/virtual-base-cast.cpp cfe/trunk/test/CodeGenObjC/gnu-exceptions.m cfe/trunk/test/CodeGenObjC/objc-literal-tests.m cfe/trunk/test/CodeGenObjCXX/lambda-expressions.mm cfe/trunk/test/CodeGenOpenCL/amdgpu-attrs.cl cfe/trunk/test/Driver/darwin-iphone-defaults.m cfe/trunk/test/PCH/objc_container.m Index: cfe/trunk/include/clang/Frontend/CodeGenOptions.def === --- cfe/trunk/include/clang/Frontend/CodeGenOptions.def +++ cfe/trunk/include/clang/Frontend/CodeGenOptions.def @@ -104,8 +104,6 @@ CODEGENOPT(NoSignedZeros , 1, 0) ///< Allow ignoring the signedness of FP zero CODEGENOPT(ReciprocalMath, 1, 0) ///< Allow FP divisions to be reassociated. CODEGENOPT(NoTrappingMath, 1, 0) ///< Set when -fno-trapping-math is enabled. -CODEGENOPT(NoInline , 1, 0) ///< Set when -fno-inline is enabled. - ///< Disables use of the inline keyword. CODEGENOPT(NoNaNsFPMath , 1, 0) ///< Assume FP arguments, results not NaN. CODEGENOPT(FlushDenorm , 1, 0) ///< Allow FP denorm numbers to be flushed to zero CODEGENOPT(CorrectlyRoundedDivSqrt, 1, 0) ///< -cl-fp32-correctly-rounded-divide-sqrt @@ -233,7 +231,7 @@ CODEGENOPT(EmitCodeView, 1, 0) /// The kind of inlining to perform. -ENUM_CODEGENOPT(Inlining, InliningMethod, 2, NoInlining) +ENUM_CODEGENOPT(Inlining, InliningMethod, 2, NormalInlining) // Vector functions library to use. ENUM_CODEGENOPT(VecLib, VectorLibrary, 2, NoLibrary) Index: cfe/trunk/include/clang/Frontend/CodeGenOptions.h === --- cfe/trunk/include/clang/Frontend/CodeGenOptions.h +++ cfe/trunk/include/clang/Frontend/CodeGenOptions.h @@ -44,7 +44,6 @@ class CodeGenOptions : public CodeGenOptionsBase { public: enum InliningMethod { -NoInlining, // Perform no inlining whatsoever. NormalInlining, // Use the standard function inlining pass. OnlyHintInlining, // Inline only (implicitly) hinted functions. OnlyAlwaysInlining // Only run the always inlining pass. Index: cfe/trunk/test/Driver/darwin-iphone-defaults.m === --- cfe/trunk/test/Driver/darwin-iphone-defaults.m +++ cfe/trunk/test/Driver/darwin-iphone-defaults.m @@ -26,4 +26,4 @@ [I1 alloc]; } -// CHECK: attributes [[F0]] = { ssp{{.*}} } +// CHECK: attributes [[F0]] = { noinline ssp{{.*}} } Index: cfe/trunk/test/CodeGenObjCXX/lambda-expressions.mm === --- cfe/trunk/test/CodeGenObjCXX/lambda-expressions.mm +++
r290399 - When merging two deduced non-type template arguments for the same parameter,
Author: rsmith Date: Thu Dec 22 19:30:39 2016 New Revision: 290399 URL: http://llvm.org/viewvc/llvm-project?rev=290399&view=rev Log: When merging two deduced non-type template arguments for the same parameter, fail the merge if the arguments have different types (except if one of them was deduced from an array bound, in which case take the type from the other). This is correct because (except in the array bound case) the type of the template argument in each deduction must match the type of the parameter, so at least one of the two deduced arguments must have a mismatched type. This is necessary because we would otherwise lose the type information for the discarded template argument in the merge, and fail to diagnose the mismatch. In order to power this, we now properly retain the type of a deduced non-type template argument deduced from a declaration, rather than giving it the type of the template parameter; we'll convert it to the template parameter type when checking the deduced arguments. Modified: cfe/trunk/include/clang/AST/TemplateBase.h cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/lib/AST/TemplateBase.cpp cfe/trunk/lib/Sema/SemaOverload.cpp cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp cfe/trunk/test/CXX/temp/temp.fct.spec/temp.deduct/p9.cpp cfe/trunk/test/SemaTemplate/deduction.cpp Modified: cfe/trunk/include/clang/AST/TemplateBase.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/TemplateBase.h?rev=290399&r1=290398&r2=290399&view=diff == --- cfe/trunk/include/clang/AST/TemplateBase.h (original) +++ cfe/trunk/include/clang/AST/TemplateBase.h Thu Dec 22 19:30:39 2016 @@ -301,6 +301,10 @@ public: Integer.Type = T.getAsOpaquePtr(); } + /// \brief If this is a non-type template argument, get its type. Otherwise, + /// returns a null QualType. + QualType getNonTypeTemplateArgumentType() const; + /// \brief Retrieve the template argument as an expression. Expr *getAsExpr() const { assert(getKind() == Expression && "Unexpected kind"); Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=290399&r1=290398&r2=290399&view=diff == --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Dec 22 19:30:39 2016 @@ -3343,6 +3343,10 @@ def note_ovl_candidate_incomplete_deduct def note_ovl_candidate_inconsistent_deduction : Note< "candidate template ignored: deduced conflicting %select{types|values|" "templates}0 for parameter %1%diff{ ($ vs. $)|}2,3">; +def note_ovl_candidate_inconsistent_deduction_types : Note< +"candidate template ignored: deduced values %diff{" +"of conflicting types for parameter %0 (%1 of type $ vs. %3 of type $)|" +"%1 and %3 of conflicting types for parameter %0|}2,4">; def note_ovl_candidate_explicit_arg_mismatch_named : Note< "candidate template ignored: invalid explicitly-specified argument " "for template parameter %0">; @@ -3840,7 +3844,7 @@ def err_non_type_template_parm_type_dedu "non-type template parameter %0 with type %1 has incompatible initializer of type %2">; def err_deduced_non_type_template_arg_type_mismatch : Error< "deduced non-type template argument does not have the same type as the " - "its corresponding template parameter%diff{ ($ vs $)|}0,1">; + "corresponding template parameter%diff{ ($ vs $)|}0,1">; def err_non_type_template_arg_subobject : Error< "non-type template argument refers to subobject '%0'">; def err_non_type_template_arg_addr_label_diff : Error< Modified: cfe/trunk/lib/AST/TemplateBase.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/TemplateBase.cpp?rev=290399&r1=290398&r2=290399&view=diff == --- cfe/trunk/lib/AST/TemplateBase.cpp (original) +++ cfe/trunk/lib/AST/TemplateBase.cpp Thu Dec 22 19:30:39 2016 @@ -243,6 +243,31 @@ Optional TemplateArgument::get return None; } +QualType TemplateArgument::getNonTypeTemplateArgumentType() const { + switch (getKind()) { + case TemplateArgument::Null: + case TemplateArgument::Type: + case TemplateArgument::Template: + case TemplateArgument::TemplateExpansion: + case TemplateArgument::Pack: +return QualType(); + + case TemplateArgument::Integral: +return getIntegralType(); + + case TemplateArgument::Expression: +return getAsExpr()->getType(); + + case TemplateArgument::Declaration: +return getParamTypeForDecl(); + + case TemplateArgument::NullPtr: +return getNullPtrType(); + } + + llvm_unreachable("Invalid TemplateArgument Kind!"); +} + void TemplateArgument::Profile(llvm::FoldingSetNodeID &ID,
r290403 - Only substitute into type of non-type template parameter once, rather than
Author: rsmith Date: Thu Dec 22 20:00:24 2016 New Revision: 290403 URL: http://llvm.org/viewvc/llvm-project?rev=290403&view=rev Log: Only substitute into type of non-type template parameter once, rather than twice, in finalization of template argument deduction. This is a re-commit of r290310 (reverted in r290329); the bug found by the buildbots was fixed in r290399 (we would sometimes build a deduced template argument with a bogus type). Modified: cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp Modified: cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp?rev=290403&r1=290402&r2=290403&view=diff == --- cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp (original) +++ cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp Thu Dec 22 20:00:24 2016 @@ -2072,7 +2072,8 @@ static bool isSameTemplateArg(ASTContext /// /// \param NTTPType For a declaration template argument, the type of /// the non-type template parameter that corresponds to this template -/// argument. +/// argument. Can be null if no type sugar is available to add to the +/// type from the template argument. /// /// \param Loc The source location to use for the resulting template /// argument. @@ -2088,12 +2089,16 @@ Sema::getTrivialTemplateArgumentLoc(cons Arg, Context.getTrivialTypeSourceInfo(Arg.getAsType(), Loc)); case TemplateArgument::Declaration: { +if (NTTPType.isNull()) + NTTPType = Arg.getParamTypeForDecl(); Expr *E = BuildExpressionFromDeclTemplateArgument(Arg, NTTPType, Loc) .getAs(); return TemplateArgumentLoc(TemplateArgument(E), E); } case TemplateArgument::NullPtr: { +if (NTTPType.isNull()) + NTTPType = Arg.getNullPtrType(); Expr *E = BuildExpressionFromDeclTemplateArgument(Arg, NTTPType, Loc) .getAs(); return TemplateArgumentLoc(TemplateArgument(NTTPType, /*isNullPtr*/true), @@ -2144,31 +2149,13 @@ ConvertDeducedTemplateArgument(Sema &S, TemplateDeductionInfo &Info, bool InFunctionTemplate, SmallVectorImpl &Output) { - // First, for a non-type template parameter type that is - // initialized by a declaration, we need the type of the - // corresponding non-type template parameter. - QualType NTTPType; - if (NonTypeTemplateParmDecl *NTTP = - dyn_cast(Param)) { -NTTPType = NTTP->getType(); -if (NTTPType->isDependentType()) { - TemplateArgumentList TemplateArgs(TemplateArgumentList::OnStack, Output); - NTTPType = S.SubstType(NTTPType, - MultiLevelTemplateArgumentList(TemplateArgs), - NTTP->getLocation(), - NTTP->getDeclName()); - if (NTTPType.isNull()) -return true; -} - } - auto ConvertArg = [&](DeducedTemplateArgument Arg, unsigned ArgumentPackIndex) { // Convert the deduced template argument into a template // argument that we can check, almost as if the user had written // the template argument explicitly. TemplateArgumentLoc ArgLoc = -S.getTrivialTemplateArgumentLoc(Arg, NTTPType, Info.getLocation()); +S.getTrivialTemplateArgumentLoc(Arg, QualType(), Info.getLocation()); // Check the template argument, converting it as necessary. return S.CheckTemplateArgument( @@ -2200,22 +2187,28 @@ ConvertDeducedTemplateArgument(Sema &S, } // If the pack is empty, we still need to substitute into the parameter -// itself, in case that substitution fails. For non-type parameters, we did -// this above. For type parameters, no substitution is ever required. -auto *TTP = dyn_cast(Param); -if (TTP && PackedArgsBuilder.empty()) { - // Set up a template instantiation context. +// itself, in case that substitution fails. +if (PackedArgsBuilder.empty()) { LocalInstantiationScope Scope(S); - Sema::InstantiatingTemplate Inst(S, Template->getLocation(), Template, - TTP, Output, - Template->getSourceRange()); - if (Inst.isInvalid()) -return true; - TemplateArgumentList TemplateArgs(TemplateArgumentList::OnStack, Output); - if (!S.SubstDecl(TTP, S.CurContext, - MultiLevelTemplateArgumentList(TemplateArgs))) -return true; + MultiLevelTemplateArgumentList Args(TemplateArgs); + + if (auto *NTTP = dyn_cast(Param)) { +Sema::InstantiatingTemplate Inst(S, Template->getLocation(), Template, + NTTP, Output, + Template->getSourceRange()); +if (Inst.isInvalid() || +S.SubstType(NTTP->getType(), Args, NTTP->getLocation(), +
r290405 - Move generation of injected template arguments for a template parameter list
Author: rsmith Date: Thu Dec 22 20:10:11 2016 New Revision: 290405 URL: http://llvm.org/viewvc/llvm-project?rev=290405&view=rev Log: Move generation of injected template arguments for a template parameter list out of an internal function and into ASTContext; this is needed in template argument deduction for P0522R0. Modified: cfe/trunk/include/clang/AST/ASTContext.h cfe/trunk/lib/AST/ASTContext.cpp cfe/trunk/lib/AST/DeclTemplate.cpp Modified: cfe/trunk/include/clang/AST/ASTContext.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTContext.h?rev=290405&r1=290404&r2=290405&view=diff == --- cfe/trunk/include/clang/AST/ASTContext.h (original) +++ cfe/trunk/include/clang/AST/ASTContext.h Thu Dec 22 20:10:11 2016 @@ -1355,6 +1355,12 @@ public: ElaboratedTypeKeyword Keyword, NestedNameSpecifier *NNS, const IdentifierInfo *Name, ArrayRef Args) const; + /// Get a template argument list with one argument per template parameter + /// in a template parameter list, such as for the injected class name of + /// a class template. + void getInjectedTemplateArgs(const TemplateParameterList *Params, + SmallVectorImpl &Args); + QualType getPackExpansionType(QualType Pattern, Optional NumExpansions); Modified: cfe/trunk/lib/AST/ASTContext.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=290405&r1=290404&r2=290405&view=diff == --- cfe/trunk/lib/AST/ASTContext.cpp (original) +++ cfe/trunk/lib/AST/ASTContext.cpp Thu Dec 22 20:10:11 2016 @@ -3872,6 +3872,44 @@ ASTContext::getDependentTemplateSpeciali return QualType(T, 0); } +void +ASTContext::getInjectedTemplateArgs(const TemplateParameterList *Params, +SmallVectorImpl &Args) { + Args.reserve(Args.size() + Params->size()); + + for (NamedDecl *Param : *Params) { +TemplateArgument Arg; +if (auto *TTP = dyn_cast(Param)) { + QualType ArgType = getTypeDeclType(TTP); + if (TTP->isParameterPack()) +ArgType = getPackExpansionType(ArgType, None); + + Arg = TemplateArgument(ArgType); +} else if (auto *NTTP = dyn_cast(Param)) { + Expr *E = new (*this) DeclRefExpr( + NTTP, /*enclosing*/false, + NTTP->getType().getNonLValueExprType(*this), + Expr::getValueKindForType(NTTP->getType()), NTTP->getLocation()); + + if (NTTP->isParameterPack()) +E = new (*this) PackExpansionExpr(DependentTy, E, NTTP->getLocation(), + None); + Arg = TemplateArgument(E); +} else { + auto *TTP = cast(Param); + if (TTP->isParameterPack()) +Arg = TemplateArgument(TemplateName(TTP), Optional()); + else +Arg = TemplateArgument(TemplateName(TTP)); +} + +if (Param->isTemplateParameterPack()) + Arg = TemplateArgument::CreatePackCopy(*this, Arg); + +Args.push_back(Arg); + } +} + QualType ASTContext::getPackExpansionType(QualType Pattern, Optional NumExpansions) { llvm::FoldingSetNodeID ID; Modified: cfe/trunk/lib/AST/DeclTemplate.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclTemplate.cpp?rev=290405&r1=290404&r2=290405&view=diff == --- cfe/trunk/lib/AST/DeclTemplate.cpp (original) +++ cfe/trunk/lib/AST/DeclTemplate.cpp Thu Dec 22 20:10:11 2016 @@ -204,44 +204,6 @@ void RedeclarableTemplateDecl::addSpecia SETraits::getDecl(Entry)); } -/// \brief Generate the injected template arguments for the given template -/// parameter list, e.g., for the injected-class-name of a class template. -static void GenerateInjectedTemplateArgs(ASTContext &Context, - TemplateParameterList *Params, - TemplateArgument *Args) { - for (NamedDecl *Param : *Params) { -TemplateArgument Arg; -if (auto *TTP = dyn_cast(Param)) { - QualType ArgType = Context.getTypeDeclType(TTP); - if (TTP->isParameterPack()) -ArgType = Context.getPackExpansionType(ArgType, None); - - Arg = TemplateArgument(ArgType); -} else if (auto *NTTP = dyn_cast(Param)) { - Expr *E = new (Context) DeclRefExpr(NTTP, /*enclosing*/ false, - NTTP->getType().getNonLValueExprType(Context), - Expr::getValueKindForType(NTTP->getType()), - NTTP->getLocation()); - - if (NTTP->isParameterPack()) -E = new (Context) PackExpansionExpr(Context.DependentTy, E, -NTTP->getLocation(), None); - Arg = TemplateAr
[PATCH] D25556: [Sema] Add variable captured by a block to the enclosing lambda's potential capture list
ahatanak updated this revision to Diff 82394. ahatanak added a comment. Rebase. r286584 changed getCurLambda to optionally skip CapturedRegionScopeInfos. This patch changes it to skip BlockScopeInfos too. https://reviews.llvm.org/D25556 Files: include/clang/Sema/Sema.h lib/Sema/Sema.cpp lib/Sema/SemaExpr.cpp test/SemaObjCXX/blocks.mm Index: test/SemaObjCXX/blocks.mm === --- test/SemaObjCXX/blocks.mm +++ test/SemaObjCXX/blocks.mm @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -fsyntax-only -verify -fblocks -Wno-objc-root-class %s +// RUN: %clang_cc1 -fsyntax-only -verify -fblocks -Wno-objc-root-class -std=c++14 %s @protocol NSObject; void bar(id(^)(void)); @@ -144,3 +144,14 @@ template void f(X); } + +namespace GenericLambdaCapture { + int test(int outerp) { +auto lambda =[&](auto p) { + return ^{ +return p + outerp; + }(); +}; +return lambda(1); + } +} Index: lib/Sema/SemaExpr.cpp === --- lib/Sema/SemaExpr.cpp +++ lib/Sema/SemaExpr.cpp @@ -14126,8 +14126,9 @@ (SemaRef.CurContext != Var->getDeclContext() && Var->getDeclContext()->isFunctionOrMethod() && Var->hasLocalStorage()); if (RefersToEnclosingScope) { - if (LambdaScopeInfo *const LSI = - SemaRef.getCurLambda(/*IgnoreCapturedRegions=*/true)) { + LambdaScopeInfo *const LSI = + SemaRef.getCurLambda(/*IgnoreNonLambdaCapturingScope=*/true); + if (LSI && !LSI->CallOperator->Encloses(Var->getDeclContext())) { // If a variable could potentially be odr-used, defer marking it so // until we finish analyzing the full expression for any // lvalue-to-rvalue Index: lib/Sema/Sema.cpp === --- lib/Sema/Sema.cpp +++ lib/Sema/Sema.cpp @@ -1243,14 +1243,14 @@ return CurBSI; } -LambdaScopeInfo *Sema::getCurLambda(bool IgnoreCapturedRegions) { +LambdaScopeInfo *Sema::getCurLambda(bool IgnoreNonLambdaCapturingScope) { if (FunctionScopes.empty()) return nullptr; auto I = FunctionScopes.rbegin(); - if (IgnoreCapturedRegions) { + if (IgnoreNonLambdaCapturingScope) { auto E = FunctionScopes.rend(); -while (I != E && isa(*I)) +while (I != E && isa(*I) && !isa(*I)) ++I; if (I == E) return nullptr; Index: include/clang/Sema/Sema.h === --- include/clang/Sema/Sema.h +++ include/clang/Sema/Sema.h @@ -1226,9 +1226,11 @@ sema::BlockScopeInfo *getCurBlock(); /// Retrieve the current lambda scope info, if any. - /// \param IgnoreCapturedRegions true if should find the top-most lambda scope - /// info ignoring all inner captured regions scope infos. - sema::LambdaScopeInfo *getCurLambda(bool IgnoreCapturedRegions = false); + /// \param IgnoreNonLambdaCapturingScope true if should find the top-most + /// lambda scope info ignoring all inner capturing scopes that are not + /// lambda scopes. + sema::LambdaScopeInfo * + getCurLambda(bool IgnoreNonLambdaCapturingScope = false); /// \brief Retrieve the current generic lambda info, if any. sema::LambdaScopeInfo *getCurGenericLambda(); Index: test/SemaObjCXX/blocks.mm === --- test/SemaObjCXX/blocks.mm +++ test/SemaObjCXX/blocks.mm @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -fsyntax-only -verify -fblocks -Wno-objc-root-class %s +// RUN: %clang_cc1 -fsyntax-only -verify -fblocks -Wno-objc-root-class -std=c++14 %s @protocol NSObject; void bar(id(^)(void)); @@ -144,3 +144,14 @@ template void f(X); } + +namespace GenericLambdaCapture { + int test(int outerp) { +auto lambda =[&](auto p) { + return ^{ +return p + outerp; + }(); +}; +return lambda(1); + } +} Index: lib/Sema/SemaExpr.cpp === --- lib/Sema/SemaExpr.cpp +++ lib/Sema/SemaExpr.cpp @@ -14126,8 +14126,9 @@ (SemaRef.CurContext != Var->getDeclContext() && Var->getDeclContext()->isFunctionOrMethod() && Var->hasLocalStorage()); if (RefersToEnclosingScope) { - if (LambdaScopeInfo *const LSI = - SemaRef.getCurLambda(/*IgnoreCapturedRegions=*/true)) { + LambdaScopeInfo *const LSI = + SemaRef.getCurLambda(/*IgnoreNonLambdaCapturingScope=*/true); + if (LSI && !LSI->CallOperator->Encloses(Var->getDeclContext())) { // If a variable could potentially be odr-used, defer marking it so // until we finish analyzing the full expression for any // lvalue-to-rvalue Index: lib/Sema/Sema.cpp === --- lib/Sema/Sema.cpp +++ lib/Sema/Sema.cpp @@ -1243,14 +1243,14 @@ return CurBSI; } -LambdaScopeInfo *Sema::getCurLambda(bool IgnoreCapturedRe
[PATCH] D27680: [CodeGen] Move lifetime.start of a variable when goto jumps back past its declaration
rjmccall added a comment. In https://reviews.llvm.org/D27680#629182, @ahatanak wrote: > In https://reviews.llvm.org/D27680#628272, @rjmccall wrote: > > > Wouldn't it be simpler to just record an insertion point for the beginning > > of the current lexical scope and insert the lifetime.begin there instead of > > at the current IP? > > > I'm not sure I understood your comment, but it seems to me that simply moving > the lifetime.start intrinsics to the current lexical scope wouldn't work in a > case like this: I'm suggesting that, instead of moving instructions retroactively when you see a goto, you just insert the lifetime.start intrinsics at the start of the current lexical scope when you're emitting the variable. That more exactly models the C language rule, I think, and shouldn't have any significant negative impact on optimization. https://reviews.llvm.org/D27680 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r290413 - Fix for PR15623. The patch eliminates unwanted ProgramState checker data propagation from an operand of the logical operation to operation result.
Author: ayartsev Date: Thu Dec 22 21:31:00 2016 New Revision: 290413 URL: http://llvm.org/viewvc/llvm-project?rev=290413&view=rev Log: Fix for PR15623. The patch eliminates unwanted ProgramState checker data propagation from an operand of the logical operation to operation result. The patch also simplifies an assume of a constraint of the form: "(exp comparison_op expr) != 0" to true into an assume of "exp comparison_op expr" to true. (And similarly, an assume of the form "(exp comparison_op expr) == 0" to true as an assume of exp comparison_op expr to false.) which improves precision overall. https://reviews.llvm.org/D22862 Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp cfe/trunk/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp cfe/trunk/test/Analysis/malloc.c cfe/trunk/test/Analysis/misc-ps.c Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp?rev=290413&r1=290412&r2=290413&view=diff == --- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp Thu Dec 22 21:31:00 2016 @@ -665,23 +665,13 @@ void ExprEngine::VisitLogicalExpr(const if (RHSVal.isUndef()) { X = RHSVal; } else { - DefinedOrUnknownSVal DefinedRHS = RHSVal.castAs(); - ProgramStateRef StTrue, StFalse; - std::tie(StTrue, StFalse) = N->getState()->assume(DefinedRHS); - if (StTrue) { -if (StFalse) { - // We can't constrain the value to 0 or 1. - // The best we can do is a cast. - X = getSValBuilder().evalCast(RHSVal, B->getType(), RHS->getType()); -} else { - // The value is known to be true. - X = getSValBuilder().makeIntVal(1, B->getType()); -} - } else { -// The value is known to be false. -assert(StFalse && "Infeasible path!"); -X = getSValBuilder().makeIntVal(0, B->getType()); - } + // We evaluate "RHSVal != 0" expression which result in 0 if the value is + // known to be false, 1 if the value is known to be true and a new symbol + // when the assumption is unknown. + nonloc::ConcreteInt Zero(getBasicVals().getValue(0, B->getType())); + X = evalBinOp(N->getState(), BO_NE, +svalBuilder.evalCast(RHSVal, B->getType(), RHS->getType()), +Zero, B->getType()); } } Bldr.generateNode(B, Pred, state->BindExpr(B, Pred->getLocationContext(), X)); Modified: cfe/trunk/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp?rev=290413&r1=290412&r2=290413&view=diff == --- cfe/trunk/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp Thu Dec 22 21:31:00 2016 @@ -254,6 +254,21 @@ ProgramStateRef SimpleConstraintManager: assert(BinaryOperator::isComparisonOp(Op) && "Non-comparison ops should be rewritten as comparisons to zero."); + SymbolRef Sym = LHS; + + // Simplification: translate an assume of a constraint of the form + // "(exp comparison_op expr) != 0" to true into an assume of + // "exp comparison_op expr" to true. (And similarly, an assume of the form + // "(exp comparison_op expr) == 0" to true into an assume of + // "exp comparison_op expr" to false.) + if (Int == 0 && (Op == BO_EQ || Op == BO_NE)) { +if (const BinarySymExpr *SE = dyn_cast(Sym)) { + BinaryOperator::Opcode Op = SE->getOpcode(); + if (BinaryOperator::isComparisonOp(Op)) +return assume(State, nonloc::SymbolVal(Sym), (Op == BO_NE ? true : false)); +} + } + // Get the type used for calculating wraparound. BasicValueFactory &BVF = getBasicVals(); APSIntType WraparoundType = BVF.getAPSIntType(LHS->getType()); @@ -265,7 +280,6 @@ ProgramStateRef SimpleConstraintManager: // x < 4 has the solution [0, 3]. x+2 < 4 has the solution [0-2, 3-2], which // in modular arithmetic is [0, 1] U [UINT_MAX-1, UINT_MAX]. It's up to // the subclasses of SimpleConstraintManager to handle the adjustment. - SymbolRef Sym = LHS; llvm::APSInt Adjustment = WraparoundType.getZeroValue(); computeAdjustment(Sym, Adjustment); Modified: cfe/trunk/test/Analysis/malloc.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/malloc.c?rev=290413&r1=290412&r2=290413&view=diff == --- cfe/trunk/test/Analysis/malloc.c (original) +++ cfe/trunk/test/Analysis/malloc.c Thu Dec 22 21:31:00 2016 @@ -1763,6 +1763,17 @@ void testConstEscapeThroughAnotherField( constEscape(&(s.x)); // could free s->p! } // no-warning +// PR15623 +in
r290415 - Revert changes made by r290413 until regression is fixed.
Author: ayartsev Date: Thu Dec 22 22:09:18 2016 New Revision: 290415 URL: http://llvm.org/viewvc/llvm-project?rev=290415&view=rev Log: Revert changes made by r290413 until regression is fixed. Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp cfe/trunk/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp cfe/trunk/test/Analysis/malloc.c cfe/trunk/test/Analysis/misc-ps.c Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp?rev=290415&r1=290414&r2=290415&view=diff == --- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp Thu Dec 22 22:09:18 2016 @@ -665,13 +665,23 @@ void ExprEngine::VisitLogicalExpr(const if (RHSVal.isUndef()) { X = RHSVal; } else { - // We evaluate "RHSVal != 0" expression which result in 0 if the value is - // known to be false, 1 if the value is known to be true and a new symbol - // when the assumption is unknown. - nonloc::ConcreteInt Zero(getBasicVals().getValue(0, B->getType())); - X = evalBinOp(N->getState(), BO_NE, -svalBuilder.evalCast(RHSVal, B->getType(), RHS->getType()), -Zero, B->getType()); + DefinedOrUnknownSVal DefinedRHS = RHSVal.castAs(); + ProgramStateRef StTrue, StFalse; + std::tie(StTrue, StFalse) = N->getState()->assume(DefinedRHS); + if (StTrue) { +if (StFalse) { + // We can't constrain the value to 0 or 1. + // The best we can do is a cast. + X = getSValBuilder().evalCast(RHSVal, B->getType(), RHS->getType()); +} else { + // The value is known to be true. + X = getSValBuilder().makeIntVal(1, B->getType()); +} + } else { +// The value is known to be false. +assert(StFalse && "Infeasible path!"); +X = getSValBuilder().makeIntVal(0, B->getType()); + } } } Bldr.generateNode(B, Pred, state->BindExpr(B, Pred->getLocationContext(), X)); Modified: cfe/trunk/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp?rev=290415&r1=290414&r2=290415&view=diff == --- cfe/trunk/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp Thu Dec 22 22:09:18 2016 @@ -254,21 +254,6 @@ ProgramStateRef SimpleConstraintManager: assert(BinaryOperator::isComparisonOp(Op) && "Non-comparison ops should be rewritten as comparisons to zero."); - SymbolRef Sym = LHS; - - // Simplification: translate an assume of a constraint of the form - // "(exp comparison_op expr) != 0" to true into an assume of - // "exp comparison_op expr" to true. (And similarly, an assume of the form - // "(exp comparison_op expr) == 0" to true into an assume of - // "exp comparison_op expr" to false.) - if (Int == 0 && (Op == BO_EQ || Op == BO_NE)) { -if (const BinarySymExpr *SE = dyn_cast(Sym)) { - BinaryOperator::Opcode Op = SE->getOpcode(); - if (BinaryOperator::isComparisonOp(Op)) -return assume(State, nonloc::SymbolVal(Sym), (Op == BO_NE ? true : false)); -} - } - // Get the type used for calculating wraparound. BasicValueFactory &BVF = getBasicVals(); APSIntType WraparoundType = BVF.getAPSIntType(LHS->getType()); @@ -280,6 +265,7 @@ ProgramStateRef SimpleConstraintManager: // x < 4 has the solution [0, 3]. x+2 < 4 has the solution [0-2, 3-2], which // in modular arithmetic is [0, 1] U [UINT_MAX-1, UINT_MAX]. It's up to // the subclasses of SimpleConstraintManager to handle the adjustment. + SymbolRef Sym = LHS; llvm::APSInt Adjustment = WraparoundType.getZeroValue(); computeAdjustment(Sym, Adjustment); Modified: cfe/trunk/test/Analysis/malloc.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/malloc.c?rev=290415&r1=290414&r2=290415&view=diff == --- cfe/trunk/test/Analysis/malloc.c (original) +++ cfe/trunk/test/Analysis/malloc.c Thu Dec 22 22:09:18 2016 @@ -1763,17 +1763,6 @@ void testConstEscapeThroughAnotherField( constEscape(&(s.x)); // could free s->p! } // no-warning -// PR15623 -int testNoCheckerDataPropogationFromLogicalOpOperandToOpResult(void) { - char *param = malloc(10); - char *value = malloc(10); - int ok = (param && value); - free(param); - free(value); - // Previously we ended up with 'Use of memory after it is freed' on return. - return ok; // no warning -} - // // False negatives. Modified: cfe/trunk/test/Analysi
[PATCH] D27680: [CodeGen] Move lifetime.start of a variable when goto jumps back past its declaration
ahatanak added a comment. Ah, good idea. That sounds like a much simpler and less invasive approach. I agree that the performance impact would probably be small, and if it turns out to have a significant impact, we can reduce the number of times we move the lifetime.start (without moving it retroactively) by checking whether a label was seen. I'll see if I can come with a new patch. https://reviews.llvm.org/D27680 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r290417 - Add an assert to catch improperly constructed %diff sequences in
Author: chandlerc Date: Thu Dec 22 23:19:47 2016 New Revision: 290417 URL: http://llvm.org/viewvc/llvm-project?rev=290417&view=rev Log: Add an assert to catch improperly constructed %diff sequences in diagnostics and fix one such diagnostic. Sadly, this assert doesn't catch this bug because we have no tests that emit this diagnostic! Doh! I'm following up on the commit that introduces it to get that fixed. Then this assert will help in a more direct way. Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/lib/Basic/Diagnostic.cpp Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=290417&r1=290416&r2=290417&view=diff == --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Dec 22 23:19:47 2016 @@ -3346,7 +3346,7 @@ def note_ovl_candidate_inconsistent_dedu def note_ovl_candidate_inconsistent_deduction_types : Note< "candidate template ignored: deduced values %diff{" "of conflicting types for parameter %0 (%1 of type $ vs. %3 of type $)|" -"%1 and %3 of conflicting types for parameter %0|}2,4">; +"%1 and %3 of conflicting types for parameter %0}2,4">; def note_ovl_candidate_explicit_arg_mismatch_named : Note< "candidate template ignored: invalid explicitly-specified argument " "for template parameter %0">; Modified: cfe/trunk/lib/Basic/Diagnostic.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Diagnostic.cpp?rev=290417&r1=290416&r2=290417&view=diff == --- cfe/trunk/lib/Basic/Diagnostic.cpp (original) +++ cfe/trunk/lib/Basic/Diagnostic.cpp Thu Dec 22 23:19:47 2016 @@ -742,7 +742,10 @@ FormatDiagnostic(const char *DiagStr, co // "%diff{compare $ to $|other text}1,2" // treat it as: // "compare %1 to %2" -const char *Pipe = ScanFormat(Argument, Argument + ArgumentLen, '|'); +const char *ArgumentEnd = Argument + ArgumentLen; +const char *Pipe = ScanFormat(Argument, ArgumentEnd, '|'); +assert(ScanFormat(Pipe + 1, ArgumentEnd, '|') == ArgumentEnd && + "Found too many '|'s in a %diff modifier!"); const char *FirstDollar = ScanFormat(Argument, Pipe, '$'); const char *SecondDollar = ScanFormat(FirstDollar + 1, Pipe, '$'); const char ArgStr1[] = { '%', static_cast('0' + ArgNo) }; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r290399 - When merging two deduced non-type template arguments for the same parameter,
On Thu, Dec 22, 2016 at 5:41 PM Richard Smith via cfe-commits < cfe-commits@lists.llvm.org> wrote: > > == > --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) > +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Dec 22 > 19:30:39 2016 > @@ -3343,6 +3343,10 @@ def note_ovl_candidate_incomplete_deduct > def note_ovl_candidate_inconsistent_deduction : Note< > "candidate template ignored: deduced conflicting > %select{types|values|" > "templates}0 for parameter %1%diff{ ($ vs. $)|}2,3">; > +def note_ovl_candidate_inconsistent_deduction_types : Note< > +"candidate template ignored: deduced values %diff{" > +"of conflicting types for parameter %0 (%1 of type $ vs. %3 of type > $)|" > +"%1 and %3 of conflicting types for parameter %0|}2,4">; > So, this new diagnostic isn't actually covered by any test. Not just is the wording not covered, literally it isn't emitted. There is a bug in the format that we didn't defend against -- having too many '|'s in the %diff alternatives. I added an assert to try to catch this in r290417 and it didn't trigger here because we never process this format. Could you add a test covering this? I went ahead and fixed the %diff format syntax by inspection in r290417. ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits