[PATCH] D26137: [clang-tidy] Add check name to YAML export

2016-11-28 Thread Alpha Abdoulaye via Phabricator via cfe-commits
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

2016-11-28 Thread Manuel Klimek via Phabricator via cfe-commits
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

2016-11-28 Thread Alex Lorenz via Phabricator via cfe-commits
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

2016-11-28 Thread Alex Lorenz via Phabricator via cfe-commits
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.

2016-11-28 Thread Kirill Romanenkov via Phabricator via cfe-commits
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

2016-11-28 Thread Malcolm Parsons via Phabricator via cfe-commits
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

2016-11-28 Thread Malcolm Parsons via cfe-commits
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

2016-11-28 Thread Joerg Sonnenberger via Phabricator via cfe-commits
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

2016-11-28 Thread Manuel Klimek via Phabricator via cfe-commits
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.

2016-11-28 Thread Joshua Hurwitz via cfe-commits
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.

2016-11-28 Thread Artem Dergachev via Phabricator via cfe-commits
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

2016-11-28 Thread Alex Lorenz via Phabricator via cfe-commits
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

2016-11-28 Thread Alex Lorenz via Phabricator via cfe-commits
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.

2016-11-28 Thread NAKAMURA Takumi via cfe-commits
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

2016-11-28 Thread Yaxun Liu via Phabricator via cfe-commits
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

2016-11-28 Thread Joey Gouly via Phabricator via cfe-commits
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.

2016-11-28 Thread Anastasia Stulova via Phabricator via cfe-commits
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

2016-11-28 Thread Michał Górny via Phabricator via cfe-commits
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)

2016-11-28 Thread Michał Górny via Phabricator via cfe-commits
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

2016-11-28 Thread Joerg Sonnenberger via Phabricator via cfe-commits
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

2016-11-28 Thread Michał Górny via Phabricator via cfe-commits
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)

2016-11-28 Thread Michał Górny via Phabricator via cfe-commits
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

2016-11-28 Thread Manuel Klimek via Phabricator via cfe-commits
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

2016-11-28 Thread Anastasia Stulova via Phabricator via cfe-commits
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

2016-11-28 Thread Joerg Sonnenberger via Phabricator via cfe-commits
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

2016-11-28 Thread Alex Lorenz via Phabricator via cfe-commits
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.

2016-11-28 Thread Alexey Bataev via cfe-commits
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

2016-11-28 Thread Yaxun Liu via Phabricator via cfe-commits
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

2016-11-28 Thread Alex Lorenz via Phabricator via cfe-commits
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

2016-11-28 Thread Yaxun Liu via Phabricator via cfe-commits
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

2016-11-28 Thread Malcolm Parsons via Phabricator via cfe-commits
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

2016-11-28 Thread Aleksei Sidorin via Phabricator via cfe-commits
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

2016-11-28 Thread Eric van Gyzen via Phabricator via cfe-commits
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

2016-11-28 Thread Chris Bieneman via Phabricator via cfe-commits
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

2016-11-28 Thread Eric van Gyzen via Phabricator via cfe-commits
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.

2016-11-28 Thread Benjamin Kramer via cfe-commits
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

2016-11-28 Thread Hans Wennborg via Phabricator via cfe-commits
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

2016-11-28 Thread Mehdi AMINI via Phabricator via cfe-commits
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

2016-11-28 Thread Hans Wennborg via cfe-commits
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

2016-11-28 Thread Sebastian Pop via Phabricator via cfe-commits
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

2016-11-28 Thread Michał Górny via Phabricator via cfe-commits
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

2016-11-28 Thread Sebastian Pop via Phabricator via cfe-commits
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

2016-11-28 Thread Alex Lorenz via Phabricator via cfe-commits
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

2016-11-28 Thread Joerg Sonnenberger via Phabricator via cfe-commits
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

2016-11-28 Thread Alexander Kornienko via Phabricator via cfe-commits
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

2016-11-28 Thread Marshall Clow via Phabricator via cfe-commits
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

2016-11-28 Thread Evgeniy Stepanov via Phabricator via cfe-commits
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

