[PATCH] D34272: [Tooling] A new framework for executing clang frontend actions.

2017-10-13 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment.

This is a good start, a few comments. Also I'd suggest running clang-format on 
this patch -- I saw a few places violate the code style.




Comment at: include/clang/Tooling/CommonOptionsParser.h:90
   ///
-  /// This constructor exits program in case of error.
+  /// If \p ExitOnError is set (default), This constructor exits program in 
case
+  /// of error; otherwise, this sets the error flag and stores error messages.

`... is set to true` for clarify.



Comment at: include/clang/Tooling/CommonOptionsParser.h:109
+
+  const std::string &getErrorMessage() const { return ErrorMessage; }
+

return `llvm::StringRef`?



Comment at: include/clang/Tooling/Execution.h:21
+//
+//  New executors can be registered as ToolExecutorPlugins via the
+//  `ToolExecutorPluginRegistry`. CLI tools can use

Consider moving this comment to ToolExecutor class to make it more discoverable?



Comment at: include/clang/Tooling/Execution.h:47
+virtual ~ToolResults() {}
+virtual void addResult(llvm::StringRef Key, llvm::StringRef Value) = 0;
+virtual std::vector> AllKVResults() = 
0;

How about using `std::pair` type here? It would make 
it consistent with the `allKVResults` below.

Also I think we can define an alias `KVResult` for this type.





Comment at: include/clang/Tooling/Execution.h:48
+virtual void addResult(llvm::StringRef Key, llvm::StringRef Value) = 0;
+virtual std::vector> AllKVResults() = 
0;
+virtual void

nit: `all`



Comment at: include/clang/Tooling/Execution.h:81
+  /// \brief Executes actions specified in the execution configuration.
+  virtual llvm::Error Execute(const ExecutionConfig &Config) = 0;
+

nit: `execute`



Comment at: lib/Tooling/Execution.cpp:61
+if (!Executor) {
+  OS << "Failed to create '" << I->getName()
+ << "': " << llvm::toString(Executor.takeError()) << "\n";

Consider we have two plugins, the first iteration fails, and the second one 
succeeds, the code looks like still treating it as an error here? And if the 
case where there are more than one executor being created successfully, we just 
return the first one.

My understanding of the createExecutorFromCommandLineArgs's behavior is to find 
a proper `ToolExecutorPlugin` and create a `ToolExecutor` instance.

Maybe add a command-line flag like `--executor=` to make it 
straightforward to choose which registered executor is used, and make 
`StandaloneToolExecutor` as default? 



Comment at: lib/Tooling/Execution.cpp:73
+
+const char *StandaloneToolExecutor::ExecutorName = "StandaloneToolExector";
+

nit: `Executor`



Comment at: unittests/Tooling/ToolingTest.cpp:32
 
+using testing::ElementsAre;
+

Seems that it is unused.



Comment at: unittests/Tooling/ToolingTest.cpp:638
+ llvm::cl::OptionCategory &Category) override {
+static llvm::cl::opt TestExecutor(
+"test_executor", llvm::cl::desc("Use TestToolExecutor"));

any reason to make it static?



Comment at: unittests/Tooling/ToolingTest.cpp:667
+  std::vector argv = {"prog", "--fake_flag_no_no_no", "f"};
+  int argc = argv.size();
+  auto Executor =

nit: size_t, the same below.


https://reviews.llvm.org/D34272



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D38110: [libunwind][MIPS]: Add support for unwinding in O32 and N64 processes.

2017-10-13 Thread Simon Dardis via Phabricator via cfe-commits
sdardis requested changes to this revision.
sdardis added a comment.
This revision now requires changes to proceed.

Marking this as changes required to clear up my review queue - no further 
comments.


https://reviews.llvm.org/D38110



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D34272: [Tooling] A new framework for executing clang frontend actions.

2017-10-13 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 118898.
ioeric added a comment.

- Minor cleanups.


https://reviews.llvm.org/D34272

Files:
  include/clang/Tooling/CommonOptionsParser.h
  include/clang/Tooling/Execution.h
  include/clang/Tooling/ToolExecutorPluginRegistry.h
  include/clang/Tooling/Tooling.h
  lib/Tooling/CMakeLists.txt
  lib/Tooling/CommonOptionsParser.cpp
  lib/Tooling/Execution.cpp
  lib/Tooling/Tooling.cpp
  unittests/Tooling/ToolingTest.cpp

Index: unittests/Tooling/ToolingTest.cpp
===
--- unittests/Tooling/ToolingTest.cpp
+++ unittests/Tooling/ToolingTest.cpp
@@ -7,24 +7,30 @@
 //
 //===--===//
 
+#include "clang/Tooling/Tooling.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclGroup.h"
+#include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Frontend/ASTUnit.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Tooling/CompilationDatabase.h"
-#include "clang/Tooling/Tooling.h"
+#include "clang/Tooling/Execution.h"
+#include "clang/Tooling/ToolExecutorPluginRegistry.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Config/llvm-config.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/TargetSelect.h"
+#include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
 #include 
 
+using testing::ElementsAre;
+
 namespace clang {
 namespace tooling {
 
@@ -60,6 +66,61 @@
  private:
   bool * const FoundTopLevelDecl;
 };
+
+// This traverses the AST and outputs function name as key and "1" as value for
+// each function declaration.
+class ASTConsumerWithResult : public ASTConsumer,
+  public RecursiveASTVisitor {
+ public:
+  using ASTVisitor = RecursiveASTVisitor;
+
+  explicit ASTConsumerWithResult(ToolResults *Results)
+  : Results(Results) {
+assert(Results != nullptr);
+  }
+
+  void HandleTranslationUnit(clang::ASTContext& Context) override {
+TraverseDecl(Context.getTranslationUnitDecl());
+  }
+
+  bool TraverseFunctionDecl(clang::FunctionDecl *Decl) {
+Results->addResult(Decl->getNameAsString(), "1");
+return ASTVisitor::TraverseFunctionDecl(Decl);
+  }
+
+private:
+  ToolResults* const Results;
+};
+
+class ReportResultAction : public ASTFrontendAction {
+ public:
+   explicit ReportResultAction(ToolResults *Results)
+   : Results(Results) {
+ assert(Results != nullptr);
+   }
+
+ protected:
+   std::unique_ptr
+   CreateASTConsumer(clang::CompilerInstance &compiler,
+ llvm::StringRef /* dummy */) override {
+ std::unique_ptr ast_consumer{
+ new ASTConsumerWithResult(Results)};
+ return ast_consumer;
+   }
+
+ private:
+  ToolResults* const Results;
+};
+
+class ReportResultActionFactory : public FrontendActionFactory {
+public:
+  ReportResultActionFactory(ToolResults *Results) : Results(Results) {}
+  FrontendAction *create() override { return new ReportResultAction(Results); }
+
+private:
+  ToolResults *const Results;
+};
+
 } // end namespace
 
 TEST(runToolOnCode, FindsNoTopLevelDeclOnEmptyCode) {
@@ -533,5 +594,166 @@
 }
 #endif
 
+inline llvm::Error make_string_error(const llvm::Twine &Message) {
+  return llvm::make_error(Message,
+ llvm::inconvertibleErrorCode());
+}
+
+class TestToolExecutor : public ToolExecutor {
+public:
+  static const char *ExecutorName;
+
+  TestToolExecutor(std::unique_ptr Options)
+  : OptionsParser(std::move(Options)) {}
+
+  llvm::StringRef getExecutorName() const override { return ExecutorName; }
+
+  llvm::Error Execute(const ExecutionConfig &) override {
+return llvm::Error::success();
+  }
+
+  ToolResults *getToolResults() override { return nullptr; };
+
+  llvm::ArrayRef getSourcePaths() const {
+return OptionsParser->getSourcePathList();
+  }
+
+  void mapVirtualFile(StringRef FilePath, StringRef Content) override {
+VFS[FilePath] = Content;
+  }
+
+private:
+  std::unique_ptr OptionsParser;
+  std::string SourcePaths;
+  std::map VFS;
+};
+
+const char *TestToolExecutor::ExecutorName = "TestToolExecutor";
+
+class TestToolExecutorPlugin : public ToolExecutorPlugin {
+public:
+  llvm::Expected>
+  create(int &argc, const char **argv,
+ llvm::cl::OptionCategory &Category) override {
+static llvm::cl::opt TestExecutor(
+"test_executor", llvm::cl::desc("Use TestToolExecutor"));
+auto OptionsParser = llvm::make_unique(
+argc, argv, Category, llvm::cl::OneOrMore, /*Overview=*/nullptr,
+/*ExitOnError=*/false);
+if (OptionsParser->hasError())
+  return make_string_error("[TestToolExecutorPlugin] " +
+   OptionsParser->getErrorMessage());
+if (!TestExecutor)
+  return make_string_error(
+  "

[PATCH] D38863: Typos in tutorial

2017-10-13 Thread Chad Rosier via Phabricator via cfe-commits
mcrosier closed this revision.
mcrosier added a comment.

Committed in r315652.


https://reviews.llvm.org/D38863



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r315678 - Fix regression of test/CodeGenOpenCL/address-spaces.cl on ppc

2017-10-13 Thread Yaxun Liu via cfe-commits
Author: yaxunl
Date: Fri Oct 13 06:53:06 2017
New Revision: 315678

URL: http://llvm.org/viewvc/llvm-project?rev=315678&view=rev
Log:
Fix regression of test/CodeGenOpenCL/address-spaces.cl on ppc

Modified:
cfe/trunk/test/CodeGenOpenCL/address-spaces.cl

Modified: cfe/trunk/test/CodeGenOpenCL/address-spaces.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCL/address-spaces.cl?rev=315678&r1=315677&r2=315678&view=diff
==
--- cfe/trunk/test/CodeGenOpenCL/address-spaces.cl (original)
+++ cfe/trunk/test/CodeGenOpenCL/address-spaces.cl Fri Oct 13 06:53:06 2017
@@ -102,7 +102,8 @@ void test_struct() {
 // SPIR-LABEL: define void @test_void_par()
 void test_void_par(void) {}
 
-// SPIR-LABEL: define i32 @test_func_return_type()
+// On ppc64 returns signext i32.
+// SPIR-LABEL: define{{.*}} i32 @test_func_return_type()
 int test_func_return_type(void) {
   return 0;
 }


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D38284: [clang-tidy] Fix google-readability-namespace-comments handling of C++17 nested namespaces

2017-10-13 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added inline comments.



Comment at: clang-tidy/readability/NamespaceCommentCheck.cpp:97
+  }
+  while (Lexer::getRawToken(LBracketLocation, Tok, Sources, getLangOpts()) ||
+ !Tok.is(tok::l_brace)) {

alexfh wrote:
> alexfh wrote:
> > The check started triggering an assertion failure and incredible slowness 
> > (infinite loops?) on some real files. I've not yet come up with an isolated 
> > test case, but all this seems to be happening around this loop.
> > 
> > I'm going to revert this revision. A bug report (and hopefully a test case 
> > ;) will follow.
> > 
> Reverted in r315580.
Here's a test case that demonstrates the issue:
```
#define MACRO macro_expansion
namespace MACRO {
void f(); // So that the namespace isn't empty.
// 1
// 2
// 3
// 4
// 5
// 6
// 7
// CHECK-MESSAGES: :[[@LINE+2]]:2: warning: namespace 'macro_expansion' not 
terminated with
// CHECK-MESSAGES: :[[@LINE-10]]:11: note: namespace 'macro_expansion' starts 
here
}
// CHECK-FIXES: }  // namespace macro_expansion

```

I'll commit it once Subversion on llvm.org starts working again.


https://reviews.llvm.org/D38284



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D38882: [clang-rename] Add function unit tests.

2017-10-13 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision.
Herald added subscribers: mgorny, klimek.

Also contain some fixes:

- Get rid of the "TranslationUnitDecl" special case in RenameClass, as

we switch to use USR instead of AST matcher to find symbols. A bonus
point is that now the test cases make more sense.

- Fix a false positive of renaming a using shadow function declaration.


https://reviews.llvm.org/D38882

Files:
  lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
  unittests/Rename/CMakeLists.txt
  unittests/Rename/RenameClassTest.cpp
  unittests/Rename/RenameFunctionTest.cpp

Index: unittests/Rename/RenameFunctionTest.cpp
===
--- /dev/null
+++ unittests/Rename/RenameFunctionTest.cpp
@@ -0,0 +1,508 @@
+//===-- RenameFunctionTest.cpp - unit tests for renaming functions ===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "ClangRenameTest.h"
+
+namespace clang {
+namespace clang_rename {
+namespace test {
+namespace {
+
+class RenameFunctionTest : public ClangRenameTest {
+public:
+  RenameFunctionTest() {
+AppendToHeader(R"(
+  struct A {
+static bool Foo();
+static bool Spam();
+  };
+  struct B {
+static void Same();
+static bool Foo();
+static int Eric(int x);
+  };
+  void Same(int x);
+  int Eric(int x);
+  namespace base {
+void Same();
+void ToNanoSeconds();
+void ToInt64NanoSeconds();
+  })");
+  }
+};
+
+TEST_F(RenameFunctionTest, RefactorsAFoo) {
+  std::string Before = R"(
+  void f() {
+A::Foo();
+::A::Foo();
+  })";
+  std::string Expected = R"(
+  void f() {
+A::Bar();
+::A::Bar();
+  })";
+
+  std::string After = runClangRenameOnCode(Before, "A::Foo", "A::Bar");
+  CompareSnippets(Expected, After);
+}
+
+TEST_F(RenameFunctionTest, RefactorsNonCallingAFoo) {
+  std::string Before = R"(
+  bool g(bool (*func)()) {
+return func();
+  }
+  void f() {
+auto *ref1 = A::Foo;
+auto *ref2 = ::A::Foo;
+g(A::Foo);
+  })";
+  std::string Expected = R"(
+  bool g(bool (*func)()) {
+return func();
+  }
+  void f() {
+auto *ref1 = A::Bar;
+auto *ref2 = ::A::Bar;
+g(A::Bar);
+  })";
+  std::string After = runClangRenameOnCode(Before, "A::Foo", "A::Bar");
+  CompareSnippets(Expected, After);
+}
+
+TEST_F(RenameFunctionTest, RefactorsEric) {
+  std::string Before = R"(
+  void f() {
+if (Eric(3)==4) ::Eric(2);
+  })";
+  std::string Expected = R"(
+  void f() {
+if (Larry(3)==4) ::Larry(2);
+  })";
+  std::string After = runClangRenameOnCode(Before, "Eric", "Larry");
+  CompareSnippets(Expected, After);
+}
+
+TEST_F(RenameFunctionTest, RefactorsNonCallingEric) {
+  std::string Before = R"(
+int g(int (*func)(int)) {
+  return func(1);
+}
+void f() {
+  auto *ref = ::Eric;
+  g(Eric);
+})";
+  std::string Expected = R"(
+int g(int (*func)(int)) {
+  return func(1);
+}
+void f() {
+  auto *ref = ::Larry;
+  g(Larry);
+})";
+  std::string After = runClangRenameOnCode(Before, "Eric", "Larry");
+  CompareSnippets(Expected, After);
+}
+
+TEST_F(RenameFunctionTest, DoesNotRefactorBFoo) {
+  std::string Before = R"(
+  void f() {
+B::Foo();
+  })";
+  std::string After = runClangRenameOnCode(Before, "A::Foo", "A::Bar");
+  CompareSnippets(Before, After);
+}
+
+TEST_F(RenameFunctionTest, DoesNotRefactorBEric) {
+  std::string Before = R"(
+  void f() {
+B::Eric(2);
+  })";
+  std::string After = runClangRenameOnCode(Before, "Eric", "Larry");
+  CompareSnippets(Before, After);
+}
+
+TEST_F(RenameFunctionTest, DoesNotRefactorCEric) {
+  std::string Before = R"(
+  namespace C { int Eric(int x); }
+  void f() {
+if (C::Eric(3)==4) ::C::Eric(2);
+  })";
+  std::string Expected = R"(
+  namespace C { int Eric(int x); }
+  void f() {
+if (C::Eric(3)==4) ::C::Eric(2);
+  })";
+  std::string After = runClangRenameOnCode(Before, "Eric", "Larry");
+  CompareSnippets(Expected, After);
+}
+
+TEST_F(RenameFunctionTest, DoesNotRefactorEricInNamespaceC) {
+  std::string Before = R"(
+   namespace C {
+   int Eric(int x);
+   void f() {
+ if (Eric(3)==4) Eric(2);
+   }
+   }  // namespace C)";
+  std::string After = runClangRenameOnCode(Before, "Eric", "Larry");
+  CompareSnippets(Before, After);
+}
+
+TEST_F(RenameFunctionTest, NamespaceQualified) {
+  std::string Before = R"(
+  void f() {
+base::ToNanoSeconds();
+::base::ToNanoSeconds();
+  }
+

[PATCH] D38813: [X86] Add skeleton support for "knm" cpu - clang side

2017-10-13 Thread Simon Pilgrim via Phabricator via cfe-commits
RKSimon accepted this revision.
RKSimon added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: lib/Basic/Targets/X86.cpp:232
 
+  case CK_KNM:
   case CK_KNL:

Worth adding a TODO comment for the missing features?


https://reviews.llvm.org/D38813



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D38688: [clang-tidy] Misc redundant expressions checker updated for macros

2017-10-13 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment.

Thanks for working on this, these additions look very useful to me.




Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:124
 
+  case Stmt::CXXFunctionalCastExprClass:
+return cast(Left)->getTypeAsWritten() ==

You could merge this case with the above case if you cast Left and Right to 
ExplicitCastExpr.  So both label could end up executing the same code. 



Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:330
+
+AST_MATCHER_P(Expr, expandedByMacro, std::set, Names) {
+  const SourceManager &SM = Finder->getASTContext().getSourceManager();

`llvm::StringSet` might be a more efficient choice.



Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:336
+std::string MacroName = Lexer::getImmediateMacroName(Loc, SM, LO);
+if (Names.find(MacroName) != Names.end())
+  return true;

You could use `count` here. 



Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:357
+  ConstExpr = Result.Nodes.getNodeAs(CstId);
+  return ConstExpr && ConstExpr->isIntegerConstantExpr(Value, *Result.Context);
+}

I think you could just return the pointer and return a null pointer in case it 
is not an integerConstantExpr. This way no compatibility overload needed.



Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:510
+// E.g.: from (X == 5) && (X == 5) retrieves 5 and 5
+static void retrieveConstExprFromBothSides(const BinaryOperator *&BinOp,
+   BinaryOperatorKind &MainOpcode,

I would prefer to return a pair of pointer to be returned to output parameters. 



Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:579
 
-AST_MATCHER_P(Expr, expandedByMacro, std::set, Names) {
-  const SourceManager &SM = Finder->getASTContext().getSourceManager();
-  const LangOptions &LO = Finder->getASTContext().getLangOpts();
-  SourceLocation Loc = Node.getExprLoc();
-  while (Loc.isMacroID()) {
-std::string MacroName = Lexer::getImmediateMacroName(Loc, SM, LO);
-if (Names.find(MacroName) != Names.end())
+static bool isOperatorSetMeaningful(BinaryOperatorKind &Opcode,
+BinaryOperatorKind &LhsOpcode,

I think a comment might be good here what do you mean by meaningful.



Comment at: test/clang-tidy/misc-redundant-expression.cpp:387
   // Should not match.
-  if (X == 10 && Y == 10) return 1;
-  if (X != 10 && X != 12) return 1;

Why did you remove these test cases?



Comment at: test/clang-tidy/misc-redundant-expression.cpp:404
   if (X <= 10 && X >= 10) return 1;
-  if (!X && !Y) return 1;
-  if (!X && Y) return 1;

Same comment as above.


Repository:
  rL LLVM

https://reviews.llvm.org/D38688



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D38877: [analyzer] RetainCount: Accept "safe" CFRetain wrappers.

2017-10-13 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ created this revision.
Herald added subscribers: szepet, xazax.hun.

CoreFoundation users often look for a "safe" wrapper around `CFRetain` and 
`CFRelease` that would gracefully accept (and ignore) a null CF reference. 
Users are trying to annotate it with `CF_RETURNS_RETAINED`, which misleads the 
analyzer into believing that the function kind of "creates" the object with a 
+1 retain count (such annotation is normally used on functions that create or 
copy objects), while in fact it increments the retain count on an existing 
object.

When modeling such function, RetainCountChecker first realizes that this 
function is merely a `CFRetain` (due to naming convention) and increments the 
reference count, and then sees the annotation and throws away the existing 
retain count information and resets the reference count to +1. This causes 
obvious problems, for example if the object is safely-retained and released 
more than once, it'd cause various kinds of overrelease warnings.

Tell the analyzer not to look at annotations for functions that he already 
identified as some form of `CFRetain`.


https://reviews.llvm.org/D38877

Files:
  lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
  test/Analysis/retain-release-safe.c


Index: test/Analysis/retain-release-safe.c
===
--- /dev/null
+++ test/Analysis/retain-release-safe.c
@@ -0,0 +1,72 @@
+// RUN: %clang_analyze_cc1 
-analyzer-checker=core,osx.coreFoundation.CFRetainRelease,osx.cocoa.RetainCount 
-verify %s
+
+#pragma clang arc_cf_code_audited begin
+typedef const void * CFTypeRef;
+extern CFTypeRef CFRetain(CFTypeRef cf);
+extern void CFRelease(CFTypeRef cf);
+#pragma clang arc_cf_code_audited end
+
+#define CF_RETURNS_RETAINED __attribute__((cf_returns_retained))
+#define CF_CONSUMED __attribute__((cf_consumed))
+
+extern CFTypeRef CFCreate() CF_RETURNS_RETAINED;
+
+// A "safe" variant of CFRetain that doesn't crash when a null pointer is
+// retained. This is often defined by users in a similar manner. The
+// CF_RETURNS_RETAINED annotation is misleading here, because the function
+// is not supposed to return an object with a +1 retain count. Instead, it
+// is supposed to return an object with +(N+1) retain count, where N is
+// the original retain count of 'cf'. However, there is no good annotation
+// to use in this case, and it is pointless to provide such annotation
+// because the only use cases would be CFRetain and SafeCFRetain.
+// So instead we teach the analyzer to be able to accept such code
+// and ignore the misplaced annotation.
+CFTypeRef SafeCFRetain(CFTypeRef cf) CF_RETURNS_RETAINED {
+  if (cf) {
+return CFRetain(cf);
+  }
+  return cf;
+}
+
+// A "safe" variant of CFRelease that doesn't crash when a null pointer is
+// released. The CF_CONSUMED annotation seems reasonable here.
+void SafeCFRelease(CFTypeRef CF_CONSUMED cf) {
+  if (cf)
+CFRelease(cf); // no-warning (when inlined)
+}
+
+void escape(CFTypeRef cf);
+
+void makeSureTestsWork() {
+  CFTypeRef cf = CFCreate();
+  CFRelease(cf);
+  CFRelease(cf); // expected-warning{{Reference-counted object is used after 
it is released}}
+}
+
+// Make sure we understand that the second SafeCFRetain doesn't return an
+// object with +1 retain count, which we won't be able to release twice.
+void falseOverrelease(CFTypeRef cf) {
+  SafeCFRetain(cf);
+  SafeCFRetain(cf);
+  SafeCFRelease(cf);
+  SafeCFRelease(cf); // no-warning after inlining this.
+}
+
+// Regular CFRelease() should behave similarly.
+void sameWithNormalRelease(CFTypeRef cf) {
+  SafeCFRetain(cf);
+  SafeCFRetain(cf);
+  CFRelease(cf);
+  CFRelease(cf); // no-warning
+}
+
+// Make sure we understand that the second SafeCFRetain doesn't return an
+// object with +1 retain count, which would no longer be owned by us after
+// it escapes to escape() and released once.
+void falseReleaseNotOwned(CFTypeRef cf) {
+  SafeCFRetain(cf);
+  SafeCFRetain(cf);
+  escape(cf);
+  SafeCFRelease(cf);
+  SafeCFRelease(cf); // no-warning after inlining this.
+}
Index: lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
+++ lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
@@ -1170,6 +1170,11 @@
   if (cocoa::isRefType(RetTy, "CF", FName)) {
 if (isRetain(FD, FName)) {
   S = getUnarySummary(FT, cfretain);
+  // CFRetain isn't supposed to be annotated. However, this may as well
+  // be a user-made "safe" CFRetain function that is incorrectly
+  // annotated as cf_returns_retained due to lack of better options.
+  // We want to ignore such annotation.
+  AllowAnnotations = false;
 } else if (isAutorelease(FD, FName)) {
   S = getUnarySummary(FT, cfautorelease);
   // The headers use cf_consumed, but we can fully model CFAutorelease


Index: test/Analysis/retain-re

Re: r315668 - [OpenCL] Add LangAS::opencl_private to represent private address space in AST

2017-10-13 Thread Eric Liu via cfe-commits
Hi Yaxun, this is causing failures in ppc build bots (
http://lab.llvm.org:8011/builders/clang-ppc64be-linux-multistage/builds/5486).
I'll revert the patch for now. Please take a look. Thanks!

On Fri, Oct 13, 2017 at 5:37 AM Yaxun Liu via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: yaxunl
> Date: Thu Oct 12 20:37:48 2017
> New Revision: 315668
>
> URL: http://llvm.org/viewvc/llvm-project?rev=315668&view=rev
> Log:
> [OpenCL] Add LangAS::opencl_private to represent private address space in
> AST
>
> Currently Clang uses default address space (0) to represent private
> address space for OpenCL
> in AST. There are two issues with this:
>
> Multiple address spaces including private address space cannot be
> diagnosed.
> There is no mangling for default address space. For example, if private
> int* is emitted as
> i32 addrspace(5)* in IR. It is supposed to be mangled as PUAS5i but it is
> mangled as
> Pi instead.
>
> This patch attempts to represent OpenCL private address space explicitly
> in AST. It adds
> a new enum LangAS::opencl_private and adds it to the variable types which
> are implicitly
> private:
>
> automatic variables without address space qualifier
>
> function parameter
>
> pointee type without address space qualifier (OpenCL 1.2 and below)
>
> Differential Revision: https://reviews.llvm.org/D35082
>
> Removed:
> cfe/trunk/test/SemaOpenCL/extern.cl
> Modified:
> cfe/trunk/include/clang/Basic/AddressSpaces.h
> cfe/trunk/lib/AST/ASTContext.cpp
> cfe/trunk/lib/AST/Expr.cpp
> cfe/trunk/lib/AST/ItaniumMangle.cpp
> cfe/trunk/lib/AST/TypePrinter.cpp
> cfe/trunk/lib/Basic/Targets/AMDGPU.cpp
> cfe/trunk/lib/Basic/Targets/NVPTX.h
> cfe/trunk/lib/Basic/Targets/SPIR.h
> cfe/trunk/lib/Basic/Targets/TCE.h
> cfe/trunk/lib/CodeGen/CGDecl.cpp
> cfe/trunk/lib/Sema/SemaChecking.cpp
> cfe/trunk/lib/Sema/SemaDecl.cpp
> cfe/trunk/lib/Sema/SemaType.cpp
> cfe/trunk/test/CodeGenOpenCL/address-spaces-mangling.cl
> cfe/trunk/test/CodeGenOpenCL/address-spaces.cl
> cfe/trunk/test/SemaOpenCL/address-spaces.cl
> cfe/trunk/test/SemaOpenCL/cl20-device-side-enqueue.cl
> cfe/trunk/test/SemaOpenCL/storageclass-cl20.cl
> cfe/trunk/test/SemaOpenCL/storageclass.cl
> cfe/trunk/test/SemaTemplate/address_space-dependent.cpp
>
> Modified: cfe/trunk/include/clang/Basic/AddressSpaces.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AddressSpaces.h?rev=315668&r1=315667&r2=315668&view=diff
>
> ==
> --- cfe/trunk/include/clang/Basic/AddressSpaces.h (original)
> +++ cfe/trunk/include/clang/Basic/AddressSpaces.h Thu Oct 12 20:37:48 2017
> @@ -25,16 +25,17 @@ namespace LangAS {
>  ///
>  enum ID {
>// The default value 0 is the value used in QualType for the the
> situation
> -  // where there is no address space qualifier. For most languages, this
> also
> -  // corresponds to the situation where there is no address space
> qualifier in
> -  // the source code, except for OpenCL, where the address space value 0
> in
> -  // QualType represents private address space in OpenCL source code.
> +  // where there is no address space qualifier.
>Default = 0,
>
>// OpenCL specific address spaces.
> +  // In OpenCL each l-value must have certain non-default address space,
> each
> +  // r-value must have no address space (i.e. the default address space).
> The
> +  // pointee of a pointer must have non-default address space.
>opencl_global,
>opencl_local,
>opencl_constant,
> +  opencl_private,
>opencl_generic,
>
>// CUDA specific address spaces.
>
> Modified: cfe/trunk/lib/AST/ASTContext.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=315668&r1=315667&r2=315668&view=diff
>
> ==
> --- cfe/trunk/lib/AST/ASTContext.cpp (original)
> +++ cfe/trunk/lib/AST/ASTContext.cpp Thu Oct 12 20:37:48 2017
> @@ -707,6 +707,7 @@ static const LangAS::Map *getAddressSpac
>1, // opencl_global
>3, // opencl_local
>2, // opencl_constant
> +  0, // opencl_private
>4, // opencl_generic
>5, // cuda_device
>6, // cuda_constant
>
> Modified: cfe/trunk/lib/AST/Expr.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Expr.cpp?rev=315668&r1=315667&r2=315668&view=diff
>
> ==
> --- cfe/trunk/lib/AST/Expr.cpp (original)
> +++ cfe/trunk/lib/AST/Expr.cpp Thu Oct 12 20:37:48 2017
> @@ -3293,20 +3293,20 @@ Expr::isNullPointerConstant(ASTContext &
>// Check that it is a cast to void*.
>if (const PointerType *PT = CE->getType()->getAs()) {
>  QualType Pointee = PT->getPointeeType();
> -Qualifiers Q = Pointee.getQualifiers();
> -// In OpenCL v2.0 generic address space a

[clang-tools-extra] r315682 - [clang-tidy] Add a regression test for google-readability-namespace-comments

2017-10-13 Thread Alexander Kornienko via cfe-commits
Author: alexfh
Date: Fri Oct 13 07:11:14 2017
New Revision: 315682

URL: http://llvm.org/viewvc/llvm-project?rev=315682&view=rev
Log:
[clang-tidy] Add a regression test for google-readability-namespace-comments

Add a regression test for the google-readability-namespace-comments bug
introduced in r315057 (reverted in r315580).

Modified:

clang-tools-extra/trunk/test/clang-tidy/google-readability-namespace-comments.cpp

Modified: 
clang-tools-extra/trunk/test/clang-tidy/google-readability-namespace-comments.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/google-readability-namespace-comments.cpp?rev=315682&r1=315681&r2=315682&view=diff
==
--- 
clang-tools-extra/trunk/test/clang-tidy/google-readability-namespace-comments.cpp
 (original)
+++ 
clang-tools-extra/trunk/test/clang-tidy/google-readability-namespace-comments.cpp
 Fri Oct 13 07:11:14 2017
@@ -15,6 +15,20 @@ void f(); // So that the namespace isn't
 // CHECK-FIXES: }  // namespace n2
 // CHECK-FIXES: }  // namespace n1
 
+#define MACRO macro_expansion
+namespace MACRO {
+void f(); // So that the namespace isn't empty.
+// 1
+// 2
+// 3
+// 4
+// 5
+// 6
+// 7
+// CHECK-MESSAGES: :[[@LINE+2]]:2: warning: namespace 'macro_expansion' not 
terminated with
+// CHECK-MESSAGES: :[[@LINE-10]]:11: note: namespace 'macro_expansion' starts 
here
+}
+// CHECK-FIXES: }  // namespace macro_expansion
 
 namespace short1 {
 namespace short2 {


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: r315668 - [OpenCL] Add LangAS::opencl_private to represent private address space in AST

2017-10-13 Thread Eric Liu via cfe-commits
Ah, I saw you already sent a fix in r315678. Thank you for the fix! No
revert! :)

On Fri, Oct 13, 2017 at 4:10 PM Eric Liu  wrote:

> Hi Yaxun, this is causing failures in ppc build bots (
> http://lab.llvm.org:8011/builders/clang-ppc64be-linux-multistage/builds/5486).
> I'll revert the patch for now. Please take a look. Thanks!
>
> On Fri, Oct 13, 2017 at 5:37 AM Yaxun Liu via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: yaxunl
>> Date: Thu Oct 12 20:37:48 2017
>> New Revision: 315668
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=315668&view=rev
>> Log:
>> [OpenCL] Add LangAS::opencl_private to represent private address space in
>> AST
>>
>> Currently Clang uses default address space (0) to represent private
>> address space for OpenCL
>> in AST. There are two issues with this:
>>
>> Multiple address spaces including private address space cannot be
>> diagnosed.
>> There is no mangling for default address space. For example, if private
>> int* is emitted as
>> i32 addrspace(5)* in IR. It is supposed to be mangled as PUAS5i but it is
>> mangled as
>> Pi instead.
>>
>> This patch attempts to represent OpenCL private address space explicitly
>> in AST. It adds
>> a new enum LangAS::opencl_private and adds it to the variable types which
>> are implicitly
>> private:
>>
>> automatic variables without address space qualifier
>>
>> function parameter
>>
>> pointee type without address space qualifier (OpenCL 1.2 and below)
>>
>> Differential Revision: https://reviews.llvm.org/D35082
>>
>> Removed:
>> cfe/trunk/test/SemaOpenCL/extern.cl
>> Modified:
>> cfe/trunk/include/clang/Basic/AddressSpaces.h
>> cfe/trunk/lib/AST/ASTContext.cpp
>> cfe/trunk/lib/AST/Expr.cpp
>> cfe/trunk/lib/AST/ItaniumMangle.cpp
>> cfe/trunk/lib/AST/TypePrinter.cpp
>> cfe/trunk/lib/Basic/Targets/AMDGPU.cpp
>> cfe/trunk/lib/Basic/Targets/NVPTX.h
>> cfe/trunk/lib/Basic/Targets/SPIR.h
>> cfe/trunk/lib/Basic/Targets/TCE.h
>> cfe/trunk/lib/CodeGen/CGDecl.cpp
>> cfe/trunk/lib/Sema/SemaChecking.cpp
>> cfe/trunk/lib/Sema/SemaDecl.cpp
>> cfe/trunk/lib/Sema/SemaType.cpp
>> cfe/trunk/test/CodeGenOpenCL/address-spaces-mangling.cl
>> cfe/trunk/test/CodeGenOpenCL/address-spaces.cl
>> cfe/trunk/test/SemaOpenCL/address-spaces.cl
>> cfe/trunk/test/SemaOpenCL/cl20-device-side-enqueue.cl
>> cfe/trunk/test/SemaOpenCL/storageclass-cl20.cl
>> cfe/trunk/test/SemaOpenCL/storageclass.cl
>> cfe/trunk/test/SemaTemplate/address_space-dependent.cpp
>>
>> Modified: cfe/trunk/include/clang/Basic/AddressSpaces.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AddressSpaces.h?rev=315668&r1=315667&r2=315668&view=diff
>>
>> ==
>> --- cfe/trunk/include/clang/Basic/AddressSpaces.h (original)
>> +++ cfe/trunk/include/clang/Basic/AddressSpaces.h Thu Oct 12 20:37:48 2017
>> @@ -25,16 +25,17 @@ namespace LangAS {
>>  ///
>>  enum ID {
>>// The default value 0 is the value used in QualType for the the
>> situation
>> -  // where there is no address space qualifier. For most languages, this
>> also
>> -  // corresponds to the situation where there is no address space
>> qualifier in
>> -  // the source code, except for OpenCL, where the address space value 0
>> in
>> -  // QualType represents private address space in OpenCL source code.
>> +  // where there is no address space qualifier.
>>Default = 0,
>>
>>// OpenCL specific address spaces.
>> +  // In OpenCL each l-value must have certain non-default address space,
>> each
>> +  // r-value must have no address space (i.e. the default address
>> space). The
>> +  // pointee of a pointer must have non-default address space.
>>opencl_global,
>>opencl_local,
>>opencl_constant,
>> +  opencl_private,
>>opencl_generic,
>>
>>// CUDA specific address spaces.
>>
>> Modified: cfe/trunk/lib/AST/ASTContext.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=315668&r1=315667&r2=315668&view=diff
>>
>> ==
>> --- cfe/trunk/lib/AST/ASTContext.cpp (original)
>> +++ cfe/trunk/lib/AST/ASTContext.cpp Thu Oct 12 20:37:48 2017
>> @@ -707,6 +707,7 @@ static const LangAS::Map *getAddressSpac
>>1, // opencl_global
>>3, // opencl_local
>>2, // opencl_constant
>> +  0, // opencl_private
>>4, // opencl_generic
>>5, // cuda_device
>>6, // cuda_constant
>>
>> Modified: cfe/trunk/lib/AST/Expr.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Expr.cpp?rev=315668&r1=315667&r2=315668&view=diff
>>
>> ==
>> --- cfe/trunk/lib/AST/Expr.cpp (original)
>> +++ cfe/trunk/lib/AST/Expr.cpp Thu Oct 12 20:37:48 2017
>> @@ -3293,20 +3293,20 @@ Expr::isNullPointerConstant(ASTContext &

[PATCH] D38284: [clang-tidy] Fix google-readability-namespace-comments handling of C++17 nested namespaces

2017-10-13 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added inline comments.



Comment at: clang-tidy/readability/NamespaceCommentCheck.cpp:97
+  }
+  while (Lexer::getRawToken(LBracketLocation, Tok, Sources, getLangOpts()) ||
+ !Tok.is(tok::l_brace)) {

alexfh wrote:
> alexfh wrote:
> > alexfh wrote:
> > > The check started triggering an assertion failure and incredible slowness 
> > > (infinite loops?) on some real files. I've not yet come up with an 
> > > isolated test case, but all this seems to be happening around this loop.
> > > 
> > > I'm going to revert this revision. A bug report (and hopefully a test 
> > > case ;) will follow.
> > > 
> > Reverted in r315580.
> Here's a test case that demonstrates the issue:
> ```
> #define MACRO macro_expansion
> namespace MACRO {
> void f(); // So that the namespace isn't empty.
> // 1
> // 2
> // 3
> // 4
> // 5
> // 6
> // 7
> // CHECK-MESSAGES: :[[@LINE+2]]:2: warning: namespace 'macro_expansion' not 
> terminated with
> // CHECK-MESSAGES: :[[@LINE-10]]:11: note: namespace 'macro_expansion' starts 
> here
> }
> // CHECK-FIXES: }  // namespace macro_expansion
> 
> ```
> 
> I'll commit it once Subversion on llvm.org starts working again.
Committed the regression test in r315682.


https://reviews.llvm.org/D38284



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D38688: [clang-tidy] Misc redundant expressions checker updated for macros

2017-10-13 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments.



Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:357
+  ConstExpr = Result.Nodes.getNodeAs(CstId);
+  return ConstExpr && ConstExpr->isIntegerConstantExpr(Value, *Result.Context);
+}

xazax.hun wrote:
> I think you could just return the pointer and return a null pointer in case 
> it is not an integerConstantExpr. This way no compatibility overload needed.
or `llvm::Optional` making it clear that no result is intended.


Repository:
  rL LLVM

https://reviews.llvm.org/D38688



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D38629: [clangd] Added a callback-based codeComplete in clangd.

2017-10-13 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments.



Comment at: clangd/ClangdServer.cpp:51
 
+template 
+std::future makeFutureAPIFromCallback(

I'm not sure we need a template here rather than just writing the code directly.



Comment at: clangd/ClangdServer.cpp:257
+  // A task that will be run asynchronously.
+  auto Task =
+  // 'mutable' to reassign Preamble variable.

Isn't this exactly where you'd want to use ForwardBinder?
(Yeah, it looks like the work scheduler itself has this functionality too)



Comment at: clangd/ClangdServer.h:261
+  void codeComplete(
+  UniqueFunction>)> Callback,
+  PathRef File, Position Pos,

Hmm, generally having a callback as the last parameter is nicer, but this 
doesn't play nicely with optional params...


https://reviews.llvm.org/D38629



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D38720: [clangd] Report proper kinds for 'Keyword' and 'Snippet' completion items.

2017-10-13 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision.
sammccall added a comment.

Going to land this on Ilya's behalf as he's out on vacation.


https://reviews.llvm.org/D38720



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D37808: [clang-tidy] Add new hicpp-multiway-paths-covered check for missing branches

2017-10-13 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 118911.
JonasToth added a comment.

ping: I don't understand the lack of feedback. This check should not be a 
frontend warning,
since it warns for perfectly valid code. It is supposed to give stronger 
guarantees
for developers requiring this.

- rebased to master


https://reviews.llvm.org/D37808

Files:
  clang-tidy/hicpp/CMakeLists.txt
  clang-tidy/hicpp/HICPPTidyModule.cpp
  clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp
  clang-tidy/hicpp/MultiwayPathsCoveredCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/hicpp-multiway-paths-covered.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/hicpp-multiway-paths-covered-else.cpp
  test/clang-tidy/hicpp-multiway-paths-covered.cpp

Index: test/clang-tidy/hicpp-multiway-paths-covered.cpp
===
--- /dev/null
+++ test/clang-tidy/hicpp-multiway-paths-covered.cpp
@@ -0,0 +1,406 @@
+// RUN: %check_clang_tidy %s hicpp-multiway-paths-covered %t
+
+enum OS { Mac,
+  Windows,
+  Linux };
+
+/// All of these cases will not emit a warning per default, but with explicit activation.
+void problematic_if(int i, enum OS os) {
+  if (i > 0) {
+return;
+  } else if (i < 0) {
+return;
+  }
+
+  if (os == Mac) {
+return;
+  } else if (os == Linux) {
+if (true) {
+  return;
+} else if (false) {
+  return;
+}
+return;
+  } else {
+/* unreachable */
+if (true) // check if the parent would match here as well
+  return;
+  }
+
+  // Ok, because all paths are covered
+  if (i > 0) {
+return;
+  } else if (i < 0) {
+return;
+  } else {
+/* error, maybe precondition failed */
+  }
+}
+
+int return_integer() { return 42; }
+
+void problematic_switch(int i) {
+  // No default in this switch
+  switch (i) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered codepath found; add a default branch
+  case 0:
+break;
+  case 1:
+break;
+  case 2:
+break;
+  }
+
+  // degenerate, maybe even warning
+  switch (i) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: degenerate switch statement without any cases found; add cases and a default branch
+  }
+
+  switch (int j = return_integer()) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered codepath found; add a default branch
+  case 0:
+  case 1:
+  case 2:
+break;
+  }
+
+  // Degenerated, only default branch.
+  switch (i) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: degenerated switch stmt found; only the default branch exists
+  default:
+break;
+  }
+
+  // Degenerated, only one case label and default branch -> Better as if-stmt.
+  switch (i) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: switch case could be better written with single if stmt
+  case 0:
+break;
+  default:
+break;
+  }
+
+  unsigned long long BigNumber = 0;
+  switch (BigNumber) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered codepath found; add a default branch
+  case 0:
+  case 1:
+break;
+  }
+
+  const int &IntRef = i;
+  switch (IntRef) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered codepath found; add a default branch
+  case 0:
+  case 1:
+break;
+  }
+
+  char C = 'A';
+  switch (C) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered codepath found; add a default branch
+  case 'A':
+break;
+  case 'B':
+break;
+  }
+}
+
+void unproblematic_switch(unsigned char c) {
+  // The switch stmt covers all paths explicitly.
+  // FIXME: False positive, determine the count of different possible values correctly.
+  // On the other hand, char is not defined to be one byte big, so there could be
+  // architectural differences, which would make the 'default:' case more portable.
+  switch (c) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered codepath found; add a default branch
+  case 0:
+  case 1:
+  case 2:
+  case 3:
+  case 4:
+  case 5:
+  case 6:
+  case 7:
+  case 8:
+  case 9:
+  case 10:
+  case 11:
+  case 12:
+  case 13:
+  case 14:
+  case 15:
+  case 16:
+  case 17:
+  case 18:
+  case 19:
+  case 20:
+  case 21:
+  case 22:
+  case 23:
+  case 24:
+  case 25:
+  case 26:
+  case 27:
+  case 28:
+  case 29:
+  case 30:
+  case 31:
+  case 32:
+  case 33:
+  case 34:
+  case 35:
+  case 36:
+  case 37:
+  case 38:
+  case 39:
+  case 40:
+  case 41:
+  case 42:
+  case 43:
+  case 44:
+  case 45:
+  case 46:
+  case 47:
+  case 48:
+  case 49:
+  case 50:
+  case 51:
+  case 52:
+  case 53:
+  case 54:
+  case 55:
+  case 56:
+  case 57:
+  case 58:
+  case 59:
+  case 60:
+  case 61:
+  case 62:
+  case 63:
+  case 64:
+  case 65:
+  case 66:
+  case 67:
+  case 68:
+  case 69:
+  case 70:
+  case 71:
+  case 72:
+  case 73:
+  case 74:
+  case 75:
+  case 76:
+  case 77:
+  case 78:
+  case 79:
+  case 80:
+  case 81:
+  case 82:
+  case 83:
+  case 84:
+  case 85:
+  case 86:
+  case 87:
+  case 88:
+  case 89:
+  ca

[PATCH] D38883: [CMake][OpenMP] Customize default offloading arch

2017-10-13 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld created this revision.
Herald added a subscriber: mgorny.

For the shuffle instructions in reductions we need at least sm_30
but the user may want to customize the default architecture.
Also remove some code that went in while troubleshooting broken
tests on external build bots.


https://reviews.llvm.org/D38883

Files:
  CMakeLists.txt
  include/clang/Config/config.h.cmake
  lib/Driver/ToolChains/Cuda.cpp
  lib/Driver/ToolChains/Cuda.h


Index: lib/Driver/ToolChains/Cuda.h
===
--- lib/Driver/ToolChains/Cuda.h
+++ lib/Driver/ToolChains/Cuda.h
@@ -76,17 +76,6 @@
   std::string getLibDeviceFile(StringRef Gpu) const {
 return LibDeviceMap.lookup(Gpu);
   }
-  /// \brief Get lowest available compute capability
-  /// for which a libdevice library exists.
-  std::string getLowestExistingArch() const {
-std::string LibDeviceFile;
-for (auto key : LibDeviceMap.keys()) {
-  LibDeviceFile = LibDeviceMap.lookup(key);
-  if (!LibDeviceFile.empty())
-return key;
-}
-return "sm_20";
-  }
 };
 
 namespace tools {
Index: lib/Driver/ToolChains/Cuda.cpp
===
--- lib/Driver/ToolChains/Cuda.cpp
+++ lib/Driver/ToolChains/Cuda.cpp
@@ -167,19 +167,6 @@
   }
 }
 
-// This code prevents IsValid from being set when
-// no libdevice has been found.
-bool allEmpty = true;
-std::string LibDeviceFile;
-for (auto key : LibDeviceMap.keys()) {
-  LibDeviceFile = LibDeviceMap.lookup(key);
-  if (!LibDeviceFile.empty())
-allEmpty = false;
-}
-
-if (allEmpty)
-  continue;
-
 IsValid = true;
 break;
   }
@@ -565,12 +552,8 @@
 
 StringRef Arch = DAL->getLastArgValue(options::OPT_march_EQ);
 if (Arch.empty()) {
-  // Default compute capability for CUDA toolchain is the
-  // lowest compute capability supported by the installed
-  // CUDA version.
-  DAL->AddJoinedArg(nullptr,
-  Opts.getOption(options::OPT_march_EQ),
-  CudaInstallation.getLowestExistingArch());
+  DAL->AddJoinedArg(nullptr, Opts.getOption(options::OPT_march_EQ),
+CLANG_OPENMP_NVPTX_DEFAULT_ARCH);
 }
 
 return DAL;
Index: include/clang/Config/config.h.cmake
===
--- include/clang/Config/config.h.cmake
+++ include/clang/Config/config.h.cmake
@@ -20,6 +20,9 @@
 /* Default OpenMP runtime used by -fopenmp. */
 #define CLANG_DEFAULT_OPENMP_RUNTIME "${CLANG_DEFAULT_OPENMP_RUNTIME}"
 
+/* Default architecture for OpenMP offloading to Nvidia GPUs. */
+#define CLANG_OPENMP_NVPTX_DEFAULT_ARCH "${CLANG_OPENMP_NVPTX_DEFAULT_ARCH}"
+
 /* Multilib suffix for libdir. */
 #define CLANG_LIBDIR_SUFFIX "${CLANG_LIBDIR_SUFFIX}"
 
Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -235,6 +235,16 @@
 set(CLANG_DEFAULT_OPENMP_RUNTIME "libomp" CACHE STRING
   "Default OpenMP runtime used by -fopenmp.")
 
+# OpenMP offloading requires at least sm_30 because we use shuffle instructions
+# to generate efficient code for reductions.
+set(CLANG_OPENMP_NVPTX_DEFAULT_ARCH "sm_30" CACHE STRING
+  "Default architecture for OpenMP offloading to Nvidia GPUs.")
+if (NOT("${CLANG_OPENMP_NVPTX_DEFAULT_ARCH}" MATCHES "^sm_[0-9]+$"))
+  message(WARNING "Resetting default architecture for OpenMP offloading to 
Nvidia GPUs to sm_30")
+  set(CLANG_OPENMP_NVPTX_DEFAULT_ARCH "sm_30" CACHE STRING
+"Default architecture for OpenMP offloading to Nvidia GPUs." FORCE)
+endif()
+
 set(CLANG_VENDOR ${PACKAGE_VENDOR} CACHE STRING
   "Vendor-specific text for showing with version information.")
 


Index: lib/Driver/ToolChains/Cuda.h
===
--- lib/Driver/ToolChains/Cuda.h
+++ lib/Driver/ToolChains/Cuda.h
@@ -76,17 +76,6 @@
   std::string getLibDeviceFile(StringRef Gpu) const {
 return LibDeviceMap.lookup(Gpu);
   }
-  /// \brief Get lowest available compute capability
-  /// for which a libdevice library exists.
-  std::string getLowestExistingArch() const {
-std::string LibDeviceFile;
-for (auto key : LibDeviceMap.keys()) {
-  LibDeviceFile = LibDeviceMap.lookup(key);
-  if (!LibDeviceFile.empty())
-return key;
-}
-return "sm_20";
-  }
 };
 
 namespace tools {
Index: lib/Driver/ToolChains/Cuda.cpp
===
--- lib/Driver/ToolChains/Cuda.cpp
+++ lib/Driver/ToolChains/Cuda.cpp
@@ -167,19 +167,6 @@
   }
 }
 
-// This code prevents IsValid from being set when
-// no libdevice has been found.
-bool allEmpty = true;
-std::string LibDeviceFile;
-for (auto key : LibDeviceMap.keys()) {
-  LibDeviceFile = LibDeviceMap.lookup(key);
-  if (!LibDeviceFile.empty())
-allEmpty = false;
-}
-
- 

[PATCH] D38882: [clang-rename] Add function unit tests.

2017-10-13 Thread Eric Liu via Phabricator via cfe-commits
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.

Lg


https://reviews.llvm.org/D38882



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D38816: Convert clang::LangAS to a strongly typed enum

2017-10-13 Thread Alexander Richardson via Phabricator via cfe-commits
arichardson updated this revision to Diff 118915.
arichardson edited the summary of this revision.
arichardson added a comment.
This revision is now accepted and ready to land.

- Keep old behaviour for clang_getAddressSpace()
- renamed to getLangASFromTargetAS()
- rebased on latest trunk


https://reviews.llvm.org/D38816

Files:
  include/clang/AST/ASTContext.h
  include/clang/AST/Type.h
  include/clang/Basic/AddressSpaces.h
  include/clang/Basic/TargetInfo.h
  lib/AST/ASTContext.cpp
  lib/AST/ItaniumMangle.cpp
  lib/AST/RecordLayoutBuilder.cpp
  lib/AST/TypePrinter.cpp
  lib/Basic/TargetInfo.cpp
  lib/Basic/Targets/AMDGPU.cpp
  lib/Basic/Targets/AMDGPU.h
  lib/CodeGen/CGBlocks.cpp
  lib/CodeGen/CGDecl.cpp
  lib/CodeGen/CGExprConstant.cpp
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  lib/CodeGen/CGValue.h
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/CodeGen/CodeGenModule.h
  lib/CodeGen/CodeGenTypeCache.h
  lib/CodeGen/ConstantEmitter.h
  lib/CodeGen/TargetInfo.cpp
  lib/CodeGen/TargetInfo.h
  lib/Sema/SemaChecking.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclAttr.cpp
  lib/Sema/SemaDeclObjC.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaExprCXX.cpp
  lib/Sema/SemaTemplateDeduction.cpp
  lib/Sema/SemaType.cpp
  tools/libclang/CXType.cpp

Index: tools/libclang/CXType.cpp
===
--- tools/libclang/CXType.cpp
+++ tools/libclang/CXType.cpp
@@ -403,7 +403,10 @@
   if (T.getAddressSpace() >= LangAS::FirstTargetAddressSpace) {
 return T.getQualifiers().getAddressSpaceAttributePrintValue();
   }
-  return T.getAddressSpace();
+  // FIXME: this function returns either a LangAS or a target AS
+  // Those values can overlap which makes this function rather unpredictable
+  // for any caller
+  return (unsigned)T.getAddressSpace();
 }
 
 CXString clang_getTypedefName(CXType CT) {
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -5631,7 +5631,7 @@
 // If this type is already address space qualified, reject it.
 // ISO/IEC TR 18037 S5.3 (amending C99 6.7.3): "No type shall be qualified
 // by qualifiers for two or more different address spaces."
-if (T.getAddressSpace()) {
+if (T.getAddressSpace() != LangAS::Default) {
   Diag(AttrLoc, diag::err_attribute_address_multiple_qualifiers);
   return QualType();
 }
@@ -5655,15 +5655,16 @@
 }
 
 llvm::APSInt max(addrSpace.getBitWidth());
-max = Qualifiers::MaxAddressSpace - LangAS::FirstTargetAddressSpace;
+max =
+Qualifiers::MaxAddressSpace - (unsigned)LangAS::FirstTargetAddressSpace;
 if (addrSpace > max) {
   Diag(AttrLoc, diag::err_attribute_address_space_too_high)
   << (unsigned)max.getZExtValue() << AddrSpace->getSourceRange();
   return QualType();
 }
 
-unsigned ASIdx = static_cast(addrSpace.getZExtValue()) +
- LangAS::FirstTargetAddressSpace;
+LangAS ASIdx =
+getLangASFromTargetAS(static_cast(addrSpace.getZExtValue()));
 
 return Context.getAddrSpaceQualType(T, ASIdx);
   }
@@ -5689,7 +5690,7 @@
   // If this type is already address space qualified, reject it.
   // ISO/IEC TR 18037 S5.3 (amending C99 6.7.3): "No type shall be qualified by
   // qualifiers for two or more different address spaces."
-  if (Type.getAddressSpace()) {
+  if (Type.getAddressSpace() != LangAS::Default) {
 S.Diag(Attr.getLoc(), diag::err_attribute_address_multiple_qualifiers);
 Attr.setInvalid();
 return;
@@ -5703,7 +5704,7 @@
 return;
   }
 
-  unsigned ASIdx;
+  LangAS ASIdx;
   if (Attr.getKind() == AttributeList::AT_AddressSpace) {
 
 // Check the attribute arguments.
@@ -7036,7 +7037,7 @@
   (T->isVoidType() && !IsPointee))
 return;
 
-  unsigned ImpAddr;
+  LangAS ImpAddr;
   // Put OpenCL automatic variable in private address space.
   // OpenCL v1.2 s6.5:
   // The default address space name for arguments to a function in a
Index: lib/Sema/SemaTemplateDeduction.cpp
===
--- lib/Sema/SemaTemplateDeduction.cpp
+++ lib/Sema/SemaTemplateDeduction.cpp
@@ -1870,11 +1870,10 @@
 Deduced);
   }
 
