[PATCH] D26137: [clang-tidy] Add check name to YAML export
Alpha added a comment. Ping Repository: rL LLVM https://reviews.llvm.org/D26137 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27138: Extend CompilationDatabase by a field for the output filename
klimek added inline comments. Comment at: lib/Tooling/JSONCompilationDatabase.cpp:266 +nodeToCommandLine(Syntax, std::get<2>(CommandsRef[I])), +Output ? Output->getValue(OutputStorage) : ""); } Optional: I'd probably let the nodeToCommandLine handle the null value and make this code more straight forward? Repository: rL LLVM https://reviews.llvm.org/D27138 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D21099: [Sema] Teach -Wcast-align to look at the aligned attribute of the declared variables
arphaman accepted this revision. arphaman added a reviewer: arphaman. arphaman added a comment. This revision is now accepted and ready to land. LGTM, Thanks https://reviews.llvm.org/D21099 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26849: [Sema] Set range end of constructors and destructors in template instantiations
arphaman accepted this revision. arphaman added a comment. This revision is now accepted and ready to land. I think it LG, thanks for adding the test https://reviews.llvm.org/D26849 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25475: [analyzer] Add a new SVal to support pointer-to-member operations.
kromanenkov added a comment. ping https://reviews.llvm.org/D25475 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26849: [Sema] Set range end of constructors and destructors in template instantiations
This revision was automatically updated to reflect the committed changes. Closed by commit rL288025: [Sema] Set range end of constructors and destructors in template instantiations (authored by malcolm.parsons). Changed prior to commit: https://reviews.llvm.org/D26849?vs=78522&id=79385#toc Repository: rL LLVM https://reviews.llvm.org/D26849 Files: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp cfe/trunk/test/Misc/ast-dump-decl.cpp Index: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp === --- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -1871,11 +1871,13 @@ Constructor->isExplicit(), Constructor->isInlineSpecified(), false, Constructor->isConstexpr()); +Method->setRangeEnd(Constructor->getLocEnd()); } else if (CXXDestructorDecl *Destructor = dyn_cast(D)) { Method = CXXDestructorDecl::Create(SemaRef.Context, Record, StartLoc, NameInfo, T, TInfo, Destructor->isInlineSpecified(), false); +Method->setRangeEnd(Destructor->getLocEnd()); } else if (CXXConversionDecl *Conversion = dyn_cast(D)) { Method = CXXConversionDecl::Create(SemaRef.Context, Record, StartLoc, NameInfo, T, TInfo, Index: cfe/trunk/test/Misc/ast-dump-decl.cpp === --- cfe/trunk/test/Misc/ast-dump-decl.cpp +++ cfe/trunk/test/Misc/ast-dump-decl.cpp @@ -223,6 +223,10 @@ class D { }; template class TestClassTemplate { + public: +TestClassTemplate(); +~TestClassTemplate(); +int j(); int i; }; @@ -252,10 +256,18 @@ // CHECK-NEXT: TemplateTypeParmDecl // CHECK-NEXT: CXXRecordDecl{{.*}} class TestClassTemplate // CHECK-NEXT: CXXRecordDecl{{.*}} class TestClassTemplate +// CHECK-NEXT: AccessSpecDecl{{.*}} public +// CHECK-NEXT: CXXConstructorDecl{{.*}} +// CHECK-NEXT: CXXDestructorDecl{{.*}} +// CHECK-NEXT: CXXMethodDecl{{.*}} // CHECK-NEXT: FieldDecl{{.*}} i // CHECK-NEXT: ClassTemplateSpecializationDecl{{.*}} class TestClassTemplate // CHECK-NEXT: TemplateArgument{{.*}}A // CHECK-NEXT: CXXRecordDecl{{.*}} class TestClassTemplate +// CHECK-NEXT: AccessSpecDecl{{.*}} public +// CHECK-NEXT: CXXConstructorDecl{{.*}} +// CHECK-NEXT: CXXDestructorDecl{{.*}} +// CHECK-NEXT: CXXMethodDecl{{.*}} // CHECK-NEXT: FieldDecl{{.*}} i // CHECK:ClassTemplateSpecialization{{.*}} 'TestClassTemplate' // CHECK-NEXT: ClassTemplateSpecialization{{.*}} 'TestClassTemplate' @@ -269,11 +281,19 @@ // CHECK: ClassTemplateSpecializationDecl{{.*}} class TestClassTemplate // CHECK-NEXT: TemplateArgument{{.*}}C // CHECK-NEXT: CXXRecordDecl{{.*}} class TestClassTemplate +// CHECK-NEXT: AccessSpecDecl{{.*}} public +// CHECK-NEXT: CXXConstructorDecl{{.*}} +// CHECK-NEXT: CXXDestructorDecl{{.*}} +// CHECK-NEXT: CXXMethodDecl{{.*}} // CHECK-NEXT: FieldDecl{{.*}} i // CHECK: ClassTemplateSpecializationDecl{{.*}} class TestClassTemplate // CHECK-NEXT: TemplateArgument{{.*}}D // CHECK-NEXT: CXXRecordDecl{{.*}} class TestClassTemplate +// CHECK-NEXT: AccessSpecDecl{{.*}} public +// CHECK-NEXT: CXXConstructorDecl{{.*}} +// CHECK-NEXT: CXXDestructorDecl{{.*}} +// CHECK-NEXT: CXXMethodDecl{{.*}} // CHECK-NEXT: FieldDecl{{.*}} i // CHECK: ClassTemplatePartialSpecializationDecl{{.*}} class TestClassTemplatePartial Index: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp === --- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -1871,11 +1871,13 @@ Constructor->isExplicit(), Constructor->isInlineSpecified(), false, Constructor->isConstexpr()); +Method->setRangeEnd(Constructor->getLocEnd()); } else if (CXXDestructorDecl *Destructor = dyn_cast(D)) { Method = CXXDestructorDecl::Create(SemaRef.Context, Record, StartLoc, NameInfo, T, TInfo, Destructor->isInlineSpecified(), false); +Method->setRangeEnd(Destructor->getLocEnd()); } else if (CXXConversionDecl *Conversion = dyn_cast(D)) { Method = CXXConversionDecl::Create(SemaRef.Context, Record, StartLoc, NameInfo, T, TInfo, Index: cfe/trunk/test/Misc/ast-dump-decl.cpp === --- cfe/trun
r288025 - [Sema] Set range end of constructors and destructors in template instantiations
Author: malcolm.parsons Date: Mon Nov 28 05:11:34 2016 New Revision: 288025 URL: http://llvm.org/viewvc/llvm-project?rev=288025&view=rev Log: [Sema] Set range end of constructors and destructors in template instantiations Summary: clang-tidy checks frequently use source ranges of functions. The source range of constructors and destructors in template instantiations is currently a single token. The factory method for constructors and destructors does not allow the end source location to be specified. Set end location manually after creating instantiation. Reviewers: aaron.ballman, rsmith, arphaman Subscribers: arphaman, cfe-commits Differential Revision: https://reviews.llvm.org/D26849 Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp cfe/trunk/test/Misc/ast-dump-decl.cpp Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp?rev=288025&r1=288024&r2=288025&view=diff == --- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp (original) +++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp Mon Nov 28 05:11:34 2016 @@ -1871,11 +1871,13 @@ TemplateDeclInstantiator::VisitCXXMethod Constructor->isExplicit(), Constructor->isInlineSpecified(), false, Constructor->isConstexpr()); +Method->setRangeEnd(Constructor->getLocEnd()); } else if (CXXDestructorDecl *Destructor = dyn_cast(D)) { Method = CXXDestructorDecl::Create(SemaRef.Context, Record, StartLoc, NameInfo, T, TInfo, Destructor->isInlineSpecified(), false); +Method->setRangeEnd(Destructor->getLocEnd()); } else if (CXXConversionDecl *Conversion = dyn_cast(D)) { Method = CXXConversionDecl::Create(SemaRef.Context, Record, StartLoc, NameInfo, T, TInfo, Modified: cfe/trunk/test/Misc/ast-dump-decl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Misc/ast-dump-decl.cpp?rev=288025&r1=288024&r2=288025&view=diff == --- cfe/trunk/test/Misc/ast-dump-decl.cpp (original) +++ cfe/trunk/test/Misc/ast-dump-decl.cpp Mon Nov 28 05:11:34 2016 @@ -223,6 +223,10 @@ namespace testClassTemplateDecl { class D { }; template class TestClassTemplate { + public: +TestClassTemplate(); +~TestClassTemplate(); +int j(); int i; }; @@ -252,10 +256,18 @@ namespace testClassTemplateDecl { // CHECK-NEXT: TemplateTypeParmDecl // CHECK-NEXT: CXXRecordDecl{{.*}} class TestClassTemplate // CHECK-NEXT: CXXRecordDecl{{.*}} class TestClassTemplate +// CHECK-NEXT: AccessSpecDecl{{.*}} public +// CHECK-NEXT: CXXConstructorDecl{{.*}} +// CHECK-NEXT: CXXDestructorDecl{{.*}} +// CHECK-NEXT: CXXMethodDecl{{.*}} // CHECK-NEXT: FieldDecl{{.*}} i // CHECK-NEXT: ClassTemplateSpecializationDecl{{.*}} class TestClassTemplate // CHECK-NEXT: TemplateArgument{{.*}}A // CHECK-NEXT: CXXRecordDecl{{.*}} class TestClassTemplate +// CHECK-NEXT: AccessSpecDecl{{.*}} public +// CHECK-NEXT: CXXConstructorDecl{{.*}} +// CHECK-NEXT: CXXDestructorDecl{{.*}} +// CHECK-NEXT: CXXMethodDecl{{.*}} // CHECK-NEXT: FieldDecl{{.*}} i // CHECK:ClassTemplateSpecialization{{.*}} 'TestClassTemplate' // CHECK-NEXT: ClassTemplateSpecialization{{.*}} 'TestClassTemplate' @@ -269,11 +281,19 @@ namespace testClassTemplateDecl { // CHECK: ClassTemplateSpecializationDecl{{.*}} class TestClassTemplate // CHECK-NEXT: TemplateArgument{{.*}}C // CHECK-NEXT: CXXRecordDecl{{.*}} class TestClassTemplate +// CHECK-NEXT: AccessSpecDecl{{.*}} public +// CHECK-NEXT: CXXConstructorDecl{{.*}} +// CHECK-NEXT: CXXDestructorDecl{{.*}} +// CHECK-NEXT: CXXMethodDecl{{.*}} // CHECK-NEXT: FieldDecl{{.*}} i // CHECK: ClassTemplateSpecializationDecl{{.*}} class TestClassTemplate // CHECK-NEXT: TemplateArgument{{.*}}D // CHECK-NEXT: CXXRecordDecl{{.*}} class TestClassTemplate +// CHECK-NEXT: AccessSpecDecl{{.*}} public +// CHECK-NEXT: CXXConstructorDecl{{.*}} +// CHECK-NEXT: CXXDestructorDecl{{.*}} +// CHECK-NEXT: CXXMethodDecl{{.*}} // CHECK-NEXT: FieldDecl{{.*}} i // CHECK: ClassTemplatePartialSpecializationDecl{{.*}} class TestClassTemplatePartial ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27138: Extend CompilationDatabase by a field for the output filename
joerg added inline comments. Comment at: lib/Tooling/JSONCompilationDatabase.cpp:266 +nodeToCommandLine(Syntax, std::get<2>(CommandsRef[I])), +Output ? Output->getValue(OutputStorage) : ""); } klimek wrote: > Optional: I'd probably let the nodeToCommandLine handle the null value and > make this code more straight forward? I couldn't find a way to create a synthetic node without changing the YAML API. Repository: rL LLVM https://reviews.llvm.org/D27138 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27138: Extend CompilationDatabase by a field for the output filename
klimek added inline comments. Comment at: lib/Tooling/JSONCompilationDatabase.cpp:266 +nodeToCommandLine(Syntax, std::get<2>(CommandsRef[I])), +Output ? Output->getValue(OutputStorage) : ""); } joerg wrote: > klimek wrote: > > Optional: I'd probably let the nodeToCommandLine handle the null value and > > make this code more straight forward? > I couldn't find a way to create a synthetic node without changing the YAML > API. I'm probably missing something - why would we need a synthetic node? Can't we just put nullptr into the vector? Repository: rL LLVM https://reviews.llvm.org/D27138 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] Warning for main returning a bool.
Thanks Richard for looking at the revised patch. On Mon, Nov 21, 2016 at 1:50 PM Richard Smith wrote: > This looks good to me. (While we could generalize this further, this patch > is a strict improvement, and we'll probably want to treat the 'main' case > specially regardless of whether we add a more general conversion warning.) > > On 21 November 2016 at 07:18, Joshua Hurwitz wrote: > > I modified the patch to warn only when a bool literal is returned from > main. See attached. -Josh > > > On Tue, Nov 15, 2016 at 7:50 PM Richard Smith > wrote: > > On Tue, Nov 15, 2016 at 2:55 PM, Aaron Ballman via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > > On Tue, Nov 15, 2016 at 5:44 PM, Hal Finkel wrote: > > - Original Message - > >> From: "Aaron Ballman" > >> To: "Hal Finkel" > >> Cc: "cfe-commits" , "Joshua Hurwitz" < > hurwi...@google.com> > >> Sent: Tuesday, November 15, 2016 4:42:05 PM > >> Subject: Re: [PATCH] Warning for main returning a bool. > >> > >> On Tue, Nov 15, 2016 at 5:22 PM, Hal Finkel wrote: > >> > - Original Message - > >> >> From: "Aaron Ballman via cfe-commits" > >> >> To: "Joshua Hurwitz" > >> >> Cc: "cfe-commits" > >> >> Sent: Tuesday, November 15, 2016 12:17:28 PM > >> >> Subject: Re: [PATCH] Warning for main returning a bool. > >> >> > >> >> On Fri, Oct 14, 2016 at 1:17 PM, Joshua Hurwitz via cfe-commits > >> >> wrote: > >> >> > See attached. > >> >> > > >> >> > Returning a bool from main is a special case of return type > >> >> > mismatch. The > >> >> > common convention when returning a bool is that 'true' (== 1) > >> >> > indicates > >> >> > success and 'false' (== 0) failure. But since main expects a > >> >> > return > >> >> > value of > >> >> > 0 on success, returning a bool is usually unintended. > >> >> > >> >> I am not convinced that this is a high-value diagnostic. Returning > >> >> a > >> >> Boolean from main() may or may not be a bug (the returned value is > >> >> generally a convention more than anything else). Also, why Boolean > >> >> and > >> >> not, say, long long or float? > >> > > >> > I've seen this error often enough, but I think we need to be > >> > careful about false positives here. I recommend that we check only > >> > for explicit uses of boolean immediates (i.e. return true; or > >> > return false;) -- these are often bugs. > >> > >> I could get behind that. > >> > >> > Aaron, I disagree that the return value is just some kind of > >> > convention. It has a clear meaning. > >> > >> For many hosted environments, certainly. Freestanding > >> implementations? > >> Much less so, but I suppose this behavior is still reasonable enough > >> for them (not to mention, there may not even *be* a main() for a > >> freestanding implementation). > >> > >> > Furthermore, the behavior of the system can be quite different for > >> > a non-zero exit code than otherwise, and users who don't > >> > understand what's going on can find it very difficult to > >> > understand what's going wrong. > >> > >> That's a fair point, but my question still stands -- why only Boolean > >> values, and not "return 0x12345678ULL;" or "return 1.2;"? > >> > >> Combining with your idea above, if the check flagged instances where > >> a > >> literal of non-integral type (other than Boolean) is returned from > >> main(), that seems like good value. > > > > Agreed. > > > > FWIW, if we have a function that returns 'int' and the user tries to > return '1.2' we should probably warn for any function. > > Good point, we already have -Wliteral-conversion (which catches 1.2) > and -Wconstant-conversion (which catches 0x12345678ULL), and > -Wint-conversion for C programs returning awful things like string > literals, all of which are enabled by default. Perhaps Boolean values > really are the only case we don't explicitly warn about. > > > Wow, I'm amazed that we have no warning at all for converting, say, 'true' > to int (or indeed to float). > > Even with a warning for bool literal -> non-bool conversions in place, it > would still seem valuable to factor out a check for the corresponding case > in a return statement in main, since in that case we actually know what the > return value means, and the chance of a false positive goes way down. > > So I think that means we want two or possibly three checks here: > > 1) main returns bool literal > 2) -Wliteral-conversion warning for converting bool literal to another > type (excluding 1-bit unsigned integral bit-fields) > 3) and possibly: warning for implicit conversion of bool to floating-point > (new subgroup of -Wbool-conversion?) > > (ordered from most- to least-reasonable to enable by default). > > ~Aaron > > > > > -Hal > > > >> > >> ~Aaron > >> > >> > > >> > Thanks again, > >> > Hal > >> > > >> >> > >> >> ~Aaron > >> >> > >> >> > > >> >> > ___ > >> >> > cfe-commits mailing list > >> >> > cfe-commits@lists.llvm.org > >> >> > http://lists.llvm.org/cgi-bin/mailman/lis
[PATCH] D25475: [analyzer] Add a new SVal to support pointer-to-member operations.
NoQ added a comment. This looks good to me (bikeshedded a bit), but i think Devin should have another look, because his comments were way deeper than mine. Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h:217 + + llvm::ImmutableList consCXXBase( + const CXXBaseSpecifier *CBS, Hmm, is it "construct"? Or "constrain"? I think a verbose name wouldn't hurt. In fact, i suspect that `consVals` are rather `conSVals`, where `con` stands for "concatenate". Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:882 +return UndefinedVal(); + if (const FieldDecl *FD = PTMSV->getDeclAs()) +return state->getLValue(FD, lhs); Hmm, do we need to cover `CXXMethodDecl` here? Or is it modeled as a function pointer anyway, so no action is needed? Worth commenting, i think. https://reviews.llvm.org/D25475 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27163: Introduce -f[no-]strict-return flag that controls code generation for C++'s undefined behaviour return optimisation
arphaman created this revision. arphaman added reviewers: rjmccall, rsmith, mehdi_amini. arphaman added a subscriber: cfe-commits. arphaman set the repository for this revision to rL LLVM. This patch adds a new clang flag called `-f[no-]strict-return`. The purpose of this flag is to control how the code generator applies the undefined behaviour return optimisation to value returning functions that flow-off without a required return: - If -fstrict-return is on (default for non Darwin targets), then the code generator follows the current behaviour: it emits the IR for the undefined behaviour (trap with unreachable). - Otherwise, the code generator emits the IR for the undefined behaviour only if the function avoided -Wreturn-type warnings. This avoidance is detected even if -Wreturn-type warnings are disabled (-Wno-return-type). Repository: rL LLVM https://reviews.llvm.org/D27163 Files: include/clang/AST/Decl.h include/clang/Basic/LangOptions.def include/clang/Driver/Options.td include/clang/Frontend/CodeGenOptions.def include/clang/Sema/AnalysisBasedWarnings.h include/clang/Sema/Sema.h lib/CodeGen/CodeGenFunction.cpp lib/Driver/Tools.cpp lib/Frontend/CompilerInvocation.cpp lib/Sema/AnalysisBasedWarnings.cpp lib/Sema/Sema.cpp test/CodeGenCXX/return.cpp test/Driver/clang_f_opts.c test/Driver/darwin-no-strict-return.c Index: test/Driver/darwin-no-strict-return.c === --- /dev/null +++ test/Driver/darwin-no-strict-return.c @@ -0,0 +1,7 @@ +// Darwin should have -fno-strict-return turned on by default +// rdar://13102603 + +// RUN: %clang -### -S -target x86_64-apple-macosx10.7.0 %s 2>&1 | FileCheck -check-prefix=CHECK-NO-STRICT-RETURN %s +// RUN: %clang -### -S -target x86_64-apple-macosx10.7.0 -fstrict-return %s 2>&1 | FileCheck -check-prefix=CHECK-STRICT-RETURN %s +// CHECK-NO-STRICT-RETURN: "-fno-strict-return" +// CHECK-STRICT-RETURN-NOT: "-fno-strict-return" Index: test/Driver/clang_f_opts.c === --- test/Driver/clang_f_opts.c +++ test/Driver/clang_f_opts.c @@ -469,3 +469,8 @@ // CHECK-WCHAR2: -fshort-wchar // CHECK-WCHAR2-NOT: -fno-short-wchar // DELIMITERS: {{^ *"}} + +// RUN: %clang -### -S -fstrict-return %s 2>&1 | FileCheck -check-prefix=CHECK-STRICT-RETURN %s +// RUN: %clang -### -S -fno-strict-return %s 2>&1 | FileCheck -check-prefix=CHECK-NO-STRICT-RETURN %s +// CHECK-STRICT-RETURN-NOT: "-fno-strict-return" +// CHECK-NO-STRICT-RETURN: "-fno-strict-return" Index: test/CodeGenCXX/return.cpp === --- test/CodeGenCXX/return.cpp +++ test/CodeGenCXX/return.cpp @@ -1,12 +1,61 @@ // RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple -o - %s | FileCheck %s // RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple -O -o - %s | FileCheck %s --check-prefix=CHECK-OPT +// RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple -fno-strict-return -o - %s | FileCheck %s --check-prefix=CHECK-NOSTRICT +// RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple -fno-strict-return -Wno-return-type -o - %s | FileCheck %s --check-prefix=CHECK-NOSTRICT +// RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple -fno-strict-return -O -o - %s | FileCheck %s --check-prefix=CHECK-NOSTRICT-OPT // CHECK: @_Z9no_return // CHECK-OPT: @_Z9no_return +// CHECK-NOSTRICT: @_Z9no_return +// CHECK-NOSTRICT-OPT: @_Z9no_return int no_return() { // CHECK: call void @llvm.trap // CHECK-NEXT: unreachable // CHECK-OPT-NOT: call void @llvm.trap // CHECK-OPT: unreachable + + // CHECK-NOSTRICT: entry: + // CHECK-NOSTRICT-NEXT: alloca + // CHECK-NOSTRICT-NEXT: load + // CHECK-NOSTRICT-NEXT: ret i32 + // CHECK-NOSTRICT-NEXT: } + + // CHECK-NOSTRICT-OPT: entry: + // CHECK-NOSTRICT-OPT: ret i32 undef +} + +enum Enum { + A, B +}; + +// CHECK: @_Z21alwaysOptimizedReturn4Enum +// CHECK-OPT: @_Z21alwaysOptimizedReturn4Enum +// CHECK-NOSTRICT: @_Z21alwaysOptimizedReturn4Enum +// CHECK-NOSTRICT-OPT: @_Z21alwaysOptimizedReturn4Enum +int alwaysOptimizedReturn(Enum e) { + switch (e) { + case A: return 0; + case B: return 1; + } + // CHECK-NOSTRICT: call void @llvm.trap() + // CHECK-NOSTRICT-NEXT: unreachable + + // CHECK-NOSTRICT-OPT: entry: + // CHECK-NOSTRICT-OPT-NEXT: icmp + // CHECK-NOSTRICT-OPT-NEXT: zext + // CHECK-NOSTRICT-OPT-NEXT: ret i32 +} + +// CHECK-NOSTRICT: @_Z23strictlyOptimizedReturn4Enum +// CHECK-NOSTRICT-OPT: @_Z23strictlyOptimizedReturn4Enum +int strictlyOptimizedReturn(Enum e) { + switch (e) { + case A: return 22; + } + // CHECK-NOSTRICT-NOT: call void @llvm.trap + // CHECK-NOSTRICT-NOT: unreachable + + // CHECK-NOSTRICT-OPT: entry: + // CHECK-NOSTRICT-OPT-NEXT: ret i32 22 } Index: lib/Sema/Sema.cpp === --- lib/Sema/Sema.cpp +++ lib/Sema/Sema.cpp @@ -115
[PATCH] D27163: Introduce -f[no-]strict-return flag that controls code generation for C++'s undefined behaviour return optimisation
arphaman updated this revision to Diff 79396. arphaman added a comment. Fixed comment typo. Repository: rL LLVM https://reviews.llvm.org/D27163 Files: include/clang/AST/Decl.h include/clang/Basic/LangOptions.def include/clang/Driver/Options.td include/clang/Frontend/CodeGenOptions.def include/clang/Sema/AnalysisBasedWarnings.h include/clang/Sema/Sema.h lib/CodeGen/CodeGenFunction.cpp lib/Driver/Tools.cpp lib/Frontend/CompilerInvocation.cpp lib/Sema/AnalysisBasedWarnings.cpp lib/Sema/Sema.cpp test/CodeGenCXX/return.cpp test/Driver/clang_f_opts.c test/Driver/darwin-no-strict-return.c Index: test/Driver/darwin-no-strict-return.c === --- /dev/null +++ test/Driver/darwin-no-strict-return.c @@ -0,0 +1,7 @@ +// Darwin should have -fno-strict-return turned on by default +// rdar://13102603 + +// RUN: %clang -### -S -target x86_64-apple-macosx10.7.0 %s 2>&1 | FileCheck -check-prefix=CHECK-NO-STRICT-RETURN %s +// RUN: %clang -### -S -target x86_64-apple-macosx10.7.0 -fstrict-return %s 2>&1 | FileCheck -check-prefix=CHECK-STRICT-RETURN %s +// CHECK-NO-STRICT-RETURN: "-fno-strict-return" +// CHECK-STRICT-RETURN-NOT: "-fno-strict-return" Index: test/Driver/clang_f_opts.c === --- test/Driver/clang_f_opts.c +++ test/Driver/clang_f_opts.c @@ -469,3 +469,8 @@ // CHECK-WCHAR2: -fshort-wchar // CHECK-WCHAR2-NOT: -fno-short-wchar // DELIMITERS: {{^ *"}} + +// RUN: %clang -### -S -fstrict-return %s 2>&1 | FileCheck -check-prefix=CHECK-STRICT-RETURN %s +// RUN: %clang -### -S -fno-strict-return %s 2>&1 | FileCheck -check-prefix=CHECK-NO-STRICT-RETURN %s +// CHECK-STRICT-RETURN-NOT: "-fno-strict-return" +// CHECK-NO-STRICT-RETURN: "-fno-strict-return" Index: test/CodeGenCXX/return.cpp === --- test/CodeGenCXX/return.cpp +++ test/CodeGenCXX/return.cpp @@ -1,12 +1,61 @@ // RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple -o - %s | FileCheck %s // RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple -O -o - %s | FileCheck %s --check-prefix=CHECK-OPT +// RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple -fno-strict-return -o - %s | FileCheck %s --check-prefix=CHECK-NOSTRICT +// RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple -fno-strict-return -Wno-return-type -o - %s | FileCheck %s --check-prefix=CHECK-NOSTRICT +// RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple -fno-strict-return -O -o - %s | FileCheck %s --check-prefix=CHECK-NOSTRICT-OPT // CHECK: @_Z9no_return // CHECK-OPT: @_Z9no_return +// CHECK-NOSTRICT: @_Z9no_return +// CHECK-NOSTRICT-OPT: @_Z9no_return int no_return() { // CHECK: call void @llvm.trap // CHECK-NEXT: unreachable // CHECK-OPT-NOT: call void @llvm.trap // CHECK-OPT: unreachable + + // CHECK-NOSTRICT: entry: + // CHECK-NOSTRICT-NEXT: alloca + // CHECK-NOSTRICT-NEXT: load + // CHECK-NOSTRICT-NEXT: ret i32 + // CHECK-NOSTRICT-NEXT: } + + // CHECK-NOSTRICT-OPT: entry: + // CHECK-NOSTRICT-OPT: ret i32 undef +} + +enum Enum { + A, B +}; + +// CHECK: @_Z21alwaysOptimizedReturn4Enum +// CHECK-OPT: @_Z21alwaysOptimizedReturn4Enum +// CHECK-NOSTRICT: @_Z21alwaysOptimizedReturn4Enum +// CHECK-NOSTRICT-OPT: @_Z21alwaysOptimizedReturn4Enum +int alwaysOptimizedReturn(Enum e) { + switch (e) { + case A: return 0; + case B: return 1; + } + // CHECK-NOSTRICT: call void @llvm.trap() + // CHECK-NOSTRICT-NEXT: unreachable + + // CHECK-NOSTRICT-OPT: entry: + // CHECK-NOSTRICT-OPT-NEXT: icmp + // CHECK-NOSTRICT-OPT-NEXT: zext + // CHECK-NOSTRICT-OPT-NEXT: ret i32 +} + +// CHECK-NOSTRICT: @_Z23strictlyOptimizedReturn4Enum +// CHECK-NOSTRICT-OPT: @_Z23strictlyOptimizedReturn4Enum +int strictlyOptimizedReturn(Enum e) { + switch (e) { + case A: return 22; + } + // CHECK-NOSTRICT-NOT: call void @llvm.trap + // CHECK-NOSTRICT-NOT: unreachable + + // CHECK-NOSTRICT-OPT: entry: + // CHECK-NOSTRICT-OPT-NEXT: ret i32 22 } Index: lib/Sema/Sema.cpp === --- lib/Sema/Sema.cpp +++ lib/Sema/Sema.cpp @@ -1154,7 +1154,7 @@ } void Sema::PopFunctionScopeInfo(const AnalysisBasedWarnings::Policy *WP, -const Decl *D, const BlockExpr *blkExpr) { +Decl *D, const BlockExpr *blkExpr) { FunctionScopeInfo *Scope = FunctionScopes.pop_back_val(); assert(!FunctionScopes.empty() && "mismatched push/pop!"); Index: lib/Sema/AnalysisBasedWarnings.cpp === --- lib/Sema/AnalysisBasedWarnings.cpp +++ lib/Sema/AnalysisBasedWarnings.cpp @@ -521,13 +521,22 @@ } // anonymous namespace +/// The given declaration \p D is marked as associated with a non-void return +/// warning. +static void markHasNonVoidReturnWarning(Decl *D) { + if (a
[clang-tools-extra] r288034 - ClangMoveTests.cpp: Fix a bogus comparison of iterator.
Author: chapuni Date: Mon Nov 28 08:27:37 2016 New Revision: 288034 URL: http://llvm.org/viewvc/llvm-project?rev=288034&view=rev Log: ClangMoveTests.cpp: Fix a bogus comparison of iterator. msc Debug build detected it. Modified: clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp Modified: clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp?rev=288034&r1=288033&r2=288034&view=diff == --- clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp (original) +++ clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp Mon Nov 28 08:27:37 2016 @@ -521,7 +521,8 @@ TEST(ClangMove, DumpDecls) { const auto& Results = Reporter.getDeclarationList(); auto ActualDeclIter = Results.begin(); auto ExpectedDeclIter = ExpectedDeclarations.begin(); - while (ActualDeclIter != Results.end() && ExpectedDeclIter != Results.end()) { + while (ActualDeclIter != Results.end() && + ExpectedDeclIter != ExpectedDeclarations.end()) { EXPECT_EQ(*ActualDeclIter, *ExpectedDeclIter); ++ActualDeclIter; ++ExpectedDeclIter; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27109: [OpenCL] Prevent generation of globals in non-constant address space for OpenCL
yaxunl added inline comments. Comment at: lib/CodeGen/CGDecl.cpp:954 + if (!getLangOpts().OpenCL || Ty.getAddressSpace() == LangAS::opencl_constant) +if (CGM.getCodeGenOpts().MergeAllConstants && !NRVO && !isByRef && +CGM.isTypeConstant(Ty, true)) { can we merge this if with the above if? https://reviews.llvm.org/D27109 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27049: [OpenCL] Refactor out ReadPipe/WritePipe
joey updated this revision to Diff 79399. joey added a comment. Pipe types cannot be merged by ASTContext::mergeTypes. Repository: rL LLVM https://reviews.llvm.org/D27049 Files: include/clang/AST/ASTContext.h include/clang/AST/Type.h include/clang/Serialization/ASTBitCodes.h lib/AST/ASTContext.cpp lib/Serialization/ASTReader.cpp lib/Serialization/ASTWriter.cpp test/SemaOpenCL/invalid-pipes-cl2.0.cl Index: test/SemaOpenCL/invalid-pipes-cl2.0.cl === --- test/SemaOpenCL/invalid-pipes-cl2.0.cl +++ test/SemaOpenCL/invalid-pipes-cl2.0.cl @@ -20,3 +20,12 @@ typedef pipe int pipe_int_t; pipe_int_t test6() {} // expected-error{{declaring function return value of type 'pipe_int_t' (aka 'read_only pipe int') is not allowed}} + +// Tests ASTContext::mergeTypes rejects this. +int f(pipe int x, int y); +int f(x, y) +pipe short x; +int y; +{ +return y; +} Index: lib/Serialization/ASTWriter.cpp === --- lib/Serialization/ASTWriter.cpp +++ lib/Serialization/ASTWriter.cpp @@ -516,10 +516,8 @@ void ASTTypeWriter::VisitPipeType(const PipeType *T) { Record.AddTypeRef(T->getElementType()); - if (T->isReadOnly()) -Code = TYPE_READ_PIPE; - else -Code = TYPE_WRITE_PIPE; + Record.push_back(T->isReadOnly()); + Code = TYPE_PIPE; } namespace { Index: lib/Serialization/ASTReader.cpp === --- lib/Serialization/ASTReader.cpp +++ lib/Serialization/ASTReader.cpp @@ -5793,27 +5793,21 @@ return Context.getAtomicType(ValueType); } - case TYPE_READ_PIPE: { -if (Record.size() != 1) { + case TYPE_PIPE: { +if (Record.size() != 2) { Error("Incorrect encoding of pipe type"); return QualType(); } // Reading the pipe element type. QualType ElementType = readType(*Loc.F, Record, Idx); -return Context.getReadPipeType(ElementType); +unsigned ReadOnly = Record[1]; +if (ReadOnly) + return Context.getReadPipeType(ElementType); +else + return Context.getWritePipeType(ElementType); } - case TYPE_WRITE_PIPE: { -if (Record.size() != 1) { - Error("Incorrect encoding of pipe type"); - return QualType(); -} - -// Reading the pipe element type. -QualType ElementType = readType(*Loc.F, Record, Idx); -return Context.getWritePipeType(ElementType); - } } llvm_unreachable("Invalid TypeCode!"); } Index: lib/AST/ASTContext.cpp === --- lib/AST/ASTContext.cpp +++ lib/AST/ASTContext.cpp @@ -3338,54 +3338,37 @@ return QualType(FTP, 0); } -QualType ASTContext::getReadPipeType(QualType T) const { +QualType ASTContext::getPipeType(QualType T, bool ReadOnly) const { llvm::FoldingSetNodeID ID; - ReadPipeType::Profile(ID, T); + PipeType::Profile(ID, T, ReadOnly); void *InsertPos = 0; - if (ReadPipeType *PT = ReadPipeTypes.FindNodeOrInsertPos(ID, InsertPos)) + if (PipeType *PT = PipeTypes.FindNodeOrInsertPos(ID, InsertPos)) return QualType(PT, 0); // If the pipe element type isn't canonical, this won't be a canonical type // either, so fill in the canonical type field. QualType Canonical; if (!T.isCanonical()) { -Canonical = getReadPipeType(getCanonicalType(T)); +Canonical = getPipeType(getCanonicalType(T), ReadOnly); // Get the new insert position for the node we care about. -ReadPipeType *NewIP = ReadPipeTypes.FindNodeOrInsertPos(ID, InsertPos); +PipeType *NewIP = PipeTypes.FindNodeOrInsertPos(ID, InsertPos); assert(!NewIP && "Shouldn't be in the map!"); (void)NewIP; } - ReadPipeType *New = new (*this, TypeAlignment) ReadPipeType(T, Canonical); + PipeType *New = new (*this, TypeAlignment) PipeType(T, Canonical, ReadOnly); Types.push_back(New); - ReadPipeTypes.InsertNode(New, InsertPos); + PipeTypes.InsertNode(New, InsertPos); return QualType(New, 0); } -QualType ASTContext::getWritePipeType(QualType T) const { - llvm::FoldingSetNodeID ID; - WritePipeType::Profile(ID, T); - - void *InsertPos = 0; - if (WritePipeType *PT = WritePipeTypes.FindNodeOrInsertPos(ID, InsertPos)) -return QualType(PT, 0); - - // If the pipe element type isn't canonical, this won't be a canonical type - // either, so fill in the canonical type field. - QualType Canonical; - if (!T.isCanonical()) { -Canonical = getWritePipeType(getCanonicalType(T)); +QualType ASTContext::getReadPipeType(QualType T) const { + return getPipeType(T, true); +} -// Get the new insert position for the node we care about. -WritePipeType *NewIP = WritePipeTypes.FindNodeOrInsertPos(ID, InsertPos); -assert(!NewIP && "Shouldn't be in the map!"); -(void)NewIP; - } - WritePipeType *New = new (*this, TypeAlignment) WritePipeType(T, Canonical); - Types.push_back(New); - WritePipeTypes.InsertNode(New
[PATCH] D27099: [OpenCL] Prohibit using reserve_id_t in program scope.
Anastasia accepted this revision. Anastasia added a comment. This revision is now accepted and ready to land. LGTM! Thanks! Please, address the small nitpick before committing. Comment at: lib/Sema/SemaDecl.cpp:5965 + +// OpenCL 1.2 spec, p6.9 r: +// The event type cannot be used with the __local, __constant and __global OpenCL 1.2 spec, p6.9 r -> OpenCL v1.2 s6.9.r https://reviews.llvm.org/D27099 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25686: [Driver] Support "hardfloat" vendor triples used by Gentoo
mgorny added a comment. Second ping. @hfinkel, could you suggest me how to proceed with this? https://reviews.llvm.org/D25686 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26887: [Driver] Fix finding multilib gcc install on Gentoo (with gcc-config)
mgorny added a comment. Ping. https://reviews.llvm.org/D26887 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27138: Extend CompilationDatabase by a field for the output filename
joerg added inline comments. Comment at: lib/Tooling/JSONCompilationDatabase.cpp:266 +nodeToCommandLine(Syntax, std::get<2>(CommandsRef[I])), +Output ? Output->getValue(OutputStorage) : ""); } klimek wrote: > joerg wrote: > > klimek wrote: > > > Optional: I'd probably let the nodeToCommandLine handle the null value > > > and make this code more straight forward? > > I couldn't find a way to create a synthetic node without changing the YAML > > API. > I'm probably missing something - why would we need a synthetic node? Can't we > just put nullptr into the vector? That's what I am doing and why this line checks output :) Repository: rL LLVM https://reviews.llvm.org/D27138 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26764: [compiler-rt] [cmake] Remove i686 target that is duplicate to i386
mgorny added a comment. Ping. https://reviews.llvm.org/D26764 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26796: [Driver] Use arch type to find compiler-rt libraries (on Linux)
mgorny added a comment. Ping. https://reviews.llvm.org/D26796 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27138: Extend CompilationDatabase by a field for the output filename
klimek added inline comments. Comment at: lib/Tooling/JSONCompilationDatabase.cpp:266 +nodeToCommandLine(Syntax, std::get<2>(CommandsRef[I])), +Output ? Output->getValue(OutputStorage) : ""); } joerg wrote: > klimek wrote: > > joerg wrote: > > > klimek wrote: > > > > Optional: I'd probably let the nodeToCommandLine handle the null value > > > > and make this code more straight forward? > > > I couldn't find a way to create a synthetic node without changing the > > > YAML API. > > I'm probably missing something - why would we need a synthetic node? Can't > > we just put nullptr into the vector? > That's what I am doing and why this line checks output :) Ok, let's ask differently: why is it a problem if we put a nullptr into the array? Repository: rL LLVM https://reviews.llvm.org/D27138 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27109: [OpenCL] Prevent generation of globals in non-constant address space for OpenCL
Anastasia updated this revision to Diff 79403. Anastasia added a comment. Merged if statements into one! https://reviews.llvm.org/D27109 Files: lib/CodeGen/CGDecl.cpp test/CodeGenOpenCL/constant-addr-space-globals.cl Index: test/CodeGenOpenCL/constant-addr-space-globals.cl === --- test/CodeGenOpenCL/constant-addr-space-globals.cl +++ test/CodeGenOpenCL/constant-addr-space-globals.cl @@ -6,3 +6,22 @@ kernel void test(global float *out) { *out = array[0]; } + +// Test that we don't use directly initializers for const aggregates +// but create a copy in the original address space (unless a variable itself is +// 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] +kernel void k(void) { + // CHECK-NOT: %arr1 = alloca [3 x i32] + constant const int arr1[] = {1, 2, 3}; + // CHECK: %arr2 = alloca [3 x i32] + const int arr2[] = {4, 5, 6}; + // CHECK: %arr3 = alloca [3 x i32] + int arr3[] = {7, 8, 9}; + + foo(arr1, arr2, arr3); +} Index: lib/CodeGen/CGDecl.cpp === --- lib/CodeGen/CGDecl.cpp +++ lib/CodeGen/CGDecl.cpp @@ -948,8 +948,12 @@ // If the variable's a const type, and it's neither an NRVO // candidate nor a __block variable and has no mutable members, // emit it as a global instead. - if (CGM.getCodeGenOpts().MergeAllConstants && !NRVO && !isByRef && - CGM.isTypeConstant(Ty, true)) { + // Exception is if a variable is located in non-constant address space + // in OpenCL. + if ((!getLangOpts().OpenCL || + Ty.getAddressSpace() == LangAS::opencl_constant) && + (CGM.getCodeGenOpts().MergeAllConstants && !NRVO && !isByRef && + CGM.isTypeConstant(Ty, true))) { EmitStaticVarDecl(D, llvm::GlobalValue::InternalLinkage); // Signal this condition to later callbacks. Index: test/CodeGenOpenCL/constant-addr-space-globals.cl === --- test/CodeGenOpenCL/constant-addr-space-globals.cl +++ test/CodeGenOpenCL/constant-addr-space-globals.cl @@ -6,3 +6,22 @@ kernel void test(global float *out) { *out = array[0]; } + +// Test that we don't use directly initializers for const aggregates +// but create a copy in the original address space (unless a variable itself is +// 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] +kernel void k(void) { + // CHECK-NOT: %arr1 = alloca [3 x i32] + constant const int arr1[] = {1, 2, 3}; + // CHECK: %arr2 = alloca [3 x i32] + const int arr2[] = {4, 5, 6}; + // CHECK: %arr3 = alloca [3 x i32] + int arr3[] = {7, 8, 9}; + + foo(arr1, arr2, arr3); +} Index: lib/CodeGen/CGDecl.cpp === --- lib/CodeGen/CGDecl.cpp +++ lib/CodeGen/CGDecl.cpp @@ -948,8 +948,12 @@ // If the variable's a const type, and it's neither an NRVO // candidate nor a __block variable and has no mutable members, // emit it as a global instead. - if (CGM.getCodeGenOpts().MergeAllConstants && !NRVO && !isByRef && - CGM.isTypeConstant(Ty, true)) { + // Exception is if a variable is located in non-constant address space + // in OpenCL. + if ((!getLangOpts().OpenCL || + Ty.getAddressSpace() == LangAS::opencl_constant) && + (CGM.getCodeGenOpts().MergeAllConstants && !NRVO && !isByRef && + CGM.isTypeConstant(Ty, true))) { EmitStaticVarDecl(D, llvm::GlobalValue::InternalLinkage); // Signal this condition to later callbacks. ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27138: Extend CompilationDatabase by a field for the output filename
joerg added inline comments. Comment at: lib/Tooling/JSONCompilationDatabase.cpp:266 +nodeToCommandLine(Syntax, std::get<2>(CommandsRef[I])), +Output ? Output->getValue(OutputStorage) : ""); } klimek wrote: > joerg wrote: > > klimek wrote: > > > joerg wrote: > > > > klimek wrote: > > > > > Optional: I'd probably let the nodeToCommandLine handle the null > > > > > value and make this code more straight forward? > > > > I couldn't find a way to create a synthetic node without changing the > > > > YAML API. > > > I'm probably missing something - why would we need a synthetic node? > > > Can't we just put nullptr into the vector? > > That's what I am doing and why this line checks output :) > Ok, let's ask differently: why is it a problem if we put a nullptr into the > array? I think it just adds unnecessary complexity. An empty file name is not a valid output, so "" vs Optional has the same result. I'd have prefered to keep this complexity inside the JSON parser, but that would have meant creating a synthetic node with value "" and there is no API for that at the moment. Repository: rL LLVM https://reviews.llvm.org/D27138 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27165: Add format_dynamic_key_arg attribute to improve "-Wformat" warnings for functions that load the formatting string dynamically based on a key value
arphaman created this revision. arphaman added reviewers: aaron.ballman, manmanren. arphaman added a subscriber: cfe-commits. arphaman set the repository for this revision to rL LLVM. This patch adds a new ``format_dynamic_key_arg``. It's intended to be used instead of ``format_arg`` for methods/functions that load the formatting string from some external source based on the given key value. The attribute tells Clang that it has to avoid ``-Wformat`` warnings when key strings have no formatting specifiers. Otherwise it tells Clang that it can assume that the key strings use the same formatting layout as the external formatting string values (i.e. Clang can perform the normal ``-Wformat`` related checks). This attribute is useful for certain Objective-C methods like ``NSBundle.localizedStringForKey`` that load the values dynamically based on a specified key value. Repository: rL LLVM https://reviews.llvm.org/D27165 Files: include/clang/Analysis/Analyses/FormatString.h include/clang/Basic/Attr.td include/clang/Basic/AttrDocs.td lib/Analysis/PrintfFormatString.cpp lib/Sema/SemaChecking.cpp lib/Sema/SemaDeclAttr.cpp test/Sema/attr-format_arg.c test/SemaObjC/format-strings-objc.m Index: test/SemaObjC/format-strings-objc.m === --- test/SemaObjC/format-strings-objc.m +++ test/SemaObjC/format-strings-objc.m @@ -302,3 +302,21 @@ } @end + +@interface FormatDynamicKeyArg + +- (NSString *)load:(NSString *)key __attribute__((format_dynamic_key_arg(1))); + +@end + +void testFormatDynamicKeyArg(FormatDynamicKeyArg *m) { + // Don't warn when the key has no formatting specifiers + NSLog([m load:@"key"], @"foo"); + NSLog([m load:@""]); + + // Warn normally when the key has formatting specifiers + NSLog([m load:@"correct %d"], 2); + NSLog([m load:@"warn %d"], "off"); // expected-warning {{format specifies type 'int' but the argument has type 'char *'}} + NSLog([m load:@"%@ %@"], @"name"); // expected-warning {{more '%' conversions than data arguments}} + NSLog([m load:@"Warn again %@"], @"name", @"void"); // expected-warning {{data argument not used by format string}} +} Index: test/Sema/attr-format_arg.c === --- test/Sema/attr-format_arg.c +++ test/Sema/attr-format_arg.c @@ -11,3 +11,19 @@ printf(f("%d"), 123); printf(f("%d %d"), 123); // expected-warning{{more '%' conversions than data arguments}} } + +const char *loadFormattingValue(const char *key) __attribute__((format_dynamic_key_arg(1))); + +void testFormatDynamicKeyArg() { + // Don't warn when the key has no formatting specifiers + printf(loadFormattingValue("key"), "foo"); + + // Warn normally when the key has formatting specifiers + printf(loadFormattingValue("%d"), 123); + printf(loadFormattingValue("%d %d"), 123); // expected-warning{{more '%' conversions than data arguments}} +} + +const char *formatKeyArgError1(const char *format) + __attribute__((format_dynamic_key_arg("foo"))); // expected-error{{'format_dynamic_key_arg' attribute requires parameter 1 to be an integer constant}} +const char *formatKeyArgError2(const char *format) + __attribute__((format_dynamic_key_arg(0))); // expected-error{{'format_dynamic_key_arg' attribute parameter 1 is out of bounds}} Index: lib/Sema/SemaDeclAttr.cpp === --- lib/Sema/SemaDeclAttr.cpp +++ lib/Sema/SemaDeclAttr.cpp @@ -2750,7 +2750,8 @@ /// Handle __attribute__((format_arg((idx attribute based on /// http://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html -static void handleFormatArgAttr(Sema &S, Decl *D, const AttributeList &Attr) { +static void handleFormatArgAttr(Sema &S, Decl *D, const AttributeList &Attr, +bool IsDynamicKey = false) { Expr *IdxExpr = Attr.getArgAsExpr(0); uint64_t Idx; if (!checkFunctionOrMethodParameterIndex(S, D, Attr, 1, IdxExpr, Idx)) @@ -2786,6 +2787,12 @@ llvm::APSInt Val; IdxExpr->EvaluateAsInt(Val, S.Context); + if (IsDynamicKey) { +D->addAttr(::new (S.Context) FormatDynamicKeyArgAttr( +Attr.getRange(), S.Context, Val.getZExtValue(), +Attr.getAttributeSpellingListIndex())); +return; + } D->addAttr(::new (S.Context) FormatArgAttr(Attr.getRange(), S.Context, Val.getZExtValue(), Attr.getAttributeSpellingListIndex())); @@ -5621,6 +5628,9 @@ case AttributeList::AT_FormatArg: handleFormatArgAttr(S, D, Attr); break; + case AttributeList::AT_FormatDynamicKeyArg: +handleFormatArgAttr(S, D, Attr, /*IsDynamicKey=*/true); +break; case AttributeList::AT_CUDAGlobal: handleGlobalAttr(S, D, Attr); break; Index: lib/Sema/SemaChecking.cpp === --- lib/Sema/SemaChecking.cpp +++ lib/Sema/SemaChecking.cpp @@ -4401,30 +4401,24 @@ }; } //
r288039 - [OPENMP] Fix for PR31137: Wrong DSA for members in struct.
Author: abataev Date: Mon Nov 28 09:55:15 2016 New Revision: 288039 URL: http://llvm.org/viewvc/llvm-project?rev=288039&view=rev Log: [OPENMP] Fix for PR31137: Wrong DSA for members in struct. If member expression is used in the task region and the base expression is a DeclRefExp and the variable used in this ref expression is private, it should be marked as implicitly firstprivate inside this region. Patch fixes this issue. Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp cfe/trunk/test/OpenMP/task_firstprivate_codegen.cpp Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=288039&r1=288038&r2=288039&view=diff == --- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original) +++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Mon Nov 28 09:55:15 2016 @@ -1556,7 +1556,8 @@ public: !Stack->isLoopControlVariable(FD).first) ImplicitFirstprivate.push_back(E); } -} +} else + Visit(E->getBase()); } void VisitOMPExecutableDirective(OMPExecutableDirective *S) { for (auto *C : S->clauses()) { Modified: cfe/trunk/test/OpenMP/task_firstprivate_codegen.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/task_firstprivate_codegen.cpp?rev=288039&r1=288038&r2=288039&view=diff == --- cfe/trunk/test/OpenMP/task_firstprivate_codegen.cpp (original) +++ cfe/trunk/test/OpenMP/task_firstprivate_codegen.cpp Mon Nov 28 09:55:15 2016 @@ -26,7 +26,7 @@ volatile double g; // CHECK-DAG: [[KMP_TASK_T_TY:%.+]] = type { i8*, i32 (i32, i8*)*, i32, %union{{.+}}, %union{{.+}} } // CHECK-DAG: [[S_DOUBLE_TY:%.+]] = type { double } // CHECK-DAG: [[PRIVATES_MAIN_TY:%.+]] = type {{.?}}{ [2 x [[S_DOUBLE_TY]]], [[S_DOUBLE_TY]], i32, [2 x i32] -// CHECK-DAG: [[CAP_MAIN_TY:%.+]] = type {{.*}}{ [2 x i32]*, i32, {{.*}}[2 x [[S_DOUBLE_TY]]]*, [[S_DOUBLE_TY]]*, i{{[0-9]+}} +// CHECK-DAG: [[CAP_MAIN_TY:%.+]] = type {{.*}}{ [[S_DOUBLE_TY]]*, [2 x i32]*, i32, {{.*}}[2 x [[S_DOUBLE_TY]]]*, i{{[0-9]+}} // CHECK-DAG: [[KMP_TASK_MAIN_TY:%.+]] = type { [[KMP_TASK_T_TY]], [[PRIVATES_MAIN_TY]] } // CHECK-DAG: [[S_INT_TY:%.+]] = type { i32 } // CHECK-DAG: [[CAP_TMAIN_TY:%.+]] = type { [2 x i32]*, i32*, [2 x [[S_INT_TY]]]*, [[S_INT_TY]]* } @@ -149,10 +149,11 @@ int main() { int vec[] = {1, 2}; S s_arr[] = {1, 2}; S var(3); -#pragma omp task firstprivate(var, t_var, s_arr, vec, s_arr, var, sivar) +#pragma omp task firstprivate(t_var, s_arr, vec, sivar) { +var.f = 1; vec[0] = t_var; -s_arr[0] = var; +s_arr[0] = S(3); sivar = 33; } return tmain(); @@ -172,15 +173,15 @@ int main() { // CHECK: call {{.*}} [[S_DOUBLE_TY_COPY_CONSTR:@.+]]([[S_DOUBLE_TY]]* [[TEST]], // Store original variables in capture struct. -// CHECK: [[VEC_REF:%.+]] = getelementptr inbounds [[CAP_MAIN_TY]], [[CAP_MAIN_TY]]* %{{.+}}, i{{[0-9]+}} 0, i{{[0-9]+}} 0 +// CHECK: [[VAR_REF:%.+]] = getelementptr inbounds [[CAP_MAIN_TY]], [[CAP_MAIN_TY]]* %{{.+}}, i{{[0-9]+}} 0, i{{[0-9]+}} 0 +// CHECK: store [[S_DOUBLE_TY]]* [[VAR_ADDR]], [[S_DOUBLE_TY]]** [[VAR_REF]], +// CHECK: [[VEC_REF:%.+]] = getelementptr inbounds [[CAP_MAIN_TY]], [[CAP_MAIN_TY]]* %{{.+}}, i{{[0-9]+}} 0, i{{[0-9]+}} 1 // CHECK: store [2 x i32]* [[VEC_ADDR]], [2 x i32]** [[VEC_REF]], -// CHECK: [[T_VAR_REF:%.+]] = getelementptr inbounds [[CAP_MAIN_TY]], [[CAP_MAIN_TY]]* %{{.+}}, i{{[0-9]+}} 0, i{{[0-9]+}} 1 +// CHECK: [[T_VAR_REF:%.+]] = getelementptr inbounds [[CAP_MAIN_TY]], [[CAP_MAIN_TY]]* %{{.+}}, i{{[0-9]+}} 0, i{{[0-9]+}} 2 // CHECK: [[T_VAR:%.+]] = load i32, i32* [[T_VAR_ADDR]], // CHECK: store i32 [[T_VAR]], i32* [[T_VAR_REF]], -// CHECK: [[S_ARR_REF:%.+]] = getelementptr inbounds [[CAP_MAIN_TY]], [[CAP_MAIN_TY]]* %{{.+}}, i{{[0-9]+}} 0, i{{[0-9]+}} 3 +// CHECK: [[S_ARR_REF:%.+]] = getelementptr inbounds [[CAP_MAIN_TY]], [[CAP_MAIN_TY]]* %{{.+}}, i{{[0-9]+}} 0, i{{[0-9]+}} 4 // CHECK: store [2 x [[S_DOUBLE_TY]]]* [[S_ARR_ADDR]], [2 x [[S_DOUBLE_TY]]]** [[S_ARR_REF]], -// CHECK: [[VAR_REF:%.+]] = getelementptr inbounds [[CAP_MAIN_TY]], [[CAP_MAIN_TY]]* %{{.+}}, i{{[0-9]+}} 0, i{{[0-9]+}} 4 -// CHECK: store [[S_DOUBLE_TY]]* [[VAR_ADDR]], [[S_DOUBLE_TY]]** [[VAR_REF]], // CHECK: [[SIVAR_REF:%.+]] = getelementptr inbounds [[CAP_MAIN_TY]], [[CAP_MAIN_TY]]* %{{.+}}, i{{[0-9]+}} 0, i{{[0-9]+}} 5 // CHECK: [[SIVAR_VAL:%.+]] = load i32, i32* [[SIVAR]], // CHECK: store i{{[0-9]+}} [[SIVAR_VAL]], i{{[0-9]+}}* [[SIVAR_REF]], @@ -208,7 +209,7 @@ int main() { // Constructors for s_arr and var. // s_arr; // CHECK: [[PRIVATE_S_ARR_REF:%.+]] = getelementptr inbounds [[PRIVATES_MAIN_TY]], [[PRIVATES_MAIN_TY]]* [[PRIVATES]], i{{[0-9]+}} 0, i{{[0-9]+}} 0 -// CHECK: [[S_ARR_ADDR_REF:%.+]] = getelementptr inbounds [[CAP_MAIN_TY]], [[CAP_MAIN_TY]]* [[SHAREDS]], i{{.+}} 0, i{{.+}} 3 +// CHECK: [[S_ARR_A
[PATCH] D26196: Add support for non-zero null pointer for C and OpenCL
yaxunl updated the summary for this revision. yaxunl updated this revision to Diff 79406. yaxunl added a comment. Revised by John's comments. Emit null pointer in target-specific representation in folded constant expressions. Fix casting null pointer to integer in constant expressions. https://reviews.llvm.org/D26196 Files: include/clang/AST/APValue.h include/clang/AST/ASTContext.h include/clang/Basic/TargetInfo.h lib/AST/APValue.cpp lib/AST/ASTContext.cpp lib/AST/ExprConstant.cpp lib/Basic/Targets.cpp lib/CodeGen/CGDecl.cpp lib/CodeGen/CGExprAgg.cpp lib/CodeGen/CGExprConstant.cpp lib/CodeGen/CGExprScalar.cpp lib/CodeGen/CodeGenModule.h lib/CodeGen/CodeGenTypes.cpp lib/CodeGen/TargetInfo.cpp lib/CodeGen/TargetInfo.h test/CodeGenOpenCL/amdgpu-nullptr.cl Index: test/CodeGenOpenCL/amdgpu-nullptr.cl === --- /dev/null +++ test/CodeGenOpenCL/amdgpu-nullptr.cl @@ -0,0 +1,527 @@ +// RUN: %clang_cc1 %s -cl-std=CL2.0 -include opencl-c.h -triple amdgcn -fno-common -emit-llvm -o - | FileCheck %s +// RUN: %clang_cc1 %s -O0 -cl-std=CL2.0 -include opencl-c.h -triple amdgcn -fno-common -emit-llvm -o - | FileCheck --check-prefix=NOOPT %s + +typedef struct { + private char *p1; + local char *p2; + constant char *p3; + global char *p4; + generic char *p5; +} StructTy1; + +typedef struct { + constant char *p3; + global char *p4; + generic char *p5; +} StructTy2; + +// LLVM requests global variable with common linkage to be initialized with zeroinitializer, therefore use -fno-common +// to suppress common linkage for tentative definition. + +// Test 0 as initializer. + +// CHECK: @private_p = local_unnamed_addr addrspace(1) global i8* addrspacecast (i8 addrspace(4)* null to i8*), align 4 +private char *private_p = 0; + +// CHECK: @local_p = local_unnamed_addr addrspace(1) global i8 addrspace(3)* addrspacecast (i8 addrspace(4)* null to i8 addrspace(3)*), align 4 +local char *local_p = 0; + +// CHECK: @global_p = local_unnamed_addr addrspace(1) global i8 addrspace(1)* null, align 4 +global char *global_p = 0; + +// CHECK: @constant_p = local_unnamed_addr addrspace(1) global i8 addrspace(2)* null, align 4 +constant char *constant_p = 0; + +// CHECK: @generic_p = local_unnamed_addr addrspace(1) global i8 addrspace(4)* null, align 4 +generic char *generic_p = 0; + +// Test NULL as initializer. + +// CHECK: @private_p_NULL = local_unnamed_addr addrspace(1) global i8* addrspacecast (i8 addrspace(4)* null to i8*), align 4 +private char *private_p_NULL = NULL; + +// CHECK: @local_p_NULL = local_unnamed_addr addrspace(1) global i8 addrspace(3)* addrspacecast (i8 addrspace(4)* null to i8 addrspace(3)*), align 4 +local char *local_p_NULL = NULL; + +// CHECK: @global_p_NULL = local_unnamed_addr addrspace(1) global i8 addrspace(1)* null, align 4 +global char *global_p_NULL = NULL; + +// CHECK: @constant_p_NULL = local_unnamed_addr addrspace(1) global i8 addrspace(2)* null, align 4 +constant char *constant_p_NULL = NULL; + +// CHECK: @generic_p_NULL = local_unnamed_addr addrspace(1) global i8 addrspace(4)* null, align 4 +generic char *generic_p_NULL = NULL; + +// Test constant folding of null pointer. +// A null pointer should be folded to a null pointer in the target address space. + +// CHECK: @fold_generic = local_unnamed_addr addrspace(1) global i32 addrspace(4)* null, align 4 +generic int *fold_generic = (global int*)(generic float*)(private char*)0; + +// CHECK: @fold_priv = local_unnamed_addr addrspace(1) global i16* addrspacecast (i16 addrspace(4)* null to i16*), align 4 +private short *fold_priv = (private short*)(generic int*)(global void*)0; + +// CHECK: @fold_priv_arith = local_unnamed_addr addrspace(1) global i8* inttoptr (i32 9 to i8*), align 4 +private char *fold_priv_arith = (private char*)0 + 10; + +// CHECK: @fold_int = local_unnamed_addr addrspace(1) global i32 13, align 4 +int fold_int = (int)(private void*)(generic char*)(global int*)0 + 14; + +// CHECK: @fold_int2 = local_unnamed_addr addrspace(1) global i32 12, align 4 +int fold_int2 = (int) ((private void*)0 + 13); + +// CHECK: @fold_int3 = local_unnamed_addr addrspace(1) global i32 -1, align 4 +int fold_int3 = (int) ((private int*)0); + +// CHECK: @fold_int4 = local_unnamed_addr addrspace(1) global i32 7, align 4 +int fold_int4 = (int) &((private int*)0)[2]; + +// CHECK: @fold_int5 = local_unnamed_addr addrspace(1) global i32 3, align 4 +int fold_int5 = (int) &((private StructTy1*)0)->p2; + +// Test static variable initialization. + +// NOOPT: @test_static_var.sp1 = internal addrspace(1) global i8* addrspacecast (i8 addrspace(4)* null to i8*), align 4 +// NOOPT: @test_static_var.sp2 = internal addrspace(1) global i8* addrspacecast (i8 addrspace(4)* null to i8*), align 4 +// NOOPT: @test_static_var.sp3 = internal addrspace(1) global i8* addrspacecast (i8 addrspace(4)* null to i8*), align 4 +// NOOPT: @test_static_var.sp4 = internal addrspace(1) global i8* nul
[PATCH] D26672: Avoid -Wshadow warnings for parameters that shadow fields in setter-like methods
arphaman added a comment. Ping Repository: rL LLVM https://reviews.llvm.org/D26672 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27109: [OpenCL] Prevent generation of globals in non-constant address space for OpenCL
yaxunl accepted this revision. yaxunl added a comment. This revision is now accepted and ready to land. LGTM. Thanks! https://reviews.llvm.org/D27109 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27166: [clang-tidy] Enhance modernize-use-auto to templated function casts
malcolm.parsons created this revision. malcolm.parsons added reviewers: aaron.ballman, Prazek, alexfh. malcolm.parsons added a subscriber: cfe-commits. Herald added a subscriber: JDevlieghere. Use auto when declaring variables that are initialized by calling a templated function that returns its explicit first argument. Fixes PR26763. https://reviews.llvm.org/D27166 Files: clang-tidy/modernize/UseAutoCheck.cpp docs/clang-tidy/checks/modernize-use-auto.rst test/clang-tidy/modernize-use-auto-cast-remove-stars.cpp test/clang-tidy/modernize-use-auto-cast.cpp Index: test/clang-tidy/modernize-use-auto-cast.cpp === --- test/clang-tidy/modernize-use-auto-cast.cpp +++ test/clang-tidy/modernize-use-auto-cast.cpp @@ -138,3 +138,52 @@ B b; A a = A(b); } + +template +T template_value_cast(const U &u); + +template +T *template_pointer_cast(U *u); + +template +T &template_reference_cast(U &u); + +template +const T *template_const_pointer_cast(const U *u); + +template +const T &template_const_reference_cast(const U &u); + +template +T max(T t1, T t2); + +void f_template_cast() +{ + double d = 0; + int i1 = template_value_cast(d); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a template cast to avoid duplicating the type name + // CHECK-FIXES: auto i1 = template_value_cast(d); + + A *a = new B(); + B *b1 = template_value_cast(a); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a template cast to avoid duplicating the type name + // CHECK-FIXES: auto *b1 = template_value_cast(a); + B &b2 = template_value_cast(*a); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a template cast to avoid duplicating the type name + // CHECK-FIXES: auto &b2 = template_value_cast(*a); + B *b3 = template_pointer_cast(a); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a template cast to avoid duplicating the type name + // CHECK-FIXES: auto *b3 = template_pointer_cast(a); + B &b4 = template_reference_cast(*a); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a template cast to avoid duplicating the type name + // CHECK-FIXES: auto &b4 = template_reference_cast(*a); + const B *b5 = template_const_pointer_cast(a); + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use auto when initializing with a template cast to avoid duplicating the type name + // CHECK-FIXES: const auto *b5 = template_const_pointer_cast(a); + const B &b6 = template_const_reference_cast(*a); + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use auto when initializing with a template cast to avoid duplicating the type name + // CHECK-FIXES: const auto &b6 = template_const_reference_cast(*a); + + auto i2 = template_value_cast(d); + int i3 = max(i1, i2); +} Index: test/clang-tidy/modernize-use-auto-cast-remove-stars.cpp === --- test/clang-tidy/modernize-use-auto-cast-remove-stars.cpp +++ test/clang-tidy/modernize-use-auto-cast-remove-stars.cpp @@ -136,3 +136,52 @@ B b; A a = A(b); } + +template +T template_value_cast(const U &u); + +template +T *template_pointer_cast(U *u); + +template +T &template_reference_cast(U &u); + +template +const T *template_const_pointer_cast(const U *u); + +template +const T &template_const_reference_cast(const U &u); + +template +T max(T t1, T t2); + +void f_template_cast() +{ + double d = 0; + int i1 = template_value_cast(d); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a template cast to avoid duplicating the type name + // CHECK-FIXES: auto i1 = template_value_cast(d); + + A *a = new B(); + B *b1 = template_value_cast(a); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a template cast to avoid duplicating the type name + // CHECK-FIXES: auto b1 = template_value_cast(a); + B &b2 = template_value_cast(*a); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a template cast to avoid duplicating the type name + // CHECK-FIXES: auto &b2 = template_value_cast(*a); + B *b3 = template_pointer_cast(a); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a template cast to avoid duplicating the type name + // CHECK-FIXES: auto b3 = template_pointer_cast(a); + B &b4 = template_reference_cast(*a); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a template cast to avoid duplicating the type name + // CHECK-FIXES: auto &b4 = template_reference_cast(*a); + const B *b5 = template_const_pointer_cast(a); + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use auto when initializing with a template cast to avoid duplicating the type name + // CHECK-FIXES: const auto b5 = template_const_pointer_cast(a); + const B &b6 = template_const_reference_cast(*a); + // CHECK-MESSAGES: :[[@L
[PATCH] D26904: [astimporter] Support importing CXXDependentScopeMemberExpr and FunctionTemplateDecl
a.sidorin added a comment. Hello Kareem. There are some tests in "class-template" dir that may serve as example. You don't need a warning for a function that was found: you should just return found declaration. There is already a warning for a declaration that is defined in multiple TUs. You can run these tests to see them. https://reviews.llvm.org/D26904 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27167: [libc++] Support multibyte decimal_point and thousands_sep
vangyzen created this revision. vangyzen added a subscriber: cfe-commits. Herald added a subscriber: emaste. Herald added a reviewer: EricWF. numpunct_byname assumed that decimal_point and thousands_sep were ASCII and simply copied the first byte from them. Add support for multibyte strings in these fields. I found this problem on FreeBSD 11, where thousands_sep in fr_FR.UTF-8 is a no-break space (U+00A0). https://reviews.llvm.org/D27167 Files: src/locale.cpp Index: src/locale.cpp === --- src/locale.cpp +++ src/locale.cpp @@ -4281,23 +4281,51 @@ { } +#if defined(__clang__) +#pragma clang diagnostic ignored "-Wmissing-braces" +#endif + void numpunct_byname::__init(const char* nm) { if (strcmp(nm, "C") != 0) { __locale_unique_ptr loc(newlocale(LC_ALL_MASK, nm, 0), freelocale); if (loc == nullptr) -__throw_runtime_error("numpunct_byname::numpunct_byname" -" failed to construct for " + string(nm)); +__throw_runtime_error("numpunct_byname::numpunct_byname" + " failed to construct for " + string(nm)); lconv* lc = __libcpp_localeconv_l(loc.get()); -if (*lc->decimal_point) -__decimal_point_ = *lc->decimal_point; -if (*lc->thousands_sep) -__thousands_sep_ = *lc->thousands_sep; +if (*lc->decimal_point) { +size_t len = strlen(lc->decimal_point); +mbstate_t mbs = {0}; +wchar_t wc; +size_t nb = __libcpp_mbrtowc_l(&wc, lc->decimal_point, len, &mbs, +loc.get()); +if (nb == len) { +__decimal_point_ = wc; +} else { +__throw_runtime_error("numpunct_byname: decimal_point" + " is not a valid multibyte character: " + + string(lc->decimal_point)); +} +} +if (*lc->thousands_sep) { +size_t len = strlen(lc->thousands_sep); +mbstate_t mbs = {0}; +wchar_t wc; +size_t nb = __libcpp_mbrtowc_l(&wc, lc->thousands_sep, len, &mbs, +loc.get()); +if (nb == len) { +__thousands_sep_ = wc; +} else { +__throw_runtime_error("numpunct_byname: thousands_sep" + " is not a valid multibyte character: " + + string(lc->thousands_sep)); +} +} __grouping_ = lc->grouping; -// locallization for truename and falsename is not available +// localization for truename and falsename is not available } } @@ -4861,10 +4889,6 @@ return result; } -#if defined(__clang__) -#pragma clang diagnostic ignored "-Wmissing-braces" -#endif - template <> wstring __time_get_storage::__analyze(char fmt, const ctype& ct) Index: src/locale.cpp === --- src/locale.cpp +++ src/locale.cpp @@ -4281,23 +4281,51 @@ { } +#if defined(__clang__) +#pragma clang diagnostic ignored "-Wmissing-braces" +#endif + void numpunct_byname::__init(const char* nm) { if (strcmp(nm, "C") != 0) { __locale_unique_ptr loc(newlocale(LC_ALL_MASK, nm, 0), freelocale); if (loc == nullptr) -__throw_runtime_error("numpunct_byname::numpunct_byname" -" failed to construct for " + string(nm)); +__throw_runtime_error("numpunct_byname::numpunct_byname" + " failed to construct for " + string(nm)); lconv* lc = __libcpp_localeconv_l(loc.get()); -if (*lc->decimal_point) -__decimal_point_ = *lc->decimal_point; -if (*lc->thousands_sep) -__thousands_sep_ = *lc->thousands_sep; +if (*lc->decimal_point) { +size_t len = strlen(lc->decimal_point); +mbstate_t mbs = {0}; +wchar_t wc; +size_t nb = __libcpp_mbrtowc_l(&wc, lc->decimal_point, len, &mbs, +loc.get()); +if (nb == len) { +__decimal_point_ = wc; +} else { +__throw_runtime_error("numpunct_byname: decimal_point" + " is not a valid multibyte character: " + + string(lc->decimal_point)); +} +} +if (*lc->thousands_sep) { +size_t len = strlen(lc->thousands_sep); +mbstate_t mbs = {0}; +wchar_t wc; +size_t nb = __libcpp_mbrtowc_l(&wc, lc->thousands_sep, len, &mbs, +loc.get()); +if (nb == len) { +__thousands_sep_ = wc; +} else { +__throw_runtime_error("numpunct_byname: thousands_sep
[PATCH] D26764: [compiler-rt] [cmake] Remove i686 target that is duplicate to i386
beanz added a reviewer: eugenis. beanz added a subscriber: eugenis. beanz added a comment. Looping in @eugenis, who added i686 support in r218761. eugenis, since i686 is identical to i386 generating it as a separate target is undesirable. Can you help advise as to how we can better meet your needs and not duplicate an effectively identical target? https://reviews.llvm.org/D26764 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26979: Do not hard-code locale data in unit tests: get it from the OS instead
vangyzen added inline comments. Comment at: projects/libcxx/test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/thousands_sep.pass.cpp:79 const std::numpunct& np = std::use_facet >(l); -assert(np.thousands_sep() == L','); +assert(np.thousands_sep() == wexpected); } vangyzen wrote: > This assertion now fails for me. wexpected is 0xa0 (NBSP), which is correct. > However, np.thousands_sep() is -62, which is 0xc2, which is the first byte > of a UTF-8-encoded NBSP. It looks like the library isn't correctly handling > multibyte thousands separators. D27167 adds support for multibyte strings in decimal_point and thousands_sep. With that change, this unit test passes. https://reviews.llvm.org/D26979 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r288043 - [include-fixer] Don't interfere with typo correction if we found nothing.
Author: d0k Date: Mon Nov 28 11:16:18 2016 New Revision: 288043 URL: http://llvm.org/viewvc/llvm-project?rev=288043&view=rev Log: [include-fixer] Don't interfere with typo correction if we found nothing. Just let the existing typo correction machinery handle that. Modified: clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp clang-tools-extra/trunk/test/include-fixer/yamldb_plugin.cpp Modified: clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp?rev=288043&r1=288042&r2=288043&view=diff == --- clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp (original) +++ clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp Mon Nov 28 11:16:18 2016 @@ -279,9 +279,9 @@ clang::TypoCorrection IncludeFixerSemaSo std::vector MatchedSymbols = query(QueryString, TypoScopeString, SymbolRange); - clang::TypoCorrection Correction(Typo.getName()); - Correction.setCorrectionRange(SS, Typo); if (!MatchedSymbols.empty() && GenerateDiagnostics) { +TypoCorrection Correction(Typo.getName()); +Correction.setCorrectionRange(SS, Typo); FileID FID = SM.getFileID(Typo.getLoc()); StringRef Code = SM.getBufferData(FID); SourceLocation StartOfFile = SM.getLocForStartOfFile(FID); @@ -290,8 +290,9 @@ clang::TypoCorrection IncludeFixerSemaSo getIncludeFixerContext(SM, CI->getPreprocessor().getHeaderSearchInfo(), MatchedSymbols), Code, StartOfFile, CI->getASTContext()); +return Correction; } - return Correction; + return TypoCorrection(); } /// Get the minimal include for a given path. Modified: clang-tools-extra/trunk/test/include-fixer/yamldb_plugin.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/include-fixer/yamldb_plugin.cpp?rev=288043&r1=288042&r2=288043&view=diff == --- clang-tools-extra/trunk/test/include-fixer/yamldb_plugin.cpp (original) +++ clang-tools-extra/trunk/test/include-fixer/yamldb_plugin.cpp Mon Nov 28 11:16:18 2016 @@ -10,8 +10,7 @@ unknown u; // CHECK: Number FIX-ITs = 1 // CHECK: FIX-IT: Replace [3:1 - 3:4] with "#include "foo.h" // CHECK: yamldb_plugin.cpp:4:1: -// CHECK: error: unknown type name 'unknown'; did you mean 'unknown'? -// CHECK: Number FIX-ITs = 1 -// CHECK: FIX-IT: Replace [4:1 - 4:8] with "unknown" +// CHECK: error: unknown type name 'unknown' +// CHECK: Number FIX-ITs = 0 // CHECK-NOT: error // CHECK-NOT: FIX-IT ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26335: [ms] Reinstate https://reviews.llvm.org/D14748 after https://reviews.llvm.org/D20291
hans added inline comments. Comment at: lib/Headers/x86intrin.h:49 +static __inline__ unsigned int __attribute__((__always_inline__, __nodebug__)) +__tzcnt_u32(unsigned int __X) { return __X ? __builtin_ctz(__X) : 32; } +#ifdef __x86_64__ andreadb wrote: > hans wrote: > > probinson wrote: > > > hans wrote: > > > > I'm worried about the conditional here. IIRC, ffmpeg uses TZCNT just as > > > > a faster encoding for BSF, but now we're putting a conditional in the > > > > way, so this will be slower actually. On the other hand, the > > > > alternative is weird too :-/ > > > I thought there was a peephole to notice a guard like this and do the > > > right thing? In which case having the guard is fine. > > But for non-BMI targets it won't be able to remove the guard (unless it can > > prove the argument is zero). > > > > MSVC will just emit the raw 'tzcnt' instruction here, and that's what > > ffmpeg wants. > I understand your point. However, the semantic of tzcnt_u32 (at least for > BMI) is well defined on zero input. Changing that semantic for non-BMI > targets sounds a bit odd/confusing to me. > > I guess ffmpeg is reliant on the fact that other compilers would either > preserve the intrinsic call until instruction selection, or fold it to a > meaningful value (i.e. 32 in this case) when the input is known to be zero. > If we remove the zero check from tzcnt_u32 (for non BMI targets), we let the > optimizer enable rules to aggressively fold calls to tzcnt_u32 to undef when > the input is zero. > > As a side note: I noticed that gcc provides intrinsic `__bsfd` in > ia32intrin.h. Maybe we should do something similar? Yes, ffmpeg relies on the compiler to emit the raw TZCNT instruction even for non-BMI targets and don't call it with zero inputs. They just want a faster BSF. It's all pretty weird, but maybe Nico's original patch is the way to do this after all. At least then, the intrinsic will always do what it says in the doc. https://reviews.llvm.org/D26335 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27163: Introduce -f[no-]strict-return flag that controls code generation for C++'s undefined behaviour return optimisation
mehdi_amini added a comment. What is the justification for a platform specific default change here? Repository: rL LLVM https://reviews.llvm.org/D27163 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: Upgrade and fix clang-format-vs
On Fri, Nov 25, 2016 at 6:58 PM, Antonio Maiorano wrote: > Ah, no, that's not what I meant. The required referenced assemblies are > versions that are normally installed with VS 2010. > > The first time I worked on this, I had upgraded the referenced assemblies to > the ones that ship with VS 2015, but then there was question of whether or > not the VSIX would continue to work with VS 2010/2012/2013. I'm not sure if > it would work, but I guess I can try to figure that out. Let me know if you figure this one out. It sounds like it would simplify things a lot. > In any case, what I discovered is that the "right" way to do things to make > sure your extension compiles in future versions of VS is to use NuGet to > automatically pull in the required assemblies, or to check them in and > reference them directly. The former would be better except for the problem > of CLI builds as I described in my earlier email. > > > > On Fri, 25 Nov 2016 at 21:47 Zachary Turner wrote: >> >> Sorry, i think I misunderstood the original option 1. I interpreted it as >> just committing changes to the vsix manifest to reference a specific version >> of the assembly which we assume to be present since it should be >> automatically installed with vs 2015. Is this not possible? Can't we just >> point the manifest to the version installed with vs? >> On Fri, Nov 25, 2016 at 6:20 PM Antonio Maiorano >> wrote: >>> >>> Hi again, >>> >>> I've made the changes so that the required assemblies are committed, so >>> now we can build the clang-format-vsix with just VS 2015. Since the patch >>> set is around 9 mb, I'm providing a link to it on my Dropbox (if you'd >>> rather I attach it, let me know): >>> >>> >>> https://dl.dropboxusercontent.com/u/10504225/llvm-patches/0001-Fix-VS2015-build-of-clang-format-vsix-by-committing-.patch >>> >>> Note that it's a git patch set, for which I followed the instructions >>> here. >>> >>> Thanks. >>> >>> On Thu, 24 Nov 2016 at 15:45 Antonio Maiorano >>> wrote: Okay, that's fine, I'll go for that and if all looks good, will attach a patch. Thanks. On Thu, 24 Nov 2016 at 15:09 Zachary Turner wrote: > > I would use the first solution. We lock ourselves to specific versions > of vs, so i think it's fine to do the same with the assemblies and deal > with > it only when we upgrade > On Thu, Nov 24, 2016 at 11:29 AM Antonio Maiorano > wrote: >> >> Hi Hans, >> >> I saw that on September 15th, you checked in a change: clang-format VS >> plugin: upgrade the project files to VS2015. >> >> When I open the latest version of ClangFormat.sln on a machine that >> has only VS 2015, it doesn't build. The reason is that some of the >> referenced assemblies are from VS 2010. >> >> > Include="Microsoft.VisualStudio.Editor, Version=10.0.0.0, >> Culture=neutral, >> PublicKeyToken=b03f5f7f11d50a3a, processorArchitecture=MSIL" /> >> >> What happens is that when building, these specific assemblies are not >> found. I suspect you have VS 2010 installed on your machine, which is why >> you don't get these build errors. >> >> The recommended way to handle this is to make use of NuGet to have it >> automatically download the required assemblies. I have made the changes >> locally to get this working, and it works great when building >> ClangFormat.sln from within Visual Studio; however, building from the CLI >> doesn't work out of the box because you must explicitly run 'nuget.exe >> restore ClangFormat.sln' before running msbuild (or devenv.exe in our >> case). >> The problem is that nuget.exe isn't something that's easily >> found/accessible >> on Windows, even once installed as an extension in VS. This is a known >> problem and the current best solution requires downloading and making >> nuget.exe available in PATH. >> >> So now i'm faced with figuring out how best to solve this so that the >> custom build step in this CMakeLists.txt that runs devenv doesn't fail: >> >> devenv "${CMAKE_CURRENT_SOURCE_DIR}/ClangFormat.sln" /Build Release >> >> There are a few options: >> >> 1) Forget NuGet and just commit the referenced assemblies. This is the >> simplest solution, but is more annoying to manage if we want to upgrade >> the >> versions of these assemblies later. >> >> 2) Commit nuget.exe to the repo and just use it. This is simple >> enough, but I'm not sure how people feel about committing binaries, and >> it >> would be frozen at whatever version we commit. >> >> 3) Do some form of wget to grab the latest nuget.exe from >> "https://nuget.org/nuget.exe"; in CMake and invoke it. I'm not yet sure >> what's the simplest way to do this. Powershell could be used, but there >> are >> security annoyances to deal with. >> >> That's all I can c
[PATCH] D26991: Hoist redundant load
sebpop added a comment. Let's also add a testcase and show the performance improvement. https://reviews.llvm.org/D26991 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26764: [compiler-rt] [cmake] Remove i686 target that is duplicate to i386
mgorny added a comment. In https://reviews.llvm.org/D26764#606638, @beanz wrote: > Looping in @eugenis, who added i686 support in r218761. > > eugenis, since i686 is identical to i386 generating it as a separate target > is undesirable. Can you help advise as to how we can better meet your needs > and not duplicate an effectively identical target? I suspect https://reviews.llvm.org/D26796 attempts to solve the same problem. https://reviews.llvm.org/D26764 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27068: Improve string::find
sebpop added a comment. Please also post the performance numbers from the added benchmarks with and without the change to the lib. Comment at: libcxx/benchmarks/string.bench.cpp:12 +// until the end of s1. +static void BM_StringFindPhase1(benchmark::State& state) { + // Benchmark following the length of s1. Let's call this benchmark "BM_StringFindNoMatch" Comment at: libcxx/benchmarks/string.bench.cpp:21 + +// Benchmark the __phase2 part of string search: we want the strings s1 and s2 +// to match from the first char in s1. Please reword to remove the reference to __phase2. Comment at: libcxx/benchmarks/string.bench.cpp:23 +// to match from the first char in s1. +static void BM_StringFindPhase2(benchmark::State& state) { + std::string s1(MAX_STRING_LEN, '-'); Let's call this benchmark "BM_StringFindAllMatch" https://reviews.llvm.org/D27068 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27163: Introduce -f[no-]strict-return flag that controls code generation for C++'s undefined behaviour return optimisation
arphaman added a comment. In https://reviews.llvm.org/D27163#606695, @mehdi_amini wrote: > What is the justification for a platform specific default change here? The flag itself is platform agnostic, however, the default value is different on Darwin (-fno-strict-return). The reason for this difference is because of how I interpreted the internal discussion for this issue: it seems that some of our internal builds had problems with this flag, so to me it seemed that people would've wanted this specific difference upstream. I might be wrong though and this might be not even the best approach, so let me know if you think there's a better solution. Repository: rL LLVM https://reviews.llvm.org/D27163 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27140: Allow clang to write compilation database records
joerg removed rL LLVM as the repository for this revision. joerg updated this revision to Diff 79423. joerg added a comment. Use llvm::yaml::escape. https://reviews.llvm.org/D27140 Files: include/clang/Driver/Options.td lib/Driver/Tools.cpp Index: include/clang/Driver/Options.td === --- include/clang/Driver/Options.td +++ include/clang/Driver/Options.td @@ -266,6 +266,8 @@ MetaVarName<"">; def MG : Flag<["-"], "MG">, Group, Flags<[CC1Option]>, HelpText<"Add missing headers to depfile">; +def MJ : JoinedOrSeparate<["-"], "MJ">, Group, +HelpText<"Write a compilation database entry per input">; def MP : Flag<["-"], "MP">, Group, Flags<[CC1Option]>, HelpText<"Create phony target for each dependency (other than main file)">; def MQ : JoinedOrSeparate<["-"], "MQ">, Group, Flags<[CC1Option]>, Index: lib/Driver/Tools.cpp === --- lib/Driver/Tools.cpp +++ lib/Driver/Tools.cpp @@ -43,6 +43,7 @@ #include "llvm/Support/raw_ostream.h" #include "llvm/Support/ScopedPrinter.h" #include "llvm/Support/TargetParser.h" +#include "llvm/Support/YAMLParser.h" #ifdef LLVM_ON_UNIX #include // For getuid(). @@ -4002,6 +4003,64 @@ CmdArgs.push_back("-KPIC"); } +static void QuoteJSONString(llvm::raw_fd_ostream &Stream, StringRef Str) { + Stream << "\""; + Stream << llvm::yaml::escape(Str); + Stream << "\""; +} + +static void DumpCompilationDatabase(const Driver &D, StringRef Filename, StringRef Target, const InputInfo &Output, +const InputInfo &Input, const ArgList &Args) { + std::error_code EC; + llvm::raw_fd_ostream File(Filename, EC, llvm::sys::fs::F_Text | llvm::sys::fs::F_Append); + if (EC) { +//errs() << "Failed to open " << Filename << ": " << EC.message() << "\n" ; +return; + } + SmallString<128> Buf; + if (llvm::sys::fs::current_path(Buf)) +Buf = "."; + File << "{ \"directory\": "; + QuoteJSONString(File, Buf); + File << ", \"file\": "; + QuoteJSONString(File, Input.getFilename()); + File << ", \"output\": "; + QuoteJSONString(File, Output.getFilename()); + + File << ", \"arguments\": ["; + QuoteJSONString(File, D.ClangExecutable); + File << ", "; + Buf = "-x"; + Buf += types::getTypeName(Input.getType()); + QuoteJSONString(File, Buf); + File << ", "; + QuoteJSONString(File, Input.getFilename()); + for (auto &A: Args) { +auto &O = A->getOption(); +// Skip language selection, which is positional. +if (O.getID() == options::OPT_x) + continue; +// Skip writing dependency output and the compiliation database itself. +if (O.getGroup().isValid() && O.getGroup().getID() == options::OPT_M_Group) + continue; +// Skip inputs. +if (O.getKind() == Option::InputClass) + continue; +// All other arguments are quoted and appended. +ArgStringList ASL; +A->render(Args, ASL); +for (auto &it: ASL) { + File << ", "; + QuoteJSONString(File, it); +} + } + File << ", "; + Buf = "--target="; + Buf += Target; + QuoteJSONString(File, Buf); + File << "]},\n"; +} + void Clang::ConstructJob(Compilation &C, const JobAction &JA, const InputInfo &Output, const InputInfoList &Inputs, const ArgList &Args, const char *LinkingOutput) const { @@ -4046,6 +4105,10 @@ CmdArgs.push_back("-triple"); CmdArgs.push_back(Args.MakeArgString(TripleStr)); + if (const Arg *MJ = Args.getLastArg(options::OPT_MJ)) +DumpCompilationDatabase(C.getDriver(), MJ->getValue(), TripleStr, Output, Input, Args); + Args.ClaimAllArgs(options::OPT_MJ); + if (IsCuda) { // We have to pass the triple of the host if compiling for a CUDA device and // vice-versa. ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26137: [clang-tidy] Add check name to YAML export
alexfh added a comment. I'll try to get back to this code review soon. Sorry for the delay. Repository: rL LLVM https://reviews.llvm.org/D26137 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27068: Improve string::find
mclow.lists added a comment. Definitely want to see numbers. https://reviews.llvm.org/D27068 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26764: [compiler-rt] [cmake] Remove i686 target that is duplicate to i386
eugenis added a comment. I think this is the right move together with https://reviews.llvm.org/D26796. Android build system will need to support both names for some time, depending on the toolchain version - looks doable. Technically, it can stick with the old name forever. https://reviews.llvm.org/D26764 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26991: Hoist redundant load
mclow.lists added a comment. /me wonders what the perf difference when `_LIBCPP_UNROLL_LOOPS` is defined or not. I think this (`_LIBCPP_UNROLL_LOOPS`) falls squarely into Chandler's request that we complain to him when the compiler generates sub-optimal code, instead of doing things like manually unrolling loops. https://reviews.llvm.org/D26991 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26991: Hoist redundant load
mclow.lists added a comment. __search is the only place where `_LIBCPP_UNROLL_LOOPS` is currently used. https://reviews.llvm.org/D26991 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26896: [libcxx] Make constexpr char_traits and char_traits
mclow.lists added inline comments. Comment at: include/__string:213 -static inline int compare(const char_type* __s1, const char_type* __s2, size_t __n) _NOEXCEPT -{return __n == 0 ? 0 : memcmp(__s1, __s2, __n);} -static inline size_t length(const char_type* __s) _NOEXCEPT {return strlen(__s);} -static inline const char_type* find(const char_type* __s, size_t __n, const char_type& __a) _NOEXCEPT -{return __n == 0 ? NULL : (const char_type*) memchr(__s, to_int_type(__a), __n);} +#if _LIBCPP_STD_VER > 14 && !defined(_LIBCPP_HAS_NO_CXX14_CONSTEXPR) +static inline constexpr int AntonBikineev wrote: > EricWF wrote: > > wow. This is #ifdef hell. Please find a way to do it with less (or > > hopefully no) conditional compilation blocks. > yep, this is generic hell. I want to cover as many cases as possible, i.e. > combinations of (is_constexpr x has_builtin_xxx) for every function. I'm open > to suggestions How about (for compare, say) you just forget about `memcmp`. Either call `__builtin_memcmp` if it is available, or use a hand-rolled loop. Note: gcc has had `__builtin_memcmp` since at least 4.8. (and it is constexpr) And just mark the function with `_LIBCPP_CONSTEXPR_AFTER_CXX14`. https://reviews.llvm.org/D26896 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27096: Protect locale tests under noexceptions
mclow.lists accepted this revision. mclow.lists added a comment. This revision is now accepted and ready to land. This LGTM. https://reviews.llvm.org/D27096 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27166: [clang-tidy] Enhance modernize-use-auto to templated function casts
Eugene.Zelenko added a comment. It'll be worth to mention enhancement in Release Notes. https://reviews.llvm.org/D27166 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: Upgrade and fix clang-format-vs
On Mon, Nov 28, 2016 at 10:46 AM, Antonio Maiorano wrote: > Okay, I'll see if upgrading to the 2015 assemblies would allow the VSIX to > keep working in older versions of VS. > > Still waiting on an answer to this question: > >> In either case, though, I must ask: how is the offical vsix that's >> available on http://llvm.org/builds/ get built? Is it part of an automated >> Clang build, or is it built and uploaded manually? If it's automated, then >> having to download and point to nuget.exe won't work. It's built with the script in utils/release/build_llvm_package.bat which I run manually on my machine once every few weeks. > On Mon, 28 Nov 2016 at 13:04 Hans Wennborg wrote: >> >> On Fri, Nov 25, 2016 at 6:58 PM, Antonio Maiorano >> wrote: >> > Ah, no, that's not what I meant. The required referenced assemblies are >> > versions that are normally installed with VS 2010. >> > >> > The first time I worked on this, I had upgraded the referenced >> > assemblies to >> > the ones that ship with VS 2015, but then there was question of whether >> > or >> > not the VSIX would continue to work with VS 2010/2012/2013. I'm not sure >> > if >> > it would work, but I guess I can try to figure that out. >> >> Let me know if you figure this one out. It sounds like it would >> simplify things a lot. >> >> > In any case, what I discovered is that the "right" way to do things to >> > make >> > sure your extension compiles in future versions of VS is to use NuGet to >> > automatically pull in the required assemblies, or to check them in and >> > reference them directly. The former would be better except for the >> > problem >> > of CLI builds as I described in my earlier email. >> > >> > >> > >> > On Fri, 25 Nov 2016 at 21:47 Zachary Turner wrote: >> >> >> >> Sorry, i think I misunderstood the original option 1. I interpreted it >> >> as >> >> just committing changes to the vsix manifest to reference a specific >> >> version >> >> of the assembly which we assume to be present since it should be >> >> automatically installed with vs 2015. Is this not possible? Can't we >> >> just >> >> point the manifest to the version installed with vs? >> >> On Fri, Nov 25, 2016 at 6:20 PM Antonio Maiorano >> >> wrote: >> >>> >> >>> Hi again, >> >>> >> >>> I've made the changes so that the required assemblies are committed, >> >>> so >> >>> now we can build the clang-format-vsix with just VS 2015. Since the >> >>> patch >> >>> set is around 9 mb, I'm providing a link to it on my Dropbox (if you'd >> >>> rather I attach it, let me know): >> >>> >> >>> >> >>> >> >>> https://dl.dropboxusercontent.com/u/10504225/llvm-patches/0001-Fix-VS2015-build-of-clang-format-vsix-by-committing-.patch >> >>> >> >>> Note that it's a git patch set, for which I followed the instructions >> >>> here. >> >>> >> >>> Thanks. >> >>> >> >>> On Thu, 24 Nov 2016 at 15:45 Antonio Maiorano >> >>> wrote: >> >> Okay, that's fine, I'll go for that and if all looks good, will >> attach a >> patch. >> >> Thanks. >> >> On Thu, 24 Nov 2016 at 15:09 Zachary Turner >> wrote: >> > >> > I would use the first solution. We lock ourselves to specific >> > versions >> > of vs, so i think it's fine to do the same with the assemblies and >> > deal with >> > it only when we upgrade >> > On Thu, Nov 24, 2016 at 11:29 AM Antonio Maiorano >> > >> > wrote: >> >> >> >> Hi Hans, >> >> >> >> I saw that on September 15th, you checked in a change: clang-format >> >> VS >> >> plugin: upgrade the project files to VS2015. >> >> >> >> When I open the latest version of ClangFormat.sln on a machine that >> >> has only VS 2015, it doesn't build. The reason is that some of the >> >> referenced assemblies are from VS 2010. >> >> >> >> > >> Include="Microsoft.VisualStudio.Editor, Version=10.0.0.0, >> >> Culture=neutral, >> >> PublicKeyToken=b03f5f7f11d50a3a, processorArchitecture=MSIL" /> >> >> >> >> What happens is that when building, these specific assemblies are >> >> not >> >> found. I suspect you have VS 2010 installed on your machine, which >> >> is why >> >> you don't get these build errors. >> >> >> >> The recommended way to handle this is to make use of NuGet to have >> >> it >> >> automatically download the required assemblies. I have made the >> >> changes >> >> locally to get this working, and it works great when building >> >> ClangFormat.sln from within Visual Studio; however, building from >> >> the CLI >> >> doesn't work out of the box because you must explicitly run >> >> 'nuget.exe >> >> restore ClangFormat.sln' before running msbuild (or devenv.exe in >> >> our case). >> >> The problem is that nuget.exe isn't something that's easily >> >> found/accessible >> >> on Windows, even once installed as an extension in VS. This is a >> >> known >> >> probl
[PATCH] D27093: Protect std::{, unordered_}map tests under noexceptions
mclow.lists accepted this revision. mclow.lists added a comment. This revision is now accepted and ready to land. LGTM. https://reviews.llvm.org/D27093 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27095: Protect std::array tests under noexceptions
mclow.lists accepted this revision. mclow.lists added a comment. This revision is now accepted and ready to land. Other than the missing `assert`s, (which are not your fault, but I would appreciate you fixing) this LGTM. Comment at: test/std/containers/sequences/array/at.pass.cpp:43 +#ifndef TEST_HAS_NO_EXCEPTIONS try { (void) c.at(3); } catch (const std::out_of_range &) {} we should really have an `assert(false);` after the call to `at` - to make sure that it actually throws. Here and below. https://reviews.llvm.org/D27095 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26611: Protect test for dynarray under libcpp-no-exceptions
mclow.lists accepted this revision. mclow.lists added a comment. This revision is now accepted and ready to land. LGTM. https://reviews.llvm.org/D26611 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26612: Protect std::string tests under libcpp-no-exceptions
mclow.lists accepted this revision. mclow.lists added a comment. This revision is now accepted and ready to land. LGTM. Thanks. https://reviews.llvm.org/D26612 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27167: [libc++] Support multibyte decimal_point and thousands_sep
vangyzen added a comment. A unit test change in https://reviews.llvm.org/D26979 found this and will continue to test it (on some OSes). https://reviews.llvm.org/D27167 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: Upgrade and fix clang-format-vs
On Mon, Nov 28, 2016 at 11:11 AM, Antonio Maiorano wrote: >> It's built with the script in utils/release/build_llvm_package.bat > which I run manually on my machine once every few weeks. > > Okay, that's good news. So the simplest path to success would be to require > the user to either pass the path to CMake via an arg like -DNUGET_EXE_PATH, > or if it's not defined, to assume it's already in PATH. This is the most > future-proof solution as it will work with future versions of VS (2017 RC > just came out). > > I can still look into whether a vsix built with VS 2015 references will > continue to work in older versions of VS, but even if this works, I feel > like it's a temporary solution at best. There are other advantages to using > NuGet here: it would allow us to more easily pin/upgrade which assemblies we > want to use over time. > > If you're okay with it, I'll make the changes necessary to use > -DNUGET_EXE_PATH, if defined, otherwise assume it's on PATH. This should be > a simple change at this point. That sounds good to me. There are already a bunch of prerequisites for building the plugin, so adding this one doesn't seem unreasonable. Especially since it seems it will simplify things to the point that they might even work elsewhere than my own machine :-) > On Mon, 28 Nov 2016 at 13:59 Hans Wennborg wrote: >> >> On Mon, Nov 28, 2016 at 10:46 AM, Antonio Maiorano >> wrote: >> > Okay, I'll see if upgrading to the 2015 assemblies would allow the VSIX >> > to >> > keep working in older versions of VS. >> > >> > Still waiting on an answer to this question: >> > >> >> In either case, though, I must ask: how is the offical vsix that's >> >> available on http://llvm.org/builds/ get built? Is it part of an >> >> automated >> >> Clang build, or is it built and uploaded manually? If it's automated, >> >> then >> >> having to download and point to nuget.exe won't work. >> >> It's built with the script in utils/release/build_llvm_package.bat >> which I run manually on my machine once every few weeks. >> >> >> > On Mon, 28 Nov 2016 at 13:04 Hans Wennborg wrote: >> >> >> >> On Fri, Nov 25, 2016 at 6:58 PM, Antonio Maiorano >> >> wrote: >> >> > Ah, no, that's not what I meant. The required referenced assemblies >> >> > are >> >> > versions that are normally installed with VS 2010. >> >> > >> >> > The first time I worked on this, I had upgraded the referenced >> >> > assemblies to >> >> > the ones that ship with VS 2015, but then there was question of >> >> > whether >> >> > or >> >> > not the VSIX would continue to work with VS 2010/2012/2013. I'm not >> >> > sure >> >> > if >> >> > it would work, but I guess I can try to figure that out. >> >> >> >> Let me know if you figure this one out. It sounds like it would >> >> simplify things a lot. >> >> >> >> > In any case, what I discovered is that the "right" way to do things >> >> > to >> >> > make >> >> > sure your extension compiles in future versions of VS is to use NuGet >> >> > to >> >> > automatically pull in the required assemblies, or to check them in >> >> > and >> >> > reference them directly. The former would be better except for the >> >> > problem >> >> > of CLI builds as I described in my earlier email. >> >> > >> >> > >> >> > >> >> > On Fri, 25 Nov 2016 at 21:47 Zachary Turner >> >> > wrote: >> >> >> >> >> >> Sorry, i think I misunderstood the original option 1. I interpreted >> >> >> it >> >> >> as >> >> >> just committing changes to the vsix manifest to reference a specific >> >> >> version >> >> >> of the assembly which we assume to be present since it should be >> >> >> automatically installed with vs 2015. Is this not possible? Can't we >> >> >> just >> >> >> point the manifest to the version installed with vs? >> >> >> On Fri, Nov 25, 2016 at 6:20 PM Antonio Maiorano >> >> >> >> >> >> wrote: >> >> >>> >> >> >>> Hi again, >> >> >>> >> >> >>> I've made the changes so that the required assemblies are >> >> >>> committed, >> >> >>> so >> >> >>> now we can build the clang-format-vsix with just VS 2015. Since the >> >> >>> patch >> >> >>> set is around 9 mb, I'm providing a link to it on my Dropbox (if >> >> >>> you'd >> >> >>> rather I attach it, let me know): >> >> >>> >> >> >>> >> >> >>> >> >> >>> >> >> >>> https://dl.dropboxusercontent.com/u/10504225/llvm-patches/0001-Fix-VS2015-build-of-clang-format-vsix-by-committing-.patch >> >> >>> >> >> >>> Note that it's a git patch set, for which I followed the >> >> >>> instructions >> >> >>> here. >> >> >>> >> >> >>> Thanks. >> >> >>> >> >> >>> On Thu, 24 Nov 2016 at 15:45 Antonio Maiorano >> >> >>> wrote: >> >> >> >> Okay, that's fine, I'll go for that and if all looks good, will >> >> attach a >> >> patch. >> >> >> >> Thanks. >> >> >> >> On Thu, 24 Nov 2016 at 15:09 Zachary Turner >> >> wrote: >> >> > >> >> > I would use the first solution. We lock ourselves to specific >> >> > versions >> >> > of vs, so i think it's fine to do th
[PATCH] D26991: Hoist redundant load
sebpop added a comment. In https://reviews.llvm.org/D26991#606764, @mclow.lists wrote: > /me wonders what the perf difference when `_LIBCPP_UNROLL_LOOPS` is defined > or not. > > I think this (`_LIBCPP_UNROLL_LOOPS`) falls squarely into Chandler's request > that we complain to him when the compiler generates sub-optimal code, instead > of doing things like manually unrolling loops. I think we should remove all the code in the ifdef: it looks to me like this code was left in from a previous "experiment", as it never gets executed unless one compiles the code with -D_LIBCPP_UNROLL_LOOPS, which seems wrong. Let's push this cleanup in a separate commit. https://reviews.llvm.org/D26991 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27068: Improve string::find
sebpop added a comment. In https://reviews.llvm.org/D27068#606757, @mclow.lists wrote: > Definitely want to see numbers. master: Benchmark Time CPU Iterations --- BM_StringFindPhase1/104 ns 4 ns 161568945 BM_StringFindPhase1/64 28 ns 28 ns 24937005 BM_StringFindPhase1/512 132 ns132 ns5321432 BM_StringFindPhase1/4k 956 ns956 ns 732038 BM_StringFindPhase1/32k7622 ns 7621 ns 92121 BM_StringFindPhase1/128k 30485 ns 30483 ns 22938 BM_StringFindPhase2/1 4 ns 4 ns 191884173 BM_StringFindPhase2/8 7 ns 7 ns 105129099 BM_StringFindPhase2/64 34 ns 34 ns 20582485 BM_StringFindPhase2/512 187 ns187 ns3736654 BM_StringFindPhase2/4k 1415 ns 1414 ns 495342 BM_StringFindPhase2/32k 11221 ns 11220 ns 62393 BM_StringFindPhase2/128k 44952 ns 44949 ns 15595 master + patch: Benchmark Time CPU Iterations --- BM_StringFindPhase1/103 ns 3 ns 204725158 BM_StringFindPhase1/645 ns 5 ns 146268957 BM_StringFindPhase1/512 12 ns 12 ns 60176557 BM_StringFindPhase1/4k 67 ns 67 ns 10508533 BM_StringFindPhase1/32k 503 ns503 ns100 BM_StringFindPhase1/128k 2396 ns 2396 ns 292701 BM_StringFindPhase2/1 6 ns 6 ns 127378641 BM_StringFindPhase2/8 6 ns 6 ns 117407388 BM_StringFindPhase2/647 ns 7 ns 95998016 BM_StringFindPhase2/512 18 ns 18 ns 38135928 BM_StringFindPhase2/4k 83 ns 83 ns8452337 BM_StringFindPhase2/32k 762 ns762 ns 925744 BM_StringFindPhase2/128k 3255 ns 3255 ns 215141 https://reviews.llvm.org/D27068 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27068: Improve string::find
sebpop added a comment. The numbers are from an x86_64-linux machine: Intel(R) Core(TM) i7-4790K CPU @ 4.00GHz Overall the patch is a win on the synthetic benchmark. We have also seen this in a proprietary benchmark where it accounted for about 10% speedup. https://reviews.llvm.org/D27068 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27163: Introduce -f[no-]strict-return flag that controls code generation for C++'s undefined behaviour return optimisation
mehdi_amini added a comment. In https://reviews.llvm.org/D27163#606744, @arphaman wrote: > In https://reviews.llvm.org/D27163#606695, @mehdi_amini wrote: > > > What is the justification for a platform specific default change here? > > > The flag itself is platform agnostic, however, the default value is different > on Darwin (-fno-strict-return). The reason for this difference is because of > how I interpreted the internal discussion for this issue: it seems that some > of our internal builds had problems with this flag, so to me it seemed that > people would've wanted this specific difference upstream. I was not involved in the internal discussion, and the pre-existing internal arguments should be repeated here when needed to drive the patch. I find dubious that the compiler wouldn't have specific handling for undefined behavior on a specific platform, without a strong argument that justify it (like a platform with a different hardware memory model making it more sensitive, etc.). In the current case, it seems like a "vendor specific choice" of being more conservative rather than something attached to the platform itself, so I'm not sure it makes sense to hard-code this upstream. Do we have past records of doing this? Repository: rL LLVM https://reviews.llvm.org/D27163 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D24991: Inline hot functions in libcxx shared_ptr implementation.
sebpop added a comment. @mclow.lists could you please have a last look at this patch: the change is for a performance improvement (20% uplift on a proprietary benchmark), and all the issues mentioned in the review have been addressed. The existing synthetic benchmark shows an overall improvement: master: Benchmark Time CPU Iterations BM_SharedPtrCreateDestroy 54 ns 54 ns 12388755 BM_SharedPtrIncDecRef 37 ns 37 ns 19021739 BM_WeakPtrIncDecRef 38 ns 38 ns 18421053 master + patch: Benchmark Time CPU Iterations BM_SharedPtrCreateDestroy 44 ns 44 ns 14730639 BM_SharedPtrIncDecRef 18 ns 18 ns 3889 BM_WeakPtrIncDecRef 30 ns 30 ns 23648649 https://reviews.llvm.org/D24991 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26955: Fix bitwidth for x87 extended-precision floating-point type
bruno added inline comments. Comment at: lib/Basic/TargetInfo.cpp:229 switch (BitWidth) { - case 96: + case 80: if (&getLongDoubleFormat() == &llvm::APFloat::x87DoubleExtended) The change makes sense but I believe there's some historical reason why this is 96 instead of 80? What problem have you found in practice? Do you have a testcase or unittest that exercise the issue (and that could be added to the patch)? https://reviews.llvm.org/D26955 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27157: IRGen: Remove all uses of CreateDefaultAlignedLoad.
pcc updated this revision to Diff 79436. pcc marked an inline comment as done. pcc added a comment. - Address review comments https://reviews.llvm.org/D27157 Files: clang/lib/CodeGen/CGBuilder.h clang/lib/CodeGen/CGBuiltin.cpp clang/lib/CodeGen/CGCall.cpp clang/lib/CodeGen/CGObjCGNU.cpp clang/lib/CodeGen/TargetInfo.cpp Index: clang/lib/CodeGen/TargetInfo.cpp === --- clang/lib/CodeGen/TargetInfo.cpp +++ clang/lib/CodeGen/TargetInfo.cpp @@ -3548,15 +3548,17 @@ llvm::Value *RegHiAddr = TyLo->isFPOrFPVectorTy() ? GPAddr : FPAddr; // Copy the first element. -llvm::Value *V = - CGF.Builder.CreateDefaultAlignedLoad( - CGF.Builder.CreateBitCast(RegLoAddr, PTyLo)); +// FIXME: Our choice of alignment here and below is probably pessimistic. +llvm::Value *V = CGF.Builder.CreateAlignedLoad( +TyLo, CGF.Builder.CreateBitCast(RegLoAddr, PTyLo), +CharUnits::fromQuantity(getDataLayout().getABITypeAlignment(TyLo))); CGF.Builder.CreateStore(V, CGF.Builder.CreateStructGEP(Tmp, 0, CharUnits::Zero())); // Copy the second element. -V = CGF.Builder.CreateDefaultAlignedLoad( - CGF.Builder.CreateBitCast(RegHiAddr, PTyHi)); +V = CGF.Builder.CreateAlignedLoad( +TyHi, CGF.Builder.CreateBitCast(RegHiAddr, PTyHi), +CharUnits::fromQuantity(getDataLayout().getABITypeAlignment(TyHi))); CharUnits Offset = CharUnits::fromQuantity( getDataLayout().getStructLayout(ST)->getElementOffset(1)); CGF.Builder.CreateStore(V, CGF.Builder.CreateStructGEP(Tmp, 1, Offset)); Index: clang/lib/CodeGen/CGObjCGNU.cpp === --- clang/lib/CodeGen/CGObjCGNU.cpp +++ clang/lib/CodeGen/CGObjCGNU.cpp @@ -2890,9 +2890,11 @@ if (RuntimeVersion < 10 || CGF.CGM.getTarget().getTriple().isKnownWindowsMSVCEnvironment()) return CGF.Builder.CreateZExtOrBitCast( - CGF.Builder.CreateDefaultAlignedLoad(CGF.Builder.CreateAlignedLoad( - ObjCIvarOffsetVariable(Interface, Ivar), - CGF.getPointerAlign(), "ivar")), + CGF.Builder.CreateAlignedLoad( + Int32Ty, CGF.Builder.CreateAlignedLoad( + ObjCIvarOffsetVariable(Interface, Ivar), + CGF.getPointerAlign(), "ivar"), + CharUnits::fromQuantity(4)), PtrDiffTy); std::string name = "__objc_ivar_offset_value_" + Interface->getNameAsString() +"." + Ivar->getNameAsString(); Index: clang/lib/CodeGen/CGCall.cpp === --- clang/lib/CodeGen/CGCall.cpp +++ clang/lib/CodeGen/CGCall.cpp @@ -2884,13 +2884,13 @@ // FIXME: Generate IR in one pass, rather than going back and fixing up these // placeholders. llvm::Type *IRTy = CGF.ConvertTypeForMem(Ty); - llvm::Value *Placeholder = -llvm::UndefValue::get(IRTy->getPointerTo()->getPointerTo()); - Placeholder = CGF.Builder.CreateDefaultAlignedLoad(Placeholder); + llvm::Type *IRPtrTy = IRTy->getPointerTo(); + llvm::Value *Placeholder = llvm::UndefValue::get(IRPtrTy->getPointerTo()); // FIXME: When we generate this IR in one pass, we shouldn't need // this win32-specific alignment hack. CharUnits Align = CharUnits::fromQuantity(4); + Placeholder = CGF.Builder.CreateAlignedLoad(IRPtrTy, Placeholder, Align); return AggValueSlot::forAddr(Address(Placeholder, Align), Ty.getQualifiers(), Index: clang/lib/CodeGen/CGBuiltin.cpp === --- clang/lib/CodeGen/CGBuiltin.cpp +++ clang/lib/CodeGen/CGBuiltin.cpp @@ -2191,8 +2191,9 @@ Value *IntToPtr = Builder.CreateIntToPtr(EmitScalarExpr(E->getArg(0)), llvm::PointerType::get(IntTy, 257)); -LoadInst *Load = -Builder.CreateDefaultAlignedLoad(IntToPtr, /*isVolatile=*/true); +LoadInst *Load = Builder.CreateAlignedLoad( +IntTy, IntToPtr, getContext().getTypeAlignInChars(E->getType())); +Load->setVolatile(true); return RValue::get(Load); } @@ -5440,9 +5441,11 @@ switch (BuiltinID) { default: break; case NEON::BI__builtin_neon_vldrq_p128: { -llvm::Type *Int128PTy = llvm::Type::getIntNPtrTy(getLLVMContext(), 128); +llvm::Type *Int128Ty = llvm::Type::getIntNTy(getLLVMContext(), 128); +llvm::Type *Int128PTy = llvm::PointerType::get(Int128Ty, 0); Value *Ptr = Builder.CreateBitCast(EmitScalarExpr(E->getArg(0)), Int128PTy); -return Builder.CreateDefaultAlignedLoad(Ptr); +return Builder.CreateAlignedLoad(Int128Ty, Ptr, + CharUnits::fromQuantity(16)); } case NEON::BI__builtin_neon_vstrq_p128: { llvm::Type *Int128PTy = llvm::Type::getIntNPtrTy(getLLV
[PATCH] D27157: IRGen: Remove all uses of CreateDefaultAlignedLoad.
pcc added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2195 LoadInst *Load = -Builder.CreateDefaultAlignedLoad(IntToPtr, /*isVolatile=*/true); +Builder.CreateAlignedLoad(IntTy, IntToPtr, CharUnits::fromQuantity(4)); +Load->setVolatile(true); rjmccall wrote: > Why 4? __readfsdword is a Windows intrinsic which returns an unsigned long, which always has size/alignment 4 on Windows. But now that I think about it I suppose we can get here on other platforms with MS extensions enabled, so I've changed this to query the alignment. https://reviews.llvm.org/D27157 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r288059 - [MS] Mangle a unique ID into all MS inline asm labels
Author: rnk Date: Mon Nov 28 14:52:19 2016 New Revision: 288059 URL: http://llvm.org/viewvc/llvm-project?rev=288059&view=rev Log: [MS] Mangle a unique ID into all MS inline asm labels This solves PR23715 in a way that is compatible with LTO. MSVC supports jumping to source-level labels and between inline asm blocks, but we don't. Also revert the old solution, r255201, which was to mark these calls as noduplicate. Modified: cfe/trunk/include/clang/Sema/Sema.h cfe/trunk/lib/CodeGen/CGStmt.cpp cfe/trunk/lib/Sema/Sema.cpp cfe/trunk/lib/Sema/SemaStmtAsm.cpp cfe/trunk/test/CodeGen/mozilla-ms-inline-asm.c cfe/trunk/test/CodeGen/ms-inline-asm.c cfe/trunk/test/CodeGen/ms-inline-asm.cpp Modified: cfe/trunk/include/clang/Sema/Sema.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=288059&r1=288058&r2=288059&view=diff == --- cfe/trunk/include/clang/Sema/Sema.h (original) +++ cfe/trunk/include/clang/Sema/Sema.h Mon Nov 28 14:52:19 2016 @@ -788,9 +788,6 @@ public: /// \brief will hold 'respondsToSelector:' Selector RespondsToSelectorSel; - /// \brief counter for internal MS Asm label names. - unsigned MSAsmLabelNameCounter; - /// A flag to remember whether the implicit forms of operator new and delete /// have been declared. bool GlobalNewDeleteDeclared; Modified: cfe/trunk/lib/CodeGen/CGStmt.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGStmt.cpp?rev=288059&r1=288058&r2=288059&view=diff == --- cfe/trunk/lib/CodeGen/CGStmt.cpp (original) +++ cfe/trunk/lib/CodeGen/CGStmt.cpp Mon Nov 28 14:52:19 2016 @@ -2103,15 +2103,6 @@ void CodeGenFunction::EmitAsmStmt(const Result->addAttribute(llvm::AttributeSet::FunctionIndex, llvm::Attribute::NoUnwind); - if (isa(&S)) { -// If the assembly contains any labels, mark the call noduplicate to prevent -// defining the same ASM label twice (PR23715). This is pretty hacky, but it -// works. -if (AsmString.find("__MSASMLABEL_") != std::string::npos) - Result->addAttribute(llvm::AttributeSet::FunctionIndex, - llvm::Attribute::NoDuplicate); - } - // Attach readnone and readonly attributes. if (!HasSideEffect) { if (ReadNone) Modified: cfe/trunk/lib/Sema/Sema.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.cpp?rev=288059&r1=288058&r2=288059&view=diff == --- cfe/trunk/lib/Sema/Sema.cpp (original) +++ cfe/trunk/lib/Sema/Sema.cpp Mon Nov 28 14:52:19 2016 @@ -96,7 +96,6 @@ Sema::Sema(Preprocessor &pp, ASTContext ValueWithBytesObjCTypeMethod(nullptr), NSArrayDecl(nullptr), ArrayWithObjectsMethod(nullptr), NSDictionaryDecl(nullptr), DictionaryWithObjectsMethod(nullptr), -MSAsmLabelNameCounter(0), GlobalNewDeleteDeclared(false), TUKind(TUKind), NumSFINAEErrors(0), Modified: cfe/trunk/lib/Sema/SemaStmtAsm.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmtAsm.cpp?rev=288059&r1=288058&r2=288059&view=diff == --- cfe/trunk/lib/Sema/SemaStmtAsm.cpp (original) +++ cfe/trunk/lib/Sema/SemaStmtAsm.cpp Mon Nov 28 14:52:19 2016 @@ -753,14 +753,12 @@ LabelDecl *Sema::GetOrCreateMSAsmLabel(S // Create an internal name for the label. The name should not be a valid mangled // name, and should be unique. We use a dot to make the name an invalid mangled // name. -OS << "__MSASMLABEL_." << MSAsmLabelNameCounter++ << "__"; -for (auto it = ExternalLabelName.begin(); it != ExternalLabelName.end(); - ++it) { - OS << *it; - if (*it == '$') { -// We escape '$' in asm strings by replacing it with "$$" +OS << "__MSASMLABEL_.{:uid}__"; +for (char C : ExternalLabelName) { + OS << C; + // We escape '$' in asm strings by replacing it with "$$" + if (C == '$') OS << '$'; - } } Label->setMSAsmLabel(OS.str()); } Modified: cfe/trunk/test/CodeGen/mozilla-ms-inline-asm.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/mozilla-ms-inline-asm.c?rev=288059&r1=288058&r2=288059&view=diff == --- cfe/trunk/test/CodeGen/mozilla-ms-inline-asm.c (original) +++ cfe/trunk/test/CodeGen/mozilla-ms-inline-asm.c Mon Nov 28 14:52:19 2016 @@ -20,7 +20,7 @@ void invoke(void* that, unsigned methodI // CHECK: call void asm sideeffect inteldialect // CHECK: mov edx,dword ptr $1 // CHECK: test edx,edx -// CHECK: jz {{[^_]*}}__MSASMLABEL_.0__noparams +// CHECK: jz {{[^_]*}}__MSASMLABEL_.{:uid}__noparams // ^ Can't use {{.*}} here because the matching is greedy. // CHECK: mov
[PATCH] D26916: [ObjC] Avoid a @try/@finally/@autoreleasepool fixit when parsing an expression
bruno added a comment. Hi Alex, thanks for working on this Comment at: lib/Parse/ParseObjc.cpp:2877 +if (GetLookAheadToken(1).is(tok::l_brace) && +ExprStatementTokLoc == AtLoc) { char ch = Tok.getIdentifierInfo()->getNameStart()[0]; Does this only triggers when `Res.isInvalid()` returns true in the first part of the patch? I wonder if it's also safe to allow `ExprStatementTokLoc = AtLoc;` for every path or only when it fails. Repository: rL LLVM https://reviews.llvm.org/D26916 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27163: Introduce -f[no-]strict-return flag that controls code generation for C++'s undefined behaviour return optimisation
Quuxplusone added inline comments. Comment at: lib/Sema/AnalysisBasedWarnings.cpp:530 + // Don't need to mark Objective-C methods or blocks since the undefined + // behaviour optimization isn't used for them. +} This seems like a trap waiting to spring on someone, unless there's a technical reason that methods and blocks cannot possibly use the same optimization paths as regular functions. ("Nobody's gotten around to implementing it yet" is the most obvious nontechnical reason for the current difference.) Either way, I'd expect this patch to include test cases for both methods and blocks, to verify that the behavior you expect is actually the behavior that happens. Basically, it ought to have a regression test targeting the regression that I'm predicting is going to spring on someone as soon as they implement optimizations for methods and blocks. Also, one dumb question: what about C++ lambdas? are they FunctionDecls too? test case? Comment at: test/CodeGenCXX/return.cpp:21 + // CHECK-NOSTRICT-NEXT: load + // CHECK-NOSTRICT-NEXT: ret i32 + // CHECK-NOSTRICT-NEXT: } Can you explain why a load from an uninitialized stack location would be *better* than a trap and/or `unreachable`? IIUC this is basically what Mehdi is asking: i.e., can you explain the rationale for this patch, because I don't get it either. It *seems* like a strict regression in code quality. Repository: rL LLVM https://reviews.llvm.org/D27163 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r288060 - [Driver] Refactor distro detection & classification as a separate API
Author: mgorny Date: Mon Nov 28 15:11:14 2016 New Revision: 288060 URL: http://llvm.org/viewvc/llvm-project?rev=288060&view=rev Log: [Driver] Refactor distro detection & classification as a separate API Refactor the Distro enum along with helper functions into a full-fledged Distro class, inspired by llvm::Triple, and make it a public API. The new class wraps the enum with necessary comparison operators, adding the convenience Is*() methods and a constructor performing the detection. The public API is needed to run the unit tests (D25869). Differential Revision: https://reviews.llvm.org/D25949 Added: cfe/trunk/include/clang/Driver/Distro.h cfe/trunk/lib/Driver/Distro.cpp Modified: cfe/trunk/lib/Driver/CMakeLists.txt cfe/trunk/lib/Driver/ToolChains.cpp Added: cfe/trunk/include/clang/Driver/Distro.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Distro.h?rev=288060&view=auto == --- cfe/trunk/include/clang/Driver/Distro.h (added) +++ cfe/trunk/include/clang/Driver/Distro.h Mon Nov 28 15:11:14 2016 @@ -0,0 +1,122 @@ +//===--- Distro.h - Linux distribution detection support *- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#ifndef LLVM_CLANG_DRIVER_DISTRO_H +#define LLVM_CLANG_DRIVER_DISTRO_H + +#include "clang/Basic/VirtualFileSystem.h" + +namespace clang { +namespace driver { + +/// Distro - Helper class for detecting and classifying Linux distributions. +/// +/// This class encapsulates the clang Linux distribution detection mechanism +/// as well as helper functions that match the specific (versioned) results +/// into wider distribution classes. +class Distro { +public: + enum DistroType { +// NB: Releases of a particular Linux distro should be kept together +// in this enum, because some tests are done by integer comparison against +// the first and last known member in the family, e.g. IsRedHat(). +ArchLinux, +DebianLenny, +DebianSqueeze, +DebianWheezy, +DebianJessie, +DebianStretch, +Exherbo, +RHEL5, +RHEL6, +RHEL7, +Fedora, +OpenSUSE, +UbuntuHardy, +UbuntuIntrepid, +UbuntuJaunty, +UbuntuKarmic, +UbuntuLucid, +UbuntuMaverick, +UbuntuNatty, +UbuntuOneiric, +UbuntuPrecise, +UbuntuQuantal, +UbuntuRaring, +UbuntuSaucy, +UbuntuTrusty, +UbuntuUtopic, +UbuntuVivid, +UbuntuWily, +UbuntuXenial, +UbuntuYakkety, +UbuntuZesty, +UnknownDistro + }; + +private: + /// The distribution, possibly with specific version. + DistroType DistroVal; + +public: + /// @name Constructors + /// @{ + + /// Default constructor leaves the distribution unknown. + Distro() : DistroVal() {} + + /// Constructs a Distro type for specific distribution. + Distro(DistroType D) : DistroVal(D) {} + + /// Detects the distribution using specified VFS. + explicit Distro(clang::vfs::FileSystem& VFS); + + bool operator==(const Distro &Other) const { +return DistroVal == Other.DistroVal; + } + + bool operator!=(const Distro &Other) const { +return DistroVal != Other.DistroVal; + } + + bool operator>=(const Distro &Other) const { +return DistroVal >= Other.DistroVal; + } + + bool operator<=(const Distro &Other) const { +return DistroVal <= Other.DistroVal; + } + + /// @} + /// @name Convenience Predicates + /// @{ + + bool IsRedhat() const { +return DistroVal == Fedora || (DistroVal >= RHEL5 && DistroVal <= RHEL7); + } + + bool IsOpenSUSE() const { +return DistroVal == OpenSUSE; + } + + bool IsDebian() const { +return DistroVal >= DebianLenny && DistroVal <= DebianStretch; + } + + bool IsUbuntu() const { +return DistroVal >= UbuntuHardy && DistroVal <= UbuntuZesty; + } + + /// @} +}; + +} // end namespace driver +} // end namespace clang + +#endif Modified: cfe/trunk/lib/Driver/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/CMakeLists.txt?rev=288060&r1=288059&r2=288060&view=diff == --- cfe/trunk/lib/Driver/CMakeLists.txt (original) +++ cfe/trunk/lib/Driver/CMakeLists.txt Mon Nov 28 15:11:14 2016 @@ -12,6 +12,7 @@ add_clang_library(clangDriver Action.cpp Compilation.cpp CrossWindowsToolChain.cpp + Distro.cpp Driver.cpp DriverOptions.cpp Job.cpp Added: cfe/trunk/lib/Driver/Distro.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Distro.cpp?rev=288060&view=auto == --- cfe/trunk/lib/Driver/Distro.cpp (added) +++ cfe/trunk/lib/Driver/Distro.cpp Mon Nov 28 15:11:14 2016 @@ -0,0
r288061 - [Driver] Fix recognizing newer OpenSUSE versions
Author: mgorny Date: Mon Nov 28 15:11:18 2016 New Revision: 288061 URL: http://llvm.org/viewvc/llvm-project?rev=288061&view=rev Log: [Driver] Fix recognizing newer OpenSUSE versions Fix recognizing newer OpenSUSE versions that combine the two version components into 'VERSION = x.y'. The check was written against an older version that kept those two split as VERSION and PATCHLEVEL. Differential Revision: https://reviews.llvm.org/D26850 Modified: cfe/trunk/lib/Driver/Distro.cpp Modified: cfe/trunk/lib/Driver/Distro.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Distro.cpp?rev=288061&r1=288060&r2=288061&view=diff == --- cfe/trunk/lib/Driver/Distro.cpp (original) +++ cfe/trunk/lib/Driver/Distro.cpp Mon Nov 28 15:11:18 2016 @@ -108,11 +108,14 @@ static Distro::DistroType DetectDistro(v if (!Line.trim().startswith("VERSION")) continue; std::pair SplitLine = Line.split('='); + // Old versions have split VERSION and PATCHLEVEL + // Newer versions use VERSION = x.y + std::pair SplitVer = SplitLine.second.trim().split('.'); int Version; + // OpenSUSE/SLES 10 and older are not supported and not compatible // with our rules, so just treat them as Distro::UnknownDistro. - if (!SplitLine.second.trim().getAsInteger(10, Version) && - Version > 10) + if (!SplitVer.first.getAsInteger(10, Version) && Version > 10) return Distro::OpenSUSE; return Distro::UnknownDistro; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r288062 - [Driver] Add unit tests for Distro detection
Author: mgorny Date: Mon Nov 28 15:11:22 2016 New Revision: 288062 URL: http://llvm.org/viewvc/llvm-project?rev=288062&view=rev Log: [Driver] Add unit tests for Distro detection Add a set of unit tests for the distro detection code. The tests use an in-memory virtual filesystems resembling release files for various distributions supported. All release files are provided (not only the ones directly used) in order to guarantee that one of the rules will not mistakenly recognize the distribution incorrectly due to the additional files (e.g. Ubuntu as Debian). Differential Revision: https://reviews.llvm.org/D25869 Added: cfe/trunk/unittests/Driver/DistroTest.cpp Modified: cfe/trunk/unittests/Driver/CMakeLists.txt Modified: cfe/trunk/unittests/Driver/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Driver/CMakeLists.txt?rev=288062&r1=288061&r2=288062&view=diff == --- cfe/trunk/unittests/Driver/CMakeLists.txt (original) +++ cfe/trunk/unittests/Driver/CMakeLists.txt Mon Nov 28 15:11:22 2016 @@ -3,6 +3,7 @@ set(LLVM_LINK_COMPONENTS ) add_clang_unittest(ClangDriverTests + DistroTest.cpp ToolChainTest.cpp MultilibTest.cpp ) Added: cfe/trunk/unittests/Driver/DistroTest.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Driver/DistroTest.cpp?rev=288062&view=auto == --- cfe/trunk/unittests/Driver/DistroTest.cpp (added) +++ cfe/trunk/unittests/Driver/DistroTest.cpp Mon Nov 28 15:11:22 2016 @@ -0,0 +1,305 @@ +//===- unittests/Driver/DistroTest.cpp --- ToolChains tests ---===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// +// +// Unit tests for Distro detection. +// +//===--===// + +#include "clang/Driver/Distro.h" +#include "clang/Basic/VirtualFileSystem.h" +#include "llvm/Support/raw_ostream.h" +#include "gtest/gtest.h" +using namespace clang; +using namespace clang::driver; + +namespace { + +// The tests include all release-related files for each distribution +// in the VFS, in order to make sure that earlier tests do not +// accidentally result in incorrect distribution guess. + +TEST(DistroTest, DetectUbuntu) { + vfs::InMemoryFileSystem UbuntuTrustyFileSystem; + // Ubuntu uses Debian Sid version. + UbuntuTrustyFileSystem.addFile("/etc/debian_version", 0, + llvm::MemoryBuffer::getMemBuffer("jessie/sid\n")); + UbuntuTrustyFileSystem.addFile("/etc/lsb-release", 0, + llvm::MemoryBuffer::getMemBuffer("DISTRIB_ID=Ubuntu\n" + "DISTRIB_RELEASE=14.04\n" + "DISTRIB_CODENAME=trusty\n" + "DISTRIB_DESCRIPTION=\"Ubuntu 14.04 LTS\"\n")); + UbuntuTrustyFileSystem.addFile("/etc/os-release", 0, + llvm::MemoryBuffer::getMemBuffer("NAME=\"Ubuntu\"\n" + "VERSION=\"14.04, Trusty Tahr\"\n" + "ID=ubuntu\n" + "ID_LIKE=debian\n" + "PRETTY_NAME=\"Ubuntu 14.04 LTS\"\n" + "VERSION_ID=\"14.04\"\n" + "HOME_URL=\"http://www.ubuntu.com/\"\n"; + "SUPPORT_URL=\"http://help.ubuntu.com/\"\n"; + "BUG_REPORT_URL=\"http://bugs.launchpad.net/ubuntu/\"\n";)); + + Distro UbuntuTrusty{UbuntuTrustyFileSystem}; + ASSERT_EQ(Distro(Distro::UbuntuTrusty), UbuntuTrusty); + ASSERT_TRUE(UbuntuTrusty.IsUbuntu()); + ASSERT_FALSE(UbuntuTrusty.IsRedhat()); + ASSERT_FALSE(UbuntuTrusty.IsOpenSUSE()); + ASSERT_FALSE(UbuntuTrusty.IsDebian()); + + vfs::InMemoryFileSystem UbuntuYakketyFileSystem; + UbuntuYakketyFileSystem.addFile("/etc/debian_version", 0, + llvm::MemoryBuffer::getMemBuffer("stretch/sid\n")); + UbuntuYakketyFileSystem.addFile("/etc/lsb-release", 0, + llvm::MemoryBuffer::getMemBuffer("DISTRIB_ID=Ubuntu\n" + "DISTRIB_RELEASE=16.10\n" + "DISTRIB_CODENAME=yakkety\n" + "DISTRIB_DESCRIPTION=\"Ubuntu 16.10\"\n")); + UbuntuYakketyFileSystem.addFile("/etc/os-release", 0, + llvm::MemoryBuffer::getMemBuffer("NAME=\"Ubuntu\"\n" + "VERSION=\"16.10 (Yakkety Yak)\"\n" + "ID=ubuntu\n" + "ID_LIKE=debian\n" + "PRETTY_NAME=\"Ubuntu 16.10\"\n" +
[PATCH] D25949: [Driver] Refactor distro detection & classification as a separate API
This revision was automatically updated to reflect the committed changes. Closed by commit rL288060: [Driver] Refactor distro detection & classification as a separate API (authored by mgorny). Changed prior to commit: https://reviews.llvm.org/D25949?vs=78462&id=79442#toc Repository: rL LLVM https://reviews.llvm.org/D25949 Files: cfe/trunk/include/clang/Driver/Distro.h cfe/trunk/lib/Driver/CMakeLists.txt cfe/trunk/lib/Driver/Distro.cpp cfe/trunk/lib/Driver/ToolChains.cpp Index: cfe/trunk/include/clang/Driver/Distro.h === --- cfe/trunk/include/clang/Driver/Distro.h +++ cfe/trunk/include/clang/Driver/Distro.h @@ -0,0 +1,122 @@ +//===--- Distro.h - Linux distribution detection support *- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#ifndef LLVM_CLANG_DRIVER_DISTRO_H +#define LLVM_CLANG_DRIVER_DISTRO_H + +#include "clang/Basic/VirtualFileSystem.h" + +namespace clang { +namespace driver { + +/// Distro - Helper class for detecting and classifying Linux distributions. +/// +/// This class encapsulates the clang Linux distribution detection mechanism +/// as well as helper functions that match the specific (versioned) results +/// into wider distribution classes. +class Distro { +public: + enum DistroType { +// NB: Releases of a particular Linux distro should be kept together +// in this enum, because some tests are done by integer comparison against +// the first and last known member in the family, e.g. IsRedHat(). +ArchLinux, +DebianLenny, +DebianSqueeze, +DebianWheezy, +DebianJessie, +DebianStretch, +Exherbo, +RHEL5, +RHEL6, +RHEL7, +Fedora, +OpenSUSE, +UbuntuHardy, +UbuntuIntrepid, +UbuntuJaunty, +UbuntuKarmic, +UbuntuLucid, +UbuntuMaverick, +UbuntuNatty, +UbuntuOneiric, +UbuntuPrecise, +UbuntuQuantal, +UbuntuRaring, +UbuntuSaucy, +UbuntuTrusty, +UbuntuUtopic, +UbuntuVivid, +UbuntuWily, +UbuntuXenial, +UbuntuYakkety, +UbuntuZesty, +UnknownDistro + }; + +private: + /// The distribution, possibly with specific version. + DistroType DistroVal; + +public: + /// @name Constructors + /// @{ + + /// Default constructor leaves the distribution unknown. + Distro() : DistroVal() {} + + /// Constructs a Distro type for specific distribution. + Distro(DistroType D) : DistroVal(D) {} + + /// Detects the distribution using specified VFS. + explicit Distro(clang::vfs::FileSystem& VFS); + + bool operator==(const Distro &Other) const { +return DistroVal == Other.DistroVal; + } + + bool operator!=(const Distro &Other) const { +return DistroVal != Other.DistroVal; + } + + bool operator>=(const Distro &Other) const { +return DistroVal >= Other.DistroVal; + } + + bool operator<=(const Distro &Other) const { +return DistroVal <= Other.DistroVal; + } + + /// @} + /// @name Convenience Predicates + /// @{ + + bool IsRedhat() const { +return DistroVal == Fedora || (DistroVal >= RHEL5 && DistroVal <= RHEL7); + } + + bool IsOpenSUSE() const { +return DistroVal == OpenSUSE; + } + + bool IsDebian() const { +return DistroVal >= DebianLenny && DistroVal <= DebianStretch; + } + + bool IsUbuntu() const { +return DistroVal >= UbuntuHardy && DistroVal <= UbuntuZesty; + } + + /// @} +}; + +} // end namespace driver +} // end namespace clang + +#endif Index: cfe/trunk/lib/Driver/CMakeLists.txt === --- cfe/trunk/lib/Driver/CMakeLists.txt +++ cfe/trunk/lib/Driver/CMakeLists.txt @@ -12,6 +12,7 @@ Action.cpp Compilation.cpp CrossWindowsToolChain.cpp + Distro.cpp Driver.cpp DriverOptions.cpp Job.cpp Index: cfe/trunk/lib/Driver/Distro.cpp === --- cfe/trunk/lib/Driver/Distro.cpp +++ cfe/trunk/lib/Driver/Distro.cpp @@ -0,0 +1,131 @@ +//===--- Distro.cpp - Linux distribution detection support --*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#include "clang/Driver/Distro.h" +#include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/ADT/StringSwitch.h" +#include "llvm/Support/ErrorOr.h" +#include "llvm/Support/MemoryBuffer.h" + +using namespace clang::driver; +using namespace clang; + +static Distro::DistroType DetectDistro(vfs::FileSystem &VFS) { + llvm::ErrorOr> File = + VFS.getBufferForFile("/etc/lsb-release"); + if
[PATCH] D25869: [Driver] Add unit tests for Distro detection
This revision was automatically updated to reflect the committed changes. Closed by commit rL288062: [Driver] Add unit tests for Distro detection (authored by mgorny). Changed prior to commit: https://reviews.llvm.org/D25869?vs=78625&id=79444#toc Repository: rL LLVM https://reviews.llvm.org/D25869 Files: cfe/trunk/unittests/Driver/CMakeLists.txt cfe/trunk/unittests/Driver/DistroTest.cpp Index: cfe/trunk/unittests/Driver/DistroTest.cpp === --- cfe/trunk/unittests/Driver/DistroTest.cpp +++ cfe/trunk/unittests/Driver/DistroTest.cpp @@ -0,0 +1,305 @@ +//===- unittests/Driver/DistroTest.cpp --- ToolChains tests ---===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// +// +// Unit tests for Distro detection. +// +//===--===// + +#include "clang/Driver/Distro.h" +#include "clang/Basic/VirtualFileSystem.h" +#include "llvm/Support/raw_ostream.h" +#include "gtest/gtest.h" +using namespace clang; +using namespace clang::driver; + +namespace { + +// The tests include all release-related files for each distribution +// in the VFS, in order to make sure that earlier tests do not +// accidentally result in incorrect distribution guess. + +TEST(DistroTest, DetectUbuntu) { + vfs::InMemoryFileSystem UbuntuTrustyFileSystem; + // Ubuntu uses Debian Sid version. + UbuntuTrustyFileSystem.addFile("/etc/debian_version", 0, + llvm::MemoryBuffer::getMemBuffer("jessie/sid\n")); + UbuntuTrustyFileSystem.addFile("/etc/lsb-release", 0, + llvm::MemoryBuffer::getMemBuffer("DISTRIB_ID=Ubuntu\n" + "DISTRIB_RELEASE=14.04\n" + "DISTRIB_CODENAME=trusty\n" + "DISTRIB_DESCRIPTION=\"Ubuntu 14.04 LTS\"\n")); + UbuntuTrustyFileSystem.addFile("/etc/os-release", 0, + llvm::MemoryBuffer::getMemBuffer("NAME=\"Ubuntu\"\n" + "VERSION=\"14.04, Trusty Tahr\"\n" + "ID=ubuntu\n" + "ID_LIKE=debian\n" + "PRETTY_NAME=\"Ubuntu 14.04 LTS\"\n" + "VERSION_ID=\"14.04\"\n" + "HOME_URL=\"http://www.ubuntu.com/\"\n"; + "SUPPORT_URL=\"http://help.ubuntu.com/\"\n"; + "BUG_REPORT_URL=\"http://bugs.launchpad.net/ubuntu/\"\n";)); + + Distro UbuntuTrusty{UbuntuTrustyFileSystem}; + ASSERT_EQ(Distro(Distro::UbuntuTrusty), UbuntuTrusty); + ASSERT_TRUE(UbuntuTrusty.IsUbuntu()); + ASSERT_FALSE(UbuntuTrusty.IsRedhat()); + ASSERT_FALSE(UbuntuTrusty.IsOpenSUSE()); + ASSERT_FALSE(UbuntuTrusty.IsDebian()); + + vfs::InMemoryFileSystem UbuntuYakketyFileSystem; + UbuntuYakketyFileSystem.addFile("/etc/debian_version", 0, + llvm::MemoryBuffer::getMemBuffer("stretch/sid\n")); + UbuntuYakketyFileSystem.addFile("/etc/lsb-release", 0, + llvm::MemoryBuffer::getMemBuffer("DISTRIB_ID=Ubuntu\n" + "DISTRIB_RELEASE=16.10\n" + "DISTRIB_CODENAME=yakkety\n" + "DISTRIB_DESCRIPTION=\"Ubuntu 16.10\"\n")); + UbuntuYakketyFileSystem.addFile("/etc/os-release", 0, + llvm::MemoryBuffer::getMemBuffer("NAME=\"Ubuntu\"\n" + "VERSION=\"16.10 (Yakkety Yak)\"\n" + "ID=ubuntu\n" + "ID_LIKE=debian\n" + "PRETTY_NAME=\"Ubuntu 16.10\"\n" + "VERSION_ID=\"16.10\"\n" + "HOME_URL=\"http://www.ubuntu.com/\"\n"; + "SUPPORT_URL=\"http://help.ubuntu.com/\"\n"; + "BUG_REPORT_URL=\"http://bugs.launchpad.net/ubuntu/\"\n"; + "PRIVACY_POLICY_URL=\"http://www.ubuntu.com/legal/terms-and-policies/privacy-policy\"\n"; + "VERSION_CODENAME=yakkety\n" + "UBUNTU_CODENAME=yakkety\n")); + + Distro UbuntuYakkety{UbuntuYakketyFileSystem}; + ASSERT_EQ(Distro(Distro::UbuntuYakkety), UbuntuYakkety); + ASSERT_TRUE(UbuntuYakkety.IsUbuntu()); + ASSERT_FALSE(UbuntuYakkety.IsRedhat()); + ASSERT_FALSE(UbuntuYakkety.IsOpenSUSE()); + ASSERT_FALSE(UbuntuYakkety.IsDebian()); +} + +TEST(DistroTest, DetectRedhat) { + vfs::InMemoryFileSystem Fedora25FileSystem; + Fedora25FileSystem.addFile("/etc/system-rele
[PATCH] D25928: [cfi] Enable cfi-icall on ARM and AArch64.
eugenis closed this revision. eugenis added a comment. Committed in r286613 Repository: rL LLVM https://reviews.llvm.org/D25928 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27157: IRGen: Remove all uses of CreateDefaultAlignedLoad.
rjmccall added inline comments. Comment at: clang/lib/CodeGen/CGBuilder.h:126 // FIXME: these "default-aligned" APIs should be removed, // but I don't feel like fixing all the builtin code right now. llvm::StoreInst *CreateDefaultAlignedStore(llvm::Value *Val, Oh, please remove this comment, too, since you've now achieved it. :) Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2195 LoadInst *Load = -Builder.CreateDefaultAlignedLoad(IntToPtr, /*isVolatile=*/true); +Builder.CreateAlignedLoad(IntTy, IntToPtr, CharUnits::fromQuantity(4)); +Load->setVolatile(true); pcc wrote: > rjmccall wrote: > > Why 4? > __readfsdword is a Windows intrinsic which returns an unsigned long, which > always has size/alignment 4 on Windows. > > But now that I think about it I suppose we can get here on other platforms > with MS extensions enabled, so I've changed this to query the alignment. Thanks. https://reviews.llvm.org/D27157 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27157: IRGen: Remove all uses of CreateDefaultAlignedLoad.
pcc added inline comments. Comment at: clang/lib/CodeGen/CGBuilder.h:126 // FIXME: these "default-aligned" APIs should be removed, // but I don't feel like fixing all the builtin code right now. llvm::StoreInst *CreateDefaultAlignedStore(llvm::Value *Val, rjmccall wrote: > Oh, please remove this comment, too, since you've now achieved it. :) We still have CreateDefaultAlignedStore though. https://reviews.llvm.org/D27157 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26991: Hoist redundant load
mclow.lists added a comment. There are no uses of `_LIBCPP_UNROLL_LOOPS` in LLVM (other than the ones in ``. Googling for `_LIBCPP_UNROLL_LOOPS` on github finds the ones in libc++, and no others. I think I'll just take it out, and see what happens. https://reviews.llvm.org/D26991 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27163: Introduce -f[no-]strict-return flag that controls code generation for C++'s undefined behaviour return optimisation
mehdi_amini added inline comments. Comment at: lib/Sema/AnalysisBasedWarnings.cpp:530 + // Don't need to mark Objective-C methods or blocks since the undefined + // behaviour optimization isn't used for them. +} Quuxplusone wrote: > This seems like a trap waiting to spring on someone, unless there's a > technical reason that methods and blocks cannot possibly use the same > optimization paths as regular functions. ("Nobody's gotten around to > implementing it yet" is the most obvious nontechnical reason for the current > difference.) Either way, I'd expect this patch to include test cases for both > methods and blocks, to verify that the behavior you expect is actually the > behavior that happens. Basically, it ought to have a regression test > targeting the regression that I'm predicting is going to spring on someone as > soon as they implement optimizations for methods and blocks. > > Also, one dumb question: what about C++ lambdas? are they FunctionDecls too? > test case? > This seems like a trap waiting to spring on someone, unless there's a > technical reason that methods and blocks cannot possibly use the same > optimization paths as regular functions. The optimization path in LLVM is the same. I think the difference lies in clang IRGen: there is no "unreachable" generated for these so the optimizer can't be aggressive. So this patch is not changing anything for Objective-C methods and blocks, and I expect that we *already* have a test that covers this behavior (if not we should add one). Comment at: test/CodeGenCXX/return.cpp:21 + // CHECK-NOSTRICT-NEXT: load + // CHECK-NOSTRICT-NEXT: ret i32 + // CHECK-NOSTRICT-NEXT: } Quuxplusone wrote: > Can you explain why a load from an uninitialized stack location would be > *better* than a trap and/or `unreachable`? IIUC this is basically what Mehdi > is asking: i.e., can you explain the rationale for this patch, because I > don't get it either. It *seems* like a strict regression in code quality. There is a difference. LLVM will optimize: ``` define i32 @foo() { %1 = alloca i32, align 4 %2 = load i32, i32* %1, align 4 ret i32 %2 } ``` to: ``` define i32 @foo() { ret i32 undef } ``` Which means "return an undefined value" (basically any valide bit-pattern for an i32). This is not undefined behavior if the caller does not use the value with a side-effect. However with: ``` define i32 @foo() { unreachable } ``` Just calling `foo()` is undefined behavior, even if the returned value isn't used. So what this patch does is actually making the compiled-code `safer` by inhibiting some optimizations based on this UB. Comment at: test/CodeGenCXX/return.cpp:35 +// CHECK-NOSTRICT: @_Z21alwaysOptimizedReturn4Enum +// CHECK-NOSTRICT-OPT: @_Z21alwaysOptimizedReturn4Enum +int alwaysOptimizedReturn(Enum e) { This should be `-LABEL` check lines. And instead of repeating 4 times the same check, you could add a common prefix. Comment at: test/CodeGenCXX/return.cpp:47 + // CHECK-NOSTRICT-OPT-NEXT: zext + // CHECK-NOSTRICT-OPT-NEXT: ret i32 +} Document what's going on in the tests please. Repository: rL LLVM https://reviews.llvm.org/D27163 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26904: [astimporter] Support importing CXXDependentScopeMemberExpr and FunctionTemplateDecl
spyffe requested changes to this revision. spyffe added a comment. This revision now requires changes to proceed. There are several missing imports here, as well as a few minor nits. If the unit test cases aren't catching these, I'm a little concerned. We should be catching this. Also we should definitely test that bodies of function templates (in particular, bodies that use the template arguments) get imported properly. Comment at: lib/AST/ASTImporter.cpp:2327 +void ASTNodeImporter::ImportAttributes(Decl *From, Decl *To) { + for (Decl::attr_iterator I = From->attr_begin(), E = From->attr_end(); I != E; + ++I) { Would ``` for (Attr *A : From->atrs()) { ``` work in this case? Comment at: lib/AST/ASTImporter.cpp:3564 + + FunctionTemplateDecl *ToFunc = FunctionTemplateDecl::Create( + Importer.getToContext(), DC, Loc, Name, Params, TemplatedFD); You didn't import `TemplatedFD` before installing it in `ToFunc`. This code is broken, and we should make sure the unit tests know to catch these cases. Comment at: lib/AST/ASTImporter.cpp:6482 + return CXXDependentScopeMemberExpr::Create(Importer.getToContext(), + Base, BaseType, + E->isArrow(), You're installing `Base` and `BaseType` without importing them, as well as all the `Loc`s. https://reviews.llvm.org/D26904 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26955: Fix bitwidth for x87 extended-precision floating-point type
ddcc added inline comments. Comment at: lib/Basic/TargetInfo.cpp:229 switch (BitWidth) { - case 96: + case 80: if (&getLongDoubleFormat() == &llvm::APFloat::x87DoubleExtended) bruno wrote: > The change makes sense but I believe there's some historical reason why this > is 96 instead of 80? What problem have you found in practice? Do you have a > testcase or unittest that exercise the issue (and that could be added to the > patch)? I'd be curious why it was historically set to 96; I dug up rL190044 as the original commit, but it doesn't mention 80 vs 96 for this at all. I've been implementing an alternative symbolic constraint solver backend for the static analyzer, including floating-point support, which performs some type conversion and needs to reason about bitwidth. I'm still working on those patches, since they touch a lot of code and currently break some tests. I can write up a testcase for this issue, though I've only written IR testcases before and I'm not sure how I'd directly call a clang internal function? https://reviews.llvm.org/D26955 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26800: [Sema] Make __attribute__((notnull)) inheritable through function parameters
bettinson added a comment. ping https://reviews.llvm.org/D26800 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27033: [ASTImporter] Support importing UnresolvedLookupExpr nodes
spyffe added a comment. I only have a stylistic nit to add to Aleksei's comments. Comment at: lib/AST/ASTImporter.cpp:6492 + UnresolvedSet<8> ToDecls; + for (UnresolvedLookupExpr::decls_iterator S = E->decls_begin(), +F = E->decls_end(); a.sidorin wrote: > `auto` will look nice here. Alternatively, ``` for (Decl *D : E->decls()) ``` https://reviews.llvm.org/D27033 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26955: Fix bitwidth for x87 extended-precision floating-point type
efriedma added inline comments. Comment at: lib/Basic/TargetInfo.cpp:229 switch (BitWidth) { - case 96: + case 80: if (&getLongDoubleFormat() == &llvm::APFloat::x87DoubleExtended) ddcc wrote: > bruno wrote: > > The change makes sense but I believe there's some historical reason why > > this is 96 instead of 80? What problem have you found in practice? Do you > > have a testcase or unittest that exercise the issue (and that could be > > added to the patch)? > I'd be curious why it was historically set to 96; I dug up rL190044 as the > original commit, but it doesn't mention 80 vs 96 for this at all. > > I've been implementing an alternative symbolic constraint solver backend for > the static analyzer, including floating-point support, which performs some > type conversion and needs to reason about bitwidth. I'm still working on > those patches, since they touch a lot of code and currently break some tests. > I can write up a testcase for this issue, though I've only written IR > testcases before and I'm not sure how I'd directly call a clang internal > function? It was set to 96 because that's what the only caller (handleModeAttr) expects; see https://reviews.llvm.org/rL65935 and https://reviews.llvm.org/rL190926. It's arbitrary as far as I know. https://reviews.llvm.org/D26955 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27163: Introduce -f[no-]strict-return flag that controls code generation for C++'s undefined behaviour return optimisation
rjmccall added inline comments. Comment at: test/CodeGenCXX/return.cpp:21 + // CHECK-NOSTRICT-NEXT: load + // CHECK-NOSTRICT-NEXT: ret i32 + // CHECK-NOSTRICT-NEXT: } mehdi_amini wrote: > Quuxplusone wrote: > > Can you explain why a load from an uninitialized stack location would be > > *better* than a trap and/or `unreachable`? IIUC this is basically what > > Mehdi is asking: i.e., can you explain the rationale for this patch, > > because I don't get it either. It *seems* like a strict regression in code > > quality. > There is a difference. LLVM will optimize: > > ``` > define i32 @foo() { > %1 = alloca i32, align 4 > %2 = load i32, i32* %1, align 4 > ret i32 %2 > } > ``` > > to: > > ``` > define i32 @foo() { > ret i32 undef > } > ``` > > Which means "return an undefined value" (basically any valide bit-pattern for > an i32). This is not undefined behavior if the caller does not use the value > with a side-effect. > > However with: > > ``` > define i32 @foo() { > unreachable > } > ``` > > Just calling `foo()` is undefined behavior, even if the returned value isn't > used. > > So what this patch does is actually making the compiled-code `safer` by > inhibiting some optimizations based on this UB. We've been disabling this optimization completely in Apple compilers for years, so allowing it to ever kick in is actually an improvement. I'll try to elaborate on our reasoning for this change, which *is* actually somewhat Apple-specific. Falling off the end of a non-void function is only undefined behavior in C++, not in C. It is fairly common practice to compile C code as C++, and while there's a large number of potential language incompatibilities there, they tend to be rare or trivial to fix in practice. At Apple, we have a specific need to compile C code as C++ because of the C++-based IOKit API: while drivers are overwhelmingly written as C code, at Apple they have to be compiled as C++ in order to talk to the kernel. Moreover, often Apple did not write the drivers in question, and maintaining a large set of patches in order to eliminate undefined behavior that wasn't actually UB in the originally-intended build configuration is not really seen as acceptable. It makes sense to me to expose this as a flag akin to -fno-strict-aliasing in order to support C-in-C++ code bases like Apple's drivers. It is possible that we don't actually have to change the default for the flag on Apple platforms and can instead pursue more targeted build changes. Repository: rL LLVM https://reviews.llvm.org/D27163 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26753: ASTImporter: improve support for C++ templates
spyffe accepted this revision. spyffe added a comment. This revision is now accepted and ready to land. Yeah, that test looks great! Thanks! Comment at: lib/AST/ASTImporter.cpp:496 +return false; + if (DN1->isIdentifier()) +return IsStructurallyEquivalent(DN1->getIdentifier(), spyffe wrote: > We should probably also check whether `DN1->isIdentifier() == > DN2->isIdentifier()`. Looking at my comment with fresh post Thanksgiving eyes, that would be totally wrong. The `IsStructurallyEquivalent` is fine. Comment at: test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec2.cpp:67 +template +struct Child1: public Two::Three::Parent { + char member; ooh, nice! https://reviews.llvm.org/D26753 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27163: Introduce -f[no-]strict-return flag that controls code generation for C++'s undefined behaviour return optimisation
mehdi_amini added inline comments. Comment at: test/CodeGenCXX/return.cpp:21 + // CHECK-NOSTRICT-NEXT: load + // CHECK-NOSTRICT-NEXT: ret i32 + // CHECK-NOSTRICT-NEXT: } rjmccall wrote: > mehdi_amini wrote: > > Quuxplusone wrote: > > > Can you explain why a load from an uninitialized stack location would be > > > *better* than a trap and/or `unreachable`? IIUC this is basically what > > > Mehdi is asking: i.e., can you explain the rationale for this patch, > > > because I don't get it either. It *seems* like a strict regression in > > > code quality. > > There is a difference. LLVM will optimize: > > > > ``` > > define i32 @foo() { > > %1 = alloca i32, align 4 > > %2 = load i32, i32* %1, align 4 > > ret i32 %2 > > } > > ``` > > > > to: > > > > ``` > > define i32 @foo() { > > ret i32 undef > > } > > ``` > > > > Which means "return an undefined value" (basically any valide bit-pattern > > for an i32). This is not undefined behavior if the caller does not use the > > value with a side-effect. > > > > However with: > > > > ``` > > define i32 @foo() { > > unreachable > > } > > ``` > > > > Just calling `foo()` is undefined behavior, even if the returned value > > isn't used. > > > > So what this patch does is actually making the compiled-code `safer` by > > inhibiting some optimizations based on this UB. > We've been disabling this optimization completely in Apple compilers for > years, so allowing it to ever kick in is actually an improvement. I'll try > to elaborate on our reasoning for this change, which *is* actually somewhat > Apple-specific. > > Falling off the end of a non-void function is only undefined behavior in C++, > not in C. It is fairly common practice to compile C code as C++, and while > there's a large number of potential language incompatibilities there, they > tend to be rare or trivial to fix in practice. At Apple, we have a specific > need to compile C code as C++ because of the C++-based IOKit API: while > drivers are overwhelmingly written as C code, at Apple they have to be > compiled as C++ in order to talk to the kernel. Moreover, often Apple did > not write the drivers in question, and maintaining a large set of patches in > order to eliminate undefined behavior that wasn't actually UB in the > originally-intended build configuration is not really seen as acceptable. > > It makes sense to me to expose this as a flag akin to -fno-strict-aliasing in > order to support C-in-C++ code bases like Apple's drivers. It is possible > that we don't actually have to change the default for the flag on Apple > platforms and can instead pursue more targeted build changes. I totally understand why such flag is desirable, it is just not a clear cut to make it the default on one platform only (and having this flag available upstream does not prevent the Xcode clang from enabling this by default though, if fixing the build settings isn't possible/desirable). Repository: rL LLVM https://reviews.llvm.org/D27163 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27163: Introduce -f[no-]strict-return flag that controls code generation for C++'s undefined behaviour return optimisation
rsmith added a comment. A target-specific default for this, simply because there's a lot of code on Darwin that happens to violate this language rule, doesn't make sense to me. Basing the behavior on whether a `-Wreturn-type` warning would have been emitted seems like an extremely strange heuristic: only optimizing in the cases where we provide the user no hint that we will do so seems incredibly user-hostile. Regardless of anything else, it does not make any sense to "return" stack garbage when the return type is a C++ class type, particularly one with a non-trivial copy constructor or destructor. A trap at `-O0` is the kindest thing we can do in that case. In summary, I think it could be reasonable to have such a flag to disable the trap/unreachable *only* for scalar types (or, at a push, trivially-copyable types). But it seems unreasonable to have different defaults for Darwin, or to look at whether a `-Wreturn-type` warning would have fired. Alternatively, since it seems you're only interested in the behavior of cases where `-Wreturn-type` would have fired, how about using Clang's tooling support to write a utility to add a return statement to the end of every function where it would fire, and give that to your users to fix their code? Repository: rL LLVM https://reviews.llvm.org/D27163 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25866: [Sema] Support implicit scalar to vector conversions
bruno added inline comments. Comment at: lib/Sema/SemaExpr.cpp:8064 + ScalarCast = CK_FloatingCast; +} else if (ScalarTy->isIntegralType(S.Context)) { + // Determine if the integer constant can be expressed as a floating point sdardis wrote: > bruno wrote: > > I don't see why it's necessary to check for all specific cases where the > > scalar is a constant. For all the others scenarios it should be enough to > > get the right answer via `getIntegerTypeOrder` or `getFloatTypeOrder`. For > > this is specific condition, the `else` part for the `CstScalar` below > > should also handle the constant case, right? > > > > > If we have a scalar constant, it is necessary to examine it's actual value to > see if it can be casted without truncation. The scalar constant's type is > somewhat irrelevant. Consider: > >v4f32 f(v4f32 a) { > return a + (double)1.0; >} > > In this case examining the order only works if the vector's order is greater > than or equal to the scalar constant's order. Instead, if we ask whether the > scalar constant can be represented as a constant scalar of the vector's > element type, then we know if we can convert without the loss of precision. > > For integers, the case is a little more contrived due to native integer > width, but the question is the same. Can a scalar constant X be represented > as a given floating point type. Consider: > >v4f32 f(v4f32 a) { > return a + (signed int)1; > } > > This would rejected for platforms where a signed integer's width is greater > than the precision of float if we examined it based purely on types and their > sizes. Instead, if we convert the integer to the float point's type with > APFloat and convert back to see if we have the same value, we can determine > if the scalar constant can be safely converted without the loss of precision. > > I based the logic examining the value of the constant scalar based on GCC's > behaviour, which implicitly converts scalars regardless of their type if they > can be represented in the same type of the vector's element type without the > loss of precision. If the scalar cannot be evaluated to a constant at compile > time, then the size in bits for the scalar's type determines if it can be > converted safely. Right. Can you split the scalarTy <-> vectorEltTy checking logic into separate functions; one for cst-scalar-int-to-vec-elt-int and another for scalar-int-to-vec-elt-float? Comment at: lib/Sema/SemaExpr.cpp:8267 + } + // Otherwise, use the generic diagnostic. sdardis wrote: > bruno wrote: > > This change seems orthogonal to this patch. Can you make it a separated > > patch with the changes from test/Sema/vector-cast.c? > This change is a necessary part of this patch and it can't be split out in > sensible fashion. > > For example, line 329 of test/Sema/zvector.c adds a scalar signed character > to a vector of signed characters. With just this change we would report > "cannot convert between scalar type 'signed char' and vector type '__vector > signed char' (vector of 16 'signed char' values) as implicit conversion would > cause truncation". > > This is heavily misleading for C and C++ code as we don't perform implicit > conversions and signed char could be implicitly converted to a vector of > signed char without the loss of precision. > > To make this diagnostic precise without performing implicit conversions > requires determining cases where implicit conversion would cause truncation > and reporting that failure reason, or defaulting to the generic diagnostic. > > Keeping this change as part of this patch is cleaner and simpler as it covers > both implicit conversions and more precise diagnostics. Can you double check whether we should support these GCC semantics for getLangOpts().ZVector? If not, then this should be introduced in a separate patch/commit. Comment at: lib/Sema/SemaExpr.cpp:8020 + if (Order < 0) +return true; +} Please also early return `!CstScalar && Order < 0` like you did below. Comment at: lib/Sema/SemaExpr.cpp:8064 + !ScalarTy->hasSignedIntegerRepresentation()); +bool ignored = false; +Float.convertToInteger( `ignored` => `Ignored` Comment at: lib/Sema/SemaExpr.cpp:8171 +return LHSType; +} } It's not clear to me whether clang should support the GCC semantics when in `getLangOpts().AltiVec || getLangOpts().ZVector`, do you? You can remove the curly braces for the `if` and `else` here. Comment at: lib/Sema/SemaExpr.cpp:8182 +return RHSType; +} } Also remove braces here. https://reviews.llvm.org/D25866 ___ cfe-commits mailing list cfe-commits@lists.
[PATCH] D27163: Introduce -f[no-]strict-return flag that controls code generation for C++'s undefined behaviour return optimisation
rsmith added a comment. In https://reviews.llvm.org/D27163#607078, @rsmith wrote: > A target-specific default for this, simply because there's a lot of code on > Darwin that happens to violate this language rule, doesn't make sense to me. ... but rjmccall's explanation of the problem helps. The C compatibility argument also suggests that this should only apply to trivially-copyable types, and perhaps only scalar types. The same issue presumably arises for `-fstrict-enums`, which is again only UB in C++? (Also, C has rather different and much less useful TBAA rules.) Perhaps some catch-all "I want defined behavior for things that C defines even though I'm compiling in C++" flag would make sense here? Repository: rL LLVM https://reviews.llvm.org/D27163 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26267: [Modules] Include builtins with #include instead of #import for ObjC
bruno added a comment. @rsmith ping! https://reviews.llvm.org/D26267 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27163: Introduce -f[no-]strict-return flag that controls code generation for C++'s undefined behaviour return optimisation
rjmccall added a comment. In https://reviews.llvm.org/D27163#607100, @rsmith wrote: > In https://reviews.llvm.org/D27163#607078, @rsmith wrote: > > > A target-specific default for this, simply because there's a lot of code on > > Darwin that happens to violate this language rule, doesn't make sense to me. > > > ... but rjmccall's explanation of the problem helps. The C compatibility > argument also suggests that this should only apply to trivially-copyable > types, and perhaps only scalar types. I would agree with this. The right rule is probably whether the type is trivially-destructed. > The same issue presumably arises for `-fstrict-enums`, which is again only UB > in C++? (Also, C has rather different and much less useful TBAA rules.) > Perhaps some catch-all "I want defined behavior for things that C defines > even though I'm compiling in C++" flag would make sense here? I don't think we've seen problems from any of these. Also I don't think the TBAA rules are as different as you're suggesting, except that C++ actually tries to provide specific rules for unions (that don't match what users actually want). Repository: rL LLVM https://reviews.llvm.org/D27163 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r288081 - Make CGVTables use ConstantInitBuilder. NFC.
Author: rjmccall Date: Mon Nov 28 16:18:33 2016 New Revision: 288081 URL: http://llvm.org/viewvc/llvm-project?rev=288081&view=rev Log: Make CGVTables use ConstantInitBuilder. NFC. Modified: cfe/trunk/lib/CodeGen/CGVTables.cpp cfe/trunk/lib/CodeGen/CGVTables.h cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp Modified: cfe/trunk/lib/CodeGen/CGVTables.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGVTables.cpp?rev=288081&r1=288080&r2=288081&view=diff == --- cfe/trunk/lib/CodeGen/CGVTables.cpp (original) +++ cfe/trunk/lib/CodeGen/CGVTables.cpp Mon Nov 28 16:18:33 2016 @@ -14,6 +14,7 @@ #include "CGCXXABI.h" #include "CodeGenFunction.h" #include "CodeGenModule.h" +#include "ConstantBuilder.h" #include "clang/AST/CXXInheritance.h" #include "clang/AST/RecordLayout.h" #include "clang/CodeGen/CGFunctionInfo.h" @@ -524,29 +525,29 @@ void CodeGenVTables::EmitThunks(GlobalDe emitThunk(GD, Thunk, /*ForVTable=*/false); } -llvm::Constant *CodeGenVTables::CreateVTableComponent( -unsigned Idx, const VTableLayout &VTLayout, llvm::Constant *RTTI, -unsigned &NextVTableThunkIndex) { - VTableComponent Component = VTLayout.vtable_components()[Idx]; - - auto OffsetConstant = [&](CharUnits Offset) { -return llvm::ConstantExpr::getIntToPtr( -llvm::ConstantInt::get(CGM.PtrDiffTy, Offset.getQuantity()), -CGM.Int8PtrTy); +void CodeGenVTables::addVTableComponent( +ConstantArrayBuilder &builder, const VTableLayout &layout, +unsigned idx, llvm::Constant *rtti, unsigned &nextVTableThunkIndex) { + auto &component = layout.vtable_components()[idx]; + + auto addOffsetConstant = [&](CharUnits offset) { +builder.add(llvm::ConstantExpr::getIntToPtr( +llvm::ConstantInt::get(CGM.PtrDiffTy, offset.getQuantity()), +CGM.Int8PtrTy)); }; - switch (Component.getKind()) { + switch (component.getKind()) { case VTableComponent::CK_VCallOffset: -return OffsetConstant(Component.getVCallOffset()); +return addOffsetConstant(component.getVCallOffset()); case VTableComponent::CK_VBaseOffset: -return OffsetConstant(Component.getVBaseOffset()); +return addOffsetConstant(component.getVBaseOffset()); case VTableComponent::CK_OffsetToTop: -return OffsetConstant(Component.getOffsetToTop()); +return addOffsetConstant(component.getOffsetToTop()); case VTableComponent::CK_RTTI: -return RTTI; +return builder.add(llvm::ConstantExpr::getBitCast(rtti, CGM.Int8PtrTy)); case VTableComponent::CK_FunctionPointer: case VTableComponent::CK_CompleteDtorPointer: @@ -554,17 +555,17 @@ llvm::Constant *CodeGenVTables::CreateVT GlobalDecl GD; // Get the right global decl. -switch (Component.getKind()) { +switch (component.getKind()) { default: llvm_unreachable("Unexpected vtable component kind"); case VTableComponent::CK_FunctionPointer: - GD = Component.getFunctionDecl(); + GD = component.getFunctionDecl(); break; case VTableComponent::CK_CompleteDtorPointer: - GD = GlobalDecl(Component.getDestructorDecl(), Dtor_Complete); + GD = GlobalDecl(component.getDestructorDecl(), Dtor_Complete); break; case VTableComponent::CK_DeletingDtorPointer: - GD = GlobalDecl(Component.getDestructorDecl(), Dtor_Deleting); + GD = GlobalDecl(component.getDestructorDecl(), Dtor_Deleting); break; } @@ -580,68 +581,69 @@ llvm::Constant *CodeGenVTables::CreateVT ? MD->hasAttr() : (MD->hasAttr() || !MD->hasAttr()); if (!CanEmitMethod) -return llvm::ConstantExpr::getNullValue(CGM.Int8PtrTy); +return builder.addNullPointer(CGM.Int8PtrTy); // Method is acceptable, continue processing as usual. } -auto SpecialVirtualFn = [&](llvm::Constant *&Cache, StringRef Name) { - if (!Cache) { -llvm::FunctionType *Ty = -llvm::FunctionType::get(CGM.VoidTy, /*isVarArg=*/false); -Cache = CGM.CreateRuntimeFunction(Ty, Name); -if (auto *F = dyn_cast(Cache)) - F->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global); -Cache = llvm::ConstantExpr::getBitCast(Cache, CGM.Int8PtrTy); - } - return Cache; +auto getSpecialVirtualFn = [&](StringRef name) { + llvm::FunctionType *fnTy = + llvm::FunctionType::get(CGM.VoidTy, /*isVarArg=*/false); + llvm::Constant *fn = CGM.CreateRuntimeFunction(fnTy, name); + if (auto f = dyn_cast(fn)) +f->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global); + return llvm::ConstantExpr::getBitCast(fn, CGM.Int8PtrTy); }; -if (cast(GD.getDecl())->isPure()) - // We have a pure virtual member function. - return SpecialVirtualFn(PureVirtualFn, - CGM.getCXXABI().GetPureVirtualCallName()); - -if (
r288080 - Hide the result of building a constant initializer. NFC.
Author: rjmccall Date: Mon Nov 28 16:18:30 2016 New Revision: 288080 URL: http://llvm.org/viewvc/llvm-project?rev=288080&view=rev Log: Hide the result of building a constant initializer. NFC. Modified: cfe/trunk/lib/CodeGen/CGObjCGNU.cpp cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp cfe/trunk/lib/CodeGen/CodeGenModule.cpp cfe/trunk/lib/CodeGen/ConstantBuilder.h Modified: cfe/trunk/lib/CodeGen/CGObjCGNU.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGObjCGNU.cpp?rev=288080&r1=288079&r2=288080&view=diff == --- cfe/trunk/lib/CodeGen/CGObjCGNU.cpp (original) +++ cfe/trunk/lib/CodeGen/CGObjCGNU.cpp Mon Nov 28 16:18:30 2016 @@ -1549,7 +1549,7 @@ GenerateMethodList(StringRef ClassName, Methods.add( llvm::ConstantStruct::get(ObjCMethodTy, {C, MethodTypes[i], Method})); } - MethodList.add(Methods.finish()); + Methods.finishAndAddTo(MethodList); // Create an instance of the structure return MethodList.finishAndCreateGlobal(".objc_method_list", @@ -1584,9 +1584,9 @@ GenerateIvarList(ArrayRefsecond); @@ -2498,7 +2498,7 @@ llvm::Function *CGObjCGNU::ModuleInitFun auto SelStruct = Selectors.beginStruct(SelStructTy); SelStruct.add(NULLPtr); SelStruct.add(NULLPtr); -Selectors.add(SelStruct.finish()); +SelStruct.finishAndAddTo(Selectors); } // Number of static selectors Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=288080&r1=288079&r2=288080&view=diff == --- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original) +++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Mon Nov 28 16:18:30 2016 @@ -2832,7 +2832,7 @@ CGOpenMPRuntime::createOffloadingBinaryD Dev.add(ImgEnd); Dev.add(HostEntriesBegin); Dev.add(HostEntriesEnd); -DeviceImagesEntries.add(Dev.finish()); +Dev.finishAndAddTo(DeviceImagesEntries); } // Create device images global array. Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=288080&r1=288079&r2=288080&view=diff == --- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original) +++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Mon Nov 28 16:18:30 2016 @@ -753,7 +753,7 @@ void CodeGenModule::EmitCtorList(CtorLis ctor.add(llvm::ConstantExpr::getBitCast(I.AssociatedData, VoidPtrTy)); else ctor.addNullPointer(VoidPtrTy); -ctors.add(ctor.finish()); +ctor.finishAndAddTo(ctors); } auto list = Modified: cfe/trunk/lib/CodeGen/ConstantBuilder.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ConstantBuilder.h?rev=288080&r1=288079&r2=288080&view=diff == --- cfe/trunk/lib/CodeGen/ConstantBuilder.h (original) +++ cfe/trunk/lib/CodeGen/ConstantBuilder.h Mon Nov 28 16:18:30 2016 @@ -58,10 +58,10 @@ public: assert(Buffer.empty() && "didn't claim all values out of buffer"); } - class AggregateBuilder { + class AggregateBuilderBase { protected: ConstantInitBuilder &Builder; -AggregateBuilder *Parent; +AggregateBuilderBase *Parent; size_t Begin; bool Finished = false; bool Frozen = false; @@ -74,8 +74,8 @@ public: return Builder.Buffer; } -AggregateBuilder(ConstantInitBuilder &builder, - AggregateBuilder *parent) +AggregateBuilderBase(ConstantInitBuilder &builder, + AggregateBuilderBase *parent) : Builder(builder), Parent(parent), Begin(builder.Buffer.size()) { if (parent) { assert(!parent->Frozen && "parent already has child builder active"); @@ -86,7 +86,7 @@ public: } } -~AggregateBuilder() { +~AggregateBuilderBase() { assert(Finished && "didn't claim value from aggregate builder"); } @@ -107,17 +107,17 @@ public: public: // Not copyable. -AggregateBuilder(const AggregateBuilder &) = delete; -AggregateBuilder &operator=(const AggregateBuilder &) = delete; +AggregateBuilderBase(const AggregateBuilderBase &) = delete; +AggregateBuilderBase &operator=(const AggregateBuilderBase &) = delete; // Movable, mostly to allow returning. But we have to write this out // properly to satisfy the assert in the destructor. -AggregateBuilder(AggregateBuilder &&other) +AggregateBuilderBase(AggregateBuilderBase &&other) : Builder(other.Builder), Parent(other.Parent), Begin(other.Begin), Finished(other.Finished), Frozen(other.Frozen) { other.Finished = false; } -AggregateBuilder &operator=(AggregateBuilder &&other) = delete; +AggregateBuilderBase &operator=(AggregateBuil
r288079 - ConstantBuilder -> ConstantInitBuilder for clarity, and
Author: rjmccall Date: Mon Nov 28 16:18:27 2016 New Revision: 288079 URL: http://llvm.org/viewvc/llvm-project?rev=288079&view=rev Log: ConstantBuilder -> ConstantInitBuilder for clarity, and move the member classes up to top level to allow forward declarations to name them. NFC. Modified: cfe/trunk/lib/CodeGen/CGBlocks.cpp cfe/trunk/lib/CodeGen/CGCUDANV.cpp cfe/trunk/lib/CodeGen/CGObjCGNU.cpp cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp cfe/trunk/lib/CodeGen/CodeGenModule.cpp cfe/trunk/lib/CodeGen/ConstantBuilder.h Modified: cfe/trunk/lib/CodeGen/CGBlocks.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBlocks.cpp?rev=288079&r1=288078&r2=288079&view=diff == --- cfe/trunk/lib/CodeGen/CGBlocks.cpp (original) +++ cfe/trunk/lib/CodeGen/CGBlocks.cpp Mon Nov 28 16:18:27 2016 @@ -88,7 +88,7 @@ static llvm::Constant *buildBlockDescrip else i8p = CGM.VoidPtrTy; - ConstantBuilder builder(CGM); + ConstantInitBuilder builder(CGM); auto elements = builder.beginStruct(); // reserved @@ -1076,7 +1076,7 @@ static llvm::Constant *buildGlobalBlock( assert(blockInfo.CanBeGlobal); // Generate the constants for the block literal initializer. - ConstantBuilder builder(CGM); + ConstantInitBuilder builder(CGM); auto fields = builder.beginStruct(); // isa Modified: cfe/trunk/lib/CodeGen/CGCUDANV.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCUDANV.cpp?rev=288079&r1=288078&r2=288079&view=diff == --- cfe/trunk/lib/CodeGen/CGCUDANV.cpp (original) +++ cfe/trunk/lib/CodeGen/CGCUDANV.cpp Mon Nov 28 16:18:27 2016 @@ -298,7 +298,7 @@ llvm::Function *CGNVCUDARuntime::makeMod CGM.getTriple().isMacOSX() ? "__NV_CUDA,__fatbin" : ".nvFatBinSegment"; // Create initialized wrapper structure that points to the loaded GPU binary -ConstantBuilder Builder(CGM); +ConstantInitBuilder Builder(CGM); auto Values = Builder.beginStruct(FatbinWrapperTy); // Fatbin wrapper magic. Values.addInt(IntTy, 0x466243b1); Modified: cfe/trunk/lib/CodeGen/CGObjCGNU.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGObjCGNU.cpp?rev=288079&r1=288078&r2=288079&view=diff == --- cfe/trunk/lib/CodeGen/CGObjCGNU.cpp (original) +++ cfe/trunk/lib/CodeGen/CGObjCGNU.cpp Mon Nov 28 16:18:27 2016 @@ -222,7 +222,7 @@ protected: } /// Push the property attributes into two structure fields. - void PushPropertyAttributes(ConstantBuilder::StructBuilder &Fields, + void PushPropertyAttributes(ConstantStructBuilder &Fields, ObjCPropertyDecl *property, bool isSynthesized=true, bool isDynamic=true) { int attrs = property->getPropertyAttributes(); @@ -1204,7 +1204,7 @@ llvm::Constant *CGObjCGNUstep::GetEHType llvm::Constant *typeName = ExportUniqueString(className, "__objc_eh_typename_"); - ConstantBuilder builder(CGM); + ConstantInitBuilder builder(CGM); auto fields = builder.beginStruct(); fields.add(BVtable); fields.add(typeName); @@ -1242,7 +1242,7 @@ ConstantAddress CGObjCGNU::GenerateConst else if (isa->getType() != PtrToIdTy) isa = llvm::ConstantExpr::getBitCast(isa, PtrToIdTy); - ConstantBuilder Builder(CGM); + ConstantInitBuilder Builder(CGM); auto Fields = Builder.beginStruct(); Fields.add(isa); Fields.add(MakeConstantString(Str)); @@ -1524,7 +1524,7 @@ GenerateMethodList(StringRef ClassName, if (MethodSels.empty()) return NULLPtr; - ConstantBuilder Builder(CGM); + ConstantInitBuilder Builder(CGM); auto MethodList = Builder.beginStruct(); MethodList.addNullPointer(CGM.Int8PtrTy); @@ -1564,7 +1564,7 @@ GenerateIvarList(ArrayRef Protocols) { - ConstantBuilder Builder(CGM); + ConstantInitBuilder Builder(CGM); auto ProtocolList = Builder.beginStruct(); ProtocolList.add(NULLPtr); ProtocolList.addInt(LongTy, Protocols.size()); @@ -1770,7 +1770,7 @@ CGObjCGNU::GenerateEmptyProtocol(const s llvm::Constant *MethodList = GenerateProtocolMethodList({}, {}); // Protocols are objects containing lists of the methods implemented and // protocols adopted. - ConstantBuilder Builder(CGM); + ConstantInitBuilder Builder(CGM); auto Elements = Builder.beginStruct(); // The isa pointer must be set to a magic number so the runtime knows it's @@ -1869,13 +1869,13 @@ void CGObjCGNU::GenerateProtocol(const O numReqProperties++; } -ConstantBuilder reqPropertyListBuilder(CGM); +ConstantInitBuilder reqPropertyListBuilder(CGM); auto reqPropertiesList = reqPropertyListBuilder.beginStruct(); reqPropertiesList.addInt(IntTy, numReqProperties); reqPropertiesList.add(NULLPtr); auto reqPropertiesArray = reqPropertiesList.beginArray(propertyMetadataTy); -Consta
[PATCH] D27157: IRGen: Remove all uses of CreateDefaultAlignedLoad.
rjmccall added inline comments. Comment at: clang/lib/CodeGen/CGBuilder.h:126 // FIXME: these "default-aligned" APIs should be removed, // but I don't feel like fixing all the builtin code right now. llvm::StoreInst *CreateDefaultAlignedStore(llvm::Value *Val, pcc wrote: > rjmccall wrote: > > Oh, please remove this comment, too, since you've now achieved it. :) > We still have CreateDefaultAlignedStore though. Oh, yes, of course. In that case, this LGTM. https://reviews.llvm.org/D27157 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26267: [Modules] Include builtins with #include instead of #import for ObjC
rsmith added inline comments. Comment at: lib/Frontend/FrontendActions.cpp:227 +IsBuiltin = true; + addHeaderInclude(H.NameAsWritten, Includes, LangOpts, Module->IsExternC, + IsBuiltin /*AlwaysInclude*/); I don't understand why this would be necessary. If these headers are in fact textual headers, they won't be in the `HK_Normal` or `HK_Private` lists at all (they'll be in the `HK_Private` or `HK_PrivateTextual` lists instead), so your `IsBuiltin = true;` line should be unreachable. (Checking for an absolute path also seems suspicious here.) I suspect the problem is instead that `#import` just doesn't work properly with modules (specifically, either with local submodule visibility or with modules that do not `export *`), because it doesn't care whether the previous inclusion is visible or not, and as a result it assumes "I've heard about someone #including this ever" means the same thing as "the contents of this file are visible right now". I know that `#pragma once` has this issue, so I'd expect `#import` to also suffer from it. https://reviews.llvm.org/D26267 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r288083 - IRGen: Remove all uses of CreateDefaultAlignedLoad.
Author: pcc Date: Mon Nov 28 16:30:21 2016 New Revision: 288083 URL: http://llvm.org/viewvc/llvm-project?rev=288083&view=rev Log: IRGen: Remove all uses of CreateDefaultAlignedLoad. Differential Revision: https://reviews.llvm.org/D27157 Modified: cfe/trunk/lib/CodeGen/CGBuilder.h cfe/trunk/lib/CodeGen/CGBuiltin.cpp cfe/trunk/lib/CodeGen/CGCall.cpp cfe/trunk/lib/CodeGen/CGObjCGNU.cpp cfe/trunk/lib/CodeGen/TargetInfo.cpp Modified: cfe/trunk/lib/CodeGen/CGBuilder.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuilder.h?rev=288083&r1=288082&r2=288083&view=diff == --- cfe/trunk/lib/CodeGen/CGBuilder.h (original) +++ cfe/trunk/lib/CodeGen/CGBuilder.h Mon Nov 28 16:30:21 2016 @@ -124,19 +124,6 @@ public: // FIXME: these "default-aligned" APIs should be removed, // but I don't feel like fixing all the builtin code right now. - llvm::LoadInst *CreateDefaultAlignedLoad(llvm::Value *Addr, - const llvm::Twine &Name = "") { -return CGBuilderBaseTy::CreateLoad(Addr, false, Name); - } - llvm::LoadInst *CreateDefaultAlignedLoad(llvm::Value *Addr, - const char *Name) { -return CGBuilderBaseTy::CreateLoad(Addr, false, Name); - } - llvm::LoadInst *CreateDefaultAlignedLoad(llvm::Value *Addr, bool IsVolatile, - const llvm::Twine &Name = "") { -return CGBuilderBaseTy::CreateLoad(Addr, IsVolatile, Name); - } - llvm::StoreInst *CreateDefaultAlignedStore(llvm::Value *Val, llvm::Value *Addr, bool IsVolatile = false) { Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=288083&r1=288082&r2=288083&view=diff == --- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original) +++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Mon Nov 28 16:30:21 2016 @@ -2191,8 +2191,9 @@ RValue CodeGenFunction::EmitBuiltinExpr( Value *IntToPtr = Builder.CreateIntToPtr(EmitScalarExpr(E->getArg(0)), llvm::PointerType::get(IntTy, 257)); -LoadInst *Load = -Builder.CreateDefaultAlignedLoad(IntToPtr, /*isVolatile=*/true); +LoadInst *Load = Builder.CreateAlignedLoad( +IntTy, IntToPtr, getContext().getTypeAlignInChars(E->getType())); +Load->setVolatile(true); return RValue::get(Load); } @@ -5440,9 +5441,11 @@ Value *CodeGenFunction::EmitAArch64Built switch (BuiltinID) { default: break; case NEON::BI__builtin_neon_vldrq_p128: { -llvm::Type *Int128PTy = llvm::Type::getIntNPtrTy(getLLVMContext(), 128); +llvm::Type *Int128Ty = llvm::Type::getIntNTy(getLLVMContext(), 128); +llvm::Type *Int128PTy = llvm::PointerType::get(Int128Ty, 0); Value *Ptr = Builder.CreateBitCast(EmitScalarExpr(E->getArg(0)), Int128PTy); -return Builder.CreateDefaultAlignedLoad(Ptr); +return Builder.CreateAlignedLoad(Int128Ty, Ptr, + CharUnits::fromQuantity(16)); } case NEON::BI__builtin_neon_vstrq_p128: { llvm::Type *Int128PTy = llvm::Type::getIntNPtrTy(getLLVMContext(), 128); @@ -6615,27 +6618,37 @@ Value *CodeGenFunction::EmitAArch64Built return EmitNeonCall(CGM.getIntrinsic(Int, Tys), Ops, ""); } case NEON::BI__builtin_neon_vld1_v: - case NEON::BI__builtin_neon_vld1q_v: + case NEON::BI__builtin_neon_vld1q_v: { Ops[0] = Builder.CreateBitCast(Ops[0], llvm::PointerType::getUnqual(VTy)); -return Builder.CreateDefaultAlignedLoad(Ops[0]); +auto Alignment = CharUnits::fromQuantity( +BuiltinID == NEON::BI__builtin_neon_vld1_v ? 8 : 16); +return Builder.CreateAlignedLoad(VTy, Ops[0], Alignment); + } case NEON::BI__builtin_neon_vst1_v: case NEON::BI__builtin_neon_vst1q_v: Ops[0] = Builder.CreateBitCast(Ops[0], llvm::PointerType::getUnqual(VTy)); Ops[1] = Builder.CreateBitCast(Ops[1], VTy); return Builder.CreateDefaultAlignedStore(Ops[1], Ops[0]); case NEON::BI__builtin_neon_vld1_lane_v: - case NEON::BI__builtin_neon_vld1q_lane_v: + case NEON::BI__builtin_neon_vld1q_lane_v: { Ops[1] = Builder.CreateBitCast(Ops[1], Ty); Ty = llvm::PointerType::getUnqual(VTy->getElementType()); Ops[0] = Builder.CreateBitCast(Ops[0], Ty); -Ops[0] = Builder.CreateDefaultAlignedLoad(Ops[0]); +auto Alignment = CharUnits::fromQuantity( +BuiltinID == NEON::BI__builtin_neon_vld1_lane_v ? 8 : 16); +Ops[0] = +Builder.CreateAlignedLoad(VTy->getElementType(), Ops[0], Alignment); return Builder.CreateInsertElement(Ops[1], Ops[0], Ops[2], "vld1_lane"); + } case NEON::BI__builtin_neon_vld1_dup_v: case NEON::BI__builtin_neon_vld1q_dup_v: { Value *V = UndefValue::
[PATCH] D27157: IRGen: Remove all uses of CreateDefaultAlignedLoad.
This revision was automatically updated to reflect the committed changes. Closed by commit rL288083: IRGen: Remove all uses of CreateDefaultAlignedLoad. (authored by pcc). Changed prior to commit: https://reviews.llvm.org/D27157?vs=79436&id=79462#toc Repository: rL LLVM https://reviews.llvm.org/D27157 Files: cfe/trunk/lib/CodeGen/CGBuilder.h cfe/trunk/lib/CodeGen/CGBuiltin.cpp cfe/trunk/lib/CodeGen/CGCall.cpp cfe/trunk/lib/CodeGen/CGObjCGNU.cpp cfe/trunk/lib/CodeGen/TargetInfo.cpp Index: cfe/trunk/lib/CodeGen/CGBuilder.h === --- cfe/trunk/lib/CodeGen/CGBuilder.h +++ cfe/trunk/lib/CodeGen/CGBuilder.h @@ -124,19 +124,6 @@ // FIXME: these "default-aligned" APIs should be removed, // but I don't feel like fixing all the builtin code right now. - llvm::LoadInst *CreateDefaultAlignedLoad(llvm::Value *Addr, - const llvm::Twine &Name = "") { -return CGBuilderBaseTy::CreateLoad(Addr, false, Name); - } - llvm::LoadInst *CreateDefaultAlignedLoad(llvm::Value *Addr, - const char *Name) { -return CGBuilderBaseTy::CreateLoad(Addr, false, Name); - } - llvm::LoadInst *CreateDefaultAlignedLoad(llvm::Value *Addr, bool IsVolatile, - const llvm::Twine &Name = "") { -return CGBuilderBaseTy::CreateLoad(Addr, IsVolatile, Name); - } - llvm::StoreInst *CreateDefaultAlignedStore(llvm::Value *Val, llvm::Value *Addr, bool IsVolatile = false) { Index: cfe/trunk/lib/CodeGen/CGCall.cpp === --- cfe/trunk/lib/CodeGen/CGCall.cpp +++ cfe/trunk/lib/CodeGen/CGCall.cpp @@ -2884,13 +2884,13 @@ // FIXME: Generate IR in one pass, rather than going back and fixing up these // placeholders. llvm::Type *IRTy = CGF.ConvertTypeForMem(Ty); - llvm::Value *Placeholder = -llvm::UndefValue::get(IRTy->getPointerTo()->getPointerTo()); - Placeholder = CGF.Builder.CreateDefaultAlignedLoad(Placeholder); + llvm::Type *IRPtrTy = IRTy->getPointerTo(); + llvm::Value *Placeholder = llvm::UndefValue::get(IRPtrTy->getPointerTo()); // FIXME: When we generate this IR in one pass, we shouldn't need // this win32-specific alignment hack. CharUnits Align = CharUnits::fromQuantity(4); + Placeholder = CGF.Builder.CreateAlignedLoad(IRPtrTy, Placeholder, Align); return AggValueSlot::forAddr(Address(Placeholder, Align), Ty.getQualifiers(), Index: cfe/trunk/lib/CodeGen/CGBuiltin.cpp === --- cfe/trunk/lib/CodeGen/CGBuiltin.cpp +++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp @@ -2191,8 +2191,9 @@ Value *IntToPtr = Builder.CreateIntToPtr(EmitScalarExpr(E->getArg(0)), llvm::PointerType::get(IntTy, 257)); -LoadInst *Load = -Builder.CreateDefaultAlignedLoad(IntToPtr, /*isVolatile=*/true); +LoadInst *Load = Builder.CreateAlignedLoad( +IntTy, IntToPtr, getContext().getTypeAlignInChars(E->getType())); +Load->setVolatile(true); return RValue::get(Load); } @@ -5440,9 +5441,11 @@ switch (BuiltinID) { default: break; case NEON::BI__builtin_neon_vldrq_p128: { -llvm::Type *Int128PTy = llvm::Type::getIntNPtrTy(getLLVMContext(), 128); +llvm::Type *Int128Ty = llvm::Type::getIntNTy(getLLVMContext(), 128); +llvm::Type *Int128PTy = llvm::PointerType::get(Int128Ty, 0); Value *Ptr = Builder.CreateBitCast(EmitScalarExpr(E->getArg(0)), Int128PTy); -return Builder.CreateDefaultAlignedLoad(Ptr); +return Builder.CreateAlignedLoad(Int128Ty, Ptr, + CharUnits::fromQuantity(16)); } case NEON::BI__builtin_neon_vstrq_p128: { llvm::Type *Int128PTy = llvm::Type::getIntNPtrTy(getLLVMContext(), 128); @@ -6615,27 +6618,37 @@ return EmitNeonCall(CGM.getIntrinsic(Int, Tys), Ops, ""); } case NEON::BI__builtin_neon_vld1_v: - case NEON::BI__builtin_neon_vld1q_v: + case NEON::BI__builtin_neon_vld1q_v: { Ops[0] = Builder.CreateBitCast(Ops[0], llvm::PointerType::getUnqual(VTy)); -return Builder.CreateDefaultAlignedLoad(Ops[0]); +auto Alignment = CharUnits::fromQuantity( +BuiltinID == NEON::BI__builtin_neon_vld1_v ? 8 : 16); +return Builder.CreateAlignedLoad(VTy, Ops[0], Alignment); + } case NEON::BI__builtin_neon_vst1_v: case NEON::BI__builtin_neon_vst1q_v: Ops[0] = Builder.CreateBitCast(Ops[0], llvm::PointerType::getUnqual(VTy)); Ops[1] = Builder.CreateBitCast(Ops[1], VTy); return Builder.CreateDefaultAlignedStore(Ops[1], Ops[0]); case NEON::BI__builtin_neon_vld1_lane_v: - case NEON::BI__builtin_neon_vld1q_lane_v: + case NEON::BI__builtin_neon_vld1q_lane_v: { Ops[1] = Builder.CreateBitCast