2016-11-28 Thread Marshall Clow via Phabricator via cfe-commits
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

2016-11-28 Thread Marshall Clow via Phabricator via cfe-commits
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

2016-11-28 Thread Marshall Clow via Phabricator via cfe-commits
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

2016-11-28 Thread Marshall Clow via Phabricator via cfe-commits
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

2016-11-28 Thread Eugene Zelenko via Phabricator via cfe-commits
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

2016-11-28 Thread Hans Wennborg via cfe-commits
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

2016-11-28 Thread Marshall Clow via Phabricator via cfe-commits
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

2016-11-28 Thread Marshall Clow via Phabricator via cfe-commits
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

2016-11-28 Thread Marshall Clow via Phabricator via cfe-commits
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

2016-11-28 Thread Marshall Clow via Phabricator via cfe-commits
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

2016-11-28 Thread Eric van Gyzen via Phabricator via cfe-commits
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

2016-11-28 Thread Hans Wennborg via cfe-commits
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

2016-11-28 Thread Sebastian Pop via Phabricator via cfe-commits
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

2016-11-28 Thread Sebastian Pop via Phabricator via cfe-commits
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

2016-11-28 Thread Sebastian Pop via Phabricator via cfe-commits
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

2016-11-28 Thread Mehdi AMINI via Phabricator via cfe-commits
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.

2016-11-28 Thread Sebastian Pop via Phabricator via cfe-commits
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

2016-11-28 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
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.

2016-11-28 Thread Peter Collingbourne via Phabricator via cfe-commits
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.

2016-11-28 Thread Peter Collingbourne via Phabricator via cfe-commits
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

2016-11-28 Thread Reid Kleckner via cfe-commits
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

2016-11-28 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
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

2016-11-28 Thread Arthur O'Dwyer via Phabricator via cfe-commits
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

2016-11-28 Thread Michal Gorny via cfe-commits
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

2016-11-28 Thread Michal Gorny via cfe-commits
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

2016-11-28 Thread Michal Gorny via cfe-commits
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

2016-11-28 Thread Michał Górny via Phabricator via cfe-commits
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

2016-11-28 Thread Michał Górny via Phabricator via cfe-commits
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.

2016-11-28 Thread Evgeniy Stepanov via Phabricator via cfe-commits
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.

2016-11-28 Thread John McCall via Phabricator via cfe-commits
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.

2016-11-28 Thread Peter Collingbourne via Phabricator via cfe-commits
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

2016-11-28 Thread Marshall Clow via Phabricator via cfe-commits
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

2016-11-28 Thread Mehdi AMINI via Phabricator via cfe-commits
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

2016-11-28 Thread Sean Callanan via Phabricator via cfe-commits
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

2016-11-28 Thread Dominic Chen via Phabricator via cfe-commits
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

2016-11-28 Thread Matt Bettinson via Phabricator via cfe-commits
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

2016-11-28 Thread Sean Callanan via Phabricator via cfe-commits
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

2016-11-28 Thread Eli Friedman via Phabricator via cfe-commits
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

2016-11-28 Thread John McCall via Phabricator via cfe-commits
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

2016-11-28 Thread Sean Callanan via Phabricator via cfe-commits
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

2016-11-28 Thread Mehdi AMINI via Phabricator via cfe-commits
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

2016-11-28 Thread Richard Smith via Phabricator via cfe-commits
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

2016-11-28 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
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

2016-11-28 Thread Richard Smith via Phabricator via cfe-commits
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

2016-11-28 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
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

2016-11-28 Thread John McCall via Phabricator via cfe-commits
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.

2016-11-28 Thread John McCall via cfe-commits
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.

2016-11-28 Thread John McCall via cfe-commits
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

2016-11-28 Thread John McCall via cfe-commits
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.

2016-11-28 Thread John McCall via Phabricator via cfe-commits
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

2016-11-28 Thread Richard Smith via Phabricator via cfe-commits
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.

2016-11-28 Thread Peter Collingbourne via cfe-commits
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.

2016-11-28 Thread Peter Collingbourne via Phabricator via cfe-commits
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

  1   2   >