-  if (Arg.getAddressSpace() >= LangAS::FirstTargetAddressSpace) {
+  if (isTargetAddressSpace(Arg.getAddressSpace())) {
 llvm::APSInt ArgAddressSpace(S.Context.getTypeSize(S.Context.IntTy),
  false);
-ArgAddressSpace =
-(Arg.getAddressSpace() - LangAS::FirstTargetAddressSpace);
+ArgAddressSpace = toTargetAddressSpace(Arg.getAddressSpace());
 
 // Perform deduction on the pointer types.
 if (Sema::TemplateDeductionResult Result =
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -2113,7 +

[PATCH] D38868: [OpenCL] Restrict swizzle length check to OpenCL mode

2017-10-13 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added inline comments.



Comment at: test/SemaOpenCL/vector_swizzle_length.cl:7
 void foo() {
-float8 f2 = (float8)(0, 0, 0, 0, 0, 0, 0, 0);
+float8 f2 = (float8){0, 0, 0, 0, 0, 0, 0, 0};
 

Even though this works in Clang, ideally OpenCL vector literal is with 
parenthesis (see s6.1.6).



Comment at: test/SemaOpenCL/vector_swizzle_length.cl:13
+#else
+// expected-no-diagnostics
+(void)f2.s01234;

I am not sure it's good idea to test C mode here since this is intended for 
OpenCL only functionality. I think it might be better to separate C 
functionality and keep under regular Sema tests. We could keep the same test 
name to be able to identify similar functionality.


https://reviews.llvm.org/D38868



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D37808: [clang-tidy] Add new hicpp-multiway-paths-covered check for missing branches

2017-10-13 Thread Jonas Devlieghere via Phabricator via cfe-commits
JDevlieghere added a comment.

I very much like this check. I only have a few minor comments, but maybe this 
encourages others to have a look too!




Comment at: clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp:85
+diag(ElseIfWithoutElse->getLocStart(),
+ "potential uncovered codepath found; add an ending else branch");
+return;

I'm not a big fan of the 'found', can we just omit it? The same goes for the 
other diags. 



Comment at: clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp:96
+// Only the default branch (we explicitly matched for default!) exists.
+if (CaseCount == 1) {
+  diag(SwitchWithDefault->getLocStart(),

Why not a switch?



Comment at: clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp:152
+// Should be written as an IfStmt.
+if (CaseCount == 1) {
+  diag(SwitchWithoutDefault->getLocStart(), "switch stmt with only one "

I'm aware that the message and fixme are different, but since the structure is 
so similar to the handling of the other switch case, I wonder if there is any 
chance we could extract the common parts?


https://reviews.llvm.org/D37808



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r315688 - Remove an unused variable.

2017-10-13 Thread Haojian Wu via cfe-commits
Author: hokein
Date: Fri Oct 13 08:34:03 2017
New Revision: 315688

URL: http://llvm.org/viewvc/llvm-project?rev=315688&view=rev
Log:
Remove an unused variable.

Fix -Wunused-but-set-variable warning.

Modified:
cfe/trunk/lib/Sema/SemaType.cpp

Modified: cfe/trunk/lib/Sema/SemaType.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=315688&r1=315687&r2=315688&view=diff
==
--- cfe/trunk/lib/Sema/SemaType.cpp (original)
+++ cfe/trunk/lib/Sema/SemaType.cpp Fri Oct 13 08:34:03 2017
@@ -7079,7 +7079,6 @@ static void processTypeAttrs(TypeProcess
   // type, but others can be present in the type specifiers even though they
   // apply to the decl.  Here we apply type attributes and ignore the rest.
 
-  bool hasOpenCLAddressSpace = false;
   while (attrs) {
 AttributeList &attr = *attrs;
 attrs = attr.getNext(); // reset to the next here due to early loop 
continue
@@ -7142,7 +7141,6 @@ static void processTypeAttrs(TypeProcess
 case AttributeList::AT_AddressSpace:
   HandleAddressSpaceTypeAttribute(type, attr, state.getSema());
   attr.setUsedAsTypeAttr();
-  hasOpenCLAddressSpace = true;
   break;
 OBJC_POINTER_TYPE_ATTRS_CASELIST:
   if (!handleObjCPointerTypeAttr(state, attr, type))


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r315689 - Fix an unused-variable warning.

2017-10-13 Thread Haojian Wu via cfe-commits
Author: hokein
Date: Fri Oct 13 08:37:53 2017
New Revision: 315689

URL: http://llvm.org/viewvc/llvm-project?rev=315689&view=rev
Log:
Fix an unused-variable warning.

Modified:
cfe/trunk/lib/CodeGen/CGClass.cpp

Modified: cfe/trunk/lib/CodeGen/CGClass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGClass.cpp?rev=315689&r1=315688&r2=315689&view=diff
==
--- cfe/trunk/lib/CodeGen/CGClass.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGClass.cpp Fri Oct 13 08:37:53 2017
@@ -1516,7 +1516,7 @@ namespace {
   llvm::Value *LoadThisForDtorDelete(CodeGenFunction &CGF,
  const CXXDestructorDecl *DD) {
 if (Expr *ThisArg = DD->getOperatorDeleteThisArg())
-  return CGF.EmitScalarExpr(DD->getOperatorDeleteThisArg());
+  return CGF.EmitScalarExpr(ThisArg);
 return CGF.LoadCXXThis();
   }
 


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D38816: Convert clang::LangAS to a strongly typed enum

2017-10-13 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added inline comments.



Comment at: lib/AST/TypePrinter.cpp:1323
 OS << "address_space(";
-OS << T->getEquivalentType().getAddressSpace();
+OS << T->getEquivalentType()
+  .getQualifiers()

arichardson wrote:
> Anastasia wrote:
> > Why do we need this change?
> `__attribute__((address_space(n)))` is a target address space and not a 
> language address space like `LangAS::opencl_generic`. Isn't 
> `Qualifiers::getAddressSpaceAttributePrintValue()` meant exactly for this use 
> case?
Yes, I think there are some adjustment we do in this method to get the original 
source value to be printed corerctly. Does this mean we have no tests that 
caught this issue?


https://reviews.llvm.org/D38816



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D38816: Convert clang::LangAS to a strongly typed enum

2017-10-13 Thread Alexander Richardson via Phabricator via cfe-commits
arichardson added inline comments.



Comment at: lib/AST/TypePrinter.cpp:1323
 OS << "address_space(";
-OS << T->getEquivalentType().getAddressSpace();
+OS << T->getEquivalentType()
+  .getQualifiers()

Anastasia wrote:
> arichardson wrote:
> > Anastasia wrote:
> > > Why do we need this change?
> > `__attribute__((address_space(n)))` is a target address space and not a 
> > language address space like `LangAS::opencl_generic`. Isn't 
> > `Qualifiers::getAddressSpaceAttributePrintValue()` meant exactly for this 
> > use case?
> Yes, I think there are some adjustment we do in this method to get the 
> original source value to be printed corerctly. Does this mean we have no 
> tests that caught this issue?
Seems like it, all tests pass both with and without this patch.


https://reviews.llvm.org/D38816



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D38857: [OpenCL] Improve printing and semantic check related to implicit addr space

2017-10-13 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!

Can we close https://bugs.llvm.org/show_bug.cgi?id=33418 after this commit?




Comment at: test/SemaOpenCL/null_literal.cl:38
 
-#ifdef CL20
-// Accept explicitly pointer to generic address space in OpenCL v2.0.
-global int* ptr5 = (generic void*)0;
-#endif
-
-global int* ptr6 = (local void*)0; // expected-error{{initializing '__global 
int *' with an expression of type '__local void *' changes address space of 
pointer}}
+  global int *g7 = (global void*)(generic void *)0;
+  constant int *c7 = (constant void*)(generic void *)0; 
//expected-error{{casting '__generic void *' to type '__constant void *' 
changes address space of pointer}}

Does this extra cast test anything we already miss to test?


https://reviews.llvm.org/D38857



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D38134: [OpenCL] Emit enqueued block as kernel

2017-10-13 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! Great work! Thanks!


https://reviews.llvm.org/D38134



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D37299: [Modules] Add ability to specify module name to module file mapping in a file

2017-10-13 Thread Boris Kolpackov via Phabricator via cfe-commits
boris added a comment.

Ping.


https://reviews.llvm.org/D37299



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D37808: [clang-tidy] Add new hicpp-multiway-paths-covered check for missing branches

2017-10-13 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments.



Comment at: clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp:85
+diag(ElseIfWithoutElse->getLocStart(),
+ "potential uncovered codepath found; add an ending else branch");
+return;

JDevlieghere wrote:
> I'm not a big fan of the 'found', can we just omit it? The same goes for the 
> other diags. 
Agree. The messages are better without 'found'.



Comment at: clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp:96
+// Only the default branch (we explicitly matched for default!) exists.
+if (CaseCount == 1) {
+  diag(SwitchWithDefault->getLocStart(),

JDevlieghere wrote:
> Why not a switch?
I intent to check if all cases are explicitly covered.

In the testcases there is one switch with all numbers explicitly written, 
meaning there is no need to add a default branch.

This would allow further 
```
else if (CaseCount == MaximumPossibleCases) { /* No warning */ }
```
path which is not modable with `switch`.



Comment at: clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp:152
+// Should be written as an IfStmt.
+if (CaseCount == 1) {
+  diag(SwitchWithoutDefault->getLocStart(), "switch stmt with only one "

JDevlieghere wrote:
> I'm aware that the message and fixme are different, but since the structure 
> is so similar to the handling of the other switch case, I wonder if there is 
> any chance we could extract the common parts?
I try to get something shorter.
Maybe 
```
if(CaseCount == 1 && MatchedSwitch) {}
else if(CaseCount == 1 && MatchedElseIf) {}
```
?


https://reviews.llvm.org/D37808



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r315699 - Hide "#pragma optimize("", off)" from clang when it pretends to be MSVC 2017

2017-10-13 Thread Reid Kleckner via cfe-commits
Author: rnk
Date: Fri Oct 13 09:18:32 2017
New Revision: 315699

URL: http://llvm.org/viewvc/llvm-project?rev=315699&view=rev
Log:
Hide "#pragma optimize("", off)" from clang when it pretends to be MSVC 2017

These pragmas work around a bug in VC 1911 that isn't present in clang,
and clang warns about them.

Modified:
cfe/trunk/lib/AST/StmtProfile.cpp

Modified: cfe/trunk/lib/AST/StmtProfile.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/StmtProfile.cpp?rev=315699&r1=315698&r2=315699&view=diff
==
--- cfe/trunk/lib/AST/StmtProfile.cpp (original)
+++ cfe/trunk/lib/AST/StmtProfile.cpp Fri Oct 13 09:18:32 2017
@@ -1418,7 +1418,7 @@ static Stmt::StmtClass DecodeOperatorCal
   llvm_unreachable("Invalid overloaded operator expression");
 }
 
-#if defined(_MSC_VER)
+#if defined(_MSC_VER) && !defined(__clang__)
 #if _MSC_VER == 1911
 // Work around 
https://developercommunity.visualstudio.com/content/problem/84002/clang-cl-when-built-with-vc-2017-crashes-cause-vc.html
 // MSVC 2017 update 3 miscompiles this function, and a clang built with it
@@ -1459,7 +1459,7 @@ void StmtProfiler::VisitCXXOperatorCallE
   ID.AddInteger(S->getOperator());
 }
 
-#if defined(_MSC_VER)
+#if defined(_MSC_VER) && !defined(__clang__)
 #if _MSC_VER == 1911
 #pragma optimize("", on)
 #endif


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D38816: Convert clang::LangAS to a strongly typed enum

2017-10-13 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added inline comments.



Comment at: lib/AST/TypePrinter.cpp:1323
 OS << "address_space(";
-OS << T->getEquivalentType().getAddressSpace();
+OS << T->getEquivalentType()
+  .getQualifiers()

arichardson wrote:
> Anastasia wrote:
> > arichardson wrote:
> > > Anastasia wrote:
> > > > Why do we need this change?
> > > `__attribute__((address_space(n)))` is a target address space and not a 
> > > language address space like `LangAS::opencl_generic`. Isn't 
> > > `Qualifiers::getAddressSpaceAttributePrintValue()` meant exactly for this 
> > > use case?
> > Yes, I think there are some adjustment we do in this method to get the 
> > original source value to be printed corerctly. Does this mean we have no 
> > tests that caught this issue?
> Seems like it, all tests pass both with and without this patch.
Strange considering that we have this attribute printed in some error messages 
of some Sema tests. If I compile this code without your patch:
 
```
typedef int __attribute__((address_space(1))) int_1;
typedef int __attribute__((address_space(2))) int_2;

void f0(int_1 &); 
void f0(const int_1 &);

void test_f0() {
  int i;
  static int_2 i2;
  f0(i);
  f0(i2);
}
```

I get the address spaces printed correctly inside the type:
  note: candidate function not viable: 1st argument ('int_2' (aka 
'__attribute__((address_space(2))) int')) is in address space 2, but parameter 
must be in address space 1

Perhaps @yaxunl could comment further on whether this change is needed.


https://reviews.llvm.org/D38816



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D38816: Convert clang::LangAS to a strongly typed enum

2017-10-13 Thread Alexander Richardson via Phabricator via cfe-commits
arichardson added inline comments.



Comment at: lib/AST/TypePrinter.cpp:1323
 OS << "address_space(";
-OS << T->getEquivalentType().getAddressSpace();
+OS << T->getEquivalentType()
+  .getQualifiers()

Anastasia wrote:
> arichardson wrote:
> > Anastasia wrote:
> > > arichardson wrote:
> > > > Anastasia wrote:
> > > > > Why do we need this change?
> > > > `__attribute__((address_space(n)))` is a target address space and not a 
> > > > language address space like `LangAS::opencl_generic`. Isn't 
> > > > `Qualifiers::getAddressSpaceAttributePrintValue()` meant exactly for 
> > > > this use case?
> > > Yes, I think there are some adjustment we do in this method to get the 
> > > original source value to be printed corerctly. Does this mean we have no 
> > > tests that caught this issue?
> > Seems like it, all tests pass both with and without this patch.
> Strange considering that we have this attribute printed in some error 
> messages of some Sema tests. If I compile this code without your patch:
>  
> ```
> typedef int __attribute__((address_space(1))) int_1;
> typedef int __attribute__((address_space(2))) int_2;
> 
> void f0(int_1 &); 
> void f0(const int_1 &);
> 
> void test_f0() {
>   int i;
>   static int_2 i2;
>   f0(i);
>   f0(i2);
> }
> ```
> 
> I get the address spaces printed correctly inside the type:
>   note: candidate function not viable: 1st argument ('int_2' (aka 
> '__attribute__((address_space(2))) int')) is in address space 2, but 
> parameter must be in address space 1
> 
> Perhaps @yaxunl could comment further on whether this change is needed.
My guess is that it doesn't go through that switch statement but rather through 
`Qualifiers::print()`. I'll try adding a llvm_unreachable() to see if there are 
any tests that go down this path.


https://reviews.llvm.org/D38816



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D38883: [CMake][OpenMP] Customize default offloading arch

2017-10-13 Thread Artem Belevich via Phabricator via cfe-commits
tra added inline comments.



Comment at: lib/Driver/ToolChains/Cuda.cpp:170-182
-// This code prevents IsValid from being set when
-// no libdevice has been found.
-bool allEmpty = true;
-std::string LibDeviceFile;
-for (auto key : LibDeviceMap.keys()) {
-  LibDeviceFile = LibDeviceMap.lookup(key);
-  if (!LibDeviceFile.empty())

I'd keep this code. It appears to serve useful purpose as it requires CUDA 
installation to have at least some libdevice library in it.  It gives us a 
change to find a valid installation, instead of ailing some time later when we 
ask for a libdevice file and fail because there are none.



Comment at: lib/Driver/ToolChains/Cuda.cpp:556
+  DAL->AddJoinedArg(nullptr, Opts.getOption(options::OPT_march_EQ),
+CLANG_OPENMP_NVPTX_DEFAULT_ARCH);
 }

This sets default GPU arch for *everyone* based on OPENMP requirements. Perhaps 
this should be predicated on this being openmp compilation.

IMO to avoid unnecessary surprises, the default for CUDA compilation should 
follow defaults of nvcc. sm_30 becomes default only in CUDA-9.



https://reviews.llvm.org/D38883



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D38893: [change-namespace] do not change type locs in defaulted functions.

2017-10-13 Thread Eric Liu via Phabricator via cfe-commits
ioeric created this revision.

https://reviews.llvm.org/D38893

Files:
  change-namespace/ChangeNamespace.cpp
  unittests/change-namespace/ChangeNamespaceTests.cpp


Index: unittests/change-namespace/ChangeNamespaceTests.cpp
===
--- unittests/change-namespace/ChangeNamespaceTests.cpp
+++ unittests/change-namespace/ChangeNamespaceTests.cpp
@@ -2093,6 +2093,68 @@
   EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
 }
 
+TEST_F(ChangeNamespaceTest, DefaultMoveConstructors) {
+  std::string Code = "namespace na {\n"
+ "class B {\n"
+ " public:\n"
+ "  B() = default;\n"
+ "  // Allow move only.\n"
+ "  B(B&&) = default;\n"
+ "  B& operator=(B&&) = default;\n"
+ "  B(const B&) = delete;\n"
+ "  B& operator=(const B&) = delete;\n"
+ " private:\n"
+ "  int ref_;\n"
+ "};\n"
+ "} // namespace na\n"
+ "namespace na {\n"
+ "namespace nb {\n"
+ "class A {\n"
+ "public:\n"
+ "  A() = default;\n"
+ "  A(A&&) = default;\n"
+ "  A& operator=(A&&) = default;\n"
+ "private:\n"
+ "  B b;\n"
+ "  A(const A&) = delete;\n"
+ "  A& operator=(const A&) = delete;\n"
+ "};\n"
+ "void f() { A a; a = A(); A aa = A(); }\n"
+ "} // namespace nb\n"
+ "} // namespace na\n";
+  std::string Expected = "namespace na {\n"
+ "class B {\n"
+ " public:\n"
+ "  B() = default;\n"
+ "  // Allow move only.\n"
+ "  B(B&&) = default;\n"
+ "  B& operator=(B&&) = default;\n"
+ "  B(const B&) = delete;\n"
+ "  B& operator=(const B&) = delete;\n"
+ " private:\n"
+ "  int ref_;\n"
+ "};\n"
+ "} // namespace na\n"
+ "\n"
+ "namespace x {\n"
+ "namespace y {\n"
+ "class A {\n"
+ "public:\n"
+ "  A() = default;\n"
+ "  A(A&&) = default;\n"
+ "  A& operator=(A&&) = default;\n"
+ "private:\n"
+ "  na::B b;\n"
+ "  A(const A&) = delete;\n"
+ "  A& operator=(const A&) = delete;\n"
+ "};\n"
+ "void f() { A a; a = A(); A aa = A(); }\n"
+ "} // namespace y\n"
+ "} // namespace x\n";
+  EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
+
 } // anonymous namespace
 } // namespace change_namespace
 } // namespace clang
Index: change-namespace/ChangeNamespace.cpp
===
--- change-namespace/ChangeNamespace.cpp
+++ change-namespace/ChangeNamespace.cpp
@@ -427,7 +427,8 @@
  unless(templateSpecializationType())),
hasParent(nestedNameSpecifierLoc()),
hasAncestor(isImplicit()),
-   hasAncestor(UsingShadowDeclInClass))),
+   hasAncestor(UsingShadowDeclInClass),
+   hasAncestor(functionDecl(isDefaulted(),
   hasAncestor(decl().bind("dc")))
   .bind("type"),
   this);
@@ -451,6 +452,7 @@
   specifiesType(hasDeclaration(DeclMatcher.bind("from_decl"),
   unless(anyOf(hasAncestor(isImplicit()),
hasAncestor(UsingShadowDeclInClass),
+   hasAncestor(functionDecl(isDefaulted())),
hasAncestor(typeLoc(loc(qualType(hasDeclaration(
decl(equalsBoundNode("from_decl"))
   .bind("nested_specifier_loc"),


Index: unittests/change-namespace/ChangeNamespaceTests.cpp
===
--- unittests/change-namespace/ChangeNamespaceTests.cpp
+++ unittests/change-namespace/ChangeNamespaceTests.cpp
@@ -2093,6 +2093,68 @@
   EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
 }
 
+TEST_F(ChangeNamespaceTest, DefaultMoveConstructors) {
+  std::string Code = "namespace na {\n"
+ "class B {\n"
+ " public:\n"
+ "  B() = de

r315702 - [CodeGen] EmitCXXMemberDataPointerAddress() to generate TBAA info along with LValue base info

2017-10-13 Thread Ivan A. Kosarev via cfe-commits
Author: kosarev
Date: Fri Oct 13 09:38:32 2017
New Revision: 315702

URL: http://llvm.org/viewvc/llvm-project?rev=315702&view=rev
Log:
[CodeGen] EmitCXXMemberDataPointerAddress() to generate TBAA info along with 
LValue base info

This patch should not bring in any functional changes.

Differential Revision: https://reviews.llvm.org/D38788

Modified:
cfe/trunk/lib/CodeGen/CGClass.cpp
cfe/trunk/lib/CodeGen/CGExpr.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h

Modified: cfe/trunk/lib/CodeGen/CGClass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGClass.cpp?rev=315702&r1=315701&r2=315702&view=diff
==
--- cfe/trunk/lib/CodeGen/CGClass.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGClass.cpp Fri Oct 13 09:38:32 2017
@@ -129,13 +129,16 @@ Address
 CodeGenFunction::EmitCXXMemberDataPointerAddress(const Expr *E, Address base,
  llvm::Value *memberPtr,
   const MemberPointerType *memberPtrType,
- LValueBaseInfo *BaseInfo) {
+ LValueBaseInfo *BaseInfo,
+ TBAAAccessInfo *TBAAInfo) {
   // Ask the ABI to compute the actual address.
   llvm::Value *ptr =
 CGM.getCXXABI().EmitMemberDataPointerAddress(*this, E, base,
  memberPtr, memberPtrType);
 
   QualType memberType = memberPtrType->getPointeeType();
+  if (TBAAInfo)
+*TBAAInfo = CGM.getTBAAAccessInfo(memberType);
   CharUnits memberAlign = getNaturalTypeAlignment(memberType, BaseInfo);
   memberAlign =
 CGM.getDynamicOffsetAlignment(base.getAlignment(),

Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=315702&r1=315701&r2=315702&view=diff
==
--- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp Fri Oct 13 09:38:32 2017
@@ -4566,11 +4566,12 @@ EmitPointerToDataMemberBinaryExpr(const
 = E->getRHS()->getType()->getAs();
 
   LValueBaseInfo BaseInfo;
+  TBAAAccessInfo TBAAInfo;
   Address MemberAddr =
-EmitCXXMemberDataPointerAddress(E, BaseAddr, OffsetV, MPT, &BaseInfo);
+EmitCXXMemberDataPointerAddress(E, BaseAddr, OffsetV, MPT, &BaseInfo,
+&TBAAInfo);
 
-  return MakeAddrLValue(MemberAddr, MPT->getPointeeType(), BaseInfo,
-CGM.getTBAAAccessInfo(MPT->getPointeeType()));
+  return MakeAddrLValue(MemberAddr, MPT->getPointeeType(), BaseInfo, TBAAInfo);
 }
 
 /// Given the address of a temporary variable, produce an r-value of

Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.h?rev=315702&r1=315701&r2=315702&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenFunction.h (original)
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.h Fri Oct 13 09:38:32 2017
@@ -3327,7 +3327,8 @@ public:
   Address EmitCXXMemberDataPointerAddress(const Expr *E, Address base,
   llvm::Value *memberPtr,
   const MemberPointerType 
*memberPtrType,
-  LValueBaseInfo *BaseInfo = nullptr);
+  LValueBaseInfo *BaseInfo = nullptr,
+  TBAAAccessInfo *TBAAInfo = nullptr);
   RValue EmitCXXMemberPointerCallExpr(const CXXMemberCallExpr *E,
   ReturnValueSlot ReturnValue);
 


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D38788: [CodeGen] EmitCXXMemberDataPointerAddress() to generate TBAA info along with LValue base info

2017-10-13 Thread Ivan Kosarev via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL315702: [CodeGen] EmitCXXMemberDataPointerAddress() to 
generate TBAA info along with… (authored by kosarev).

Changed prior to commit:
  https://reviews.llvm.org/D38788?vs=118560&id=118931#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D38788

Files:
  cfe/trunk/lib/CodeGen/CGClass.cpp
  cfe/trunk/lib/CodeGen/CGExpr.cpp
  cfe/trunk/lib/CodeGen/CodeGenFunction.h


Index: cfe/trunk/lib/CodeGen/CGClass.cpp
===
--- cfe/trunk/lib/CodeGen/CGClass.cpp
+++ cfe/trunk/lib/CodeGen/CGClass.cpp
@@ -129,13 +129,16 @@
 CodeGenFunction::EmitCXXMemberDataPointerAddress(const Expr *E, Address base,
  llvm::Value *memberPtr,
   const MemberPointerType *memberPtrType,
- LValueBaseInfo *BaseInfo) {
+ LValueBaseInfo *BaseInfo,
+ TBAAAccessInfo *TBAAInfo) {
   // Ask the ABI to compute the actual address.
   llvm::Value *ptr =
 CGM.getCXXABI().EmitMemberDataPointerAddress(*this, E, base,
  memberPtr, memberPtrType);
 
   QualType memberType = memberPtrType->getPointeeType();
+  if (TBAAInfo)
+*TBAAInfo = CGM.getTBAAAccessInfo(memberType);
   CharUnits memberAlign = getNaturalTypeAlignment(memberType, BaseInfo);
   memberAlign =
 CGM.getDynamicOffsetAlignment(base.getAlignment(),
Index: cfe/trunk/lib/CodeGen/CodeGenFunction.h
===
--- cfe/trunk/lib/CodeGen/CodeGenFunction.h
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.h
@@ -3327,7 +3327,8 @@
   Address EmitCXXMemberDataPointerAddress(const Expr *E, Address base,
   llvm::Value *memberPtr,
   const MemberPointerType 
*memberPtrType,
-  LValueBaseInfo *BaseInfo = nullptr);
+  LValueBaseInfo *BaseInfo = nullptr,
+  TBAAAccessInfo *TBAAInfo = nullptr);
   RValue EmitCXXMemberPointerCallExpr(const CXXMemberCallExpr *E,
   ReturnValueSlot ReturnValue);
 
Index: cfe/trunk/lib/CodeGen/CGExpr.cpp
===
--- cfe/trunk/lib/CodeGen/CGExpr.cpp
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp
@@ -4566,11 +4566,12 @@
 = E->getRHS()->getType()->getAs();
 
   LValueBaseInfo BaseInfo;
+  TBAAAccessInfo TBAAInfo;
   Address MemberAddr =
-EmitCXXMemberDataPointerAddress(E, BaseAddr, OffsetV, MPT, &BaseInfo);
+EmitCXXMemberDataPointerAddress(E, BaseAddr, OffsetV, MPT, &BaseInfo,
+&TBAAInfo);
 
-  return MakeAddrLValue(MemberAddr, MPT->getPointeeType(), BaseInfo,
-CGM.getTBAAAccessInfo(MPT->getPointeeType()));
+  return MakeAddrLValue(MemberAddr, MPT->getPointeeType(), BaseInfo, TBAAInfo);
 }
 
 /// Given the address of a temporary variable, produce an r-value of


Index: cfe/trunk/lib/CodeGen/CGClass.cpp
===
--- cfe/trunk/lib/CodeGen/CGClass.cpp
+++ cfe/trunk/lib/CodeGen/CGClass.cpp
@@ -129,13 +129,16 @@
 CodeGenFunction::EmitCXXMemberDataPointerAddress(const Expr *E, Address base,
  llvm::Value *memberPtr,
   const MemberPointerType *memberPtrType,
- LValueBaseInfo *BaseInfo) {
+ LValueBaseInfo *BaseInfo,
+ TBAAAccessInfo *TBAAInfo) {
   // Ask the ABI to compute the actual address.
   llvm::Value *ptr =
 CGM.getCXXABI().EmitMemberDataPointerAddress(*this, E, base,
  memberPtr, memberPtrType);
 
   QualType memberType = memberPtrType->getPointeeType();
+  if (TBAAInfo)
+*TBAAInfo = CGM.getTBAAAccessInfo(memberType);
   CharUnits memberAlign = getNaturalTypeAlignment(memberType, BaseInfo);
   memberAlign =
 CGM.getDynamicOffsetAlignment(base.getAlignment(),
Index: cfe/trunk/lib/CodeGen/CodeGenFunction.h
===
--- cfe/trunk/lib/CodeGen/CodeGenFunction.h
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.h
@@ -3327,7 +3327,8 @@
   Address EmitCXXMemberDataPointerAddress(const Expr *E, Address base,
   llvm::Value *memberPtr,
   const MemberPointerType *memberPtrType,
-  LValueBaseInfo *BaseInfo = nullptr);
+

[PATCH] D38816: Convert clang::LangAS to a strongly typed enum

2017-10-13 Thread Alexander Richardson via Phabricator via cfe-commits
arichardson added inline comments.



Comment at: lib/AST/TypePrinter.cpp:1323
 OS << "address_space(";
-OS << T->getEquivalentType().getAddressSpace();
+OS << T->getEquivalentType()
+  .getQualifiers()

arichardson wrote:
> Anastasia wrote:
> > arichardson wrote:
> > > Anastasia wrote:
> > > > arichardson wrote:
> > > > > Anastasia wrote:
> > > > > > Why do we need this change?
> > > > > `__attribute__((address_space(n)))` is a target address space and not 
> > > > > a language address space like `LangAS::opencl_generic`. Isn't 
> > > > > `Qualifiers::getAddressSpaceAttributePrintValue()` meant exactly for 
> > > > > this use case?
> > > > Yes, I think there are some adjustment we do in this method to get the 
> > > > original source value to be printed corerctly. Does this mean we have 
> > > > no tests that caught this issue?
> > > Seems like it, all tests pass both with and without this patch.
> > Strange considering that we have this attribute printed in some error 
> > messages of some Sema tests. If I compile this code without your patch:
> >  
> > ```
> > typedef int __attribute__((address_space(1))) int_1;
> > typedef int __attribute__((address_space(2))) int_2;
> > 
> > void f0(int_1 &); 
> > void f0(const int_1 &);
> > 
> > void test_f0() {
> >   int i;
> >   static int_2 i2;
> >   f0(i);
> >   f0(i2);
> > }
> > ```
> > 
> > I get the address spaces printed correctly inside the type:
> >   note: candidate function not viable: 1st argument ('int_2' (aka 
> > '__attribute__((address_space(2))) int')) is in address space 2, but 
> > parameter must be in address space 1
> > 
> > Perhaps @yaxunl could comment further on whether this change is needed.
> My guess is that it doesn't go through that switch statement but rather 
> through `Qualifiers::print()`. I'll try adding a llvm_unreachable() to see if 
> there are any tests that go down this path.
I just ran the clang tests with an llvm_unreachable() here and none of them 
failed. So it seems like we don't have anything testing this code path.


https://reviews.llvm.org/D38816



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r315704 - [CodeGen] EmitLoadOfPointer() to generate TBAA info along with LValue base info

2017-10-13 Thread Ivan A. Kosarev via cfe-commits
Author: kosarev
Date: Fri Oct 13 09:47:22 2017
New Revision: 315704

URL: http://llvm.org/viewvc/llvm-project?rev=315704&view=rev
Log:
[CodeGen] EmitLoadOfPointer() to generate TBAA info along with LValue base info

This patch should not bring in any functional changes.

Differential Revision: https://reviews.llvm.org/D38791

Modified:
cfe/trunk/lib/CodeGen/CGExpr.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h

Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=315704&r1=315703&r2=315704&view=diff
==
--- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp Fri Oct 13 09:47:22 2017
@@ -2165,7 +2165,11 @@ LValue CodeGenFunction::EmitLoadOfRefere
 
 Address CodeGenFunction::EmitLoadOfPointer(Address Ptr,
const PointerType *PtrTy,
-   LValueBaseInfo *BaseInfo) {
+   LValueBaseInfo *BaseInfo,
+   TBAAAccessInfo *TBAAInfo) {
+  if (TBAAInfo)
+*TBAAInfo = CGM.getTBAAAccessInfo(PtrTy->getPointeeType());
+
   llvm::Value *Addr = Builder.CreateLoad(Ptr);
   return Address(Addr, getNaturalTypeAlignment(PtrTy->getPointeeType(),
BaseInfo,
@@ -2175,9 +2179,9 @@ Address CodeGenFunction::EmitLoadOfPoint
 LValue CodeGenFunction::EmitLoadOfPointerLValue(Address PtrAddr,
 const PointerType *PtrTy) {
   LValueBaseInfo BaseInfo;
-  Address Addr = EmitLoadOfPointer(PtrAddr, PtrTy, &BaseInfo);
-  return MakeAddrLValue(Addr, PtrTy->getPointeeType(), BaseInfo,
-CGM.getTBAAAccessInfo(PtrTy->getPointeeType()));
+  TBAAAccessInfo TBAAInfo;
+  Address Addr = EmitLoadOfPointer(PtrAddr, PtrTy, &BaseInfo, &TBAAInfo);
+  return MakeAddrLValue(Addr, PtrTy->getPointeeType(), BaseInfo, TBAAInfo);
 }
 
 static LValue EmitGlobalVarDeclLValue(CodeGenFunction &CGF,

Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.h?rev=315704&r1=315703&r2=315704&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenFunction.h (original)
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.h Fri Oct 13 09:47:22 2017
@@ -1948,7 +1948,8 @@ public:
   LValue EmitLoadOfReferenceLValue(Address Ref, const ReferenceType *RefTy);
 
   Address EmitLoadOfPointer(Address Ptr, const PointerType *PtrTy,
-LValueBaseInfo *BaseInfo = nullptr);
+LValueBaseInfo *BaseInfo = nullptr,
+TBAAAccessInfo *TBAAInfo = nullptr);
   LValue EmitLoadOfPointerLValue(Address Ptr, const PointerType *PtrTy);
 
   /// CreateTempAlloca - This creates an alloca and inserts it into the entry


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D38791: [CodeGen] EmitLoadOfPointer() to generate TBAA info along with LValue base info

2017-10-13 Thread Ivan Kosarev via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL315704: [CodeGen] EmitLoadOfPointer() to generate TBAA info 
along with LValue base info (authored by kosarev).

Changed prior to commit:
  https://reviews.llvm.org/D38791?vs=118575&id=118932#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D38791

Files:
  cfe/trunk/lib/CodeGen/CGExpr.cpp
  cfe/trunk/lib/CodeGen/CodeGenFunction.h


Index: cfe/trunk/lib/CodeGen/CGExpr.cpp
===
--- cfe/trunk/lib/CodeGen/CGExpr.cpp
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp
@@ -2165,7 +2165,11 @@
 
 Address CodeGenFunction::EmitLoadOfPointer(Address Ptr,
const PointerType *PtrTy,
-   LValueBaseInfo *BaseInfo) {
+   LValueBaseInfo *BaseInfo,
+   TBAAAccessInfo *TBAAInfo) {
+  if (TBAAInfo)
+*TBAAInfo = CGM.getTBAAAccessInfo(PtrTy->getPointeeType());
+
   llvm::Value *Addr = Builder.CreateLoad(Ptr);
   return Address(Addr, getNaturalTypeAlignment(PtrTy->getPointeeType(),
BaseInfo,
@@ -2175,9 +2179,9 @@
 LValue CodeGenFunction::EmitLoadOfPointerLValue(Address PtrAddr,
 const PointerType *PtrTy) {
   LValueBaseInfo BaseInfo;
-  Address Addr = EmitLoadOfPointer(PtrAddr, PtrTy, &BaseInfo);
-  return MakeAddrLValue(Addr, PtrTy->getPointeeType(), BaseInfo,
-CGM.getTBAAAccessInfo(PtrTy->getPointeeType()));
+  TBAAAccessInfo TBAAInfo;
+  Address Addr = EmitLoadOfPointer(PtrAddr, PtrTy, &BaseInfo, &TBAAInfo);
+  return MakeAddrLValue(Addr, PtrTy->getPointeeType(), BaseInfo, TBAAInfo);
 }
 
 static LValue EmitGlobalVarDeclLValue(CodeGenFunction &CGF,
Index: cfe/trunk/lib/CodeGen/CodeGenFunction.h
===
--- cfe/trunk/lib/CodeGen/CodeGenFunction.h
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.h
@@ -1948,7 +1948,8 @@
   LValue EmitLoadOfReferenceLValue(Address Ref, const ReferenceType *RefTy);
 
   Address EmitLoadOfPointer(Address Ptr, const PointerType *PtrTy,
-LValueBaseInfo *BaseInfo = nullptr);
+LValueBaseInfo *BaseInfo = nullptr,
+TBAAAccessInfo *TBAAInfo = nullptr);
   LValue EmitLoadOfPointerLValue(Address Ptr, const PointerType *PtrTy);
 
   /// CreateTempAlloca - This creates an alloca and inserts it into the entry


Index: cfe/trunk/lib/CodeGen/CGExpr.cpp
===
--- cfe/trunk/lib/CodeGen/CGExpr.cpp
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp
@@ -2165,7 +2165,11 @@
 
 Address CodeGenFunction::EmitLoadOfPointer(Address Ptr,
const PointerType *PtrTy,
-   LValueBaseInfo *BaseInfo) {
+   LValueBaseInfo *BaseInfo,
+   TBAAAccessInfo *TBAAInfo) {
+  if (TBAAInfo)
+*TBAAInfo = CGM.getTBAAAccessInfo(PtrTy->getPointeeType());
+
   llvm::Value *Addr = Builder.CreateLoad(Ptr);
   return Address(Addr, getNaturalTypeAlignment(PtrTy->getPointeeType(),
BaseInfo,
@@ -2175,9 +2179,9 @@
 LValue CodeGenFunction::EmitLoadOfPointerLValue(Address PtrAddr,
 const PointerType *PtrTy) {
   LValueBaseInfo BaseInfo;
-  Address Addr = EmitLoadOfPointer(PtrAddr, PtrTy, &BaseInfo);
-  return MakeAddrLValue(Addr, PtrTy->getPointeeType(), BaseInfo,
-CGM.getTBAAAccessInfo(PtrTy->getPointeeType()));
+  TBAAAccessInfo TBAAInfo;
+  Address Addr = EmitLoadOfPointer(PtrAddr, PtrTy, &BaseInfo, &TBAAInfo);
+  return MakeAddrLValue(Addr, PtrTy->getPointeeType(), BaseInfo, TBAAInfo);
 }
 
 static LValue EmitGlobalVarDeclLValue(CodeGenFunction &CGF,
Index: cfe/trunk/lib/CodeGen/CodeGenFunction.h
===
--- cfe/trunk/lib/CodeGen/CodeGenFunction.h
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.h
@@ -1948,7 +1948,8 @@
   LValue EmitLoadOfReferenceLValue(Address Ref, const ReferenceType *RefTy);
 
   Address EmitLoadOfPointer(Address Ptr, const PointerType *PtrTy,
-LValueBaseInfo *BaseInfo = nullptr);
+LValueBaseInfo *BaseInfo = nullptr,
+TBAAAccessInfo *TBAAInfo = nullptr);
   LValue EmitLoadOfPointerLValue(Address Ptr, const PointerType *PtrTy);
 
   /// CreateTempAlloca - This creates an alloca and inserts it into the entry
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r315705 - [CodeGen] EmitLoadOfReference() to generate TBAA info along with LValue base info

2017-10-13 Thread Ivan A. Kosarev via cfe-commits
Author: kosarev
Date: Fri Oct 13 09:50:50 2017
New Revision: 315705

URL: http://llvm.org/viewvc/llvm-project?rev=315705&view=rev
Log:
[CodeGen] EmitLoadOfReference() to generate TBAA info along with LValue base 
info

This patch should not bring in any functional changes.

Differential Revision: https://reviews.llvm.org/D38793

Modified:
cfe/trunk/lib/CodeGen/CGExpr.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h

Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=315705&r1=315704&r2=315705&view=diff
==
--- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp Fri Oct 13 09:50:50 2017
@@ -2149,7 +2149,11 @@ static LValue EmitThreadPrivateVarDeclLV
 
 Address CodeGenFunction::EmitLoadOfReference(Address Addr,
  const ReferenceType *RefTy,
- LValueBaseInfo *BaseInfo) {
+ LValueBaseInfo *BaseInfo,
+ TBAAAccessInfo *TBAAInfo) {
+  if (TBAAInfo)
+*TBAAInfo = CGM.getTBAAAccessInfo(RefTy->getPointeeType());
+
   llvm::Value *Ptr = Builder.CreateLoad(Addr);
   return Address(Ptr, getNaturalTypeAlignment(RefTy->getPointeeType(),
   BaseInfo, /*forPointee*/ true));
@@ -2158,9 +2162,9 @@ Address CodeGenFunction::EmitLoadOfRefer
 LValue CodeGenFunction::EmitLoadOfReferenceLValue(Address RefAddr,
   const ReferenceType *RefTy) {
   LValueBaseInfo BaseInfo;
-  Address Addr = EmitLoadOfReference(RefAddr, RefTy, &BaseInfo);
-  return MakeAddrLValue(Addr, RefTy->getPointeeType(), BaseInfo,
-CGM.getTBAAAccessInfo(RefTy->getPointeeType()));
+  TBAAAccessInfo TBAAInfo;
+  Address Addr = EmitLoadOfReference(RefAddr, RefTy, &BaseInfo, &TBAAInfo);
+  return MakeAddrLValue(Addr, RefTy->getPointeeType(), BaseInfo, TBAAInfo);
 }
 
 Address CodeGenFunction::EmitLoadOfPointer(Address Ptr,

Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.h?rev=315705&r1=315704&r2=315705&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenFunction.h (original)
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.h Fri Oct 13 09:50:50 2017
@@ -1944,7 +1944,8 @@ public:
LValueBaseInfo *BaseInfo = nullptr);
 
   Address EmitLoadOfReference(Address Ref, const ReferenceType *RefTy,
-  LValueBaseInfo *BaseInfo = nullptr);
+  LValueBaseInfo *BaseInfo = nullptr,
+  TBAAAccessInfo *TBAAInfo = nullptr);
   LValue EmitLoadOfReferenceLValue(Address Ref, const ReferenceType *RefTy);
 
   Address EmitLoadOfPointer(Address Ptr, const PointerType *PtrTy,


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D38793: [CodeGen] EmitLoadOfReference() to generate TBAA info along with LValue base info

2017-10-13 Thread Ivan Kosarev via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL315705: [CodeGen] EmitLoadOfReference() to generate TBAA 
info along with LValue base… (authored by kosarev).

Changed prior to commit:
  https://reviews.llvm.org/D38793?vs=118578&id=118933#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D38793

Files:
  cfe/trunk/lib/CodeGen/CGExpr.cpp
  cfe/trunk/lib/CodeGen/CodeGenFunction.h


Index: cfe/trunk/lib/CodeGen/CGExpr.cpp
===
--- cfe/trunk/lib/CodeGen/CGExpr.cpp
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp
@@ -2149,18 +2149,22 @@
 
 Address CodeGenFunction::EmitLoadOfReference(Address Addr,
  const ReferenceType *RefTy,
- LValueBaseInfo *BaseInfo) {
+ LValueBaseInfo *BaseInfo,
+ TBAAAccessInfo *TBAAInfo) {
+  if (TBAAInfo)
+*TBAAInfo = CGM.getTBAAAccessInfo(RefTy->getPointeeType());
+
   llvm::Value *Ptr = Builder.CreateLoad(Addr);
   return Address(Ptr, getNaturalTypeAlignment(RefTy->getPointeeType(),
   BaseInfo, /*forPointee*/ true));
 }
 
 LValue CodeGenFunction::EmitLoadOfReferenceLValue(Address RefAddr,
   const ReferenceType *RefTy) {
   LValueBaseInfo BaseInfo;
-  Address Addr = EmitLoadOfReference(RefAddr, RefTy, &BaseInfo);
-  return MakeAddrLValue(Addr, RefTy->getPointeeType(), BaseInfo,
-CGM.getTBAAAccessInfo(RefTy->getPointeeType()));
+  TBAAAccessInfo TBAAInfo;
+  Address Addr = EmitLoadOfReference(RefAddr, RefTy, &BaseInfo, &TBAAInfo);
+  return MakeAddrLValue(Addr, RefTy->getPointeeType(), BaseInfo, TBAAInfo);
 }
 
 Address CodeGenFunction::EmitLoadOfPointer(Address Ptr,
Index: cfe/trunk/lib/CodeGen/CodeGenFunction.h
===
--- cfe/trunk/lib/CodeGen/CodeGenFunction.h
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.h
@@ -1944,7 +1944,8 @@
LValueBaseInfo *BaseInfo = nullptr);
 
   Address EmitLoadOfReference(Address Ref, const ReferenceType *RefTy,
-  LValueBaseInfo *BaseInfo = nullptr);
+  LValueBaseInfo *BaseInfo = nullptr,
+  TBAAAccessInfo *TBAAInfo = nullptr);
   LValue EmitLoadOfReferenceLValue(Address Ref, const ReferenceType *RefTy);
 
   Address EmitLoadOfPointer(Address Ptr, const PointerType *PtrTy,


Index: cfe/trunk/lib/CodeGen/CGExpr.cpp
===
--- cfe/trunk/lib/CodeGen/CGExpr.cpp
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp
@@ -2149,18 +2149,22 @@
 
 Address CodeGenFunction::EmitLoadOfReference(Address Addr,
  const ReferenceType *RefTy,
- LValueBaseInfo *BaseInfo) {
+ LValueBaseInfo *BaseInfo,
+ TBAAAccessInfo *TBAAInfo) {
+  if (TBAAInfo)
+*TBAAInfo = CGM.getTBAAAccessInfo(RefTy->getPointeeType());
+
   llvm::Value *Ptr = Builder.CreateLoad(Addr);
   return Address(Ptr, getNaturalTypeAlignment(RefTy->getPointeeType(),
   BaseInfo, /*forPointee*/ true));
 }
 
 LValue CodeGenFunction::EmitLoadOfReferenceLValue(Address RefAddr,
   const ReferenceType *RefTy) {
   LValueBaseInfo BaseInfo;
-  Address Addr = EmitLoadOfReference(RefAddr, RefTy, &BaseInfo);
-  return MakeAddrLValue(Addr, RefTy->getPointeeType(), BaseInfo,
-CGM.getTBAAAccessInfo(RefTy->getPointeeType()));
+  TBAAAccessInfo TBAAInfo;
+  Address Addr = EmitLoadOfReference(RefAddr, RefTy, &BaseInfo, &TBAAInfo);
+  return MakeAddrLValue(Addr, RefTy->getPointeeType(), BaseInfo, TBAAInfo);
 }
 
 Address CodeGenFunction::EmitLoadOfPointer(Address Ptr,
Index: cfe/trunk/lib/CodeGen/CodeGenFunction.h
===
--- cfe/trunk/lib/CodeGen/CodeGenFunction.h
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.h
@@ -1944,7 +1944,8 @@
LValueBaseInfo *BaseInfo = nullptr);
 
   Address EmitLoadOfReference(Address Ref, const ReferenceType *RefTy,
-  LValueBaseInfo *BaseInfo = nullptr);
+  LValueBaseInfo *BaseInfo = nullptr,
+  TBAAAccessInfo *TBAAInfo = nullptr);
   LValue EmitLoadOfReferenceLValue(Address Ref, const ReferenceType *RefTy);
 
   Address EmitLoadOfPointer(Address Ptr, const PointerType *PtrTy,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cf

r315707 - [SEH] Use the SEH personality on frontend-outlined funclets

2017-10-13 Thread Reid Kleckner via cfe-commits
Author: rnk
Date: Fri Oct 13 09:55:14 2017
New Revision: 315707

URL: http://llvm.org/viewvc/llvm-project?rev=315707&view=rev
Log:
[SEH] Use the SEH personality on frontend-outlined funclets

This allows __try inside __finally to work.

Fixes PR34939

Modified:
cfe/trunk/lib/CodeGen/CGException.cpp
cfe/trunk/test/CodeGenCXX/exceptions-seh.cpp

Modified: cfe/trunk/lib/CodeGen/CGException.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGException.cpp?rev=315707&r1=315706&r2=315707&view=diff
==
--- cfe/trunk/lib/CodeGen/CGException.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGException.cpp Fri Oct 13 09:55:14 2017
@@ -225,7 +225,12 @@ const EHPersonality &EHPersonality::get(
 }
 
 const EHPersonality &EHPersonality::get(CodeGenFunction &CGF) {
-  return get(CGF.CGM, dyn_cast_or_null(CGF.CurCodeDecl));
+  const auto *FD = CGF.CurCodeDecl;
+  // For outlined finallys and filters, use the SEH personality in case they
+  // contain more SEH. This mostly only affects finallys. Filters could
+  // hypothetically use gnu statement expressions to sneak in nested SEH.
+  FD = FD ? FD : CGF.CurSEHParent;
+  return get(CGF.CGM, dyn_cast_or_null(FD));
 }
 
 static llvm::Constant *getPersonalityFn(CodeGenModule &CGM,

Modified: cfe/trunk/test/CodeGenCXX/exceptions-seh.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/exceptions-seh.cpp?rev=315707&r1=315706&r2=315707&view=diff
==
--- cfe/trunk/test/CodeGenCXX/exceptions-seh.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/exceptions-seh.cpp Fri Oct 13 09:55:14 2017
@@ -76,6 +76,27 @@ extern "C" void use_seh() {
 // CHECK: [[cont]]
 // CHECK: br label %[[ret]]
 
+extern "C" void nested_finally() {
+  __try {
+might_throw();
+  } __finally {
+__try {
+  might_throw();
+} __finally {
+}
+  }
+}
+
+// CHECK-LABEL: define void @nested_finally() #{{[0-9]+}}
+// CHECK-SAME:  personality i8* bitcast (i32 (...)* @__C_specific_handler to 
i8*)
+// CHECK: invoke void @might_throw()
+// CHECK: call void @"\01?fin$0@0@nested_finally@@"(i8 1, i8* {{.*}})
+
+// CHECK-LABEL: define internal void @"\01?fin$0@0@nested_finally@@"
+// CHECK-SAME:  personality i8* bitcast (i32 (...)* @__C_specific_handler to 
i8*)
+// CHECK: invoke void @might_throw()
+// CHECK: call void @"\01?fin$1@0@nested_finally@@"(i8 1, i8* {{.*}})
+
 void use_seh_in_lambda() {
   ([]() {
 __try {


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r315708 - [CodeGen] getNaturalTypeAlignment() to generate TBAA info along with LValue base info

2017-10-13 Thread Ivan A. Kosarev via cfe-commits
Author: kosarev
Date: Fri Oct 13 09:58:30 2017
New Revision: 315708

URL: http://llvm.org/viewvc/llvm-project?rev=315708&view=rev
Log:
[CodeGen] getNaturalTypeAlignment() to generate TBAA info along with LValue 
base info

This patch should not bring in any functional changes.

Differential Revision: https://reviews.llvm.org/D38794

Modified:
cfe/trunk/lib/CodeGen/CGClass.cpp
cfe/trunk/lib/CodeGen/CGExpr.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h

Modified: cfe/trunk/lib/CodeGen/CGClass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGClass.cpp?rev=315708&r1=315707&r2=315708&view=diff
==
--- cfe/trunk/lib/CodeGen/CGClass.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGClass.cpp Fri Oct 13 09:58:30 2017
@@ -137,9 +137,8 @@ CodeGenFunction::EmitCXXMemberDataPointe
  memberPtr, memberPtrType);
 
   QualType memberType = memberPtrType->getPointeeType();
-  if (TBAAInfo)
-*TBAAInfo = CGM.getTBAAAccessInfo(memberType);
-  CharUnits memberAlign = getNaturalTypeAlignment(memberType, BaseInfo);
+  CharUnits memberAlign = getNaturalTypeAlignment(memberType, BaseInfo,
+  TBAAInfo);
   memberAlign =
 CGM.getDynamicOffsetAlignment(base.getAlignment(),
 memberPtrType->getClass()->getAsCXXRecordDecl(),

Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=315708&r1=315707&r2=315708&view=diff
==
--- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp Fri Oct 13 09:58:30 2017
@@ -2151,12 +2151,10 @@ Address CodeGenFunction::EmitLoadOfRefer
  const ReferenceType *RefTy,
  LValueBaseInfo *BaseInfo,
  TBAAAccessInfo *TBAAInfo) {
-  if (TBAAInfo)
-*TBAAInfo = CGM.getTBAAAccessInfo(RefTy->getPointeeType());
-
   llvm::Value *Ptr = Builder.CreateLoad(Addr);
   return Address(Ptr, getNaturalTypeAlignment(RefTy->getPointeeType(),
-  BaseInfo, /*forPointee*/ true));
+  BaseInfo, TBAAInfo,
+  /* forPointeeType= */ true));
 }
 
 LValue CodeGenFunction::EmitLoadOfReferenceLValue(Address RefAddr,
@@ -2171,12 +2169,9 @@ Address CodeGenFunction::EmitLoadOfPoint
const PointerType *PtrTy,
LValueBaseInfo *BaseInfo,
TBAAAccessInfo *TBAAInfo) {
-  if (TBAAInfo)
-*TBAAInfo = CGM.getTBAAAccessInfo(PtrTy->getPointeeType());
-
   llvm::Value *Addr = Builder.CreateLoad(Ptr);
   return Address(Addr, getNaturalTypeAlignment(PtrTy->getPointeeType(),
-   BaseInfo,
+   BaseInfo, TBAAInfo,
/*forPointeeType=*/true));
 }
 
@@ -2315,8 +2310,10 @@ LValue CodeGenFunction::EmitDeclRefLValu
   // FIXME: Eventually we will want to emit vector element references.
 
   // Should we be using the alignment of the constant pointer we emitted?
-  CharUnits Alignment = getNaturalTypeAlignment(E->getType(), nullptr,
-/*pointee*/ true);
+  CharUnits Alignment = getNaturalTypeAlignment(E->getType(),
+/* BaseInfo= */ nullptr,
+/* TBAAInfo= */ nullptr,
+/* forPointeeType= */ 
true);
   return MakeAddrLValue(Address(Val, Alignment), T, AlignmentSource::Decl);
 }
 
@@ -3729,7 +3726,8 @@ LValue CodeGenFunction::EmitLValueForFie
   type = refType->getPointeeType();
 
   CharUnits alignment =
-getNaturalTypeAlignment(type, &FieldBaseInfo, /*pointee*/ true);
+getNaturalTypeAlignment(type, &FieldBaseInfo, /* TBAAInfo= */ nullptr,
+/* forPointeeType= */ true);
   FieldBaseInfo.setMayAlias(false);
   addr = Address(load, alignment);
 

Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=315708&r1=315707&r2=315708&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Fri Oct 13 09:58:30 2017
@@ -120,12 +120,17 @@ CodeGenFunction::~CodeGenFunction() {
 CharUnits CodeGenFunction::getNaturalPoint

[PATCH] D38794: [CodeGen] getNaturalTypeAlignment() to generate TBAA info along with LValue base info

2017-10-13 Thread Ivan Kosarev via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL315708: [CodeGen] getNaturalTypeAlignment() to generate TBAA 
info along with LValue… (authored by kosarev).

Changed prior to commit:
  https://reviews.llvm.org/D38794?vs=118581&id=118934#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D38794

Files:
  cfe/trunk/lib/CodeGen/CGClass.cpp
  cfe/trunk/lib/CodeGen/CGExpr.cpp
  cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
  cfe/trunk/lib/CodeGen/CodeGenFunction.h

Index: cfe/trunk/lib/CodeGen/CodeGenFunction.h
===
--- cfe/trunk/lib/CodeGen/CodeGenFunction.h
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.h
@@ -1939,6 +1939,7 @@
   LValue MakeNaturalAlignAddrLValue(llvm::Value *V, QualType T);
   CharUnits getNaturalTypeAlignment(QualType T,
 LValueBaseInfo *BaseInfo = nullptr,
+TBAAAccessInfo *TBAAInfo = nullptr,
 bool forPointeeType = false);
   CharUnits getNaturalPointeeTypeAlignment(QualType T,
LValueBaseInfo *BaseInfo = nullptr);
Index: cfe/trunk/lib/CodeGen/CGExpr.cpp
===
--- cfe/trunk/lib/CodeGen/CGExpr.cpp
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp
@@ -2151,12 +2151,10 @@
  const ReferenceType *RefTy,
  LValueBaseInfo *BaseInfo,
  TBAAAccessInfo *TBAAInfo) {
-  if (TBAAInfo)
-*TBAAInfo = CGM.getTBAAAccessInfo(RefTy->getPointeeType());
-
   llvm::Value *Ptr = Builder.CreateLoad(Addr);
   return Address(Ptr, getNaturalTypeAlignment(RefTy->getPointeeType(),
-  BaseInfo, /*forPointee*/ true));
+  BaseInfo, TBAAInfo,
+  /* forPointeeType= */ true));
 }
 
 LValue CodeGenFunction::EmitLoadOfReferenceLValue(Address RefAddr,
@@ -2171,12 +2169,9 @@
const PointerType *PtrTy,
LValueBaseInfo *BaseInfo,
TBAAAccessInfo *TBAAInfo) {
-  if (TBAAInfo)
-*TBAAInfo = CGM.getTBAAAccessInfo(PtrTy->getPointeeType());
-
   llvm::Value *Addr = Builder.CreateLoad(Ptr);
   return Address(Addr, getNaturalTypeAlignment(PtrTy->getPointeeType(),
-   BaseInfo,
+   BaseInfo, TBAAInfo,
/*forPointeeType=*/true));
 }
 
@@ -2315,8 +2310,10 @@
   // FIXME: Eventually we will want to emit vector element references.
 
   // Should we be using the alignment of the constant pointer we emitted?
-  CharUnits Alignment = getNaturalTypeAlignment(E->getType(), nullptr,
-/*pointee*/ true);
+  CharUnits Alignment = getNaturalTypeAlignment(E->getType(),
+/* BaseInfo= */ nullptr,
+/* TBAAInfo= */ nullptr,
+/* forPointeeType= */ true);
   return MakeAddrLValue(Address(Val, Alignment), T, AlignmentSource::Decl);
 }
 
@@ -3729,7 +3726,8 @@
   type = refType->getPointeeType();
 
   CharUnits alignment =
-getNaturalTypeAlignment(type, &FieldBaseInfo, /*pointee*/ true);
+getNaturalTypeAlignment(type, &FieldBaseInfo, /* TBAAInfo= */ nullptr,
+/* forPointeeType= */ true);
   FieldBaseInfo.setMayAlias(false);
   addr = Address(load, alignment);
 
Index: cfe/trunk/lib/CodeGen/CGClass.cpp
===
--- cfe/trunk/lib/CodeGen/CGClass.cpp
+++ cfe/trunk/lib/CodeGen/CGClass.cpp
@@ -137,9 +137,8 @@
  memberPtr, memberPtrType);
 
   QualType memberType = memberPtrType->getPointeeType();
-  if (TBAAInfo)
-*TBAAInfo = CGM.getTBAAAccessInfo(memberType);
-  CharUnits memberAlign = getNaturalTypeAlignment(memberType, BaseInfo);
+  CharUnits memberAlign = getNaturalTypeAlignment(memberType, BaseInfo,
+  TBAAInfo);
   memberAlign =
 CGM.getDynamicOffsetAlignment(base.getAlignment(),
 memberPtrType->getClass()->getAsCXXRecordDecl(),
Index: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
===
--- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
@@ -120,12 +120,17 @@
 CharUnits CodeGenFunction::getNaturalPointeeTypeAlignment(QualType T,
   

r315712 - Revert "[lit] Raise the logic for enabling clang & lld substitutions to llvm."

2017-10-13 Thread Bruno Cardoso Lopes via cfe-commits
Author: bruno
Date: Fri Oct 13 10:11:13 2017
New Revision: 315712

URL: http://llvm.org/viewvc/llvm-project?rev=315712&view=rev
Log:
Revert "[lit] Raise the logic for enabling clang & lld substitutions to llvm."

This reverts commit r315627, fixing bot failures:
http://green.lab.llvm.org/green/job/clang-stage1-configure-RA

LIT is failing to properly apply substitution to debuginfo-tests
after this change.

rdar://problem/34979568

Modified:
cfe/trunk/test/lit.cfg.py

Modified: cfe/trunk/test/lit.cfg.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/lit.cfg.py?rev=315712&r1=315711&r2=315712&view=diff
==
--- cfe/trunk/test/lit.cfg.py (original)
+++ cfe/trunk/test/lit.cfg.py Fri Oct 13 10:11:13 2017
@@ -39,33 +39,71 @@ config.test_source_root = os.path.dirnam
 # test_exec_root: The root path where tests should be run.
 config.test_exec_root = os.path.join(config.clang_obj_root, 'test')
 
-llvm_config.use_default_substitutions()
+# Clear some environment variables that might affect Clang.
+#
+# This first set of vars are read by Clang, but shouldn't affect tests
+# that aren't specifically looking for these features, or are required
+# simply to run the tests at all.
+#
+# FIXME: Should we have a tool that enforces this?
 
-llvm_config.use_clang()
+# safe_env_vars = ('TMPDIR', 'TEMP', 'TMP', 'USERPROFILE', 'PWD',
+#  'MACOSX_DEPLOYMENT_TARGET', 'IPHONEOS_DEPLOYMENT_TARGET',
+#  'VCINSTALLDIR', 'VC100COMNTOOLS', 'VC90COMNTOOLS',
+#  'VC80COMNTOOLS')
+possibly_dangerous_env_vars = ['COMPILER_PATH', 'RC_DEBUG_OPTIONS',
+   'CINDEXTEST_PREAMBLE_FILE', 'LIBRARY_PATH',
+   'CPATH', 'C_INCLUDE_PATH', 'CPLUS_INCLUDE_PATH',
+   'OBJC_INCLUDE_PATH', 'OBJCPLUS_INCLUDE_PATH',
+   'LIBCLANG_TIMING', 'LIBCLANG_OBJTRACKING',
+   'LIBCLANG_LOGGING', 'LIBCLANG_BGPRIO_INDEX',
+   'LIBCLANG_BGPRIO_EDIT', 'LIBCLANG_NOTHREADS',
+   'LIBCLANG_RESOURCE_USAGE',
+   'LIBCLANG_CODE_COMPLETION_LOGGING']
+# Clang/Win32 may refer to %INCLUDE%. vsvarsall.bat sets it.
+if platform.system() != 'Windows':
+possibly_dangerous_env_vars.append('INCLUDE')
+
+llvm_config.clear_environment(possibly_dangerous_env_vars)
+
+# Tweak the PATH to include the tools dir and the scripts dir.
+llvm_config.with_environment(
+'PATH', [config.llvm_tools_dir, config.clang_tools_dir], append_path=True)
+
+llvm_config.with_environment('LD_LIBRARY_PATH', [
+ config.llvm_shlib_dir, config.llvm_libs_dir], 
append_path=True)
 
 # Propagate path to symbolizer for ASan/MSan.
 llvm_config.with_system_environment(
 ['ASAN_SYMBOLIZER_PATH', 'MSAN_SYMBOLIZER_PATH'])
 
-config.substitutions.append(('%PATH%', config.environment['PATH']))
+llvm_config.use_default_substitutions()
 
+# Discover the 'clang' and 'clangcc' to use.
 
-# For each occurrence of a clang tool name, replace it with the full path to
-# the build directory holding that tool.  We explicitly specify the directories
-# to search to ensure that we get the tools just built and not some random
-# tools that might happen to be in the user's PATH.
-tool_dirs = [config.clang_tools_dir, config.llvm_tools_dir]
 
-tools = [
-'c-index-test', 'clang-check', 'clang-diff', 'clang-format', 'opt',
-ToolSubst('%clang_func_map', command=FindTool(
-'clang-func-mapping'), unresolved='ignore'),
-]
+def inferClang(PATH):
+# Determine which clang to use.
+clang = os.getenv('CLANG')
 
-if config.clang_examples:
-tools.append('clang-interpreter')
+# If the user set clang in the environment, definitely use that and don't
+# try to validate.
+if clang:
+return clang
 
-llvm_config.add_tool_substitutions(tools, tool_dirs)
+# Otherwise look in the path.
+clang = lit.util.which('clang', PATH)
+
+if not clang:
+lit_config.fatal("couldn't find 'clang' program, try setting "
+ 'CLANG in your environment')
+
+return clang
+
+
+config.clang = inferClang(config.environment['PATH']).replace('\\', '/')
+if not lit_config.quiet:
+lit_config.note('using clang: %r' % config.clang)
 
 # Plugins (loadable modules)
 # TODO: This should be supplied by Makefile or autoconf.
@@ -77,6 +115,87 @@ else:
 if has_plugins and config.llvm_plugin_ext:
 config.available_features.add('plugins')
 
+config.substitutions.append(('%llvmshlibdir', config.llvm_shlib_dir))
+config.substitutions.append(('%pluginext', config.llvm_plugin_ext))
+config.substitutions.append(('%PATH%', config.environment['PATH']))
+
+if config.clang_examples:
+config.available_features.add('examples')
+
+builtin_include_dir = llvm_config.get_clang_builtin_include_dir(config.clang)
+
+tools = 

[PATCH] D38794: [CodeGen] getNaturalTypeAlignment() to generate TBAA info along with LValue base info

2017-10-13 Thread Ivan Kosarev via Phabricator via cfe-commits
kosarev added a comment.

Right. Switching back to returning just AlignmentSource would make its name 
more adequate, but I suspect we intentionally want full-size lvalue base info 
here.


Repository:
  rL LLVM

https://reviews.llvm.org/D38794



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: r315627 - [lit] Raise the logic for enabling clang & lld substitutions to llvm.

2017-10-13 Thread Bruno Cardoso Lopes via cfe-commits
Hi Zachary,

I reverted this in r315712, since it's making one of our bots red
since yesterday (more explanations in the commit):
http://green.lab.llvm.org/green/job/clang-stage1-configure-RA

Cheers,

On Thu, Oct 12, 2017 at 2:56 PM, Zachary Turner via cfe-commits
 wrote:
> Author: zturner
> Date: Thu Oct 12 14:56:05 2017
> New Revision: 315627
>
> URL: http://llvm.org/viewvc/llvm-project?rev=315627&view=rev
> Log:
> [lit] Raise the logic for enabling clang & lld substitutions to llvm.
>
> This paves the way for other projects which might /use/ clang or
> lld but not necessarily need to the full set of functionality
> available to clang and lld tests to be able to have a basic set
> of substitutions that allow a project to run the clang or lld
> executables.
>
> Modified:
> cfe/trunk/test/lit.cfg.py
>
> Modified: cfe/trunk/test/lit.cfg.py
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/lit.cfg.py?rev=315627&r1=315626&r2=315627&view=diff
> ==
> --- cfe/trunk/test/lit.cfg.py (original)
> +++ cfe/trunk/test/lit.cfg.py Thu Oct 12 14:56:05 2017
> @@ -39,162 +39,43 @@ config.test_source_root = os.path.dirnam
>  # test_exec_root: The root path where tests should be run.
>  config.test_exec_root = os.path.join(config.clang_obj_root, 'test')
>
> -# Clear some environment variables that might affect Clang.
> -#
> -# This first set of vars are read by Clang, but shouldn't affect tests
> -# that aren't specifically looking for these features, or are required
> -# simply to run the tests at all.
> -#
> -# FIXME: Should we have a tool that enforces this?
> -
> -# safe_env_vars = ('TMPDIR', 'TEMP', 'TMP', 'USERPROFILE', 'PWD',
> -#  'MACOSX_DEPLOYMENT_TARGET', 'IPHONEOS_DEPLOYMENT_TARGET',
> -#  'VCINSTALLDIR', 'VC100COMNTOOLS', 'VC90COMNTOOLS',
> -#  'VC80COMNTOOLS')
> -possibly_dangerous_env_vars = ['COMPILER_PATH', 'RC_DEBUG_OPTIONS',
> -   'CINDEXTEST_PREAMBLE_FILE', 'LIBRARY_PATH',
> -   'CPATH', 'C_INCLUDE_PATH', 
> 'CPLUS_INCLUDE_PATH',
> -   'OBJC_INCLUDE_PATH', 'OBJCPLUS_INCLUDE_PATH',
> -   'LIBCLANG_TIMING', 'LIBCLANG_OBJTRACKING',
> -   'LIBCLANG_LOGGING', 'LIBCLANG_BGPRIO_INDEX',
> -   'LIBCLANG_BGPRIO_EDIT', 'LIBCLANG_NOTHREADS',
> -   'LIBCLANG_RESOURCE_USAGE',
> -   'LIBCLANG_CODE_COMPLETION_LOGGING']
> -# Clang/Win32 may refer to %INCLUDE%. vsvarsall.bat sets it.
> -if platform.system() != 'Windows':
> -possibly_dangerous_env_vars.append('INCLUDE')
> -
> -llvm_config.clear_environment(possibly_dangerous_env_vars)
> -
> -# Tweak the PATH to include the tools dir and the scripts dir.
> -llvm_config.with_environment(
> -'PATH', [config.llvm_tools_dir, config.clang_tools_dir], 
> append_path=True)
> +llvm_config.use_default_substitutions()
>
> -llvm_config.with_environment('LD_LIBRARY_PATH', [
> - config.llvm_shlib_dir, config.llvm_libs_dir], 
> append_path=True)
> +llvm_config.use_clang()
>
>  # Propagate path to symbolizer for ASan/MSan.
>  llvm_config.with_system_environment(
>  ['ASAN_SYMBOLIZER_PATH', 'MSAN_SYMBOLIZER_PATH'])
>
> -llvm_config.use_default_substitutions()
> -
> -# Discover the 'clang' and 'clangcc' to use.
> -
> -
> -def inferClang(PATH):
> -# Determine which clang to use.
> -clang = os.getenv('CLANG')
> -
> -# If the user set clang in the environment, definitely use that and don't
> -# try to validate.
> -if clang:
> -return clang
> -
> -# Otherwise look in the path.
> -clang = lit.util.which('clang', PATH)
> -
> -if not clang:
> -lit_config.fatal("couldn't find 'clang' program, try setting "
> - 'CLANG in your environment')
> -
> -return clang
> -
> -
> -config.clang = inferClang(config.environment['PATH']).replace('\\', '/')
> -if not lit_config.quiet:
> -lit_config.note('using clang: %r' % config.clang)
> -
> -# Plugins (loadable modules)
> -# TODO: This should be supplied by Makefile or autoconf.
> -if sys.platform in ['win32', 'cygwin']:
> -has_plugins = config.enable_shared
> -else:
> -has_plugins = True
> -
> -if has_plugins and config.llvm_plugin_ext:
> -config.available_features.add('plugins')
> -
> -config.substitutions.append(('%llvmshlibdir', config.llvm_shlib_dir))
> -config.substitutions.append(('%pluginext', config.llvm_plugin_ext))
>  config.substitutions.append(('%PATH%', config.environment['PATH']))
>
> -if config.clang_examples:
> -config.available_features.add('examples')
>
> -builtin_include_dir = llvm_config.get_clang_builtin_include_dir(config.clang)
> +# For each occurrence of a clang tool name, replace it with the full path to
> +# the build directory holding that tool. 

[PATCH] D38883: [CMake][OpenMP] Customize default offloading arch

2017-10-13 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld marked an inline comment as done.
Hahnfeld added inline comments.



Comment at: lib/Driver/ToolChains/Cuda.cpp:170-182
-// This code prevents IsValid from being set when
-// no libdevice has been found.
-bool allEmpty = true;
-std::string LibDeviceFile;
-for (auto key : LibDeviceMap.keys()) {
-  LibDeviceFile = LibDeviceMap.lookup(key);
-  if (!LibDeviceFile.empty())

tra wrote:
> I'd keep this code. It appears to serve useful purpose as it requires CUDA 
> installation to have at least some libdevice library in it.  It gives us a 
> change to find a valid installation, instead of ailing some time later when 
> we ask for a libdevice file and fail because there are none.
We had some internal discussions about this after I submitted the patch here.

The main question is: Do we want to support CUDA installations without 
libdevice and are there use cases for that? I'd say that the user should be 
able to use a toolchain without libdevice together with `-nocudalib`.



Comment at: lib/Driver/ToolChains/Cuda.cpp:540
   // Also append the compute capability.
   if (DeviceOffloadKind == Action::OFK_OpenMP) {
 for (Arg *A : Args){

This check guards the whole block.



Comment at: lib/Driver/ToolChains/Cuda.cpp:556
+  DAL->AddJoinedArg(nullptr, Opts.getOption(options::OPT_march_EQ),
+CLANG_OPENMP_NVPTX_DEFAULT_ARCH);
 }

tra wrote:
> This sets default GPU arch for *everyone* based on OPENMP requirements. 
> Perhaps this should be predicated on this being openmp compilation.
> 
> IMO to avoid unnecessary surprises, the default for CUDA compilation should 
> follow defaults of nvcc. sm_30 becomes default only in CUDA-9.
> 
This branch is only executed for OpenMP, see above


https://reviews.llvm.org/D38883



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r315715 - [CodeGen] emitOMPArraySectionBase() to generate TBAA info along with LValue base info

2017-10-13 Thread Ivan A. Kosarev via cfe-commits
Author: kosarev
Date: Fri Oct 13 10:34:18 2017
New Revision: 315715

URL: http://llvm.org/viewvc/llvm-project?rev=315715&view=rev
Log:
[CodeGen] emitOMPArraySectionBase() to generate TBAA info along with LValue 
base info

Differential Revision: https://reviews.llvm.org/D38795

Modified:
cfe/trunk/lib/CodeGen/CGExpr.cpp

Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=315715&r1=315714&r2=315715&view=diff
==
--- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp Fri Oct 13 10:34:18 2017
@@ -3318,8 +3318,11 @@ LValue CodeGenFunction::EmitArraySubscri
 
 static Address emitOMPArraySectionBase(CodeGenFunction &CGF, const Expr *Base,
LValueBaseInfo &BaseInfo,
+   TBAAAccessInfo &TBAAInfo,
QualType BaseTy, QualType ElTy,
bool IsLowerBound) {
+  TBAAInfo = CGF.CGM.getTBAAAccessInfo(ElTy);
+
   LValue BaseLVal;
   if (auto *ASE = dyn_cast(Base->IgnoreParenImpCasts())) {
 BaseLVal = CGF.EmitOMPArraySectionExpr(ASE, IsLowerBound);
@@ -3449,13 +3452,14 @@ LValue CodeGenFunction::EmitOMPArraySect
 
   Address EltPtr = Address::invalid();
   LValueBaseInfo BaseInfo;
+  TBAAAccessInfo TBAAInfo;
   if (auto *VLA = getContext().getAsVariableArrayType(ResultExprTy)) {
 // The base must be a pointer, which is not an aggregate.  Emit
 // it.  It needs to be emitted first in case it's what captures
 // the VLA bounds.
 Address Base =
-emitOMPArraySectionBase(*this, E->getBase(), BaseInfo, BaseTy,
-VLA->getElementType(), IsLowerBound);
+emitOMPArraySectionBase(*this, E->getBase(), BaseInfo, TBAAInfo,
+BaseTy, VLA->getElementType(), IsLowerBound);
 // The element count here is the total number of non-VLA elements.
 llvm::Value *NumElements = getVLASize(VLA).first;
 
@@ -3491,16 +3495,17 @@ LValue CodeGenFunction::EmitOMPArraySect
 ResultExprTy, !getLangOpts().isSignedOverflowDefined(),
 /*SignedIndices=*/false, E->getExprLoc());
 BaseInfo = ArrayLV.getBaseInfo();
+TBAAInfo = CGM.getTBAAAccessInfo(ResultExprTy);
   } else {
 Address Base = emitOMPArraySectionBase(*this, E->getBase(), BaseInfo,
-   BaseTy, ResultExprTy, IsLowerBound);
+   TBAAInfo, BaseTy, ResultExprTy,
+   IsLowerBound);
 EltPtr = emitArraySubscriptGEP(*this, Base, Idx, ResultExprTy,
!getLangOpts().isSignedOverflowDefined(),
/*SignedIndices=*/false, E->getExprLoc());
   }
 
-  return MakeAddrLValue(EltPtr, ResultExprTy, BaseInfo,
-CGM.getTBAAAccessInfo(ResultExprTy));
+  return MakeAddrLValue(EltPtr, ResultExprTy, BaseInfo, TBAAInfo);
 }
 
 LValue CodeGenFunction::


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D38795: [CodeGen] emitOMPArraySectionBase() to generate TBAA info along with LValue base info

2017-10-13 Thread Ivan Kosarev via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL315715: [CodeGen] emitOMPArraySectionBase() to generate TBAA 
info along with LValue… (authored by kosarev).

Changed prior to commit:
  https://reviews.llvm.org/D38795?vs=118585&id=118940#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D38795

Files:
  cfe/trunk/lib/CodeGen/CGExpr.cpp


Index: cfe/trunk/lib/CodeGen/CGExpr.cpp
===
--- cfe/trunk/lib/CodeGen/CGExpr.cpp
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp
@@ -3318,8 +3318,11 @@
 
 static Address emitOMPArraySectionBase(CodeGenFunction &CGF, const Expr *Base,
LValueBaseInfo &BaseInfo,
+   TBAAAccessInfo &TBAAInfo,
QualType BaseTy, QualType ElTy,
bool IsLowerBound) {
+  TBAAInfo = CGF.CGM.getTBAAAccessInfo(ElTy);
+
   LValue BaseLVal;
   if (auto *ASE = dyn_cast(Base->IgnoreParenImpCasts())) {
 BaseLVal = CGF.EmitOMPArraySectionExpr(ASE, IsLowerBound);
@@ -3449,13 +3452,14 @@
 
   Address EltPtr = Address::invalid();
   LValueBaseInfo BaseInfo;
+  TBAAAccessInfo TBAAInfo;
   if (auto *VLA = getContext().getAsVariableArrayType(ResultExprTy)) {
 // The base must be a pointer, which is not an aggregate.  Emit
 // it.  It needs to be emitted first in case it's what captures
 // the VLA bounds.
 Address Base =
-emitOMPArraySectionBase(*this, E->getBase(), BaseInfo, BaseTy,
-VLA->getElementType(), IsLowerBound);
+emitOMPArraySectionBase(*this, E->getBase(), BaseInfo, TBAAInfo,
+BaseTy, VLA->getElementType(), IsLowerBound);
 // The element count here is the total number of non-VLA elements.
 llvm::Value *NumElements = getVLASize(VLA).first;
 
@@ -3491,16 +3495,17 @@
 ResultExprTy, !getLangOpts().isSignedOverflowDefined(),
 /*SignedIndices=*/false, E->getExprLoc());
 BaseInfo = ArrayLV.getBaseInfo();
+TBAAInfo = CGM.getTBAAAccessInfo(ResultExprTy);
   } else {
 Address Base = emitOMPArraySectionBase(*this, E->getBase(), BaseInfo,
-   BaseTy, ResultExprTy, IsLowerBound);
+   TBAAInfo, BaseTy, ResultExprTy,
+   IsLowerBound);
 EltPtr = emitArraySubscriptGEP(*this, Base, Idx, ResultExprTy,
!getLangOpts().isSignedOverflowDefined(),
/*SignedIndices=*/false, E->getExprLoc());
   }
 
-  return MakeAddrLValue(EltPtr, ResultExprTy, BaseInfo,
-CGM.getTBAAAccessInfo(ResultExprTy));
+  return MakeAddrLValue(EltPtr, ResultExprTy, BaseInfo, TBAAInfo);
 }
 
 LValue CodeGenFunction::


Index: cfe/trunk/lib/CodeGen/CGExpr.cpp
===
--- cfe/trunk/lib/CodeGen/CGExpr.cpp
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp
@@ -3318,8 +3318,11 @@
 
 static Address emitOMPArraySectionBase(CodeGenFunction &CGF, const Expr *Base,
LValueBaseInfo &BaseInfo,
+   TBAAAccessInfo &TBAAInfo,
QualType BaseTy, QualType ElTy,
bool IsLowerBound) {
+  TBAAInfo = CGF.CGM.getTBAAAccessInfo(ElTy);
+
   LValue BaseLVal;
   if (auto *ASE = dyn_cast(Base->IgnoreParenImpCasts())) {
 BaseLVal = CGF.EmitOMPArraySectionExpr(ASE, IsLowerBound);
@@ -3449,13 +3452,14 @@
 
   Address EltPtr = Address::invalid();
   LValueBaseInfo BaseInfo;
+  TBAAAccessInfo TBAAInfo;
   if (auto *VLA = getContext().getAsVariableArrayType(ResultExprTy)) {
 // The base must be a pointer, which is not an aggregate.  Emit
 // it.  It needs to be emitted first in case it's what captures
 // the VLA bounds.
 Address Base =
-emitOMPArraySectionBase(*this, E->getBase(), BaseInfo, BaseTy,
-VLA->getElementType(), IsLowerBound);
+emitOMPArraySectionBase(*this, E->getBase(), BaseInfo, TBAAInfo,
+BaseTy, VLA->getElementType(), IsLowerBound);
 // The element count here is the total number of non-VLA elements.
 llvm::Value *NumElements = getVLASize(VLA).first;
 
@@ -3491,16 +3495,17 @@
 ResultExprTy, !getLangOpts().isSignedOverflowDefined(),
 /*SignedIndices=*/false, E->getExprLoc());
 BaseInfo = ArrayLV.getBaseInfo();
+TBAAInfo = CGM.getTBAAAccessInfo(ResultExprTy);
   } else {
 Address Base = emitOMPArraySectionBase(*this, E->getBase(), BaseInfo,
-   BaseTy, ResultExprTy, IsLowerBound);
+   TBAAInfo, BaseTy, ResultExprTy,
+   IsLowerBou

[PATCH] D38877: [analyzer] RetainCount: Accept "safe" CFRetain wrappers.

2017-10-13 Thread Devin Coughlin via Phabricator via cfe-commits
dcoughlin accepted this revision.
dcoughlin added a comment.
This revision is now accepted and ready to land.

Looks good to me!


https://reviews.llvm.org/D38877



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D38883: [CMake][OpenMP] Customize default offloading arch

2017-10-13 Thread Artem Belevich via Phabricator via cfe-commits
tra added inline comments.



Comment at: lib/Driver/ToolChains/Cuda.cpp:170-182
-// This code prevents IsValid from being set when
-// no libdevice has been found.
-bool allEmpty = true;
-std::string LibDeviceFile;
-for (auto key : LibDeviceMap.keys()) {
-  LibDeviceFile = LibDeviceMap.lookup(key);
-  if (!LibDeviceFile.empty())

Hahnfeld wrote:
> tra wrote:
> > I'd keep this code. It appears to serve useful purpose as it requires CUDA 
> > installation to have at least some libdevice library in it.  It gives us a 
> > change to find a valid installation, instead of ailing some time later when 
> > we ask for a libdevice file and fail because there are none.
> We had some internal discussions about this after I submitted the patch here.
> 
> The main question is: Do we want to support CUDA installations without 
> libdevice and are there use cases for that? I'd say that the user should be 
> able to use a toolchain without libdevice together with `-nocudalib`.
Sounds reasonable. How about keeping the code but putting it under 
`if(!hasArg(nocudalib))`?




Comment at: lib/Driver/ToolChains/Cuda.cpp:556
+  DAL->AddJoinedArg(nullptr, Opts.getOption(options::OPT_march_EQ),
+CLANG_OPENMP_NVPTX_DEFAULT_ARCH);
 }

Hahnfeld wrote:
> tra wrote:
> > This sets default GPU arch for *everyone* based on OPENMP requirements. 
> > Perhaps this should be predicated on this being openmp compilation.
> > 
> > IMO to avoid unnecessary surprises, the default for CUDA compilation should 
> > follow defaults of nvcc. sm_30 becomes default only in CUDA-9.
> > 
> This branch is only executed for OpenMP, see above
OK. I've missed that 'if'.


https://reviews.llvm.org/D38883



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D38883: [CMake][OpenMP] Customize default offloading arch

2017-10-13 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld marked 4 inline comments as done.
Hahnfeld added inline comments.



Comment at: lib/Driver/ToolChains/Cuda.cpp:170-182
-// This code prevents IsValid from being set when
-// no libdevice has been found.
-bool allEmpty = true;
-std::string LibDeviceFile;
-for (auto key : LibDeviceMap.keys()) {
-  LibDeviceFile = LibDeviceMap.lookup(key);
-  if (!LibDeviceFile.empty())

tra wrote:
> Hahnfeld wrote:
> > tra wrote:
> > > I'd keep this code. It appears to serve useful purpose as it requires 
> > > CUDA installation to have at least some libdevice library in it.  It 
> > > gives us a change to find a valid installation, instead of ailing some 
> > > time later when we ask for a libdevice file and fail because there are 
> > > none.
> > We had some internal discussions about this after I submitted the patch 
> > here.
> > 
> > The main question is: Do we want to support CUDA installations without 
> > libdevice and are there use cases for that? I'd say that the user should be 
> > able to use a toolchain without libdevice together with `-nocudalib`.
> Sounds reasonable. How about keeping the code but putting it under 
> `if(!hasArg(nocudalib))`?
> 
Ok, I'll do that in a separate patch and keep the code here for now.


https://reviews.llvm.org/D38883



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D37808: [clang-tidy] Add new hicpp-multiway-paths-covered check for missing branches

2017-10-13 Thread Jonas Devlieghere via Phabricator via cfe-commits
JDevlieghere added inline comments.



Comment at: clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp:96
+// Only the default branch (we explicitly matched for default!) exists.
+if (CaseCount == 1) {
+  diag(SwitchWithDefault->getLocStart(),

JonasToth wrote:
> JDevlieghere wrote:
> > Why not a switch?
> I intent to check if all cases are explicitly covered.
> 
> In the testcases there is one switch with all numbers explicitly written, 
> meaning there is no need to add a default branch.
> 
> This would allow further 
> ```
> else if (CaseCount == MaximumPossibleCases) { /* No warning */ }
> ```
> path which is not modable with `switch`.
Sounds good



Comment at: clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp:152
+// Should be written as an IfStmt.
+if (CaseCount == 1) {
+  diag(SwitchWithoutDefault->getLocStart(), "switch stmt with only one "

JonasToth wrote:
> JDevlieghere wrote:
> > I'm aware that the message and fixme are different, but since the structure 
> > is so similar to the handling of the other switch case, I wonder if there 
> > is any chance we could extract the common parts?
> I try to get something shorter.
> Maybe 
> ```
> if(CaseCount == 1 && MatchedSwitch) {}
> else if(CaseCount == 1 && MatchedElseIf) {}
> ```
> ?
Wouldn't it be easier to have a function and pass as arguments the stuff that's 
different? Both are `SwitchStmt`s so if you pass that + the diagnostic message 
you should be able to share the other logic. 


https://reviews.llvm.org/D37808



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D38883: [CMake][OpenMP] Customize default offloading arch

2017-10-13 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits
gtbercea added inline comments.



Comment at: lib/Driver/ToolChains/Cuda.cpp:170-182
-// This code prevents IsValid from being set when
-// no libdevice has been found.
-bool allEmpty = true;
-std::string LibDeviceFile;
-for (auto key : LibDeviceMap.keys()) {
-  LibDeviceFile = LibDeviceMap.lookup(key);
-  if (!LibDeviceFile.empty())

Hahnfeld wrote:
> tra wrote:
> > Hahnfeld wrote:
> > > tra wrote:
> > > > I'd keep this code. It appears to serve useful purpose as it requires 
> > > > CUDA installation to have at least some libdevice library in it.  It 
> > > > gives us a change to find a valid installation, instead of ailing some 
> > > > time later when we ask for a libdevice file and fail because there are 
> > > > none.
> > > We had some internal discussions about this after I submitted the patch 
> > > here.
> > > 
> > > The main question is: Do we want to support CUDA installations without 
> > > libdevice and are there use cases for that? I'd say that the user should 
> > > be able to use a toolchain without libdevice together with `-nocudalib`.
> > Sounds reasonable. How about keeping the code but putting it under 
> > `if(!hasArg(nocudalib))`?
> > 
> Ok, I'll do that in a separate patch and keep the code here for now.
The problem with nocudalib is that if for example you write a test, which looks 
to verify some device facing feature that requires a libdevice to be found (so 
you don't want to use nocudalib), it will probably work on your machine which 
has the correct CUDA setup but fail on another machine which does not (which is 
where you want to use nocudalib). You can see the contradiction there.


https://reviews.llvm.org/D38883



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D38883: [CMake][OpenMP] Customize default offloading arch

2017-10-13 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits
gtbercea added inline comments.



Comment at: lib/Driver/ToolChains/Cuda.cpp:170-182
-// This code prevents IsValid from being set when
-// no libdevice has been found.
-bool allEmpty = true;
-std::string LibDeviceFile;
-for (auto key : LibDeviceMap.keys()) {
-  LibDeviceFile = LibDeviceMap.lookup(key);
-  if (!LibDeviceFile.empty())

gtbercea wrote:
> Hahnfeld wrote:
> > tra wrote:
> > > Hahnfeld wrote:
> > > > tra wrote:
> > > > > I'd keep this code. It appears to serve useful purpose as it requires 
> > > > > CUDA installation to have at least some libdevice library in it.  It 
> > > > > gives us a change to find a valid installation, instead of ailing 
> > > > > some time later when we ask for a libdevice file and fail because 
> > > > > there are none.
> > > > We had some internal discussions about this after I submitted the patch 
> > > > here.
> > > > 
> > > > The main question is: Do we want to support CUDA installations without 
> > > > libdevice and are there use cases for that? I'd say that the user 
> > > > should be able to use a toolchain without libdevice together with 
> > > > `-nocudalib`.
> > > Sounds reasonable. How about keeping the code but putting it under 
> > > `if(!hasArg(nocudalib))`?
> > > 
> > Ok, I'll do that in a separate patch and keep the code here for now.
> The problem with nocudalib is that if for example you write a test, which 
> looks to verify some device facing feature that requires a libdevice to be 
> found (so you don't want to use nocudalib), it will probably work on your 
> machine which has the correct CUDA setup but fail on another machine which 
> does not (which is where you want to use nocudalib). You can see the 
> contradiction there.
Just to be clear I am arguing for keeping this code :)


https://reviews.llvm.org/D38883



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D38883: [CMake][OpenMP] Customize default offloading arch

2017-10-13 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits
gtbercea added inline comments.



Comment at: lib/Driver/ToolChains/Cuda.h:90
-  }
 };
 

I would also like to keep the spirit of this code if not in this exact form at 
least something that performs the same functionality.


https://reviews.llvm.org/D38883



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D38883: [CMake][OpenMP] Customize default offloading arch

2017-10-13 Thread Artem Belevich via Phabricator via cfe-commits
tra added inline comments.



Comment at: lib/Driver/ToolChains/Cuda.cpp:170-182
-// This code prevents IsValid from being set when
-// no libdevice has been found.
-bool allEmpty = true;
-std::string LibDeviceFile;
-for (auto key : LibDeviceMap.keys()) {
-  LibDeviceFile = LibDeviceMap.lookup(key);
-  if (!LibDeviceFile.empty())

gtbercea wrote:
> gtbercea wrote:
> > Hahnfeld wrote:
> > > tra wrote:
> > > > Hahnfeld wrote:
> > > > > tra wrote:
> > > > > > I'd keep this code. It appears to serve useful purpose as it 
> > > > > > requires CUDA installation to have at least some libdevice library 
> > > > > > in it.  It gives us a change to find a valid installation, instead 
> > > > > > of ailing some time later when we ask for a libdevice file and fail 
> > > > > > because there are none.
> > > > > We had some internal discussions about this after I submitted the 
> > > > > patch here.
> > > > > 
> > > > > The main question is: Do we want to support CUDA installations 
> > > > > without libdevice and are there use cases for that? I'd say that the 
> > > > > user should be able to use a toolchain without libdevice together 
> > > > > with `-nocudalib`.
> > > > Sounds reasonable. How about keeping the code but putting it under 
> > > > `if(!hasArg(nocudalib))`?
> > > > 
> > > Ok, I'll do that in a separate patch and keep the code here for now.
> > The problem with nocudalib is that if for example you write a test, which 
> > looks to verify some device facing feature that requires a libdevice to be 
> > found (so you don't want to use nocudalib), it will probably work on your 
> > machine which has the correct CUDA setup but fail on another machine which 
> > does not (which is where you want to use nocudalib). You can see the 
> > contradiction there.
> Just to be clear I am arguing for keeping this code :)
@gtbercea: I'm not sure I follow your example. If you're talking about clang 
tests, we do have fake CUDA installation setup under test/Driver/Inputs which 
removes dependency on whatever CUDA you may or may not have installed on your 
machine. I also don't see a contradiction -- you you do need libdevice, it 
makes no point picking a broken CUDA installation which does not have any 
libdevice files. If you explicitly tell compiler that you don't need libdevice, 
that would make CUDA w/o libdevice acceptable. With --cuda-path you do have a 
way to tell clang which installation you want it to use. What do I miss?




https://reviews.llvm.org/D38883



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D38883: [CMake][OpenMP] Customize default offloading arch

2017-10-13 Thread Artem Belevich via Phabricator via cfe-commits
tra added inline comments.



Comment at: lib/Driver/ToolChains/Cuda.cpp:170-182
-// This code prevents IsValid from being set when
-// no libdevice has been found.
-bool allEmpty = true;
-std::string LibDeviceFile;
-for (auto key : LibDeviceMap.keys()) {
-  LibDeviceFile = LibDeviceMap.lookup(key);
-  if (!LibDeviceFile.empty())

tra wrote:
> gtbercea wrote:
> > gtbercea wrote:
> > > Hahnfeld wrote:
> > > > tra wrote:
> > > > > Hahnfeld wrote:
> > > > > > tra wrote:
> > > > > > > I'd keep this code. It appears to serve useful purpose as it 
> > > > > > > requires CUDA installation to have at least some libdevice 
> > > > > > > library in it.  It gives us a change to find a valid 
> > > > > > > installation, instead of ailing some time later when we ask for a 
> > > > > > > libdevice file and fail because there are none.
> > > > > > We had some internal discussions about this after I submitted the 
> > > > > > patch here.
> > > > > > 
> > > > > > The main question is: Do we want to support CUDA installations 
> > > > > > without libdevice and are there use cases for that? I'd say that 
> > > > > > the user should be able to use a toolchain without libdevice 
> > > > > > together with `-nocudalib`.
> > > > > Sounds reasonable. How about keeping the code but putting it under 
> > > > > `if(!hasArg(nocudalib))`?
> > > > > 
> > > > Ok, I'll do that in a separate patch and keep the code here for now.
> > > The problem with nocudalib is that if for example you write a test, which 
> > > looks to verify some device facing feature that requires a libdevice to 
> > > be found (so you don't want to use nocudalib), it will probably work on 
> > > your machine which has the correct CUDA setup but fail on another machine 
> > > which does not (which is where you want to use nocudalib). You can see 
> > > the contradiction there.
> > Just to be clear I am arguing for keeping this code :)
> @gtbercea: I'm not sure I follow your example. If you're talking about clang 
> tests, we do have fake CUDA installation setup under test/Driver/Inputs which 
> removes dependency on whatever CUDA you may or may not have installed on your 
> machine. I also don't see a contradiction -- you you do need libdevice, it 
> makes no point picking a broken CUDA installation which does not have any 
> libdevice files. If you explicitly tell compiler that you don't need 
> libdevice, that would make CUDA w/o libdevice acceptable. With --cuda-path 
> you do have a way to tell clang which installation you want it to use. What 
> do I miss?
> 
> 
Ah, you were arguing with Hahnfeld@'s -nocudalib example. Then I guess we're in 
violent agreement.


https://reviews.llvm.org/D38883



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D38813: [X86] Add skeleton support for "knm" cpu - clang side

2017-10-13 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL315723: [X86] Add skeleton support for knm cpu (authored by 
ctopper).

Changed prior to commit:
  https://reviews.llvm.org/D38813?vs=118650&id=118945#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D38813

Files:
  cfe/trunk/lib/Basic/Targets/X86.cpp
  cfe/trunk/lib/Basic/Targets/X86.h
  cfe/trunk/test/Driver/x86-march.c
  cfe/trunk/test/Preprocessor/predefined-arch-macros.c

Index: cfe/trunk/test/Preprocessor/predefined-arch-macros.c
===
--- cfe/trunk/test/Preprocessor/predefined-arch-macros.c
+++ cfe/trunk/test/Preprocessor/predefined-arch-macros.c
@@ -783,6 +783,81 @@
 // CHECK_KNL_M64: #define __tune_knl__ 1
 // CHECK_KNL_M64: #define __x86_64 1
 // CHECK_KNL_M64: #define __x86_64__ 1
+
+// RUN: %clang -march=knm -m32 -E -dM %s -o - 2>&1 \
+// RUN: -target i386-unknown-linux \
+// RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_KNM_M32
+// CHECK_KNM_M32: #define __AES__ 1
+// CHECK_KNM_M32: #define __AVX2__ 1
+// CHECK_KNM_M32: #define __AVX512CD__ 1
+// CHECK_KNM_M32: #define __AVX512ER__ 1
+// CHECK_KNM_M32: #define __AVX512F__ 1
+// CHECK_KNM_M32: #define __AVX512PF__ 1
+// CHECK_KNM_M32: #define __AVX__ 1
+// CHECK_KNM_M32: #define __BMI2__ 1
+// CHECK_KNM_M32: #define __BMI__ 1
+// CHECK_KNM_M32: #define __F16C__ 1
+// CHECK_KNM_M32: #define __FMA__ 1
+// CHECK_KNM_M32: #define __LZCNT__ 1
+// CHECK_KNM_M32: #define __MMX__ 1
+// CHECK_KNM_M32: #define __PCLMUL__ 1
+// CHECK_KNM_M32: #define __POPCNT__ 1
+// CHECK_KNM_M32: #define __PREFETCHWT1__ 1
+// CHECK_KNM_M32: #define __RDRND__ 1
+// CHECK_KNM_M32: #define __RTM__ 1
+// CHECK_KNM_M32: #define __SSE2__ 1
+// CHECK_KNM_M32: #define __SSE3__ 1
+// CHECK_KNM_M32: #define __SSE4_1__ 1
+// CHECK_KNM_M32: #define __SSE4_2__ 1
+// CHECK_KNM_M32: #define __SSE__ 1
+// CHECK_KNM_M32: #define __SSSE3__ 1
+// CHECK_KNM_M32: #define __XSAVEOPT__ 1
+// CHECK_KNM_M32: #define __XSAVE__ 1
+// CHECK_KNM_M32: #define __i386 1
+// CHECK_KNM_M32: #define __i386__ 1
+// CHECK_KNM_M32: #define __knm 1
+// CHECK_KNM_M32: #define __knm__ 1
+// CHECK_KNM_M32: #define __tune_knm__ 1
+// CHECK_KNM_M32: #define i386 1
+
+// RUN: %clang -march=knm -m64 -E -dM %s -o - 2>&1 \
+// RUN: -target i386-unknown-linux \
+// RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_KNM_M64
+// CHECK_KNM_M64: #define __AES__ 1
+// CHECK_KNM_M64: #define __AVX2__ 1
+// CHECK_KNM_M64: #define __AVX512CD__ 1
+// CHECK_KNM_M64: #define __AVX512ER__ 1
+// CHECK_KNM_M64: #define __AVX512F__ 1
+// CHECK_KNM_M64: #define __AVX512PF__ 1
+// CHECK_KNM_M64: #define __AVX__ 1
+// CHECK_KNM_M64: #define __BMI2__ 1
+// CHECK_KNM_M64: #define __BMI__ 1
+// CHECK_KNM_M64: #define __F16C__ 1
+// CHECK_KNM_M64: #define __FMA__ 1
+// CHECK_KNM_M64: #define __LZCNT__ 1
+// CHECK_KNM_M64: #define __MMX__ 1
+// CHECK_KNM_M64: #define __PCLMUL__ 1
+// CHECK_KNM_M64: #define __POPCNT__ 1
+// CHECK_KNM_M64: #define __PREFETCHWT1__ 1
+// CHECK_KNM_M64: #define __RDRND__ 1
+// CHECK_KNM_M64: #define __RTM__ 1
+// CHECK_KNM_M64: #define __SSE2_MATH__ 1
+// CHECK_KNM_M64: #define __SSE2__ 1
+// CHECK_KNM_M64: #define __SSE3__ 1
+// CHECK_KNM_M64: #define __SSE4_1__ 1
+// CHECK_KNM_M64: #define __SSE4_2__ 1
+// CHECK_KNM_M64: #define __SSE_MATH__ 1
+// CHECK_KNM_M64: #define __SSE__ 1
+// CHECK_KNM_M64: #define __SSSE3__ 1
+// CHECK_KNM_M64: #define __XSAVEOPT__ 1
+// CHECK_KNM_M64: #define __XSAVE__ 1
+// CHECK_KNM_M64: #define __amd64 1
+// CHECK_KNM_M64: #define __amd64__ 1
+// CHECK_KNM_M64: #define __knm 1
+// CHECK_KNM_M64: #define __knm__ 1
+// CHECK_KNM_M64: #define __tune_knm__ 1
+// CHECK_KNM_M64: #define __x86_64 1
+// CHECK_KNM_M64: #define __x86_64__ 1
 //
 // RUN: %clang -march=skylake-avx512 -m32 -E -dM %s -o - 2>&1 \
 // RUN: -target i386-unknown-linux \
Index: cfe/trunk/test/Driver/x86-march.c
===
--- cfe/trunk/test/Driver/x86-march.c
+++ cfe/trunk/test/Driver/x86-march.c
@@ -52,6 +52,10 @@
 // RUN:   | FileCheck %s -check-prefix=knl
 // knl: "-target-cpu" "knl"
 //
+// RUN: %clang -target x86_64-unknown-unknown -c -### %s -march=knm 2>&1 \
+// RUN:   | FileCheck %s -check-prefix=knm
+// knm: "-target-cpu" "knm"
+//
 // RUN: %clang -target x86_64-unknown-unknown -c -### %s -march=cannonlake 2>&1 \
 // RUN:   | FileCheck %s -check-prefix=cannonlake
 // cannonlake: "-target-cpu" "cannonlake"
Index: cfe/trunk/lib/Basic/Targets/X86.cpp
===
--- cfe/trunk/lib/Basic/Targets/X86.cpp
+++ cfe/trunk/lib/Basic/Targets/X86.cpp
@@ -229,6 +229,8 @@
 setFeatureEnabledImpl(Features, "cx16", true);
 break;
 
+  case CK_KNM:
+// TODO: Add avx5124fmaps/avx5124vnniw.
   case CK_KNL:
 setFeatureEnabledImpl(Features, "avx512f", true);
 setFeatureEnabledImpl(Features, "avx512cd", true)

r315723 - [X86] Add skeleton support for knm cpu

2017-10-13 Thread Craig Topper via cfe-commits
Author: ctopper
Date: Fri Oct 13 11:14:24 2017
New Revision: 315723

URL: http://llvm.org/viewvc/llvm-project?rev=315723&view=rev
Log:
[X86] Add skeleton support for knm cpu

This adds support Knights Mill CPU. Preprocessor defines match gcc's 
implementation.

Differential Revision: https://reviews.llvm.org/D38813

Modified:
cfe/trunk/lib/Basic/Targets/X86.cpp
cfe/trunk/lib/Basic/Targets/X86.h
cfe/trunk/test/Driver/x86-march.c
cfe/trunk/test/Preprocessor/predefined-arch-macros.c

Modified: cfe/trunk/lib/Basic/Targets/X86.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/X86.cpp?rev=315723&r1=315722&r2=315723&view=diff
==
--- cfe/trunk/lib/Basic/Targets/X86.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/X86.cpp Fri Oct 13 11:14:24 2017
@@ -229,6 +229,8 @@ bool X86TargetInfo::initFeatureMap(
 setFeatureEnabledImpl(Features, "cx16", true);
 break;
 
+  case CK_KNM:
+// TODO: Add avx5124fmaps/avx5124vnniw.
   case CK_KNL:
 setFeatureEnabledImpl(Features, "avx512f", true);
 setFeatureEnabledImpl(Features, "avx512cd", true);
@@ -853,6 +855,8 @@ void X86TargetInfo::getTargetDefines(con
   case CK_KNL:
 defineCPUMacros(Builder, "knl");
 break;
+  case CK_KNM:
+break;
   case CK_Lakemont:
 Builder.defineMacro("__tune_lakemont__");
 break;
@@ -1553,6 +1557,7 @@ X86TargetInfo::CPUKind X86TargetInfo::ge
   .Cases("skylake-avx512", "skx", CK_SkylakeServer)
   .Case("cannonlake", CK_Cannonlake)
   .Case("knl", CK_KNL)
+  .Case("knm", CK_KNM)
   .Case("lakemont", CK_Lakemont)
   .Case("k6", CK_K6)
   .Case("k6-2", CK_K6_2)

Modified: cfe/trunk/lib/Basic/Targets/X86.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/X86.h?rev=315723&r1=315722&r2=315723&view=diff
==
--- cfe/trunk/lib/Basic/Targets/X86.h (original)
+++ cfe/trunk/lib/Basic/Targets/X86.h Fri Oct 13 11:14:24 2017
@@ -203,6 +203,10 @@ class LLVM_LIBRARY_VISIBILITY X86TargetI
 /// Knights Landing processor.
 CK_KNL,
 
+/// \name Knights Mill
+/// Knights Mill processor.
+CK_KNM,
+
 /// \name Lakemont
 /// Lakemont microarchitecture based processors.
 CK_Lakemont,
@@ -321,6 +325,7 @@ class LLVM_LIBRARY_VISIBILITY X86TargetI
 case CK_SkylakeServer:
 case CK_Cannonlake:
 case CK_KNL:
+case CK_KNM:
 case CK_K8:
 case CK_K8SSE3:
 case CK_AMDFAM10:

Modified: cfe/trunk/test/Driver/x86-march.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/x86-march.c?rev=315723&r1=315722&r2=315723&view=diff
==
--- cfe/trunk/test/Driver/x86-march.c (original)
+++ cfe/trunk/test/Driver/x86-march.c Fri Oct 13 11:14:24 2017
@@ -52,6 +52,10 @@
 // RUN:   | FileCheck %s -check-prefix=knl
 // knl: "-target-cpu" "knl"
 //
+// RUN: %clang -target x86_64-unknown-unknown -c -### %s -march=knm 2>&1 \
+// RUN:   | FileCheck %s -check-prefix=knm
+// knm: "-target-cpu" "knm"
+//
 // RUN: %clang -target x86_64-unknown-unknown -c -### %s -march=cannonlake 
2>&1 \
 // RUN:   | FileCheck %s -check-prefix=cannonlake
 // cannonlake: "-target-cpu" "cannonlake"

Modified: cfe/trunk/test/Preprocessor/predefined-arch-macros.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/predefined-arch-macros.c?rev=315723&r1=315722&r2=315723&view=diff
==
--- cfe/trunk/test/Preprocessor/predefined-arch-macros.c (original)
+++ cfe/trunk/test/Preprocessor/predefined-arch-macros.c Fri Oct 13 11:14:24 
2017
@@ -783,6 +783,81 @@
 // CHECK_KNL_M64: #define __tune_knl__ 1
 // CHECK_KNL_M64: #define __x86_64 1
 // CHECK_KNL_M64: #define __x86_64__ 1
+
+// RUN: %clang -march=knm -m32 -E -dM %s -o - 2>&1 \
+// RUN: -target i386-unknown-linux \
+// RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_KNM_M32
+// CHECK_KNM_M32: #define __AES__ 1
+// CHECK_KNM_M32: #define __AVX2__ 1
+// CHECK_KNM_M32: #define __AVX512CD__ 1
+// CHECK_KNM_M32: #define __AVX512ER__ 1
+// CHECK_KNM_M32: #define __AVX512F__ 1
+// CHECK_KNM_M32: #define __AVX512PF__ 1
+// CHECK_KNM_M32: #define __AVX__ 1
+// CHECK_KNM_M32: #define __BMI2__ 1
+// CHECK_KNM_M32: #define __BMI__ 1
+// CHECK_KNM_M32: #define __F16C__ 1
+// CHECK_KNM_M32: #define __FMA__ 1
+// CHECK_KNM_M32: #define __LZCNT__ 1
+// CHECK_KNM_M32: #define __MMX__ 1
+// CHECK_KNM_M32: #define __PCLMUL__ 1
+// CHECK_KNM_M32: #define __POPCNT__ 1
+// CHECK_KNM_M32: #define __PREFETCHWT1__ 1
+// CHECK_KNM_M32: #define __RDRND__ 1
+// CHECK_KNM_M32: #define __RTM__ 1
+// CHECK_KNM_M32: #define __SSE2__ 1
+// CHECK_KNM_M32: #define __SSE3__ 1
+// CHECK_KNM_M32: #define __SSE4_1__ 1
+// CHECK_KNM_M32: #define __SSE4_2__ 1
+// CHECK_KNM_M32: #define __SSE__ 1
+// CHECK_K

[PATCH] D38883: [CMake][OpenMP] Customize default offloading arch

2017-10-13 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits
gtbercea added inline comments.



Comment at: lib/Driver/ToolChains/Cuda.cpp:170-182
-// This code prevents IsValid from being set when
-// no libdevice has been found.
-bool allEmpty = true;
-std::string LibDeviceFile;
-for (auto key : LibDeviceMap.keys()) {
-  LibDeviceFile = LibDeviceMap.lookup(key);
-  if (!LibDeviceFile.empty())

tra wrote:
> tra wrote:
> > gtbercea wrote:
> > > gtbercea wrote:
> > > > Hahnfeld wrote:
> > > > > tra wrote:
> > > > > > Hahnfeld wrote:
> > > > > > > tra wrote:
> > > > > > > > I'd keep this code. It appears to serve useful purpose as it 
> > > > > > > > requires CUDA installation to have at least some libdevice 
> > > > > > > > library in it.  It gives us a change to find a valid 
> > > > > > > > installation, instead of ailing some time later when we ask for 
> > > > > > > > a libdevice file and fail because there are none.
> > > > > > > We had some internal discussions about this after I submitted the 
> > > > > > > patch here.
> > > > > > > 
> > > > > > > The main question is: Do we want to support CUDA installations 
> > > > > > > without libdevice and are there use cases for that? I'd say that 
> > > > > > > the user should be able to use a toolchain without libdevice 
> > > > > > > together with `-nocudalib`.
> > > > > > Sounds reasonable. How about keeping the code but putting it under 
> > > > > > `if(!hasArg(nocudalib))`?
> > > > > > 
> > > > > Ok, I'll do that in a separate patch and keep the code here for now.
> > > > The problem with nocudalib is that if for example you write a test, 
> > > > which looks to verify some device facing feature that requires a 
> > > > libdevice to be found (so you don't want to use nocudalib), it will 
> > > > probably work on your machine which has the correct CUDA setup but fail 
> > > > on another machine which does not (which is where you want to use 
> > > > nocudalib). You can see the contradiction there.
> > > Just to be clear I am arguing for keeping this code :)
> > @gtbercea: I'm not sure I follow your example. If you're talking about 
> > clang tests, we do have fake CUDA installation setup under 
> > test/Driver/Inputs which removes dependency on whatever CUDA you may or may 
> > not have installed on your machine. I also don't see a contradiction -- you 
> > you do need libdevice, it makes no point picking a broken CUDA installation 
> > which does not have any libdevice files. If you explicitly tell compiler 
> > that you don't need libdevice, that would make CUDA w/o libdevice 
> > acceptable. With --cuda-path you do have a way to tell clang which 
> > installation you want it to use. What do I miss?
> > 
> > 
> Ah, you were arguing with Hahnfeld@'s -nocudalib example. Then I guess we're 
> in violent agreement.
I fully agree with this: "you do need libdevice, it makes no point picking a 
broken CUDA installation which does not have any libdevice files. If you 
explicitly tell compiler that you don't need libdevice, that would make CUDA 
w/o libdevice acceptable."

I was trying to show an example of a situation where you have your code 
compiled using nocudalib on one machine and then the same code will error on a 
machine which requires the nocudalib flag to be passed to make up for the 
absence of libdevice.




https://reviews.llvm.org/D38883



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D38883: [CMake][OpenMP] Customize default offloading arch

2017-10-13 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits
gtbercea added inline comments.



Comment at: lib/Driver/ToolChains/Cuda.cpp:170-182
-// This code prevents IsValid from being set when
-// no libdevice has been found.
-bool allEmpty = true;
-std::string LibDeviceFile;
-for (auto key : LibDeviceMap.keys()) {
-  LibDeviceFile = LibDeviceMap.lookup(key);
-  if (!LibDeviceFile.empty())

gtbercea wrote:
> tra wrote:
> > tra wrote:
> > > gtbercea wrote:
> > > > gtbercea wrote:
> > > > > Hahnfeld wrote:
> > > > > > tra wrote:
> > > > > > > Hahnfeld wrote:
> > > > > > > > tra wrote:
> > > > > > > > > I'd keep this code. It appears to serve useful purpose as it 
> > > > > > > > > requires CUDA installation to have at least some libdevice 
> > > > > > > > > library in it.  It gives us a change to find a valid 
> > > > > > > > > installation, instead of ailing some time later when we ask 
> > > > > > > > > for a libdevice file and fail because there are none.
> > > > > > > > We had some internal discussions about this after I submitted 
> > > > > > > > the patch here.
> > > > > > > > 
> > > > > > > > The main question is: Do we want to support CUDA installations 
> > > > > > > > without libdevice and are there use cases for that? I'd say 
> > > > > > > > that the user should be able to use a toolchain without 
> > > > > > > > libdevice together with `-nocudalib`.
> > > > > > > Sounds reasonable. How about keeping the code but putting it 
> > > > > > > under `if(!hasArg(nocudalib))`?
> > > > > > > 
> > > > > > Ok, I'll do that in a separate patch and keep the code here for now.
> > > > > The problem with nocudalib is that if for example you write a test, 
> > > > > which looks to verify some device facing feature that requires a 
> > > > > libdevice to be found (so you don't want to use nocudalib), it will 
> > > > > probably work on your machine which has the correct CUDA setup but 
> > > > > fail on another machine which does not (which is where you want to 
> > > > > use nocudalib). You can see the contradiction there.
> > > > Just to be clear I am arguing for keeping this code :)
> > > @gtbercea: I'm not sure I follow your example. If you're talking about 
> > > clang tests, we do have fake CUDA installation setup under 
> > > test/Driver/Inputs which removes dependency on whatever CUDA you may or 
> > > may not have installed on your machine. I also don't see a contradiction 
> > > -- you you do need libdevice, it makes no point picking a broken CUDA 
> > > installation which does not have any libdevice files. If you explicitly 
> > > tell compiler that you don't need libdevice, that would make CUDA w/o 
> > > libdevice acceptable. With --cuda-path you do have a way to tell clang 
> > > which installation you want it to use. What do I miss?
> > > 
> > > 
> > Ah, you were arguing with Hahnfeld@'s -nocudalib example. Then I guess 
> > we're in violent agreement.
> I fully agree with this: "you do need libdevice, it makes no point picking a 
> broken CUDA installation which does not have any libdevice files. If you 
> explicitly tell compiler that you don't need libdevice, that would make CUDA 
> w/o libdevice acceptable."
> 
> I was trying to show an example of a situation where you have your code 
> compiled using nocudalib on one machine and then the same code will error on 
> a machine which requires the nocudalib flag to be passed to make up for the 
> absence of libdevice.
> 
> 
Yes it was a counter argument to that! :) 


https://reviews.llvm.org/D38883



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D38883: [CMake][OpenMP] Customize default offloading arch

2017-10-13 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits
gtbercea added inline comments.



Comment at: lib/Driver/ToolChains/Cuda.h:90
-  }
 };
 

gtbercea wrote:
> I would also like to keep the spirit of this code if not in this exact form 
> at least something that performs the same functionality.
@tra what's your opinion on this code? Should this stay, stay but modified to 
be more robust or taken out completely?


https://reviews.llvm.org/D38883



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D38883: [CMake][OpenMP] Customize default offloading arch

2017-10-13 Thread Artem Belevich via Phabricator via cfe-commits
tra added inline comments.



Comment at: lib/Driver/ToolChains/Cuda.h:90
-  }
 };
 

gtbercea wrote:
> gtbercea wrote:
> > I would also like to keep the spirit of this code if not in this exact form 
> > at least something that performs the same functionality.
> @tra what's your opinion on this code? Should this stay, stay but modified to 
> be more robust or taken out completely?
There are currently no users for this. In general, I would rather not have 
magically-changing default GPU based on how broken your CUDA installation is. 
IMO it would be better to keep defaults static and fail if prerequisites are 
not met.


https://reviews.llvm.org/D38883



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D38101: [Sema] Diagnose tautological comparison with type's min/max values

2017-10-13 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments.



Comment at: cfe/trunk/test/Sema/tautological-constant-compare.c:23
+
+  if (c > macro(255))
+  return;

I'm having second thoughts about macro handling.
Right now we completely ignore the comparisons when the constant is anyhow 
involved with macros.
I.e.
```
unsigned char c;
if (c > 255) // expected-warning {{comparison 'unsigned char' > 255 is always 
false}}
  return;  // would be diagnosed correctly
// but
assert (c <= 255); // would be completely ignored.
```
Perhaps we should be a bit more strict, and should not bailout in such cases?

But i *guess* this case we still should ignore
```
#define macro(val) (val > 255)
if (macro(c))
  return;
```

(Even though gcc warns in the last variant too)


Repository:
  rL LLVM

https://reviews.llvm.org/D38101



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D37897: [StaticAnalyzer] Fix ProgramState for static variables that are not written

2017-10-13 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki updated this revision to Diff 118947.
danielmarjamaki added a comment.

Track modification of global static variables in CallGraph construction


Repository:
  rL LLVM

https://reviews.llvm.org/D37897

Files:
  include/clang/AST/Decl.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  lib/AST/Decl.cpp
  lib/Analysis/CallGraph.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/Analysis/global-vars.c

Index: test/Analysis/global-vars.c
===
--- test/Analysis/global-vars.c
+++ test/Analysis/global-vars.c
@@ -0,0 +1,18 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha,core -verify %s
+
+// Avoid false positive.
+static char *allv[] = {"rpcgen", "-s", "udp", "-s", "tcp"};
+static int allc = sizeof(allv) / sizeof(allv[0]);
+static void falsePositive1(void) {
+  int i;
+  for (i = 1; i < allc; i++) {
+const char *p = allv[i]; // no-warning
+i++;
+  }
+}
+
+// Detect division by zero.
+static int zero = 0;
+void truePositive1() {
+  int x = 1000 / zero; // expected-warning{{Division by zero}}
+}
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -103,8 +103,71 @@
 // Utility methods.
 //===--===//
 
+/** Get initial state for global static variable */
+ProgramStateRef ExprEngine::getInitialStateForGlobalStaticVar(
+const LocationContext *LCtx, ProgramStateRef State, const VarDecl *VD) {
+  // Is variable changed anywhere in TU?
+  if (VD->isModified())
+return State;
+
+  // What is the initialized value?
+  llvm::APSInt InitVal;
+  if (const Expr *I = VD->getInit()) {
+if (!I->EvaluateAsInt(InitVal, getContext()))
+  return State;
+  } else {
+InitVal = 0;
+  }
+
+  const MemRegion *R = State->getRegion(VD, LCtx);
+  if (!R)
+return State;
+  SVal V = State->getSVal(loc::MemRegionVal(R));
+  SVal Constraint_untested =
+  evalBinOp(State, BO_EQ, V, svalBuilder.makeIntVal(InitVal),
+svalBuilder.getConditionType());
+  Optional Constraint =
+  Constraint_untested.getAs();
+  if (!Constraint)
+return State;
+  return State->assume(*Constraint, true);
+}
+
+static void getGlobalStaticVars(const Stmt *FuncBody,
+llvm::SmallSet *Vars) {
+  std::stack Children;
+  Children.push(FuncBody);
+  while (!Children.empty()) {
+const Stmt *Child = Children.top();
+Children.pop();
+if (!Child)
+  continue;
+for (const Stmt *C : Child->children()) {
+  Children.push(C);
+}
+if (const DeclRefExpr *D = dyn_cast(Child)) {
+  const VarDecl *VD = dyn_cast(D->getDecl());
+  if (VD && VD->isDefinedOutsideFunctionOrMethod() &&
+  VD->getType()->isIntegerType() &&
+  VD->getStorageClass() == SC_Static &&
+  !VD->getType()->isPointerType()) {
+Vars->insert(VD);
+  }
+}
+  }
+}
+
 ProgramStateRef ExprEngine::getInitialState(const LocationContext *InitLoc) {
   ProgramStateRef state = StateMgr.getInitialState(InitLoc);
+  // Get initial states for static global variables.
+  if (const auto *FD = dyn_cast(InitLoc->getDecl())) {
+llvm::SmallSet Vars;
+getGlobalStaticVars(FD->getBody(), &Vars);
+for (const VarDecl *VD : Vars) {
+  state = getInitialStateForGlobalStaticVar(InitLoc, state, VD);
+}
+  }
+
   const Decl *D = InitLoc->getDecl();
 
   // Preconditions.
Index: lib/Analysis/CallGraph.cpp
===
--- lib/Analysis/CallGraph.cpp
+++ lib/Analysis/CallGraph.cpp
@@ -33,10 +33,12 @@
   CallGraphNode *CallerNode;
 
 public:
-  CGBuilder(CallGraph *g, CallGraphNode *N)
-: G(g), CallerNode(N) {}
+  CGBuilder(CallGraph *g, CallGraphNode *N) : G(g), CallerNode(N) {}
 
-  void VisitStmt(Stmt *S) { VisitChildren(S); }
+  void VisitStmt(Stmt *S) {
+markModifiedVars(S);
+VisitChildren(S);
+  }
 
   Decl *getDeclFromCall(CallExpr *CE) {
 if (FunctionDecl *CalleeDecl = CE->getDirectCallee())
@@ -63,13 +65,14 @@
 if (Decl *D = getDeclFromCall(CE))
   addCalledDecl(D);
 VisitChildren(CE);
+markModifiedVars(CE);
   }
 
   // Adds may-call edges for the ObjC message sends.
   void VisitObjCMessageExpr(ObjCMessageExpr *ME) {
 if (ObjCInterfaceDecl *IDecl = ME->getReceiverInterface()) {
   Selector Sel = ME->getSelector();
-  
+
   // Find the callee definition within the same translation unit.
   Decl *D = nullptr;
   if (ME->isInstanceMessage())
@@ -88,6 +91,53 @@
   if (SubStmt)
 this->Visit(SubStmt);
   }
+
+  void modifyVar(Expr *E) {
+auto *D = dyn_cast(E->IgnoreParenCasts());
+if (!D)
+  return;
+VarDecl *VD = dyn_cast(D->getDecl());
+if (VD)
+  VD->setModified();
+  }
+
+  void markModifiedVars(Stmt *

r315729 - [X86] Remove 'knm' defines from predefined-arch-macros.c test.

2017-10-13 Thread Craig Topper via cfe-commits
Author: ctopper
Date: Fri Oct 13 11:38:10 2017
New Revision: 315729

URL: http://llvm.org/viewvc/llvm-project?rev=315729&view=rev
Log:
[X86] Remove 'knm' defines from predefined-arch-macros.c test.

Direction seems to be that we dont' want to keep adding these, but I forgot to 
remove it from the test before I committed r315723.

Modified:
cfe/trunk/test/Preprocessor/predefined-arch-macros.c

Modified: cfe/trunk/test/Preprocessor/predefined-arch-macros.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/predefined-arch-macros.c?rev=315729&r1=315728&r2=315729&view=diff
==
--- cfe/trunk/test/Preprocessor/predefined-arch-macros.c (original)
+++ cfe/trunk/test/Preprocessor/predefined-arch-macros.c Fri Oct 13 11:38:10 
2017
@@ -815,9 +815,6 @@
 // CHECK_KNM_M32: #define __XSAVE__ 1
 // CHECK_KNM_M32: #define __i386 1
 // CHECK_KNM_M32: #define __i386__ 1
-// CHECK_KNM_M32: #define __knm 1
-// CHECK_KNM_M32: #define __knm__ 1
-// CHECK_KNM_M32: #define __tune_knm__ 1
 // CHECK_KNM_M32: #define i386 1
 
 // RUN: %clang -march=knm -m64 -E -dM %s -o - 2>&1 \
@@ -853,9 +850,6 @@
 // CHECK_KNM_M64: #define __XSAVE__ 1
 // CHECK_KNM_M64: #define __amd64 1
 // CHECK_KNM_M64: #define __amd64__ 1
-// CHECK_KNM_M64: #define __knm 1
-// CHECK_KNM_M64: #define __knm__ 1
-// CHECK_KNM_M64: #define __tune_knm__ 1
 // CHECK_KNM_M64: #define __x86_64 1
 // CHECK_KNM_M64: #define __x86_64__ 1
 //


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D38883: [CMake][OpenMP] Customize default offloading arch

2017-10-13 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits
gtbercea added inline comments.



Comment at: lib/Driver/ToolChains/Cuda.h:90
-  }
 };
 

tra wrote:
> gtbercea wrote:
> > gtbercea wrote:
> > > I would also like to keep the spirit of this code if not in this exact 
> > > form at least something that performs the same functionality.
> > @tra what's your opinion on this code? Should this stay, stay but modified 
> > to be more robust or taken out completely?
> There are currently no users for this. In general, I would rather not have 
> magically-changing default GPU based on how broken your CUDA installation is. 
> IMO it would be better to keep defaults static and fail if prerequisites are 
> not met.
I would have thought that it is up to the compiler to select, as default, the 
lowest viable compute capability. This is what this code aims to do (whether it 
actually does that's a separate issue :) ).



https://reviews.llvm.org/D38883



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r315731 - [CodeGen] EmitPointerWithAlignment() to generate TBAA info along with LValue base info

2017-10-13 Thread Ivan A. Kosarev via cfe-commits
Author: kosarev
Date: Fri Oct 13 11:40:18 2017
New Revision: 315731

URL: http://llvm.org/viewvc/llvm-project?rev=315731&view=rev
Log:
[CodeGen] EmitPointerWithAlignment() to generate TBAA info along with LValue 
base info

Differential Revision: https://reviews.llvm.org/D38796

Modified:
cfe/trunk/lib/CodeGen/CGExpr.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/lib/CodeGen/CodeGenModule.h
cfe/trunk/lib/CodeGen/CodeGenTBAA.cpp
cfe/trunk/lib/CodeGen/CodeGenTBAA.h

Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=315731&r1=315730&r2=315731&view=diff
==
--- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp Fri Oct 13 11:40:18 2017
@@ -916,7 +916,8 @@ void CodeGenModule::EmitExplicitCastExpr
 /// EmitPointerWithAlignment - Given an expression of pointer type, try to
 /// derive a more accurate bound on the alignment of the pointer.
 Address CodeGenFunction::EmitPointerWithAlignment(const Expr *E,
-  LValueBaseInfo *BaseInfo) {
+  LValueBaseInfo *BaseInfo,
+  TBAAAccessInfo *TBAAInfo) {
   // We allow this with ObjC object pointers because of fragile ABIs.
   assert(E->getType()->isPointerType() ||
  E->getType()->isObjCObjectPointerType());
@@ -936,19 +937,28 @@ Address CodeGenFunction::EmitPointerWith
 if (PtrTy->getPointeeType()->isVoidType())
   break;
 
-LValueBaseInfo InnerInfo;
-Address Addr = EmitPointerWithAlignment(CE->getSubExpr(), &InnerInfo);
-if (BaseInfo) *BaseInfo = InnerInfo;
+LValueBaseInfo InnerBaseInfo;
+TBAAAccessInfo InnerTBAAInfo;
+Address Addr = EmitPointerWithAlignment(CE->getSubExpr(),
+&InnerBaseInfo,
+&InnerTBAAInfo);
+if (BaseInfo) *BaseInfo = InnerBaseInfo;
+if (TBAAInfo) *TBAAInfo = InnerTBAAInfo;
 
 // If this is an explicit bitcast, and the source l-value is
 // opaque, honor the alignment of the casted-to type.
 if (isa(CE) &&
-InnerInfo.getAlignmentSource() != AlignmentSource::Decl) {
-  LValueBaseInfo ExpInfo;
+InnerBaseInfo.getAlignmentSource() != AlignmentSource::Decl) {
+  LValueBaseInfo TargetTypeBaseInfo;
+  TBAAAccessInfo TargetTypeTBAAInfo;
   CharUnits Align = getNaturalPointeeTypeAlignment(E->getType(),
-   &ExpInfo);
+   &TargetTypeBaseInfo,
+   
&TargetTypeTBAAInfo);
   if (BaseInfo)
-BaseInfo->mergeForCast(ExpInfo);
+BaseInfo->mergeForCast(TargetTypeBaseInfo);
+  if (TBAAInfo)
+*TBAAInfo = CGM.mergeTBAAInfoForCast(*TBAAInfo,
+ TargetTypeTBAAInfo);
   Addr = Address(Addr.getPointer(), Align);
 }
 
@@ -969,12 +979,13 @@ Address CodeGenFunction::EmitPointerWith
 
 // Array-to-pointer decay.
 case CK_ArrayToPointerDecay:
-  return EmitArrayToPointerDecay(CE->getSubExpr(), BaseInfo);
+  return EmitArrayToPointerDecay(CE->getSubExpr(), BaseInfo, TBAAInfo);
 
 // Derived-to-base conversions.
 case CK_UncheckedDerivedToBase:
 case CK_DerivedToBase: {
-  Address Addr = EmitPointerWithAlignment(CE->getSubExpr(), BaseInfo);
+  Address Addr = EmitPointerWithAlignment(CE->getSubExpr(), BaseInfo,
+  TBAAInfo);
   auto Derived = CE->getSubExpr()->getType()->getPointeeCXXRecordDecl();
   return GetAddressOfBaseClass(Addr, Derived,
CE->path_begin(), CE->path_end(),
@@ -994,6 +1005,7 @@ Address CodeGenFunction::EmitPointerWith
 if (UO->getOpcode() == UO_AddrOf) {
   LValue LV = EmitLValue(UO->getSubExpr());
   if (BaseInfo) *BaseInfo = LV.getBaseInfo();
+  if (TBAAInfo) *TBAAInfo = LV.getTBAAInfo();
   return LV.getAddress();
 }
   }
@@ -1001,7 +1013,8 @@ Address CodeGenFunction::EmitPointerWith
   // TODO: conditional operators, comma.
 
   // Otherwise, use the alignment of the type.
-  CharUnits Align = getNaturalPointeeTypeAlignment(E->getType(), BaseInfo);
+  CharUnits Align = getNaturalPointeeTypeAlignment(E->getType(), BaseInfo,
+   TBAAInfo);
   return Address(EmitScalarExpr(E), Align);
 }
 
@@ -2447,8 +2460,10 @@ LValue CodeGenFunction::EmitUnaryOpLValu
 assert(!T.isNull() && "CodeGenFunction::EmitUnaryOpL

[PATCH] D38796: [CodeGen] EmitPointerWithAlignment() to generate TBAA info along with LValue base info

2017-10-13 Thread Ivan Kosarev via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL315731: [CodeGen] EmitPointerWithAlignment() to generate 
TBAA info along with LValue… (authored by kosarev).

Changed prior to commit:
  https://reviews.llvm.org/D38796?vs=118599&id=118950#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D38796

Files:
  cfe/trunk/lib/CodeGen/CGExpr.cpp
  cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
  cfe/trunk/lib/CodeGen/CodeGenFunction.h
  cfe/trunk/lib/CodeGen/CodeGenModule.cpp
  cfe/trunk/lib/CodeGen/CodeGenModule.h
  cfe/trunk/lib/CodeGen/CodeGenTBAA.cpp
  cfe/trunk/lib/CodeGen/CodeGenTBAA.h

Index: cfe/trunk/lib/CodeGen/CodeGenTBAA.h
===
--- cfe/trunk/lib/CodeGen/CodeGenTBAA.h
+++ cfe/trunk/lib/CodeGen/CodeGenTBAA.h
@@ -47,6 +47,12 @@
 : TBAAAccessInfo(/* AccessType= */ nullptr)
   {}
 
+  bool operator==(const TBAAAccessInfo &Other) const {
+return BaseType == Other.BaseType &&
+   AccessType == Other.AccessType &&
+   Offset == Other.Offset;
+  }
+
   /// BaseType - The base/leading access type. May be null if this access
   /// descriptor represents an access that is not considered to be an access
   /// to an aggregate or union member.
@@ -136,6 +142,11 @@
   /// getMayAliasAccessInfo - Get TBAA information that represents may-alias
   /// accesses.
   TBAAAccessInfo getMayAliasAccessInfo();
+
+  /// mergeTBAAInfoForCast - Get merged TBAA information for the purpose of
+  /// type casts.
+  TBAAAccessInfo mergeTBAAInfoForCast(TBAAAccessInfo SourceInfo,
+  TBAAAccessInfo TargetInfo);
 };
 
 }  // end namespace CodeGen
Index: cfe/trunk/lib/CodeGen/CodeGenFunction.h
===
--- cfe/trunk/lib/CodeGen/CodeGenFunction.h
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.h
@@ -1942,7 +1942,8 @@
 TBAAAccessInfo *TBAAInfo = nullptr,
 bool forPointeeType = false);
   CharUnits getNaturalPointeeTypeAlignment(QualType T,
-   LValueBaseInfo *BaseInfo = nullptr);
+   LValueBaseInfo *BaseInfo = nullptr,
+   TBAAAccessInfo *TBAAInfo = nullptr);
 
   Address EmitLoadOfReference(Address Ref, const ReferenceType *RefTy,
   LValueBaseInfo *BaseInfo = nullptr,
@@ -3188,7 +3189,8 @@
   RValue EmitRValueForField(LValue LV, const FieldDecl *FD, SourceLocation Loc);
 
   Address EmitArrayToPointerDecay(const Expr *Array,
-  LValueBaseInfo *BaseInfo = nullptr);
+  LValueBaseInfo *BaseInfo = nullptr,
+  TBAAAccessInfo *TBAAInfo = nullptr);
 
   class ConstantEmission {
 llvm::PointerIntPair ValueAndIsReference;
@@ -3910,7 +3912,8 @@
   /// reasonable to just ignore the returned alignment when it isn't from an
   /// explicit source.
   Address EmitPointerWithAlignment(const Expr *Addr,
-   LValueBaseInfo *BaseInfo = nullptr);
+   LValueBaseInfo *BaseInfo = nullptr,
+   TBAAAccessInfo *TBAAInfo = nullptr);
 
   void EmitSanitizerStatReport(llvm::SanitizerStatKind SSK);
 
Index: cfe/trunk/lib/CodeGen/CGExpr.cpp
===
--- cfe/trunk/lib/CodeGen/CGExpr.cpp
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp
@@ -916,7 +916,8 @@
 /// EmitPointerWithAlignment - Given an expression of pointer type, try to
 /// derive a more accurate bound on the alignment of the pointer.
 Address CodeGenFunction::EmitPointerWithAlignment(const Expr *E,
-  LValueBaseInfo *BaseInfo) {
+  LValueBaseInfo *BaseInfo,
+  TBAAAccessInfo *TBAAInfo) {
   // We allow this with ObjC object pointers because of fragile ABIs.
   assert(E->getType()->isPointerType() ||
  E->getType()->isObjCObjectPointerType());
@@ -936,19 +937,28 @@
 if (PtrTy->getPointeeType()->isVoidType())
   break;
 
-LValueBaseInfo InnerInfo;
-Address Addr = EmitPointerWithAlignment(CE->getSubExpr(), &InnerInfo);
-if (BaseInfo) *BaseInfo = InnerInfo;
+LValueBaseInfo InnerBaseInfo;
+TBAAAccessInfo InnerTBAAInfo;
+Address Addr = EmitPointerWithAlignment(CE->getSubExpr(),
+&InnerBaseInfo,
+&InnerTBAAInfo);
+if (BaseInfo) *BaseInfo = InnerBaseInfo;
+if (TBAAInfo) *TBAAInfo = InnerTBAAInfo;
 
 // If this is an explicit bitcast, and the source l-value is
 // opaque, honor the alignment of the casted-to type

[PATCH] D38883: [CMake][OpenMP] Customize default offloading arch

2017-10-13 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits
gtbercea added inline comments.



Comment at: lib/Driver/ToolChains/Cuda.h:90
-  }
 };
 

gtbercea wrote:
> tra wrote:
> > gtbercea wrote:
> > > gtbercea wrote:
> > > > I would also like to keep the spirit of this code if not in this exact 
> > > > form at least something that performs the same functionality.
> > > @tra what's your opinion on this code? Should this stay, stay but 
> > > modified to be more robust or taken out completely?
> > There are currently no users for this. In general, I would rather not have 
> > magically-changing default GPU based on how broken your CUDA installation 
> > is. IMO it would be better to keep defaults static and fail if 
> > prerequisites are not met.
> I would have thought that it is up to the compiler to select, as default, the 
> lowest viable compute capability. This is what this code aims to do (whether 
> it actually does that's a separate issue :) ).
> 
The reason I added this code in the first place was to overcome the fact that 
something like a default of sm_30 may work on the K40 but once you go to newer 
Pascal, Volta GPUs then you need a new minimum compute capability that is 
supported.


https://reviews.llvm.org/D38883



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D38868: [OpenCL] Restrict swizzle length check to OpenCL mode

2017-10-13 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
bruno marked 2 inline comments as done.
bruno added inline comments.



Comment at: test/SemaOpenCL/vector_swizzle_length.cl:7
 void foo() {
-float8 f2 = (float8)(0, 0, 0, 0, 0, 0, 0, 0);
+float8 f2 = (float8){0, 0, 0, 0, 0, 0, 0, 0};
 

Anastasia wrote:
> Even though this works in Clang, ideally OpenCL vector literal is with 
> parenthesis (see s6.1.6).
Ok. I changed this to avoid warnings in C mode. Gonna change it back,



Comment at: test/SemaOpenCL/vector_swizzle_length.cl:13
+#else
+// expected-no-diagnostics
+(void)f2.s01234;

Anastasia wrote:
> I am not sure it's good idea to test C mode here since this is intended for 
> OpenCL only functionality. I think it might be better to separate C 
> functionality and keep under regular Sema tests. We could keep the same test 
> name to be able to identify similar functionality.
Ok!


https://reviews.llvm.org/D38868



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D38101: [Sema] Diagnose tautological comparison with type's min/max values

2017-10-13 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

Forgot to add, i really noticed/though about it just now, in 
https://reviews.llvm.org/D38871, because i did not encountered any warnings in 
that code in stage-2 builds.


Repository:
  rL LLVM

https://reviews.llvm.org/D38101



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D37897: [StaticAnalyzer] Fix ProgramState for static variables that are not written

2017-10-13 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added inline comments.



Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:123
+  // Is variable changed anywhere in TU?
+  for (const Decl *D : AMgr.getASTContext().getTranslationUnitDecl()->decls()) 
{
+if (isChanged(D, VD))

danielmarjamaki wrote:
> danielmarjamaki wrote:
> > dcoughlin wrote:
> > > Since you are calling `getInitialStateForGlobalStaticVar()` in 
> > > `getInitialState()` for each static variable declaration and 
> > > `getInitialState()` is called for each top-level function, you are doing 
> > > an n^3 operation in the size of the translation unit, which is going to 
> > > be very, very expensive for large translation units.
> > > 
> > > Have you considered doing the analysis for static variables that are 
> > > never changed during call-graph construction? This should be a linear 
> > > operation and doing it during call-graph construction would avoid an 
> > > extra walk of the entire translation unit.
> > hmm... could you tell me where the call-graph construction is that I can 
> > tweak?
> I think I found it: `clang/lib/Analysis/CallGraph.cpp`
I now track variable modifications in call-graph construction instead.



Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:155
+  Children.push(FuncBody);
+  while (!Children.empty()) {
+const Stmt *Child = Children.top();

szepet wrote:
> I think instead of this logic it would be better to use ConstStmtVisitor. In 
> this case it does quite the same thing in a (maybe?) more structured manner. 
> What do you think?
As far as I see ConstStmtVisitor is also recursive. Imho let's have readable 
code instead..


Repository:
  rL LLVM

https://reviews.llvm.org/D37897



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D37436: Initial implementation of C attributes (WG14 N2137)

2017-10-13 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

Ping.


https://reviews.llvm.org/D37436



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D37808: [clang-tidy] Add new hicpp-multiway-paths-covered check for missing branches

2017-10-13 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 118951.
JonasToth marked 2 inline comments as done.
JonasToth added a comment.

- major codechange


https://reviews.llvm.org/D37808

Files:
  clang-tidy/hicpp/CMakeLists.txt
  clang-tidy/hicpp/HICPPTidyModule.cpp
  clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp
  clang-tidy/hicpp/MultiwayPathsCoveredCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/hicpp-multiway-paths-covered.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/hicpp-multiway-paths-covered-else.cpp
  test/clang-tidy/hicpp-multiway-paths-covered.cpp

Index: test/clang-tidy/hicpp-multiway-paths-covered.cpp
===
--- /dev/null
+++ test/clang-tidy/hicpp-multiway-paths-covered.cpp
@@ -0,0 +1,445 @@
+// RUN: %check_clang_tidy %s hicpp-multiway-paths-covered %t
+
+enum OS { Mac,
+  Windows,
+  Linux };
+
+struct Bitfields {
+  unsigned UInt : 3;
+  int SInt : 1;
+};
+
+int return_integer() { return 42; }
+
+void bad_switch(int i) {
+  switch (i) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: switch with only one case; use if statement
+  case 0:
+break;
+  }
+  // No default in this switch
+  switch (i) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered codepath; add a default case
+  case 0:
+break;
+  case 1:
+break;
+  case 2:
+break;
+  }
+
+  // degenerate, maybe even warning
+  switch (i) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: degenerated switch without labels
+  }
+
+  switch (int j = return_integer()) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered codepath; add a default case
+  case 0:
+  case 1:
+  case 2:
+break;
+  }
+
+  // Degenerated, only default case.
+  switch (i) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: degenerated switch with default label only
+  default:
+break;
+  }
+
+  // Degenerated, only one case label and default case -> Better as if-stmt.
+  switch (i) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: switch could be better written as if/else statement
+  case 0:
+break;
+  default:
+break;
+  }
+
+  unsigned long long BigNumber = 0;
+  switch (BigNumber) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered codepath; add a default case
+  case 0:
+  case 1:
+break;
+  }
+
+  const int &IntRef = i;
+  switch (IntRef) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered codepath; add a default case
+  case 0:
+  case 1:
+break;
+  }
+
+  char C = 'A';
+  switch (C) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered codepath; add a default case
+  case 'A':
+break;
+  case 'B':
+break;
+  }
+
+  Bitfields Bf;
+  // UInt has 3 bits size.
+  switch (Bf.UInt) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered codepath; add a default case
+  case 0:
+  case 1:
+break;
+  }
+  // All paths explicitly covered.
+  switch (Bf.UInt) {
+  case 0:
+  case 1:
+  case 2:
+  case 3:
+  case 4:
+  case 5:
+  case 6:
+  case 7:
+break;
+  }
+  // SInt has 1 bit size, so this is somewhat degenerated.
+  switch (Bf.SInt) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: switch with only one case; use if statement
+  case 0:
+break;
+  }
+  // All paths explicitly covered.
+  switch (Bf.SInt) {
+  case 0:
+  case 1:
+break;
+  }
+}
+
+void unproblematic_switch(unsigned char c) {
+  switch (c) {
+  case 0:
+  case 1:
+  case 2:
+  case 3:
+  case 4:
+  case 5:
+  case 6:
+  case 7:
+  case 8:
+  case 9:
+  case 10:
+  case 11:
+  case 12:
+  case 13:
+  case 14:
+  case 15:
+  case 16:
+  case 17:
+  case 18:
+  case 19:
+  case 20:
+  case 21:
+  case 22:
+  case 23:
+  case 24:
+  case 25:
+  case 26:
+  case 27:
+  case 28:
+  case 29:
+  case 30:
+  case 31:
+  case 32:
+  case 33:
+  case 34:
+  case 35:
+  case 36:
+  case 37:
+  case 38:
+  case 39:
+  case 40:
+  case 41:
+  case 42:
+  case 43:
+  case 44:
+  case 45:
+  case 46:
+  case 47:
+  case 48:
+  case 49:
+  case 50:
+  case 51:
+  case 52:
+  case 53:
+  case 54:
+  case 55:
+  case 56:
+  case 57:
+  case 58:
+  case 59:
+  case 60:
+  case 61:
+  case 62:
+  case 63:
+  case 64:
+  case 65:
+  case 66:
+  case 67:
+  case 68:
+  case 69:
+  case 70:
+  case 71:
+  case 72:
+  case 73:
+  case 74:
+  case 75:
+  case 76:
+  case 77:
+  case 78:
+  case 79:
+  case 80:
+  case 81:
+  case 82:
+  case 83:
+  case 84:
+  case 85:
+  case 86:
+  case 87:
+  case 88:
+  case 89:
+  case 90:
+  case 91:
+  case 92:
+  case 93:
+  case 94:
+  case 95:
+  case 96:
+  case 97:
+  case 98:
+  case 99:
+  case 100:
+  case 101:
+  case 102:
+  case 103:
+  case 104:
+  case 105:
+  case 106:
+  case 107:
+  case 108:
+  case 109:
+  case 110:
+  case 111:
+  case 112:
+  case 113:
+  case 114:
+  case 115:
+  case 116:
+  case 117:
+  case 118:
+  case 119:
+  case 120:
+  case 121:
+  case 122:
+  case 123:
+  case 124:
+  case 125:
+  case 126:
+  case 127:
+  cas

[PATCH] D37808: [clang-tidy] Add new hicpp-multiway-paths-covered check for missing branches

2017-10-13 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth marked 6 inline comments as done.
JonasToth added inline comments.



Comment at: clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp:96
+// Only the default branch (we explicitly matched for default!) exists.
+if (CaseCount == 1) {
+  diag(SwitchWithDefault->getLocStart(),

JonasToth wrote:
> JDevlieghere wrote:
> > Why not a switch?
> I intent to check if all cases are explicitly covered.
> 
> In the testcases there is one switch with all numbers explicitly written, 
> meaning there is no need to add a default branch.
> 
> This would allow further 
> ```
> else if (CaseCount == MaximumPossibleCases) { /* No warning */ }
> ```
> path which is not modable with `switch`.
Woops. I mixed codeplaces (since bad duplication from my side). I did merge the 
diagnostics better, making this a natural fit for a switch.



Comment at: clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp:152
+// Should be written as an IfStmt.
+if (CaseCount == 1) {
+  diag(SwitchWithoutDefault->getLocStart(), "switch stmt with only one "

JDevlieghere wrote:
> JonasToth wrote:
> > JDevlieghere wrote:
> > > I'm aware that the message and fixme are different, but since the 
> > > structure is so similar to the handling of the other switch case, I 
> > > wonder if there is any chance we could extract the common parts?
> > I try to get something shorter.
> > Maybe 
> > ```
> > if(CaseCount == 1 && MatchedSwitch) {}
> > else if(CaseCount == 1 && MatchedElseIf) {}
> > ```
> > ?
> Wouldn't it be easier to have a function and pass as arguments the stuff 
> that's different? Both are `SwitchStmt`s so if you pass that + the diagnostic 
> message you should be able to share the other logic. 
I kinda rewrote the whole checking part. 
Updated diff comes in a sec. I found that bitfields are not handled as they 
should. 




https://reviews.llvm.org/D37808



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D37808: [clang-tidy] Add new hicpp-multiway-paths-covered check for missing branches

2017-10-13 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth marked 4 inline comments as done.
JonasToth added a comment.

Improved check a lot. Hope reviewers now have an easier time reading it.


https://reviews.llvm.org/D37808



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r315736 - [analyzer] RetainCount: Ignore annotations on user-made CFRetain wrappers.

2017-10-13 Thread Artem Dergachev via cfe-commits
Author: dergachev
Date: Fri Oct 13 12:10:42 2017
New Revision: 315736

URL: http://llvm.org/viewvc/llvm-project?rev=315736&view=rev
Log:
[analyzer] RetainCount: Ignore annotations on user-made CFRetain wrappers.

It is not uncommon for the users to make their own wrappers around
CoreFoundation's CFRetain and CFRelease functions that are defensive
against null references. In such cases CFRetain is often incorrectly
marked as CF_RETURNS_RETAINED. Ignore said annotation and treat such
wrappers similarly to the regular CFRetain.

rdar://problem/31699502
Differential Revision: https://reviews.llvm.org/D38877

Added:
cfe/trunk/test/Analysis/retain-release-safe.c
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp?rev=315736&r1=315735&r2=315736&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp Fri Oct 13 
12:10:42 2017
@@ -1170,6 +1170,11 @@ RetainSummaryManager::getFunctionSummary
   if (cocoa::isRefType(RetTy, "CF", FName)) {
 if (isRetain(FD, FName)) {
   S = getUnarySummary(FT, cfretain);
+  // CFRetain isn't supposed to be annotated. However, this may as well
+  // be a user-made "safe" CFRetain function that is incorrectly
+  // annotated as cf_returns_retained due to lack of better options.
+  // We want to ignore such annotation.
+  AllowAnnotations = false;
 } else if (isAutorelease(FD, FName)) {
   S = getUnarySummary(FT, cfautorelease);
   // The headers use cf_consumed, but we can fully model CFAutorelease

Added: cfe/trunk/test/Analysis/retain-release-safe.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/retain-release-safe.c?rev=315736&view=auto
==
--- cfe/trunk/test/Analysis/retain-release-safe.c (added)
+++ cfe/trunk/test/Analysis/retain-release-safe.c Fri Oct 13 12:10:42 2017
@@ -0,0 +1,72 @@
+// RUN: %clang_analyze_cc1 
-analyzer-checker=core,osx.coreFoundation.CFRetainRelease,osx.cocoa.RetainCount 
-verify %s
+
+#pragma clang arc_cf_code_audited begin
+typedef const void * CFTypeRef;
+extern CFTypeRef CFRetain(CFTypeRef cf);
+extern void CFRelease(CFTypeRef cf);
+#pragma clang arc_cf_code_audited end
+
+#define CF_RETURNS_RETAINED __attribute__((cf_returns_retained))
+#define CF_CONSUMED __attribute__((cf_consumed))
+
+extern CFTypeRef CFCreate() CF_RETURNS_RETAINED;
+
+// A "safe" variant of CFRetain that doesn't crash when a null pointer is
+// retained. This is often defined by users in a similar manner. The
+// CF_RETURNS_RETAINED annotation is misleading here, because the function
+// is not supposed to return an object with a +1 retain count. Instead, it
+// is supposed to return an object with +(N+1) retain count, where N is
+// the original retain count of 'cf'. However, there is no good annotation
+// to use in this case, and it is pointless to provide such annotation
+// because the only use cases would be CFRetain and SafeCFRetain.
+// So instead we teach the analyzer to be able to accept such code
+// and ignore the misplaced annotation.
+CFTypeRef SafeCFRetain(CFTypeRef cf) CF_RETURNS_RETAINED {
+  if (cf) {
+return CFRetain(cf);
+  }
+  return cf;
+}
+
+// A "safe" variant of CFRelease that doesn't crash when a null pointer is
+// released. The CF_CONSUMED annotation seems reasonable here.
+void SafeCFRelease(CFTypeRef CF_CONSUMED cf) {
+  if (cf)
+CFRelease(cf); // no-warning (when inlined)
+}
+
+void escape(CFTypeRef cf);
+
+void makeSureTestsWork() {
+  CFTypeRef cf = CFCreate();
+  CFRelease(cf);
+  CFRelease(cf); // expected-warning{{Reference-counted object is used after 
it is released}}
+}
+
+// Make sure we understand that the second SafeCFRetain doesn't return an
+// object with +1 retain count, which we won't be able to release twice.
+void falseOverrelease(CFTypeRef cf) {
+  SafeCFRetain(cf);
+  SafeCFRetain(cf);
+  SafeCFRelease(cf);
+  SafeCFRelease(cf); // no-warning after inlining this.
+}
+
+// Regular CFRelease() should behave similarly.
+void sameWithNormalRelease(CFTypeRef cf) {
+  SafeCFRetain(cf);
+  SafeCFRetain(cf);
+  CFRelease(cf);
+  CFRelease(cf); // no-warning
+}
+
+// Make sure we understand that the second SafeCFRetain doesn't return an
+// object with +1 retain count, which would no longer be owned by us after
+// it escapes to escape() and released once.
+void falseReleaseNotOwned(CFTypeRef cf) {
+  SafeCFRetain(cf);
+  SafeCFRetain(cf);
+  escape(cf);
+  SafeCFRelease(cf);
+  SafeCFRelease(cf); // no-warning after inlining this.
+}


___
cfe-comm

[PATCH] D38877: [analyzer] RetainCount: Accept "safe" CFRetain wrappers.

2017-10-13 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL315736: [analyzer] RetainCount: Ignore annotations on 
user-made CFRetain wrappers. (authored by dergachev).

Changed prior to commit:
  https://reviews.llvm.org/D38877?vs=118891&id=118953#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D38877

Files:
  cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
  cfe/trunk/test/Analysis/retain-release-safe.c


Index: cfe/trunk/test/Analysis/retain-release-safe.c
===
--- cfe/trunk/test/Analysis/retain-release-safe.c
+++ cfe/trunk/test/Analysis/retain-release-safe.c
@@ -0,0 +1,72 @@
+// RUN: %clang_analyze_cc1 
-analyzer-checker=core,osx.coreFoundation.CFRetainRelease,osx.cocoa.RetainCount 
-verify %s
+
+#pragma clang arc_cf_code_audited begin
+typedef const void * CFTypeRef;
+extern CFTypeRef CFRetain(CFTypeRef cf);
+extern void CFRelease(CFTypeRef cf);
+#pragma clang arc_cf_code_audited end
+
+#define CF_RETURNS_RETAINED __attribute__((cf_returns_retained))
+#define CF_CONSUMED __attribute__((cf_consumed))
+
+extern CFTypeRef CFCreate() CF_RETURNS_RETAINED;
+
+// A "safe" variant of CFRetain that doesn't crash when a null pointer is
+// retained. This is often defined by users in a similar manner. The
+// CF_RETURNS_RETAINED annotation is misleading here, because the function
+// is not supposed to return an object with a +1 retain count. Instead, it
+// is supposed to return an object with +(N+1) retain count, where N is
+// the original retain count of 'cf'. However, there is no good annotation
+// to use in this case, and it is pointless to provide such annotation
+// because the only use cases would be CFRetain and SafeCFRetain.
+// So instead we teach the analyzer to be able to accept such code
+// and ignore the misplaced annotation.
+CFTypeRef SafeCFRetain(CFTypeRef cf) CF_RETURNS_RETAINED {
+  if (cf) {
+return CFRetain(cf);
+  }
+  return cf;
+}
+
+// A "safe" variant of CFRelease that doesn't crash when a null pointer is
+// released. The CF_CONSUMED annotation seems reasonable here.
+void SafeCFRelease(CFTypeRef CF_CONSUMED cf) {
+  if (cf)
+CFRelease(cf); // no-warning (when inlined)
+}
+
+void escape(CFTypeRef cf);
+
+void makeSureTestsWork() {
+  CFTypeRef cf = CFCreate();
+  CFRelease(cf);
+  CFRelease(cf); // expected-warning{{Reference-counted object is used after 
it is released}}
+}
+
+// Make sure we understand that the second SafeCFRetain doesn't return an
+// object with +1 retain count, which we won't be able to release twice.
+void falseOverrelease(CFTypeRef cf) {
+  SafeCFRetain(cf);
+  SafeCFRetain(cf);
+  SafeCFRelease(cf);
+  SafeCFRelease(cf); // no-warning after inlining this.
+}
+
+// Regular CFRelease() should behave similarly.
+void sameWithNormalRelease(CFTypeRef cf) {
+  SafeCFRetain(cf);
+  SafeCFRetain(cf);
+  CFRelease(cf);
+  CFRelease(cf); // no-warning
+}
+
+// Make sure we understand that the second SafeCFRetain doesn't return an
+// object with +1 retain count, which would no longer be owned by us after
+// it escapes to escape() and released once.
+void falseReleaseNotOwned(CFTypeRef cf) {
+  SafeCFRetain(cf);
+  SafeCFRetain(cf);
+  escape(cf);
+  SafeCFRelease(cf);
+  SafeCFRelease(cf); // no-warning after inlining this.
+}
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
@@ -1170,6 +1170,11 @@
   if (cocoa::isRefType(RetTy, "CF", FName)) {
 if (isRetain(FD, FName)) {
   S = getUnarySummary(FT, cfretain);
+  // CFRetain isn't supposed to be annotated. However, this may as well
+  // be a user-made "safe" CFRetain function that is incorrectly
+  // annotated as cf_returns_retained due to lack of better options.
+  // We want to ignore such annotation.
+  AllowAnnotations = false;
 } else if (isAutorelease(FD, FName)) {
   S = getUnarySummary(FT, cfautorelease);
   // The headers use cf_consumed, but we can fully model CFAutorelease


Index: cfe/trunk/test/Analysis/retain-release-safe.c
===
--- cfe/trunk/test/Analysis/retain-release-safe.c
+++ cfe/trunk/test/Analysis/retain-release-safe.c
@@ -0,0 +1,72 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,osx.coreFoundation.CFRetainRelease,osx.cocoa.RetainCount -verify %s
+
+#pragma clang arc_cf_code_audited begin
+typedef const void * CFTypeRef;
+extern CFTypeRef CFRetain(CFTypeRef cf);
+extern void CFRelease(CFTypeRef cf);
+#pragma clang arc_cf_code_audited end
+
+#define CF_RETURNS_RETAINED __attribute__((cf_returns_retained))
+#define CF_CONSUMED __attribute__((cf_consumed))
+
+extern CFTypeRef CFCreate() CF_RETURNS_RETAINED;
+
+// 

[PATCH] D38773: [Sema] Add support for flexible array members in Obj-C.

2017-10-13 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:15055
   }
+  // If it is the last field is checked elsewhere.
 }

rjmccall wrote:
> vsapsai wrote:
> > rjmccall wrote:
> > > "Whether" rather than "If", please.  You should also leave a comment 
> > > about *why* we can't check this here — I assume because you also want to 
> > > complain about the last explicit ivar if there are synthesized ivars?  I 
> > > think we could at least still check this for `@interface` ivars.
> > Will change s/If/Whether/
> > 
> > Main reason for checking elsewhere is to check after ivars are synthesized, 
> > you are right. At some point I had this check done here but for detecting 
> > ivar-after-flexible-array on interface/extension, interface/implementation 
> > border I am relying on chained ObjCIvarDecl. But here I have `ArrayRef > *> Fields` so implementation will be different. I decided that it would be 
> > cleaner to perform the check only in DiagnoseVariableSizedIvars.
> Is there a reason to do any of the checking here, then?
No objective reason. I updated isIncompleteArrayType branch to avoid flexible 
array members rejected at line 15023

```lang=c++
} else if (!FDTy->isDependentType() &&
   RequireCompleteType(FD->getLocation(), FD->getType(),
   diag::err_field_incomplete)) {
```

and here I've added it for consistency. Will move to DiagnoseVariableSizedIvars 
and see if it works fine, don't expect any problems.


https://reviews.llvm.org/D38773



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D37808: [clang-tidy] Add new hicpp-multiway-paths-covered check for missing branches

2017-10-13 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment.

F5426271: llvm_lib_target_x86_paths 
Example output for `llvm/lib/Target/X86`
Running it over the whole `llvm/lib` codebase generates a lot of warnings. 
Please note, that it seems to be common to write code like this:

  int Val;
  switch(Val) {
case 1: // something
case 2: // something else
case 16: // magic
  }
  llvm_unreachable("reason");

In some cases it has been taken care that no fallthrough happens, but it's not 
so simple to spot. Using `default: llvm_unreachable("reason");` would comply 
with the check.


https://reviews.llvm.org/D37808



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D38900: libunwind: document tested FreeBSD configs and sort OS list

2017-10-13 Thread Ed Maste via Phabricator via cfe-commits
emaste created this revision.
Herald added subscribers: kristof.beyls, krytarowski, aemerson.

libunwind is known to work on FreeBSD/amd64 and FreeBSD/arm64, and is the 
default unwinder on both of those architectures.

While here sort the OS list.


https://reviews.llvm.org/D38900

Files:
  docs/index.rst


Index: docs/index.rst
===
--- docs/index.rst
+++ docs/index.rst
@@ -44,13 +44,14 @@
    
 OS   Arch CompilersUnwind Info
    
-Mac OS X i386, x86_64 Clang, GCC   DWARF CFI
+Any  i386, x86_64, ARMClangSjLj
+Bare Metal   ARM  Clang, GCC   EHABI
+FreeBSD  x86_64, ARM64ClangDWARF CFI
 iOS  ARM  ClangSjLj
-Linuxi386, x86_64, ARM64  Clang, GCC   DWARF CFI
 LinuxARM  Clang, GCC   EHABI
-Bare Metal   ARM  Clang, GCC   EHABI
+Linuxi386, x86_64, ARM64  Clang, GCC   DWARF CFI
+Mac OS X i386, x86_64 Clang, GCC   DWARF CFI
 NetBSD   x86_64   Clang, GCC   DWARF CFI
-Any  i386, x86_64, ARMClangSjLj
    
 
 The following minimum compiler versions are strongly recommended.


Index: docs/index.rst
===
--- docs/index.rst
+++ docs/index.rst
@@ -44,13 +44,14 @@
    
 OS   Arch CompilersUnwind Info
    
-Mac OS X i386, x86_64 Clang, GCC   DWARF CFI
+Any  i386, x86_64, ARMClangSjLj
+Bare Metal   ARM  Clang, GCC   EHABI
+FreeBSD  x86_64, ARM64ClangDWARF CFI
 iOS  ARM  ClangSjLj
-Linuxi386, x86_64, ARM64  Clang, GCC   DWARF CFI
 LinuxARM  Clang, GCC   EHABI
-Bare Metal   ARM  Clang, GCC   EHABI
+Linuxi386, x86_64, ARM64  Clang, GCC   DWARF CFI
+Mac OS X i386, x86_64 Clang, GCC   DWARF CFI
 NetBSD   x86_64   Clang, GCC   DWARF CFI
-Any  i386, x86_64, ARMClangSjLj
    
 
 The following minimum compiler versions are strongly recommended.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D38900: libunwind: document tested FreeBSD configs and sort OS list

2017-10-13 Thread Kamil Rytarowski via Phabricator via cfe-commits
krytarowski added a subscriber: joerg.
krytarowski added a comment.

@joerg might have insight on ppc, sparc64, arm on NetBSD.


https://reviews.llvm.org/D38900



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r315738 - [clang-refactor] Apply source replacements

2017-10-13 Thread Alex Lorenz via cfe-commits
Author: arphaman
Date: Fri Oct 13 12:42:05 2017
New Revision: 315738

URL: http://llvm.org/viewvc/llvm-project?rev=315738&view=rev
Log:
[clang-refactor] Apply source replacements

This commit actually brings clang-refactor to a usable state as it can now
apply the refactoring changes to source files.
The -selection option is now also fully supported.

Differential Revision: https://reviews.llvm.org/D38402

Added:
cfe/trunk/test/Refactor/tool-apply-replacements.cpp
cfe/trunk/test/Refactor/tool-selection-option.c
Modified:
cfe/trunk/include/clang/Frontend/CommandLineSourceLoc.h
cfe/trunk/tools/clang-refactor/ClangRefactor.cpp

Modified: cfe/trunk/include/clang/Frontend/CommandLineSourceLoc.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CommandLineSourceLoc.h?rev=315738&r1=315737&r2=315738&view=diff
==
--- cfe/trunk/include/clang/Frontend/CommandLineSourceLoc.h (original)
+++ cfe/trunk/include/clang/Frontend/CommandLineSourceLoc.h Fri Oct 13 12:42:05 
2017
@@ -51,6 +51,51 @@ public:
   }
 };
 
+/// A source range that has been parsed on the command line.
+struct ParsedSourceRange {
+  std::string FileName;
+  /// The starting location of the range. The first element is the line and
+  /// the second element is the column.
+  std::pair Begin;
+  /// The ending location of the range. The first element is the line and the
+  /// second element is the column.
+  std::pair End;
+
+  /// Returns a parsed source range from a string or None if the string is
+  /// invalid.
+  ///
+  /// These source string has the following format:
+  ///
+  /// file:start_line:start_column[-end_line:end_column]
+  ///
+  /// If the end line and column are omitted, the starting line and columns
+  /// are used as the end values.
+  static Optional fromString(StringRef Str) {
+std::pair RangeSplit;
+// Avoid splitting '-' when there's no end line & column as '-' might be
+// part of the filename.
+if (Str.count(':') > 2)
+  RangeSplit = Str.rsplit('-');
+else
+  RangeSplit = {Str, ""};
+auto Begin = ParsedSourceLocation::FromString(RangeSplit.first);
+if (Begin.FileName.empty())
+  return None;
+unsigned EndLine, EndColumn;
+if (RangeSplit.second.empty()) {
+  EndLine = Begin.Line;
+  EndColumn = Begin.Column;
+} else {
+  std::pair Split = RangeSplit.second.rsplit(':');
+  if (Split.first.getAsInteger(10, EndLine) ||
+  Split.second.getAsInteger(10, EndColumn))
+return None;
+}
+return ParsedSourceRange{std::move(Begin.FileName),
+ {Begin.Line, Begin.Column},
+ {EndLine, EndColumn}};
+  }
+};
 }
 
 namespace llvm {

Added: cfe/trunk/test/Refactor/tool-apply-replacements.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Refactor/tool-apply-replacements.cpp?rev=315738&view=auto
==
--- cfe/trunk/test/Refactor/tool-apply-replacements.cpp (added)
+++ cfe/trunk/test/Refactor/tool-apply-replacements.cpp Fri Oct 13 12:42:05 2017
@@ -0,0 +1,11 @@
+// RUN: rm -f %t.cp.cpp
+// RUN: cp %s %t.cp.cpp
+// RUN: clang-refactor local-rename -selection=%t.cp.cpp:9:7 -new-name=test 
%t.cp.cpp --
+// RUN: grep -v CHECK %t.cp.cpp | FileCheck %t.cp.cpp
+// RUN: cp %s %t.cp.cpp
+// RUN: clang-refactor local-rename -selection=%t.cp.cpp:9:7-9:15 
-new-name=test %t.cp.cpp --
+// RUN: grep -v CHECK %t.cp.cpp | FileCheck %t.cp.cpp
+
+class RenameMe {
+// CHECK: class test {
+};

Added: cfe/trunk/test/Refactor/tool-selection-option.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Refactor/tool-selection-option.c?rev=315738&view=auto
==
--- cfe/trunk/test/Refactor/tool-selection-option.c (added)
+++ cfe/trunk/test/Refactor/tool-selection-option.c Fri Oct 13 12:42:05 2017
@@ -0,0 +1,15 @@
+// RUN: rm -f %t.cp.c
+// RUN: cp %s %t.cp.c
+// RUN: clang-refactor local-rename -selection=%t.cp.c:6:5 -new-name=test -v 
%t.cp.c -- | FileCheck --check-prefix=CHECK1 %s
+// RUN: clang-refactor local-rename -selection=%t.cp.c:6:5-6:9 -new-name=test 
-v %t.cp.c -- | FileCheck --check-prefix=CHECK2 %s
+
+int test;
+
+// CHECK1: invoking action 'local-rename':
+// CHECK1-NEXT: -selection={{.*}}.cp.c:6:5 -> {{.*}}.cp.c:6:5
+
+// CHECK2: invoking action 'local-rename':
+// CHECK2-NEXT: -selection={{.*}}.cp.c:6:5 -> {{.*}}.cp.c:6:9
+
+// RUN: not clang-refactor local-rename -selection=%s:6:5 -new-name=test -v 
%t.cp.c -- 2>&1 | FileCheck --check-prefix=CHECK-FILE-ERR %s
+// CHECK-FILE-ERR: given file is not in the target TU

Modified: cfe/trunk/tools/clang-refactor/ClangRefactor.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-refactor/ClangRefactor.cpp?rev=315738&r1=315737&r2=315738&view=diff
=

[PATCH] D38402: [clang-refactor] Apply source replacements

2017-10-13 Thread Alex Lorenz via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL315738: [clang-refactor] Apply source replacements (authored 
by arphaman).

Changed prior to commit:
  https://reviews.llvm.org/D38402?vs=118471&id=118959#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D38402

Files:
  cfe/trunk/include/clang/Frontend/CommandLineSourceLoc.h
  cfe/trunk/test/Refactor/tool-apply-replacements.cpp
  cfe/trunk/test/Refactor/tool-selection-option.c
  cfe/trunk/tools/clang-refactor/ClangRefactor.cpp

Index: cfe/trunk/include/clang/Frontend/CommandLineSourceLoc.h
===
--- cfe/trunk/include/clang/Frontend/CommandLineSourceLoc.h
+++ cfe/trunk/include/clang/Frontend/CommandLineSourceLoc.h
@@ -51,6 +51,51 @@
   }
 };
 
+/// A source range that has been parsed on the command line.
+struct ParsedSourceRange {
+  std::string FileName;
+  /// The starting location of the range. The first element is the line and
+  /// the second element is the column.
+  std::pair Begin;
+  /// The ending location of the range. The first element is the line and the
+  /// second element is the column.
+  std::pair End;
+
+  /// Returns a parsed source range from a string or None if the string is
+  /// invalid.
+  ///
+  /// These source string has the following format:
+  ///
+  /// file:start_line:start_column[-end_line:end_column]
+  ///
+  /// If the end line and column are omitted, the starting line and columns
+  /// are used as the end values.
+  static Optional fromString(StringRef Str) {
+std::pair RangeSplit;
+// Avoid splitting '-' when there's no end line & column as '-' might be
+// part of the filename.
+if (Str.count(':') > 2)
+  RangeSplit = Str.rsplit('-');
+else
+  RangeSplit = {Str, ""};
+auto Begin = ParsedSourceLocation::FromString(RangeSplit.first);
+if (Begin.FileName.empty())
+  return None;
+unsigned EndLine, EndColumn;
+if (RangeSplit.second.empty()) {
+  EndLine = Begin.Line;
+  EndColumn = Begin.Column;
+} else {
+  std::pair Split = RangeSplit.second.rsplit(':');
+  if (Split.first.getAsInteger(10, EndLine) ||
+  Split.second.getAsInteger(10, EndColumn))
+return None;
+}
+return ParsedSourceRange{std::move(Begin.FileName),
+ {Begin.Line, Begin.Column},
+ {EndLine, EndColumn}};
+  }
+};
 }
 
 namespace llvm {
Index: cfe/trunk/test/Refactor/tool-selection-option.c
===
--- cfe/trunk/test/Refactor/tool-selection-option.c
+++ cfe/trunk/test/Refactor/tool-selection-option.c
@@ -0,0 +1,15 @@
+// RUN: rm -f %t.cp.c
+// RUN: cp %s %t.cp.c
+// RUN: clang-refactor local-rename -selection=%t.cp.c:6:5 -new-name=test -v %t.cp.c -- | FileCheck --check-prefix=CHECK1 %s
+// RUN: clang-refactor local-rename -selection=%t.cp.c:6:5-6:9 -new-name=test -v %t.cp.c -- | FileCheck --check-prefix=CHECK2 %s
+
+int test;
+
+// CHECK1: invoking action 'local-rename':
+// CHECK1-NEXT: -selection={{.*}}.cp.c:6:5 -> {{.*}}.cp.c:6:5
+
+// CHECK2: invoking action 'local-rename':
+// CHECK2-NEXT: -selection={{.*}}.cp.c:6:5 -> {{.*}}.cp.c:6:9
+
+// RUN: not clang-refactor local-rename -selection=%s:6:5 -new-name=test -v %t.cp.c -- 2>&1 | FileCheck --check-prefix=CHECK-FILE-ERR %s
+// CHECK-FILE-ERR: given file is not in the target TU
Index: cfe/trunk/test/Refactor/tool-apply-replacements.cpp
===
--- cfe/trunk/test/Refactor/tool-apply-replacements.cpp
+++ cfe/trunk/test/Refactor/tool-apply-replacements.cpp
@@ -0,0 +1,11 @@
+// RUN: rm -f %t.cp.cpp
+// RUN: cp %s %t.cp.cpp
+// RUN: clang-refactor local-rename -selection=%t.cp.cpp:9:7 -new-name=test %t.cp.cpp --
+// RUN: grep -v CHECK %t.cp.cpp | FileCheck %t.cp.cpp
+// RUN: cp %s %t.cp.cpp
+// RUN: clang-refactor local-rename -selection=%t.cp.cpp:9:7-9:15 -new-name=test %t.cp.cpp --
+// RUN: grep -v CHECK %t.cp.cpp | FileCheck %t.cp.cpp
+
+class RenameMe {
+// CHECK: class test {
+};
Index: cfe/trunk/tools/clang-refactor/ClangRefactor.cpp
===
--- cfe/trunk/tools/clang-refactor/ClangRefactor.cpp
+++ cfe/trunk/tools/clang-refactor/ClangRefactor.cpp
@@ -14,6 +14,7 @@
 //===--===//
 
 #include "TestSupport.h"
+#include "clang/Frontend/CommandLineSourceLoc.h"
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Refactoring.h"
@@ -54,7 +55,7 @@
 
   /// Prints any additional state associated with the selection argument to
   /// the given output stream.
-  virtual void print(raw_ostream &OS) = 0;
+  virtual void print(raw_ostream &OS) {}
 
   /// Returns a replacement refactoring result consumer (if any) that should
   /// consume the results of a r

[PATCH] D38110: [libunwind][MIPS]: Add support for unwinding in O32 and N64 processes.

2017-10-13 Thread John Baldwin via Phabricator via cfe-commits
bsdjhb updated this revision to Diff 118960.
bsdjhb added a comment.

- Check _ABI* rather than _MIPS_SIM.
- Save and restore lo/hi.
- Expand FIXME comment for more missing registers.
- Return UNW_SUCCESS from unw_getcontext().
- Use correct DWARF numbers for hi and lo and put hi first.
- Bump highest DWARF number for hi and lo.


https://reviews.llvm.org/D38110

Files:
  include/__libunwind_config.h
  include/libunwind.h
  src/Registers.hpp
  src/UnwindCursor.hpp
  src/UnwindRegistersRestore.S
  src/UnwindRegistersSave.S
  src/config.h
  src/libunwind.cpp

Index: src/libunwind.cpp
===
--- src/libunwind.cpp
+++ src/libunwind.cpp
@@ -58,8 +58,12 @@
 # define REGISTER_KIND Registers_arm
 #elif defined(__or1k__)
 # define REGISTER_KIND Registers_or1k
+#elif defined(__mips__) && defined(_ABIO32) && defined(__mips_soft_float)
+# define REGISTER_KIND Registers_mips_o32
+#elif defined(__mips__) && defined(_ABI64) && defined(__mips_soft_float)
+# define REGISTER_KIND Registers_mips_n64
 #elif defined(__mips__)
-# warning The MIPS architecture is not supported.
+# warning The MIPS architecture is not supported with this ABI and environment!
 #else
 # error Architecture not supported
 #endif
Index: src/config.h
===
--- src/config.h
+++ src/config.h
@@ -65,7 +65,7 @@
 defined(__ppc__) || defined(__ppc64__) ||  \
 (!defined(__APPLE__) && defined(__arm__)) ||   \
 (defined(__arm64__) || defined(__aarch64__)) ||\
-(defined(__APPLE__) && defined(__mips__))
+defined(__mips__)
 #define _LIBUNWIND_BUILD_ZERO_COST_APIS
 #endif
 
Index: src/UnwindRegistersSave.S
===
--- src/UnwindRegistersSave.S
+++ src/UnwindRegistersSave.S
@@ -87,6 +87,90 @@
   xorl  %eax, %eax# return UNW_ESUCCESS
   ret
 
+#elif defined(__mips__) && defined(_ABIO32) && defined(__mips_soft_float)
+
+#
+# extern int unw_getcontext(unw_context_t* thread_state)
+#
+# On entry:
+#  thread_state pointer is in a0 ($4)
+#
+# Only save registers preserved across calls.
+#
+DEFINE_LIBUNWIND_FUNCTION(unw_getcontext)
+  .set push
+  .set noat
+  .set noreorder
+  .set nomacro
+  # hi and lo
+  mfhi  $8
+  sw$8,  (4 * 33)($4)
+  mflo  $8
+  sw$8,  (4 * 34)($4)
+  # s0 - s7
+  sw$16, (4 * 16)($4)
+  sw$17, (4 * 17)($4)
+  sw$18, (4 * 18)($4)
+  sw$19, (4 * 19)($4)
+  sw$20, (4 * 20)($4)
+  sw$21, (4 * 21)($4)
+  sw$22, (4 * 22)($4)
+  sw$23, (4 * 23)($4)
+  # gp
+  sw$28, (4 * 28)($4)
+  # sp
+  sw$29, (4 * 29)($4)
+  # fp
+  sw$30, (4 * 30)($4)
+  # Store return address to pc
+  sw$31, (4 * 32)($4)
+  jr	$31
+  # return UNW_ESUCCESS
+  or$2, $0, $0
+  .set pop
+
+#elif defined(__mips__) && defined(_ABI64) && defined(__mips_soft_float)
+
+#
+# extern int unw_getcontext(unw_context_t* thread_state)
+#
+# On entry:
+#  thread_state pointer is in a0 ($4)
+#
+# Only save registers preserved across calls.
+#
+DEFINE_LIBUNWIND_FUNCTION(unw_getcontext)
+  .set push
+  .set noat
+  .set noreorder
+  .set nomacro
+  # hi and lo
+  mfhi  $8
+  sd$8,  (8 * 33)($4)
+  mflo  $8
+  sd$8,  (8 * 34)($4)
+  # s0 - s7
+  sd$16, (8 * 16)($4)
+  sd$17, (8 * 17)($4)
+  sd$18, (8 * 18)($4)
+  sd$19, (8 * 19)($4)
+  sd$20, (8 * 20)($4)
+  sd$21, (8 * 21)($4)
+  sd$22, (8 * 22)($4)
+  sd$23, (8 * 23)($4)
+  # gp
+  sd$28, (8 * 28)($4)
+  # sp
+  sd$29, (8 * 29)($4)
+  # fp
+  sd$30, (8 * 30)($4)
+  # Store return address to pc
+  sd$31, (8 * 32)($4)
+  jr	$31
+  # return UNW_ESUCCESS
+  or$2, $0, $0
+  .set pop
+
 # elif defined(__mips__)
 
 #
Index: src/UnwindRegistersRestore.S
===
--- src/UnwindRegistersRestore.S
+++ src/UnwindRegistersRestore.S
@@ -489,6 +489,118 @@
   l.jr r9
l.nop
 
+#elif defined(__mips__) && defined(_ABIO32) && defined(__mips_soft_float)
+
+//
+// void libunwind::Registers_mips_o32::jumpto()
+//
+// On entry:
+//  thread state pointer is in a0 ($4)
+//
+DEFINE_LIBUNWIND_PRIVATE_FUNCTION(_ZN9libunwind18Registers_mips_o326jumptoEv)
+  .set push
+  .set noat
+  .set noreorder
+  .set nomacro
+  // restore hi and lo
+  lw$8, (4 * 33)($4)
+  mthi  $8
+  lw$8, (4 * 34)($4)
+  mtlo  $8
+  // r0 is zero
+  lw$1, (4 * 1)($4)
+  lw$2, (4 * 2)($4)
+  lw$3, (4 * 3)($4)
+  // skip a0 for now
+  lw$5, (4 * 5)($4)
+  lw$6, (4 * 6)($4)
+  lw$7, (4 * 7)($4)
+  lw$8, (4 * 8)($4)
+  lw$9, (4 * 9)($4)
+  lw$10, (4 * 10)($4)
+  lw$11, (4 * 11)($4)
+  lw$12, (4 * 12)($4)
+  lw$13, (4 * 13)($4)
+  lw$14, (4 * 14)($4)
+  lw$15, (4 * 15)($4)
+  lw$16, (4 * 16)($4)
+  lw$17, (4 * 17)($4)
+  lw$18, (4 * 18)($4)
+  lw$19, (4 * 19)(

r315739 - Revert "[CodeGen] EmitPointerWithAlignment() to generate TBAA info along with LValue base info", r315731.

2017-10-13 Thread Ivan A. Kosarev via cfe-commits
Author: kosarev
Date: Fri Oct 13 12:55:01 2017
New Revision: 315739

URL: http://llvm.org/viewvc/llvm-project?rev=315739&view=rev
Log:
Revert "[CodeGen] EmitPointerWithAlignment() to generate TBAA info along with 
LValue base info", r315731.

With this change we fail on the clang-x86_64-linux-selfhost-modules builder.

Differential Revision: https://reviews.llvm.org/D38796

Modified:
cfe/trunk/lib/CodeGen/CGExpr.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/lib/CodeGen/CodeGenModule.h
cfe/trunk/lib/CodeGen/CodeGenTBAA.cpp
cfe/trunk/lib/CodeGen/CodeGenTBAA.h

Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=315739&r1=315738&r2=315739&view=diff
==
--- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp Fri Oct 13 12:55:01 2017
@@ -916,8 +916,7 @@ void CodeGenModule::EmitExplicitCastExpr
 /// EmitPointerWithAlignment - Given an expression of pointer type, try to
 /// derive a more accurate bound on the alignment of the pointer.
 Address CodeGenFunction::EmitPointerWithAlignment(const Expr *E,
-  LValueBaseInfo *BaseInfo,
-  TBAAAccessInfo *TBAAInfo) {
+  LValueBaseInfo *BaseInfo) {
   // We allow this with ObjC object pointers because of fragile ABIs.
   assert(E->getType()->isPointerType() ||
  E->getType()->isObjCObjectPointerType());
@@ -937,28 +936,19 @@ Address CodeGenFunction::EmitPointerWith
 if (PtrTy->getPointeeType()->isVoidType())
   break;
 
-LValueBaseInfo InnerBaseInfo;
-TBAAAccessInfo InnerTBAAInfo;
-Address Addr = EmitPointerWithAlignment(CE->getSubExpr(),
-&InnerBaseInfo,
-&InnerTBAAInfo);
-if (BaseInfo) *BaseInfo = InnerBaseInfo;
-if (TBAAInfo) *TBAAInfo = InnerTBAAInfo;
+LValueBaseInfo InnerInfo;
+Address Addr = EmitPointerWithAlignment(CE->getSubExpr(), &InnerInfo);
+if (BaseInfo) *BaseInfo = InnerInfo;
 
 // If this is an explicit bitcast, and the source l-value is
 // opaque, honor the alignment of the casted-to type.
 if (isa(CE) &&
-InnerBaseInfo.getAlignmentSource() != AlignmentSource::Decl) {
-  LValueBaseInfo TargetTypeBaseInfo;
-  TBAAAccessInfo TargetTypeTBAAInfo;
+InnerInfo.getAlignmentSource() != AlignmentSource::Decl) {
+  LValueBaseInfo ExpInfo;
   CharUnits Align = getNaturalPointeeTypeAlignment(E->getType(),
-   &TargetTypeBaseInfo,
-   
&TargetTypeTBAAInfo);
+   &ExpInfo);
   if (BaseInfo)
-BaseInfo->mergeForCast(TargetTypeBaseInfo);
-  if (TBAAInfo)
-*TBAAInfo = CGM.mergeTBAAInfoForCast(*TBAAInfo,
- TargetTypeTBAAInfo);
+BaseInfo->mergeForCast(ExpInfo);
   Addr = Address(Addr.getPointer(), Align);
 }
 
@@ -979,13 +969,12 @@ Address CodeGenFunction::EmitPointerWith
 
 // Array-to-pointer decay.
 case CK_ArrayToPointerDecay:
-  return EmitArrayToPointerDecay(CE->getSubExpr(), BaseInfo, TBAAInfo);
+  return EmitArrayToPointerDecay(CE->getSubExpr(), BaseInfo);
 
 // Derived-to-base conversions.
 case CK_UncheckedDerivedToBase:
 case CK_DerivedToBase: {
-  Address Addr = EmitPointerWithAlignment(CE->getSubExpr(), BaseInfo,
-  TBAAInfo);
+  Address Addr = EmitPointerWithAlignment(CE->getSubExpr(), BaseInfo);
   auto Derived = CE->getSubExpr()->getType()->getPointeeCXXRecordDecl();
   return GetAddressOfBaseClass(Addr, Derived,
CE->path_begin(), CE->path_end(),
@@ -1005,7 +994,6 @@ Address CodeGenFunction::EmitPointerWith
 if (UO->getOpcode() == UO_AddrOf) {
   LValue LV = EmitLValue(UO->getSubExpr());
   if (BaseInfo) *BaseInfo = LV.getBaseInfo();
-  if (TBAAInfo) *TBAAInfo = LV.getTBAAInfo();
   return LV.getAddress();
 }
   }
@@ -1013,8 +1001,7 @@ Address CodeGenFunction::EmitPointerWith
   // TODO: conditional operators, comma.
 
   // Otherwise, use the alignment of the type.
-  CharUnits Align = getNaturalPointeeTypeAlignment(E->getType(), BaseInfo,
-   TBAAInfo);
+  CharUnits Align = getNaturalPointeeTypeAlignment(E->getType(), BaseInfo);
   return Address(EmitScalarExpr(E), Align);
 }
 
@@ -2460,10 +2447,8 @@ L

[PATCH] D38883: [CMake][OpenMP] Customize default offloading arch

2017-10-13 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld updated this revision to Diff 118961.
Hahnfeld marked an inline comment as done.
Hahnfeld edited the summary of this revision.
Hahnfeld added a comment.

Check that the user didn't specify a value lower than `sm_30` and re-add some 
code as discussed.


https://reviews.llvm.org/D38883

Files:
  CMakeLists.txt
  include/clang/Config/config.h.cmake
  lib/Driver/ToolChains/Cuda.cpp
  lib/Driver/ToolChains/Cuda.h


Index: lib/Driver/ToolChains/Cuda.h
===
--- lib/Driver/ToolChains/Cuda.h
+++ lib/Driver/ToolChains/Cuda.h
@@ -76,17 +76,6 @@
   std::string getLibDeviceFile(StringRef Gpu) const {
 return LibDeviceMap.lookup(Gpu);
   }
-  /// \brief Get lowest available compute capability
-  /// for which a libdevice library exists.
-  std::string getLowestExistingArch() const {
-std::string LibDeviceFile;
-for (auto key : LibDeviceMap.keys()) {
-  LibDeviceFile = LibDeviceMap.lookup(key);
-  if (!LibDeviceFile.empty())
-return key;
-}
-return "sm_20";
-  }
 };
 
 namespace tools {
Index: lib/Driver/ToolChains/Cuda.cpp
===
--- lib/Driver/ToolChains/Cuda.cpp
+++ lib/Driver/ToolChains/Cuda.cpp
@@ -551,9 +551,9 @@
   // flags are not duplicated.
   // Also append the compute capability.
   if (DeviceOffloadKind == Action::OFK_OpenMP) {
-for (Arg *A : Args){
+for (Arg *A : Args) {
   bool IsDuplicate = false;
-  for (Arg *DALArg : *DAL){
+  for (Arg *DALArg : *DAL) {
 if (A == DALArg) {
   IsDuplicate = true;
   break;
@@ -564,14 +564,9 @@
 }
 
 StringRef Arch = DAL->getLastArgValue(options::OPT_march_EQ);
-if (Arch.empty()) {
-  // Default compute capability for CUDA toolchain is the
-  // lowest compute capability supported by the installed
-  // CUDA version.
-  DAL->AddJoinedArg(nullptr,
-  Opts.getOption(options::OPT_march_EQ),
-  CudaInstallation.getLowestExistingArch());
-}
+if (Arch.empty())
+  DAL->AddJoinedArg(nullptr, Opts.getOption(options::OPT_march_EQ),
+CLANG_OPENMP_NVPTX_DEFAULT_ARCH);
 
 return DAL;
   }
Index: include/clang/Config/config.h.cmake
===
--- include/clang/Config/config.h.cmake
+++ include/clang/Config/config.h.cmake
@@ -20,6 +20,9 @@
 /* Default OpenMP runtime used by -fopenmp. */
 #define CLANG_DEFAULT_OPENMP_RUNTIME "${CLANG_DEFAULT_OPENMP_RUNTIME}"
 
+/* Default architecture for OpenMP offloading to Nvidia GPUs. */
+#define CLANG_OPENMP_NVPTX_DEFAULT_ARCH "${CLANG_OPENMP_NVPTX_DEFAULT_ARCH}"
+
 /* Multilib suffix for libdir. */
 #define CLANG_LIBDIR_SUFFIX "${CLANG_LIBDIR_SUFFIX}"
 
Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -235,6 +235,17 @@
 set(CLANG_DEFAULT_OPENMP_RUNTIME "libomp" CACHE STRING
   "Default OpenMP runtime used by -fopenmp.")
 
+# OpenMP offloading requires at least sm_30 because we use shuffle instructions
+# to generate efficient code for reductions.
+set(CLANG_OPENMP_NVPTX_DEFAULT_ARCH "sm_30" CACHE STRING
+  "Default architecture for OpenMP offloading to Nvidia GPUs.")
+string(REGEX MATCH "^sm_([0-9]+)$" MATCHED_ARCH 
"${CLANG_OPENMP_NVPTX_DEFAULT_ARCH}")
+if (NOT DEFINED MATCHED_ARCH OR "${CMAKE_MATCH_1}" LESS 30)
+  message(WARNING "Resetting default architecture for OpenMP offloading to 
Nvidia GPUs to sm_30")
+  set(CLANG_OPENMP_NVPTX_DEFAULT_ARCH "sm_30" CACHE STRING
+"Default architecture for OpenMP offloading to Nvidia GPUs." FORCE)
+endif()
+
 set(CLANG_VENDOR ${PACKAGE_VENDOR} CACHE STRING
   "Vendor-specific text for showing with version information.")
 


Index: lib/Driver/ToolChains/Cuda.h
===
--- lib/Driver/ToolChains/Cuda.h
+++ lib/Driver/ToolChains/Cuda.h
@@ -76,17 +76,6 @@
   std::string getLibDeviceFile(StringRef Gpu) const {
 return LibDeviceMap.lookup(Gpu);
   }
-  /// \brief Get lowest available compute capability
-  /// for which a libdevice library exists.
-  std::string getLowestExistingArch() const {
-std::string LibDeviceFile;
-for (auto key : LibDeviceMap.keys()) {
-  LibDeviceFile = LibDeviceMap.lookup(key);
-  if (!LibDeviceFile.empty())
-return key;
-}
-return "sm_20";
-  }
 };
 
 namespace tools {
Index: lib/Driver/ToolChains/Cuda.cpp
===
--- lib/Driver/ToolChains/Cuda.cpp
+++ lib/Driver/ToolChains/Cuda.cpp
@@ -551,9 +551,9 @@
   // flags are not duplicated.
   // Also append the compute capability.
   if (DeviceOffloadKind == Action::OFK_OpenMP) {
-for (Arg *A : Args){
+for (Arg *A : Args) {
   bool IsDuplicate = false;
-  for (Arg *DALArg : *DAL){
+  for (Arg *DALArg : *DAL) {
 if (A == DALArg) {
 

[PATCH] D38901: [CUDA] Require libdevice only if needed

2017-10-13 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld created this revision.

If the user passes -nocudalib, we can live without it being present.
Simplify the code by just checking whether LibDeviceMap is empty.


https://reviews.llvm.org/D38901

Files:
  lib/Driver/ToolChains/Cuda.cpp


Index: lib/Driver/ToolChains/Cuda.cpp
===
--- lib/Driver/ToolChains/Cuda.cpp
+++ lib/Driver/ToolChains/Cuda.cpp
@@ -167,17 +167,9 @@
   }
 }
 
-// This code prevents IsValid from being set when
-// no libdevice has been found.
-bool allEmpty = true;
-std::string LibDeviceFile;
-for (auto key : LibDeviceMap.keys()) {
-  LibDeviceFile = LibDeviceMap.lookup(key);
-  if (!LibDeviceFile.empty())
-allEmpty = false;
-}
-
-if (allEmpty)
+// Check that we have found at least one libdevice that we can link in if
+// -nocudalib hasn't been specified.
+if (LibDeviceMap.empty() && !Args.hasArg(options::OPT_nocudalib))
   continue;
 
 IsValid = true;


Index: lib/Driver/ToolChains/Cuda.cpp
===
--- lib/Driver/ToolChains/Cuda.cpp
+++ lib/Driver/ToolChains/Cuda.cpp
@@ -167,17 +167,9 @@
   }
 }
 
-// This code prevents IsValid from being set when
-// no libdevice has been found.
-bool allEmpty = true;
-std::string LibDeviceFile;
-for (auto key : LibDeviceMap.keys()) {
-  LibDeviceFile = LibDeviceMap.lookup(key);
-  if (!LibDeviceFile.empty())
-allEmpty = false;
-}
-
-if (allEmpty)
+// Check that we have found at least one libdevice that we can link in if
+// -nocudalib hasn't been specified.
+if (LibDeviceMap.empty() && !Args.hasArg(options::OPT_nocudalib))
   continue;
 
 IsValid = true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D38901: [CUDA] Require libdevice only if needed

2017-10-13 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

The change could use a test.


https://reviews.llvm.org/D38901



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r315741 - Basic: adjust attributes on `nan` LIBBUILTINs

2017-10-13 Thread Saleem Abdulrasool via cfe-commits
Author: compnerd
Date: Fri Oct 13 13:07:30 2017
New Revision: 315741

URL: http://llvm.org/viewvc/llvm-project?rev=315741&view=rev
Log:
Basic: adjust attributes on `nan` LIBBUILTINs

The `nan` family of functions will inspect the contents of the parameter
that they are passed. As a result, the function cannot be annotated as
`const`.  The documentation of the `const` attribute explicitly states
this:
  Note that a function that has pointer arguments and examines the data
  pointed to must not be declared const.
Adjust the annotations on this family of functions.

Modified:
cfe/trunk/include/clang/Basic/Builtins.def
cfe/trunk/test/CodeGen/libcall-declarations.c

Modified: cfe/trunk/include/clang/Basic/Builtins.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Builtins.def?rev=315741&r1=315740&r2=315741&view=diff
==
--- cfe/trunk/include/clang/Basic/Builtins.def (original)
+++ cfe/trunk/include/clang/Basic/Builtins.def Fri Oct 13 13:07:30 2017
@@ -1008,9 +1008,9 @@ LIBBUILTIN(modf, "ddd*", "fn", "math.h",
 LIBBUILTIN(modff, "fff*", "fn", "math.h", ALL_LANGUAGES)
 LIBBUILTIN(modfl, "LdLdLd*", "fn", "math.h", ALL_LANGUAGES)
 
-LIBBUILTIN(nan,  "dcC*", "fnc", "math.h", ALL_LANGUAGES)
-LIBBUILTIN(nanf, "fcC*", "fnc", "math.h", ALL_LANGUAGES)
-LIBBUILTIN(nanl, "LdcC*", "fnc", "math.h", ALL_LANGUAGES)
+LIBBUILTIN(nan,  "dcC*", "fn", "math.h", ALL_LANGUAGES)
+LIBBUILTIN(nanf, "fcC*", "fn", "math.h", ALL_LANGUAGES)
+LIBBUILTIN(nanl, "LdcC*", "fn", "math.h", ALL_LANGUAGES)
 
 LIBBUILTIN(pow, "ddd", "fne", "math.h", ALL_LANGUAGES)
 LIBBUILTIN(powf, "fff", "fne", "math.h", ALL_LANGUAGES)

Modified: cfe/trunk/test/CodeGen/libcall-declarations.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/libcall-declarations.c?rev=315741&r1=315740&r2=315741&view=diff
==
--- cfe/trunk/test/CodeGen/libcall-declarations.c (original)
+++ cfe/trunk/test/CodeGen/libcall-declarations.c Fri Oct 13 13:07:30 2017
@@ -312,314 +312,314 @@ void *use[] = {
   F(__cospif),   F(__tanpi),F(__tanpif),   F(__exp10), F(__exp10f)
 };
 
-// CHECK-NOERRNO: declare double @atan2(double, double) [[NUW:#[0-9]+]]
-// CHECK-NOERRNO: declare float @atan2f(float, float) [[NUW]]
-// CHECK-NOERRNO: declare x86_fp80 @atan2l(x86_fp80, x86_fp80) [[NUW]]
-// CHECK-NOERRNO: declare i32 @abs(i32) [[NUW]]
-// CHECK-NOERRNO: declare i64 @labs(i64) [[NUW]]
-// CHECK-NOERRNO: declare i64 @llabs(i64) [[NUW]]
-// CHECK-NOERRNO: declare double @copysign(double, double) [[NUW]]
-// CHECK-NOERRNO: declare float @copysignf(float, float) [[NUW]]
-// CHECK-NOERRNO: declare x86_fp80 @copysignl(x86_fp80, x86_fp80) [[NUW]]
-// CHECK-NOERRNO: declare double @fabs(double) [[NUW]]
-// CHECK-NOERRNO: declare float @fabsf(float) [[NUW]]
-// CHECK-NOERRNO: declare x86_fp80 @fabsl(x86_fp80) [[NUW]]
-// CHECK-NOERRNO: declare double @fmod(double, double) [[NUW]]
-// CHECK-NOERRNO: declare float @fmodf(float, float) [[NUW]]
-// CHECK-NOERRNO: declare x86_fp80 @fmodl(x86_fp80, x86_fp80) [[NUW]]
-// CHECK-NOERRNO: declare double @ldexp(double, i32) [[NUW]]
-// CHECK-NOERRNO: declare float @ldexpf(float, i32) [[NUW]]
-// CHECK-NOERRNO: declare x86_fp80 @ldexpl(x86_fp80, i32) [[NUW]]
-// CHECK-NOERRNO: declare double @nan(i8*) [[NUW]]
+// CHECK-NOERRNO: declare double @atan2(double, double) [[NUWRN:#[0-9]+]]
+// CHECK-NOERRNO: declare float @atan2f(float, float) [[NUWRN]]
+// CHECK-NOERRNO: declare x86_fp80 @atan2l(x86_fp80, x86_fp80) [[NUWRN]]
+// CHECK-NOERRNO: declare i32 @abs(i32) [[NUWRN]]
+// CHECK-NOERRNO: declare i64 @labs(i64) [[NUWRN]]
+// CHECK-NOERRNO: declare i64 @llabs(i64) [[NUWRN]]
+// CHECK-NOERRNO: declare double @copysign(double, double) [[NUWRN]]
+// CHECK-NOERRNO: declare float @copysignf(float, float) [[NUWRN]]
+// CHECK-NOERRNO: declare x86_fp80 @copysignl(x86_fp80, x86_fp80) [[NUWRN]]
+// CHECK-NOERRNO: declare double @fabs(double) [[NUWRN]]
+// CHECK-NOERRNO: declare float @fabsf(float) [[NUWRN]]
+// CHECK-NOERRNO: declare x86_fp80 @fabsl(x86_fp80) [[NUWRN]]
+// CHECK-NOERRNO: declare double @fmod(double, double) [[NUWRN]]
+// CHECK-NOERRNO: declare float @fmodf(float, float) [[NUWRN]]
+// CHECK-NOERRNO: declare x86_fp80 @fmodl(x86_fp80, x86_fp80) [[NUWRN]]
+// CHECK-NOERRNO: declare double @ldexp(double, i32) [[NUWRN]]
+// CHECK-NOERRNO: declare float @ldexpf(float, i32) [[NUWRN]]
+// CHECK-NOERRNO: declare x86_fp80 @ldexpl(x86_fp80, i32) [[NUWRN]]
+// CHECK-NOERRNO: declare double @nan(i8*) [[NUW:#[0-9]+]]
 // CHECK-NOERRNO: declare float @nanf(i8*) [[NUW]]
 // CHECK-NOERRNO: declare x86_fp80 @nanl(i8*) [[NUW]]
-// CHECK-NOERRNO: declare double @pow(double, double) [[NUW]]
-// CHECK-NOERRNO: declare float @powf(float, float) [[NUW]]
-// CHECK-NOERRNO: declare x86_fp80 @powl(x86_fp80, x86_fp80) [[NUW]]
-// CHECK-NOERRNO: declare double @acos(double) [[NUW]]
-// CHE

[PATCH] D38797: [analyzer] CStringChecker: pr34460: Admit that some casts are hard to model.

2017-10-13 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL315742: [analyzer] CStringChecker: pr34460: Avoid a crash 
when a cast is not modeled. (authored by dergachev).

Changed prior to commit:
  https://reviews.llvm.org/D38797?vs=118597&id=118964#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D38797

Files:
  cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  cfe/trunk/test/Analysis/bstring.cpp
  cfe/trunk/test/Analysis/casts.c

Index: cfe/trunk/test/Analysis/casts.c
===
--- cfe/trunk/test/Analysis/casts.c
+++ cfe/trunk/test/Analysis/casts.c
@@ -123,3 +123,29 @@
   int x = (int) p;
   clang_analyzer_eval(++x < 10); // no-crash // expected-warning{{UNKNOWN}}
 }
+
+void multiDimensionalArrayPointerCasts() {
+  static int x[10][10];
+  int *y1 = &(x[3][5]);
+  char *z = ((char *) y1) + 2;
+  int *y2 = (int *)(z - 2);
+  int *y3 = ((int *)x) + 35; // This is offset for [3][5].
+
+  clang_analyzer_eval(y1 == y2); // expected-warning{{TRUE}}
+
+  // FIXME: should be FALSE (i.e. equal pointers).
+  clang_analyzer_eval(y1 - y2); // expected-warning{{UNKNOWN}}
+  // FIXME: should be TRUE (i.e. same symbol).
+  clang_analyzer_eval(*y1 == *y2); // expected-warning{{UNKNOWN}}
+
+  clang_analyzer_eval(*((char *)y1) == *((char *) y2)); // expected-warning{{TRUE}}
+
+  clang_analyzer_eval(y1 == y3); // expected-warning{{TRUE}}
+
+  // FIXME: should be FALSE (i.e. equal pointers).
+  clang_analyzer_eval(y1 - y3); // expected-warning{{UNKNOWN}}
+  // FIXME: should be TRUE (i.e. same symbol).
+  clang_analyzer_eval(*y1 == *y3); // expected-warning{{UNKNOWN}}
+
+  clang_analyzer_eval(*((char *)y1) == *((char *) y3)); // expected-warning{{TRUE}}
+}
Index: cfe/trunk/test/Analysis/bstring.cpp
===
--- cfe/trunk/test/Analysis/bstring.cpp
+++ cfe/trunk/test/Analysis/bstring.cpp
@@ -1,8 +1,35 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.cstring,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -verify %s
+// RUN: %clang_analyze_cc1 -DUSE_BUILTINS -analyzer-checker=core,unix.cstring,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -verify %s
+// RUN: %clang_analyze_cc1 -DVARIANT -analyzer-checker=core,unix.cstring,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -verify %s
+// RUN: %clang_analyze_cc1 -DUSE_BUILTINS -DVARIANT -analyzer-checker=core,unix.cstring,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -verify %s
 
 #include "Inputs/system-header-simulator-cxx.h"
 #include "Inputs/system-header-simulator-for-malloc.h"
 
+// This provides us with four possible mempcpy() definitions.
+// See also comments in bstring.c.
+
+#ifdef USE_BUILTINS
+#define BUILTIN(f) __builtin_##f
+#else /* USE_BUILTINS */
+#define BUILTIN(f) f
+#endif /* USE_BUILTINS */
+
+#ifdef VARIANT
+
+#define __mempcpy_chk BUILTIN(__mempcpy_chk)
+void *__mempcpy_chk(void *__restrict__ s1, const void *__restrict__ s2,
+size_t n, size_t destlen);
+
+#define mempcpy(a,b,c) __mempcpy_chk(a,b,c,(size_t)-1)
+
+#else /* VARIANT */
+
+#define mempcpy BUILTIN(mempcpy)
+void *mempcpy(void *__restrict__ s1, const void *__restrict__ s2, size_t n);
+
+#endif /* VARIANT */
+
 void clang_analyzer_eval(int);
 
 int *testStdCopyInvalidatesBuffer(std::vector v) {
@@ -36,3 +63,17 @@
 
   return buf;
 }
+
+namespace pr34460 {
+short a;
+class b {
+  int c;
+  long g;
+  void d() {
+int e = c;
+f += e;
+mempcpy(f, &a, g);
+  }
+  unsigned *f;
+};
+}
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -1050,31 +1050,22 @@
 // If this is mempcpy, get the byte after the last byte copied and
 // bind the expr.
 if (IsMempcpy) {
-  loc::MemRegionVal destRegVal = destVal.castAs();
-
-  // Get the length to copy.
-  if (Optional lenValNonLoc = sizeVal.getAs()) {
-// Get the byte after the last byte copied.
-SValBuilder &SvalBuilder = C.getSValBuilder();
-ASTContext &Ctx = SvalBuilder.getContext();
-QualType CharPtrTy = Ctx.getPointerType(Ctx.CharTy);
-loc::MemRegionVal DestRegCharVal = SvalBuilder.evalCast(destRegVal,
-  CharPtrTy, Dest->getType()).castAs();
-SVal lastElement = C.getSValBuilder().evalBinOpLN(state, BO_Add,
-  DestRegCharVal,
-  *lenValNonLoc,
-  Dest->getType());
-
-// The byte after the last byte copied is the return value.
-state = state->BindExpr(CE, LCtx, lastElement);
-  } else {
-// If we don't know how much we copied,

r315742 - [analyzer] CStringChecker: pr34460: Avoid a crash when a cast is not modeled.

2017-10-13 Thread Artem Dergachev via cfe-commits
Author: dergachev
Date: Fri Oct 13 13:11:00 2017
New Revision: 315742

URL: http://llvm.org/viewvc/llvm-project?rev=315742&view=rev
Log:
[analyzer] CStringChecker: pr34460: Avoid a crash when a cast is not modeled.

The checker used to crash when a mempcpy's length argument is symbolic. In this
case the cast from 'void *' to 'char *' failed because the respective
ElementRegion that represents cast is hard to add on top of the existing
ElementRegion that represents the offset to the last copied byte, while
preseving a sane memory region structure.

Additionally, a few test cases are added (to casts.c) which demonstrate problems
caused by existing sloppy work we do with multi-layer ElementRegions. If said
cast would be modeled properly in the future, these tests would need to be
taken into account.

Differential Revision: https://reviews.llvm.org/D38797

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
cfe/trunk/test/Analysis/bstring.cpp
cfe/trunk/test/Analysis/casts.c

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp?rev=315742&r1=315741&r2=315742&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp Fri Oct 13 
13:11:00 2017
@@ -1050,31 +1050,22 @@ void CStringChecker::evalCopyCommon(Chec
 // If this is mempcpy, get the byte after the last byte copied and
 // bind the expr.
 if (IsMempcpy) {
-  loc::MemRegionVal destRegVal = destVal.castAs();
-
-  // Get the length to copy.
-  if (Optional lenValNonLoc = sizeVal.getAs()) {
-// Get the byte after the last byte copied.
-SValBuilder &SvalBuilder = C.getSValBuilder();
-ASTContext &Ctx = SvalBuilder.getContext();
-QualType CharPtrTy = Ctx.getPointerType(Ctx.CharTy);
-loc::MemRegionVal DestRegCharVal = SvalBuilder.evalCast(destRegVal,
-  CharPtrTy, Dest->getType()).castAs();
-SVal lastElement = C.getSValBuilder().evalBinOpLN(state, BO_Add,
-  DestRegCharVal,
-  *lenValNonLoc,
-  Dest->getType());
-
-// The byte after the last byte copied is the return value.
-state = state->BindExpr(CE, LCtx, lastElement);
-  } else {
-// If we don't know how much we copied, we can at least
-// conjure a return value for later.
-SVal result = C.getSValBuilder().conjureSymbolVal(nullptr, CE, LCtx,
+  // Get the byte after the last byte copied.
+  SValBuilder &SvalBuilder = C.getSValBuilder();
+  ASTContext &Ctx = SvalBuilder.getContext();
+  QualType CharPtrTy = Ctx.getPointerType(Ctx.CharTy);
+  SVal DestRegCharVal =
+  SvalBuilder.evalCast(destVal, CharPtrTy, Dest->getType());
+  SVal lastElement = C.getSValBuilder().evalBinOp(
+  state, BO_Add, DestRegCharVal, sizeVal, Dest->getType());
+  // If we don't know how much we copied, we can at least
+  // conjure a return value for later.
+  if (lastElement.isUnknown())
+lastElement = C.getSValBuilder().conjureSymbolVal(nullptr, CE, LCtx,
   C.blockCount());
-state = state->BindExpr(CE, LCtx, result);
-  }
 
+  // The byte after the last byte copied is the return value.
+  state = state->BindExpr(CE, LCtx, lastElement);
 } else {
   // All other copies return the destination buffer.
   // (Well, bcopy() has a void return type, but this won't hurt.)

Modified: cfe/trunk/test/Analysis/bstring.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/bstring.cpp?rev=315742&r1=315741&r2=315742&view=diff
==
--- cfe/trunk/test/Analysis/bstring.cpp (original)
+++ cfe/trunk/test/Analysis/bstring.cpp Fri Oct 13 13:11:00 2017
@@ -1,8 +1,35 @@
 // RUN: %clang_analyze_cc1 
-analyzer-checker=core,unix.cstring,alpha.unix.cstring,debug.ExprInspection 
-analyzer-store=region -verify %s
+// RUN: %clang_analyze_cc1 -DUSE_BUILTINS 
-analyzer-checker=core,unix.cstring,alpha.unix.cstring,debug.ExprInspection 
-analyzer-store=region -verify %s
+// RUN: %clang_analyze_cc1 -DVARIANT 
-analyzer-checker=core,unix.cstring,alpha.unix.cstring,debug.ExprInspection 
-analyzer-store=region -verify %s
+// RUN: %clang_analyze_cc1 -DUSE_BUILTINS -DVARIANT 
-analyzer-checker=core,unix.cstring,alpha.unix.cstring,debug.ExprInspection 
-analyzer-store=region -verify %s
 
 #include "Inputs/system-header-simulator-cxx.h"
 #include "Inputs/system-header-simulator-for-malloc.h"
 
+// This provides us with four possible mempcpy() definitions.
+// S

[PATCH] D38883: [CMake][OpenMP] Customize default offloading arch

2017-10-13 Thread Artem Belevich via Phabricator via cfe-commits
tra added inline comments.



Comment at: lib/Driver/ToolChains/Cuda.h:90
-  }
 };
 

gtbercea wrote:
> gtbercea wrote:
> > tra wrote:
> > > gtbercea wrote:
> > > > gtbercea wrote:
> > > > > I would also like to keep the spirit of this code if not in this 
> > > > > exact form at least something that performs the same functionality.
> > > > @tra what's your opinion on this code? Should this stay, stay but 
> > > > modified to be more robust or taken out completely?
> > > There are currently no users for this. In general, I would rather not 
> > > have magically-changing default GPU based on how broken your CUDA 
> > > installation is. IMO it would be better to keep defaults static and fail 
> > > if prerequisites are not met.
> > I would have thought that it is up to the compiler to select, as default, 
> > the lowest viable compute capability. This is what this code aims to do 
> > (whether it actually does that's a separate issue :) ).
> > 
> The reason I added this code in the first place was to overcome the fact that 
> something like a default of sm_30 may work on the K40 but once you go to 
> newer Pascal, Volta GPUs then you need a new minimum compute capability that 
> is supported.
> Should this stay, stay but modified to be more robust or taken out completely?

I'd take it out, at least for now as you've removed the only user of that 
function.

In general, though, compilers tend to use conservative defaults and for CUDA 
that would be the lowest GPU variant supported by compiler. In case of CUDA 
it's determined by the CUDA SDK version. Figuring lowers supported version via 
libdevice mapping we've created is wrong. E.g. with this patch and -nocudalib 
you may end up using CUDA-9 without any libdevice and would return sm_20.

If/when we need to figure out minimum supported version, it should be based 
directly on the value returned by version().


https://reviews.llvm.org/D38883



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D38868: [OpenCL] Restrict swizzle length check to OpenCL mode

2017-10-13 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
bruno updated this revision to Diff 118966.
bruno marked 2 inline comments as done.
bruno added a comment.

Update patch after Anastasia's suggestions


https://reviews.llvm.org/D38868

Files:
  lib/Sema/SemaExprMember.cpp
  test/Sema/vector_swizzle_length.c
  test/SemaOpenCL/vector_swizzle_length.cl


Index: test/SemaOpenCL/vector_swizzle_length.cl
===
--- test/SemaOpenCL/vector_swizzle_length.cl
+++ test/SemaOpenCL/vector_swizzle_length.cl
@@ -1,9 +1,9 @@
-// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only
+// RUN: %clang_cc1 -x cl %s -verify -pedantic -fsyntax-only
 
 typedef float float8 __attribute__((ext_vector_type(8)));
 
 void foo() {
-float8 f2 = (float8)(0, 0, 0, 0, 0, 0, 0, 0);
+float8 f2 = (float8){0, 0, 0, 0, 0, 0, 0, 0};
 
 f2.s01234; // expected-error {{vector component access has invalid length 
5.  Supported: 1,2,3,4,8,16}}
 f2.xyzxy; // expected-error {{vector component access has invalid length 
5.  Supported: 1,2,3,4,8,16}}
Index: test/Sema/vector_swizzle_length.c
===
--- /dev/null
+++ test/Sema/vector_swizzle_length.c
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -x c %s -verify -pedantic -fsyntax-only
+// expected-no-diagnostics
+
+typedef float float8 __attribute__((ext_vector_type(8)));
+
+void foo() {
+float8 f2 = (float8){0, 0, 0, 0, 0, 0, 0, 0};
+(void)f2.s01234;
+(void)f2.xyzxy;
+}
Index: lib/Sema/SemaExprMember.cpp
===
--- lib/Sema/SemaExprMember.cpp
+++ lib/Sema/SemaExprMember.cpp
@@ -384,7 +384,9 @@
 }
   }
 
-  if (!HalvingSwizzle) {
+  // OpenCL mode requires swizzle length to be in accordance with accepted
+  // sizes. Clang however supports arbitrary lengths for other languages.
+  if (S.getLangOpts().OpenCL && !HalvingSwizzle) {
 unsigned SwizzleLength = CompName->getLength();
 
 if (HexSwizzle)


Index: test/SemaOpenCL/vector_swizzle_length.cl
===
--- test/SemaOpenCL/vector_swizzle_length.cl
+++ test/SemaOpenCL/vector_swizzle_length.cl
@@ -1,9 +1,9 @@
-// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only
+// RUN: %clang_cc1 -x cl %s -verify -pedantic -fsyntax-only
 
 typedef float float8 __attribute__((ext_vector_type(8)));
 
 void foo() {
-float8 f2 = (float8)(0, 0, 0, 0, 0, 0, 0, 0);
+float8 f2 = (float8){0, 0, 0, 0, 0, 0, 0, 0};
 
 f2.s01234; // expected-error {{vector component access has invalid length 5.  Supported: 1,2,3,4,8,16}}
 f2.xyzxy; // expected-error {{vector component access has invalid length 5.  Supported: 1,2,3,4,8,16}}
Index: test/Sema/vector_swizzle_length.c
===
--- /dev/null
+++ test/Sema/vector_swizzle_length.c
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -x c %s -verify -pedantic -fsyntax-only
+// expected-no-diagnostics
+
+typedef float float8 __attribute__((ext_vector_type(8)));
+
+void foo() {
+float8 f2 = (float8){0, 0, 0, 0, 0, 0, 0, 0};
+(void)f2.s01234;
+(void)f2.xyzxy;
+}
Index: lib/Sema/SemaExprMember.cpp
===
--- lib/Sema/SemaExprMember.cpp
+++ lib/Sema/SemaExprMember.cpp
@@ -384,7 +384,9 @@
 }
   }
 
-  if (!HalvingSwizzle) {
+  // OpenCL mode requires swizzle length to be in accordance with accepted
+  // sizes. Clang however supports arbitrary lengths for other languages.
+  if (S.getLangOpts().OpenCL && !HalvingSwizzle) {
 unsigned SwizzleLength = CompName->getLength();
 
 if (HexSwizzle)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D38667: AMDGPU: Parse r600 CPU name early and expose FMAF capability

2017-10-13 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm accepted this revision.
arsenm added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rL LLVM

https://reviews.llvm.org/D38667



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


  1   2   >