[PATCH] D60808: [analyzer] pr41335: NoStoreFuncVisitor: Fix crash when no-store event is in a body-farmed function.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Hmm, i think i'd love to know why doesn't the uninitialized variable checker 
fire on the if-statement as farmed by the body farm:

  592   // Signature:
  593   // _Bool OSAtomicCompareAndSwapPtr(void *__oldValue,
  594   // void *__newValue,
  595   // void * volatile *__theValue)
  596   // Generate body:
  597   //   if (oldValue == *theValue) {
  598   //*theValue = newValue;
  599   //return YES;
  600   //   }
  601   //   else return NO;

(closing brace accidentally omitted in the original comment as well)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60808/new/

https://reviews.llvm.org/D60808



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

[PATCH] D60736: [Sema][ObjC] Don't warn about a block implicitly retaining self if the block is marked noescape

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/Sema/SemaExpr.cpp:2580
+if (const BlockDecl *BD = CurContext->getInnerMostBlockDecl())
+  if (!getDiagnostics().isIgnored(diag::warn_implicitly_retains_self, 
Loc))
+ImplicitlyRetainedSelfLocs.push_back({Loc, BD});

IIRC this check can be expensive enough that it's probably not worth doing if 
you expect these entries to typically not result in diagnostics.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60736/new/

https://reviews.llvm.org/D60736



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

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added a comment.

In D59806#1469257 , @aaron.ballman 
wrote:

> LGTM! You can either land this now and refactor after the AST matcher lands, 
> or you can wait until the AST matcher lands and land this patch after -- your 
> call.


I will plan on landing this and then follow up after the new AST matcher lands. 
I want to make sure the design for the new AST matcher is solid and evaluate 
the implications of the different approaches. I plan on updating both 
`ForbiddenSubclassingCheck` 

 and `SuperSelfCheck` once the new AST matcher lands 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59806/new/

https://reviews.llvm.org/D59806



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

[PATCH] D59712: [APSInt][OpenMP] Fix isNegative, etc. for unsigned types

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

In D59712#1469392 , @lebedev.ri wrote:

> In D59712#1469358 , @jdenny wrote:
>
> > In D59712#1469301 , @lebedev.ri 
> > wrote:
> >
> > > In D59712#1469295 , 
> > > @craig.topper wrote:
> > >
> > > > Wondering if it would be better to assert for asking for the sign of an 
> > > > unsigned APSInt. I could see a caller just wanting to get the msb for 
> > > > some reason and not knowing that isNegative won’t work.
> > >
> > >
> > > Yes, i, too, would think an assert is much better solution (since i 
> > > literally just tripped over this in this review.)
> >
>
>
> Does this pass `check-all`? `check-all` of stage-2? test-suite?


No.  The assert breaks cases in at least ExprConstant.cpp and SemaExpr.cpp.

However, the current version of the patch does not break check-all.  I have not 
tried the others.

Moreover, I see now that this patch fixes behavior for the following:

  #include 
  #include 
  constexpr unsigned foo() {
unsigned i = INT_MAX;
++i; // should not warn value is outside range
return i;
  }
  int main() {
std::cout << foo() << '\n';
std::cout << (1 << (unsigned)-1) << '\n'; // should not warn about neg 
shift count
return 0;
  }

I'll integrate these into the tests later.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59712/new/

https://reviews.llvm.org/D59712



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

[clang-tools-extra] r358549 - clangd: Change Windows.h to windows.h.

[Peter Collingbourne via cfe-commits]

Author: pcc
Date: Tue Apr 16 20:02:18 2019
New Revision: 358549

URL: http://llvm.org/viewvc/llvm-project?rev=358549=rev
Log:
clangd: Change Windows.h to windows.h.

This makes the file more cross compilation friendly.

Modified:
clang-tools-extra/trunk/clangd/Threading.cpp

Modified: clang-tools-extra/trunk/clangd/Threading.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Threading.cpp?rev=358549=358548=358549=diff
==
--- clang-tools-extra/trunk/clangd/Threading.cpp (original)
+++ clang-tools-extra/trunk/clangd/Threading.cpp Tue Apr 16 20:02:18 2019
@@ -10,7 +10,7 @@
 #elif defined(__APPLE__)
 #include 
 #elif defined (_WIN32)
-#include 
+#include 
 #endif
 
 namespace clang {


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

[PATCH] D59923: [Driver] Simplify -g level computation and its interaction with -gsplit-dwarf

[Fangrui Song via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL358544: [Driver] Simplify -g level computation and its 
interaction with -gsplit-dwarf (authored by MaskRay, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59923/new/

https://reviews.llvm.org/D59923

Files:
  cfe/trunk/lib/Driver/ToolChains/Clang.cpp
  cfe/trunk/test/Driver/split-debug.c

Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp
@@ -3170,35 +3170,24 @@
 SplitDWARFInlining = false;
   }
 
-  if (const Arg *A = Args.getLastArg(options::OPT_g_Group)) {
-if (checkDebugInfoOption(A, Args, D, TC)) {
-  // If the last option explicitly specified a debug-info level, use it.
-  if (A->getOption().matches(options::OPT_gN_Group)) {
-DebugInfoKind = DebugLevelToInfoKind(*A);
-// If you say "-gsplit-dwarf -gline-tables-only", -gsplit-dwarf loses.
-// But -gsplit-dwarf is not a g_group option, hence we have to check the
-// order explicitly. If -gsplit-dwarf wins, we fix DebugInfoKind later.
-// This gets a bit more complicated if you've disabled inline info in
-// the skeleton CUs (SplitDWARFInlining) - then there's value in
-// composing split-dwarf and line-tables-only, so let those compose
-// naturally in that case. And if you just turned off debug info,
-// (-gsplit-dwarf -g0) - do that.
-if (DwarfFission != DwarfFissionKind::None) {
-  if (A->getIndex() > SplitDWARFArg->getIndex()) {
-if (DebugInfoKind == codegenoptions::NoDebugInfo ||
-DebugInfoKind == codegenoptions::DebugDirectivesOnly ||
-(DebugInfoKind == codegenoptions::DebugLineTablesOnly &&
- SplitDWARFInlining))
-  DwarfFission = DwarfFissionKind::None;
-  } else if (SplitDWARFInlining)
-DebugInfoKind = codegenoptions::NoDebugInfo;
-}
-  } else {
-// For any other 'g' option, use Limited.
-DebugInfoKind = codegenoptions::LimitedDebugInfo;
-  }
-} else {
-  DebugInfoKind = codegenoptions::LimitedDebugInfo;
+  if (const Arg *A =
+  Args.getLastArg(options::OPT_g_Group, options::OPT_gsplit_dwarf,
+  options::OPT_gsplit_dwarf_EQ)) {
+DebugInfoKind = codegenoptions::LimitedDebugInfo;
+
+// If the last option explicitly specified a debug-info level, use it.
+if (checkDebugInfoOption(A, Args, D, TC) &&
+A->getOption().matches(options::OPT_gN_Group)) {
+  DebugInfoKind = DebugLevelToInfoKind(*A);
+  // For -g0 or -gline-tables-only, drop -gsplit-dwarf. This gets a bit more
+  // complicated if you've disabled inline info in the skeleton CUs
+  // (SplitDWARFInlining) - then there's value in composing split-dwarf and
+  // line-tables-only, so let those compose naturally in that case.
+  if (DebugInfoKind == codegenoptions::NoDebugInfo ||
+  DebugInfoKind == codegenoptions::DebugDirectivesOnly ||
+  (DebugInfoKind == codegenoptions::DebugLineTablesOnly &&
+   SplitDWARFInlining))
+DwarfFission = DwarfFissionKind::None;
 }
   }
 
@@ -3273,16 +3262,12 @@
   }
 }
 
-  // -gsplit-dwarf should turn on -g and enable the backend dwarf
-  // splitting and extraction.
+  // -gsplit-dwarf enables the backend dwarf splitting and extraction.
   if (T.isOSBinFormatELF()) {
 if (!SplitDWARFInlining)
   CmdArgs.push_back("-fno-split-dwarf-inlining");
 
 if (DwarfFission != DwarfFissionKind::None) {
-  if (DebugInfoKind == codegenoptions::NoDebugInfo)
-DebugInfoKind = codegenoptions::LimitedDebugInfo;
-
   if (DwarfFission == DwarfFissionKind::Single)
 CmdArgs.push_back("-enable-split-dwarf=single");
   else
Index: cfe/trunk/test/Driver/split-debug.c
===
--- cfe/trunk/test/Driver/split-debug.c
+++ cfe/trunk/test/Driver/split-debug.c
@@ -71,7 +71,7 @@
 // RUN: FileCheck -check-prefix=CHECK-SPLIT-WITH-GMLT < %t %s
 //
 // CHECK-SPLIT-WITH-GMLT: "-enable-split-dwarf"
-// CHECK-SPLIT-WITH-GMLT: "-debug-info-kind=line-tables-only"
+// CHECK-SPLIT-WITH-GMLT: "-debug-info-kind=limited"
 // CHECK-SPLIT-WITH-GMLT: "-split-dwarf-file"
 
 // RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf -fno-split-dwarf-inlining -S -### %s 2> %t
@@ -103,6 +103,8 @@
 
 // RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf -g0 -S -### %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-G0-OVER-SPLIT < %t %s
+// RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf=split -g0 -S -### %s 2> %t
+// RUN: FileCheck -check-prefix=CHECK-G0-OVER-SPLIT < %t %s
 //
 // 

r358544 - [Driver] Simplify -g level computation and its interaction with -gsplit-dwarf

[Fangrui Song via cfe-commits]

Author: maskray
Date: Tue Apr 16 18:46:27 2019
New Revision: 358544

URL: http://llvm.org/viewvc/llvm-project?rev=358544=rev
Log:
[Driver] Simplify -g level computation and its interaction with -gsplit-dwarf

Summary:
When -gsplit-dwarf is used together with other -g options, in most cases
the computed debug info level is decided by the last -g option, with one
special case (see below). This patch drops that special case and thus
makes it easy to reason about:

// If a lower debug level -g comes after -gsplit-dwarf, in some cases
// -gsplit-dwarf is cancelled.
-gsplit-dwarf -g0 => 0
-gsplit-dwarf -gline-directives-only => DebugDirectivesOnly
-gsplit-dwarf -gmlt -fsplit-dwarf-inlining => 1
-gsplit-dwarf -gmlt -fno-split-dwarf-inlining => 1 + split

// If -gsplit-dwarf comes after -g options, with this patch, the net
// effect is 2 + split for all combinations
-g0 -gsplit-dwarf => 2 + split
-gline-directives-only -gsplit-dwarf => 2 + split
-gmlt -gsplit-dwarf -fsplit-dwarf-inlining => 2 + split
-gmlt -gsplit-dwarf -fno-split-dwarf-inlining => 1 + split (before) 2 + split 
(after)

The last case has been changed. In general, if the user intends to lower
debug info level, place that -g option after -gsplit-dwarf.

Some context:

In gcc, the last of -gsplit-dwarf -g0 -g1 -g2 -g3 -ggdb[0-3] -gdwarf-*
... decides the debug info level (-gsplit-dwarf -gdwarf-* have level 2).
It is a bit unfortunate that -gsplit-dwarf -gdwarf-* ... participate in
the level computation but that is the status quo.

Reviewers: dblaikie, echristo, probinson

Reviewed By: dblaikie, probinson

Subscribers: probinson, aprantl, jdoerfert, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/test/Driver/split-debug.c

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=358544=358543=358544=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Tue Apr 16 18:46:27 2019
@@ -3170,35 +3170,24 @@ static void RenderDebugOptions(const Too
 SplitDWARFInlining = false;
   }
 
-  if (const Arg *A = Args.getLastArg(options::OPT_g_Group)) {
-if (checkDebugInfoOption(A, Args, D, TC)) {
-  // If the last option explicitly specified a debug-info level, use it.
-  if (A->getOption().matches(options::OPT_gN_Group)) {
-DebugInfoKind = DebugLevelToInfoKind(*A);
-// If you say "-gsplit-dwarf -gline-tables-only", -gsplit-dwarf loses.
-// But -gsplit-dwarf is not a g_group option, hence we have to check 
the
-// order explicitly. If -gsplit-dwarf wins, we fix DebugInfoKind later.
-// This gets a bit more complicated if you've disabled inline info in
-// the skeleton CUs (SplitDWARFInlining) - then there's value in
-// composing split-dwarf and line-tables-only, so let those compose
-// naturally in that case. And if you just turned off debug info,
-// (-gsplit-dwarf -g0) - do that.
-if (DwarfFission != DwarfFissionKind::None) {
-  if (A->getIndex() > SplitDWARFArg->getIndex()) {
-if (DebugInfoKind == codegenoptions::NoDebugInfo ||
-DebugInfoKind == codegenoptions::DebugDirectivesOnly ||
-(DebugInfoKind == codegenoptions::DebugLineTablesOnly &&
- SplitDWARFInlining))
-  DwarfFission = DwarfFissionKind::None;
-  } else if (SplitDWARFInlining)
-DebugInfoKind = codegenoptions::NoDebugInfo;
-}
-  } else {
-// For any other 'g' option, use Limited.
-DebugInfoKind = codegenoptions::LimitedDebugInfo;
-  }
-} else {
-  DebugInfoKind = codegenoptions::LimitedDebugInfo;
+  if (const Arg *A =
+  Args.getLastArg(options::OPT_g_Group, options::OPT_gsplit_dwarf,
+  options::OPT_gsplit_dwarf_EQ)) {
+DebugInfoKind = codegenoptions::LimitedDebugInfo;
+
+// If the last option explicitly specified a debug-info level, use it.
+if (checkDebugInfoOption(A, Args, D, TC) &&
+A->getOption().matches(options::OPT_gN_Group)) {
+  DebugInfoKind = DebugLevelToInfoKind(*A);
+  // For -g0 or -gline-tables-only, drop -gsplit-dwarf. This gets a bit 
more
+  // complicated if you've disabled inline info in the skeleton CUs
+  // (SplitDWARFInlining) - then there's value in composing split-dwarf and
+  // line-tables-only, so let those compose naturally in that case.
+  if (DebugInfoKind == codegenoptions::NoDebugInfo ||
+  DebugInfoKind == codegenoptions::DebugDirectivesOnly ||
+  (DebugInfoKind == codegenoptions::DebugLineTablesOnly &&
+   SplitDWARFInlining))
+DwarfFission = DwarfFissionKind::None;
 }
   }
 
@@ -3273,16 +3262,12 @@ static 

[PATCH] D60742: [analyzer] RegionStore: Enable loading default bindings from variables.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 195500.
NoQ added a comment.

Fxd constructors and destructors.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60742/new/

https://reviews.llvm.org/D60742

Files:
  clang/lib/StaticAnalyzer/Core/RegionStore.cpp
  clang/test/Analysis/string.c
  clang/unittests/StaticAnalyzer/CMakeLists.txt
  clang/unittests/StaticAnalyzer/StoreTest.cpp

Index: clang/unittests/StaticAnalyzer/StoreTest.cpp
===
--- /dev/null
+++ clang/unittests/StaticAnalyzer/StoreTest.cpp
@@ -0,0 +1,105 @@
+//===- unittests/StaticAnalyzer/StoreTest.cpp -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Reusables.h"
+
+#include "clang/Tooling/Tooling.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace ento {
+namespace {
+
+// Test that we can put a value into an int-type variable and load it
+// back from that variable. Test what happens if default bindings are used.
+class VariableBindConsumer : public ExprEngineConsumer {
+  void performTest(const Decl *D) {
+StoreManager  = Eng.getStoreManager();
+SValBuilder  = Eng.getSValBuilder();
+MemRegionManager  = StMgr.getRegionManager();
+const ASTContext  = Eng.getContext();
+
+const auto *VDX0 = findDeclByName(D, "x0");
+const auto *VDY0 = findDeclByName(D, "y0");
+const auto *VDZ0 = findDeclByName(D, "z0");
+const auto *VDX1 = findDeclByName(D, "x1");
+const auto *VDY1 = findDeclByName(D, "y1");
+assert(VDX0 && VDY0 && VDZ0 && VDX1 && VDY1);
+
+const StackFrameContext *SFC =
+Eng.getAnalysisDeclContextManager().getStackFrame(D);
+
+Loc LX0 = loc::MemRegionVal(MRMgr.getVarRegion(VDX0, SFC));
+Loc LY0 = loc::MemRegionVal(MRMgr.getVarRegion(VDY0, SFC));
+Loc LZ0 = loc::MemRegionVal(MRMgr.getVarRegion(VDZ0, SFC));
+Loc LX1 = loc::MemRegionVal(MRMgr.getVarRegion(VDX1, SFC));
+Loc LY1 = loc::MemRegionVal(MRMgr.getVarRegion(VDY1, SFC));
+
+Store StInit = StMgr.getInitialStore(SFC).getStore();
+SVal Zero = SVB.makeZeroVal(ACtx.IntTy);
+SVal One = SVB.makeIntVal(1, ACtx.IntTy);
+SVal NarrowZero = SVB.makeZeroVal(ACtx.CharTy);
+
+// Bind(Zero)
+Store StX0 =
+StMgr.Bind(StInit, LX0, Zero).getStore();
+ASSERT_EQ(Zero, StMgr.getBinding(StX0, LX0, ACtx.IntTy));
+
+// BindDefaultInitial(Zero)
+Store StY0 =
+StMgr.BindDefaultInitial(StInit, LY0.getAsRegion(), Zero).getStore();
+ASSERT_EQ(Zero, StMgr.getBinding(StY0, LY0, ACtx.IntTy));
+ASSERT_EQ(Zero, *StMgr.getDefaultBinding(StY0, LY0.getAsRegion()));
+
+// BindDefaultZero()
+Store StZ0 =
+StMgr.BindDefaultZero(StInit, LZ0.getAsRegion()).getStore();
+// BindDefaultZero wipes the region with '0 S8b', not with out Zero.
+// Direct load, however, does give us back the object of the type
+// that we specify for loading.
+ASSERT_EQ(Zero, StMgr.getBinding(StZ0, LZ0, ACtx.IntTy));
+ASSERT_EQ(NarrowZero, *StMgr.getDefaultBinding(StZ0, LZ0.getAsRegion()));
+
+// Bind(One)
+Store StX1 =
+StMgr.Bind(StInit, LX1, One).getStore();
+ASSERT_EQ(One, StMgr.getBinding(StX1, LX1, ACtx.IntTy));
+
+// BindDefaultInitial(One)
+Store StY1 =
+StMgr.BindDefaultInitial(StInit, LY1.getAsRegion(), One).getStore();
+ASSERT_EQ(One, StMgr.getBinding(StY1, LY1, ACtx.IntTy));
+ASSERT_EQ(One, *StMgr.getDefaultBinding(StY1, LY1.getAsRegion()));
+  }
+
+public:
+  VariableBindConsumer(CompilerInstance ) : ExprEngineConsumer(C) {}
+
+  bool HandleTopLevelDecl(DeclGroupRef DG) override {
+for (const auto *D : DG)
+  performTest(D);
+return true;
+  }
+};
+
+class VariableBindAction : public ASTFrontendAction {
+public:
+  std::unique_ptr CreateASTConsumer(CompilerInstance ,
+ StringRef File) override {
+return llvm::make_unique(Compiler);
+  }
+};
+
+TEST(Store, VariableBind) {
+  EXPECT_TRUE(tooling::runToolOnCode(
+  new VariableBindAction, "void foo() { int x0, y0, z0, x1, y1; }"));
+}
+
+} // namespace
+} // namespace ento
+} // namespace clang
Index: clang/unittests/StaticAnalyzer/CMakeLists.txt
===
--- clang/unittests/StaticAnalyzer/CMakeLists.txt
+++ clang/unittests/StaticAnalyzer/CMakeLists.txt
@@ -4,6 +4,7 @@
 
 add_clang_unittest(StaticAnalysisTests
   AnalyzerOptionsTest.cpp
+  StoreTest.cpp
   RegisterCustomCheckersTest.cpp
   SymbolReaperTest.cpp
   )
Index: clang/test/Analysis/string.c
===
--- clang/test/Analysis/string.c
+++ clang/test/Analysis/string.c
@@ -1554,3 +1554,9 @@
   // 

[PATCH] D60742: [analyzer] RegionStore: Enable loading default bindings from variables.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ marked an inline comment as done.
NoQ added inline comments.



Comment at: clang/unittests/StaticAnalyzer/StoreTest.cpp:49
+// Bind(Zero)
+Store StX0 =
+StMgr.Bind(StInit, LX0, Zero).getStore();

a_sidorin wrote:
> This can fit one line.
Just wanted to make line breaks similar in all tests, for the sake of 
OCD-friendliness.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60742/new/

https://reviews.llvm.org/D60742



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

[PATCH] D60732: [analyzer] NFC: Use -verify=... in MoveChecker tests.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D60732#1469026 , @jdenny wrote:

> The original patch added documentation to `-cc1 -help`.  Doxygen for 
> VerifyDiagnosticConsumer is another possibility except it currently doesn't 
> mention command-line usage at all.  Maybe it should?


I would have definitely discovered this feature earlier if it was mentioned on 
the doxygen page for VerifyDiagnosticConsumer!

In D60732#1467504 , @Charusso wrote:

> I think this functionality is unused because you would split the file into 
> six to reduce the overhead/scroll and that is it.


The current test tests all 6 modes on all functions, that's a lot of coverage. 
You can't obtain the same coverage by splitting the file into mode-specific 
tests; you'd have to duplicate all code in all files, is much harder to 
maintain than `#ifdef`s. It would probably still be easier to read, but the 
constant fear that the files aren't actually identical pretty much defeats the 
purpose.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60732/new/

https://reviews.llvm.org/D60732



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

[PATCH] D60736: [Sema][ObjC] Don't warn about a block implicitly retaining self if the block is marked noescape

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak updated this revision to Diff 195498.
ahatanak added a comment.

Keep a list of pairs of BlockDecl and SourceLocation and, after the body of an 
ObjC method is parsed, emit a diagnostic if a SourceLocation in the list is 
inside an escaping block.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60736/new/

https://reviews.llvm.org/D60736

Files:
  include/clang/AST/DeclBase.h
  include/clang/Sema/Sema.h
  lib/Parse/ParseObjc.cpp
  lib/Sema/SemaDeclObjC.cpp
  lib/Sema/SemaExpr.cpp
  test/SemaObjC/warn-implicit-self-in-block.m
  test/SemaObjCXX/warn-implicit-self-in-block.mm

Index: test/SemaObjCXX/warn-implicit-self-in-block.mm
===
--- /dev/null
+++ test/SemaObjCXX/warn-implicit-self-in-block.mm
@@ -0,0 +1,42 @@
+// RUN: %clang_cc1 -x objective-c++ -std=c++11 -fobjc-arc -fblocks -Wimplicit-retain-self -verify %s
+// rdar://11194874
+
+typedef void (^BlockTy)();
+
+void noescapeFunc(__attribute__((noescape)) BlockTy);
+void escapeFunc(BlockTy);
+
+@interface Root @end
+
+@interface I : Root
+{
+  int _bar;
+}
+@end
+
+@implementation I
+  - (void)foo{
+  ^{
+   _bar = 3; // expected-warning {{block implicitly retains 'self'; explicitly mention 'self' to indicate this is intended behavior}}
+   }();
+  }
+
+  - (void)testNested{
+noescapeFunc(^{
+  (void)_bar;
+  escapeFunc(^{
+(void)_bar; // expected-warning {{block implicitly retains 'self'; explicitly mention 'self' to indicate this is intended behavior}}
+noescapeFunc(^{
+  (void)_bar; // expected-warning {{block implicitly retains 'self'; explicitly mention 'self' to indicate this is intended behavior}}
+});
+(void)_bar; // expected-warning {{block implicitly retains 'self'; explicitly mention 'self' to indicate this is intended behavior}}
+  });
+  (void)_bar;
+});
+  }
+
+  - (void)testLambdaInBlock{
+noescapeFunc(^{ [&](){ (void)_bar; }(); });
+escapeFunc(^{ [&](){ (void)_bar; }(); }); // expected-warning {{block implicitly retains 'self'; explicitly mention 'self' to indicate this is intended behavior}}
+  }
+@end
Index: test/SemaObjC/warn-implicit-self-in-block.m
===
--- test/SemaObjC/warn-implicit-self-in-block.m
+++ /dev/null
@@ -1,18 +0,0 @@
-// RUN: %clang_cc1 -x objective-c -fobjc-arc -fblocks -Wimplicit-retain-self -verify %s
-// rdar://11194874
-
-@interface Root @end
-
-@interface I : Root
-{
-  int _bar;
-}
-@end
-
-@implementation I
-  - (void)foo{
-  ^{
-   _bar = 3; // expected-warning {{block implicitly retains 'self'; explicitly mention 'self' to indicate this is intended behavior}}
-   }();
-  }
-@end
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -2575,11 +2575,10 @@
 !Diags.isIgnored(diag::warn_arc_repeated_use_of_weak, Loc))
   getCurFunction()->recordUseOfWeak(Result);
   }
-  if (getLangOpts().ObjCAutoRefCount) {
-if (CurContext->isClosure())
-  Diag(Loc, diag::warn_implicitly_retains_self)
-<< FixItHint::CreateInsertion(Loc, "self->");
-  }
+  if (getLangOpts().ObjCAutoRefCount)
+if (const BlockDecl *BD = CurContext->getInnerMostBlockDecl())
+  if (!getDiagnostics().isIgnored(diag::warn_implicitly_retains_self, Loc))
+ImplicitlyRetainedSelfLocs.push_back({Loc, BD});
 
   return Result;
 }
Index: lib/Sema/SemaDeclObjC.cpp
===
--- lib/Sema/SemaDeclObjC.cpp
+++ lib/Sema/SemaDeclObjC.cpp
@@ -359,6 +359,7 @@
 /// ActOnStartOfObjCMethodDef - This routine sets up parameters; invisible
 /// and user declared, in the method definition's AST.
 void Sema::ActOnStartOfObjCMethodDef(Scope *FnBodyScope, Decl *D) {
+  ImplicitlyRetainedSelfLocs.clear();
   assert((getCurMethodDecl() == nullptr) && "Methodparsing confused");
   ObjCMethodDecl *MDecl = dyn_cast_or_null(D);
 
@@ -494,6 +495,35 @@
   }
 }
 
+void Sema::ActOnEndOfObjCMethodDef() {
+  llvm::DenseMap EscapeInfo;
+
+  auto IsOrNestedInEscapingBlock = [&](const BlockDecl *BD) {
+if (EscapeInfo.count(BD))
+  return EscapeInfo[BD];
+
+bool R = false;
+const BlockDecl *CurBD = BD;
+
+do {
+  R = R || !CurBD->doesNotEscape();
+  if (R)
+break;
+  CurBD = CurBD->getParent()->getInnerMostBlockDecl();
+} while (CurBD);
+
+return EscapeInfo[BD] = R;
+  };
+
+  // If the location where 'self' is implicitly retained is inside a escaping
+  // block, emit a diagnostic.
+  for (const std::pair  :
+   ImplicitlyRetainedSelfLocs)
+if (IsOrNestedInEscapingBlock(P.second))
+  Diag(P.first, diag::warn_implicitly_retains_self)
+  << FixItHint::CreateInsertion(P.first, "self->");
+}
+
 namespace {
 
 // 

[PATCH] D60807: Test for Oz fail

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

Interesting, it looks like the difference here is almost entirely due to 
different inlining decisions.

And according to the tool I posted here:
http://lists.llvm.org/pipermail/llvm-dev/2019-April/131811.html
in the few functions in which we made the same inlining decisions, the -Oz 
assembly is almost always smaller.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60807/new/

https://reviews.llvm.org/D60807



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

[PATCH] D60808: [analyzer] pr41335: NoStoreFuncVisitor: Fix crash when no-store event is in a body-farmed function.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs, 
mikhail.ramalho, Szelethus, baloghadamsoftware, Charusso.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, jfb, a.sidorin, 
szepet, kristof.beyls, javed.absar.
Herald added a project: clang.
NoQ updated this revision to Diff 195497.
NoQ added a comment.

Fix comments.


It is getting increasingly annoying that it's so hard to construct a 
`PathDiagnosticLocation` correctly. I think we should eventually make the API 
more defensive to invalid source locations.


https://reviews.llvm.org/D60808

Files:
  clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/test/Analysis/diagnostics/body-farm-crashes.c


Index: clang/test/Analysis/diagnostics/body-farm-crashes.c
===
--- /dev/null
+++ clang/test/Analysis/diagnostics/body-farm-crashes.c
@@ -0,0 +1,15 @@
+// RUN: %clang_analyze_cc1 -w -analyzer-checker=core\
+// RUN:-analyzer-output=text -verify %s
+
+int OSAtomicCompareAndSwapPtrBarrier(*, *, **);
+int OSAtomicCompareAndSwapPtrBarrier() {}
+int *atomicInvalidSLocOnRedecl() {
+  int *b; // expected-note{{'b' declared without an initial value}}
+
+   // FIXME: These notes shouldn't be there because there's nothing 
between them.
+  OSAtomicCompareAndSwapPtrBarrier(0, 0, ); // expected-note{{Calling 
'OSAtomicCompareAndSwapPtrBarrier'}}
+   

// expected-note@-1{{Returning from 
'OSAtomicCompareAndSwapPtrBarrier'}}
+
+  return b; // expected-warning{{Undefined or garbage value returned to 
caller}}
+// expected-note@-1{{Undefined or garbage value returned to 
caller}}
+}
Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -578,6 +578,11 @@
 
 PathDiagnosticLocation L =
 PathDiagnosticLocation::create(N->getLocation(), SM);
+if (!L.hasValidLocation()) {
+  // Do we need to suppress our report for body-farmed functions as well?
+  // Or maybe attach the note to the call site instead?
+  return nullptr;
+}
 
 SmallString<256> sbuf;
 llvm::raw_svector_ostream os(sbuf);
Index: clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
===
--- clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
+++ clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
@@ -313,6 +313,8 @@
 
   bool hasRange() const { return K == StmtK || K == RangeK || K == DeclK; }
 
+  bool hasValidLocation() const { return asLocation().isValid(); }
+
   void invalidate() {
 *this = PathDiagnosticLocation();
   }
@@ -468,7 +470,7 @@
   PathDiagnosticPiece::Kind k,
   bool addPosRange = true)
   : PathDiagnosticPiece(s, k), Pos(pos) {
-assert(Pos.isValid() && Pos.asLocation().isValid() &&
+assert(Pos.isValid() && Pos.hasValidLocation() &&
"PathDiagnosticSpotPiece's must have a valid location.");
 if (addPosRange && Pos.hasRange()) addRange(Pos.asRange());
   }


Index: clang/test/Analysis/diagnostics/body-farm-crashes.c
===
--- /dev/null
+++ clang/test/Analysis/diagnostics/body-farm-crashes.c
@@ -0,0 +1,15 @@
+// RUN: %clang_analyze_cc1 -w -analyzer-checker=core\
+// RUN:-analyzer-output=text -verify %s
+
+int OSAtomicCompareAndSwapPtrBarrier(*, *, **);
+int OSAtomicCompareAndSwapPtrBarrier() {}
+int *atomicInvalidSLocOnRedecl() {
+  int *b; // expected-note{{'b' declared without an initial value}}
+
+	// FIXME: These notes shouldn't be there because there's nothing between them.
+  OSAtomicCompareAndSwapPtrBarrier(0, 0, ); // expected-note{{Calling 'OSAtomicCompareAndSwapPtrBarrier'}}
+			// expected-note@-1{{Returning from 'OSAtomicCompareAndSwapPtrBarrier'}}
+
+  return b; // expected-warning{{Undefined or garbage value returned to caller}}
+// expected-note@-1{{Undefined or garbage value returned to caller}}
+}
Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -578,6 +578,11 @@
 
 PathDiagnosticLocation L =
 PathDiagnosticLocation::create(N->getLocation(), SM);
+if (!L.hasValidLocation()) {
+  // Do we need to suppress our report for body-farmed 

[PATCH] D60808: [analyzer] pr41335: NoStoreFuncVisitor: Fix crash when no-store event is in a body-farmed function.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 195497.
NoQ added a comment.

Fix comments.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60808/new/

https://reviews.llvm.org/D60808

Files:
  clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/test/Analysis/diagnostics/body-farm-crashes.c


Index: clang/test/Analysis/diagnostics/body-farm-crashes.c
===
--- /dev/null
+++ clang/test/Analysis/diagnostics/body-farm-crashes.c
@@ -0,0 +1,15 @@
+// RUN: %clang_analyze_cc1 -w -analyzer-checker=core\
+// RUN:-analyzer-output=text -verify %s
+
+int OSAtomicCompareAndSwapPtrBarrier(*, *, **);
+int OSAtomicCompareAndSwapPtrBarrier() {}
+int *atomicInvalidSLocOnRedecl() {
+  int *b; // expected-note{{'b' declared without an initial value}}
+
+   // FIXME: These notes shouldn't be there because there's nothing 
between them.
+  OSAtomicCompareAndSwapPtrBarrier(0, 0, ); // expected-note{{Calling 
'OSAtomicCompareAndSwapPtrBarrier'}}
+   

// expected-note@-1{{Returning from 
'OSAtomicCompareAndSwapPtrBarrier'}}
+
+  return b; // expected-warning{{Undefined or garbage value returned to 
caller}}
+// expected-note@-1{{Undefined or garbage value returned to 
caller}}
+}
Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -578,6 +578,11 @@
 
 PathDiagnosticLocation L =
 PathDiagnosticLocation::create(N->getLocation(), SM);
+if (!L.hasValidLocation()) {
+  // Do we need to suppress our report for body-farmed functions as well?
+  // Or maybe attach the note to the call site instead?
+  return nullptr;
+}
 
 SmallString<256> sbuf;
 llvm::raw_svector_ostream os(sbuf);
Index: clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
===
--- clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
+++ clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
@@ -313,6 +313,8 @@
 
   bool hasRange() const { return K == StmtK || K == RangeK || K == DeclK; }
 
+  bool hasValidLocation() const { return asLocation().isValid(); }
+
   void invalidate() {
 *this = PathDiagnosticLocation();
   }
@@ -468,7 +470,7 @@
   PathDiagnosticPiece::Kind k,
   bool addPosRange = true)
   : PathDiagnosticPiece(s, k), Pos(pos) {
-assert(Pos.isValid() && Pos.asLocation().isValid() &&
+assert(Pos.isValid() && Pos.hasValidLocation() &&
"PathDiagnosticSpotPiece's must have a valid location.");
 if (addPosRange && Pos.hasRange()) addRange(Pos.asRange());
   }


Index: clang/test/Analysis/diagnostics/body-farm-crashes.c
===
--- /dev/null
+++ clang/test/Analysis/diagnostics/body-farm-crashes.c
@@ -0,0 +1,15 @@
+// RUN: %clang_analyze_cc1 -w -analyzer-checker=core\
+// RUN:-analyzer-output=text -verify %s
+
+int OSAtomicCompareAndSwapPtrBarrier(*, *, **);
+int OSAtomicCompareAndSwapPtrBarrier() {}
+int *atomicInvalidSLocOnRedecl() {
+  int *b; // expected-note{{'b' declared without an initial value}}
+
+	// FIXME: These notes shouldn't be there because there's nothing between them.
+  OSAtomicCompareAndSwapPtrBarrier(0, 0, ); // expected-note{{Calling 'OSAtomicCompareAndSwapPtrBarrier'}}
+			// expected-note@-1{{Returning from 'OSAtomicCompareAndSwapPtrBarrier'}}
+
+  return b; // expected-warning{{Undefined or garbage value returned to caller}}
+// expected-note@-1{{Undefined or garbage value returned to caller}}
+}
Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -578,6 +578,11 @@
 
 PathDiagnosticLocation L =
 PathDiagnosticLocation::create(N->getLocation(), SM);
+if (!L.hasValidLocation()) {
+  // Do we need to suppress our report for body-farmed functions as well?
+  // Or maybe attach the note to the call site instead?
+  return nullptr;
+}
 
 SmallString<256> sbuf;
 llvm::raw_svector_ostream os(sbuf);
Index: clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
===
--- clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
+++ 

[PATCH] D60274: [ELF] Implement Dependent Libraries Feature

[ben via Phabricator via cfe-commits]

bd1976llvm updated this revision to Diff 195495.
bd1976llvm added a comment.

No longer shortening "dependent libraries" to "deplibs" except for the .deplibs 
section (as this takes up bytes on disk).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60274/new/

https://reviews.llvm.org/D60274

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/dependent-lib.c
  clang/test/CodeGen/elf-linker-options.c
  clang/test/CodeGen/pragma-comment.c
  clang/test/Modules/autolink.m
  lld/ELF/Config.h
  lld/ELF/Driver.cpp
  lld/ELF/Driver.h
  lld/ELF/DriverUtils.cpp
  lld/ELF/InputFiles.cpp
  lld/ELF/Options.td
  lld/test/ELF/Inputs/deplibs-lib_bar.s
  lld/test/ELF/Inputs/deplibs-lib_foo.s
  lld/test/ELF/deplibs-colon-prefix.s
  lld/test/ELF/deplibs-corrupt.s
  lld/test/ELF/deplibs.s
  lld/test/ELF/lto/deplibs.s
  llvm/docs/Extensions.rst
  llvm/docs/LangRef.rst
  llvm/include/llvm-c/lto.h
  llvm/include/llvm/BinaryFormat/ELF.h
  llvm/include/llvm/LTO/LTO.h
  llvm/include/llvm/LTO/legacy/LTOModule.h
  llvm/include/llvm/Object/IRSymtab.h
  llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
  llvm/lib/LTO/LTO.cpp
  llvm/lib/LTO/LTOModule.cpp
  llvm/lib/MC/MCParser/ELFAsmParser.cpp
  llvm/lib/MC/MCSectionELF.cpp
  llvm/lib/Object/ELF.cpp
  llvm/lib/Object/IRSymtab.cpp
  llvm/lib/ObjectYAML/ELFYAML.cpp
  llvm/test/Feature/elf-deplibs.ll
  llvm/test/LTO/Resolution/X86/symtab-elf.ll
  llvm/test/MC/ELF/section.s
  llvm/test/Object/X86/irsymtab.ll
  llvm/tools/llvm-lto2/llvm-lto2.cpp
  llvm/tools/llvm-readobj/ELFDumper.cpp
  llvm/tools/lto/lto.cpp

Index: llvm/tools/lto/lto.cpp
===
--- llvm/tools/lto/lto.cpp
+++ llvm/tools/lto/lto.cpp
@@ -325,6 +325,10 @@
   return unwrap(mod)->getLinkerOpts().data();
 }
 
+const char* lto_module_get_dependent_libraries(lto_module_t mod) {
+return unwrap(mod)->getDependentLibraries().data();
+}
+
 void lto_codegen_set_diagnostic_handler(lto_code_gen_t cg,
 lto_diagnostic_handler_t diag_handler,
 void *ctxt) {
Index: llvm/tools/llvm-readobj/ELFDumper.cpp
===
--- llvm/tools/llvm-readobj/ELFDumper.cpp
+++ llvm/tools/llvm-readobj/ELFDumper.cpp
@@ -2830,6 +2830,8 @@
 return "LLVM_CALL_GRAPH_PROFILE";
   case SHT_LLVM_ADDRSIG:
 return "LLVM_ADDRSIG";
+  case SHT_LLVM_DEPENDENT_LIBRARIES:
+return "LLVM_DEPENDENT_LIBRARIES";
   // FIXME: Parse processor specific GNU attributes
   case SHT_GNU_ATTRIBUTES:
 return "ATTRIBUTES";
Index: llvm/tools/llvm-lto2/llvm-lto2.cpp
===
--- llvm/tools/llvm-lto2/llvm-lto2.cpp
+++ llvm/tools/llvm-lto2/llvm-lto2.cpp
@@ -360,6 +360,13 @@
 if (TT.isOSBinFormatCOFF())
   outs() << "linker opts: " << Input->getCOFFLinkerOpts() << '\n';
 
+if (TT.isOSBinFormatELF()) {
+  outs() << "dependent libraries:";
+  for (auto L : Input->getDependentLibraries())
+outs() << " \"" << L << "\"";
+  outs() << '\n';
+}
+
 std::vector ComdatTable = Input->getComdatTable();
 for (const InputFile::Symbol  : Input->symbols()) {
   switch (Sym.getVisibility()) {
Index: llvm/test/Object/X86/irsymtab.ll
===
--- llvm/test/Object/X86/irsymtab.ll
+++ llvm/test/Object/X86/irsymtab.ll
@@ -9,16 +9,17 @@
 
 ; BCA:   blob data = '\x01\x00\x00\x00\x06\x00\x00\x00\x08\x00\x00\x00D\x00\x00\x00\x01\x00\x00\x00P\x00\x00\x00\x00\x00\x00\x00P\x00\x00\x00\x02\x00\x00\x00\x80\x00\x00\x00\x00\x00\x00\x00\x0E\x00\x00\x00\x18\x00\x00\x00&\x00\x00\x00\x0B\x00\x00\x001\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x03\x00\x00\x00\x00\x00\x00\x00\x03\x00\x00\x00\xFF\xFF\xFF\xFF\x00$\x00\x00\x03\x00\x00\x00\x03\x00\x00\x00\x03\x00\x00\x00\x03\x00\x00\x00\xFF\xFF\xFF\xFF\x08$\x00\x00'
+; BCA-NEXT:blob data = '\x02\x00\x00\x00\x06\x00\x00\x00\x08\x00\x00\x00L\x00\x00\x00\x01\x00\x00\x00X\x00\x00\x00\x00\x00\x00\x00X\x00\x00\x00\x02\x00\x00\x00\x88\x00\x00\x00\x00\x00\x00\x00\x0E\x00\x00\x00\x18\x00\x00\x00&\x00\x00\x00\x0B\x00\x00\x001\x00\x00\x00\x00\x00\x00\x00\x88\x00\x00\x00\x02\x00\x00\x00\x00\x00\x00\x00\x02\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x03\x00\x00\x00\x00\x00\x00\x00\x03\x00\x00\x00\xFF\xFF\xFF\xFF\x00$\x00\x00\x03\x00\x00\x00\x03\x00\x00\x00\x03\x00\x00\x00\x03\x00\x00\x00\xFF\xFF\xFF\xFF\x08$\x00\x00\x00\x00\x00\x00\x03\x00\x00\x00\x03\x00\x00\x00\x03\x00\x00\x00'
 ; BCA-NEXT: 
 ; BCA-NEXT:  blob data = 'foobarproducerx86_64-unknown-linux-gnuirsymtab.ll'
 ; BCA-NEXT: 
 
-; SYMTAB:  version: 1
+; SYMTAB:  version: 2
 ; SYMTAB-NEXT: producer: producer
 ; SYMTAB-NEXT: target triple: x86_64-unknown-linux-gnu
 ; SYMTAB-NEXT: source filename: irsymtab.ll
+; 

[PATCH] D60807: Test for Oz fail

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

-Oz builds are usually significantly large than -Os build


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D60807

Files:
  clang/test/CodeGen/oz.cpp




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

[PATCH] D53076: [analyzer] ConditionBRVisitor: Enhance to write out more information

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.

Yay!! I'm happy with this change and redirected all my wrath on its 
dependencies.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53076/new/

https://reviews.llvm.org/D53076



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

[PATCH] D58033: Add option for emitting dbg info for call sites

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

And if we plan to enable it by default, too, perhaps not adding a driver option 
(CC1 only) is preferable, since we need to maintain compatibility with driver 
options indefinitely.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58033/new/

https://reviews.llvm.org/D58033



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

[PATCH] D60800: [MS] Emit S_HEAPALLOCSITE debug info

[Amy Huang via Phabricator via cfe-commits]

akhuang updated this revision to Diff 195480.
akhuang added a comment.

Fix test case


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60800/new/

https://reviews.llvm.org/D60800

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGen/debug-info-codeview-heapallocsite.c
  llvm/include/llvm/CodeGen/MachineFunction.h
  llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
  llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h
  llvm/lib/CodeGen/MachineFunction.cpp
  llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
  llvm/test/CodeGen/X86/label-heapallocsite.ll

Index: llvm/test/CodeGen/X86/label-heapallocsite.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/label-heapallocsite.ll
@@ -0,0 +1,73 @@
+; RUN: llc -O0 < %s | FileCheck %s
+; FIXME: Add test for llc with optimizations once it is implemented.
+
+; Source to regenerate:
+; $ clang --target=x86_64-windows-msvc -S heapallocsite.c  -g -gcodeview -o t.ll \
+;  -emit-llvm -O0 -Xclang -disable-llvm-passes -fms-extensions
+; __declspec(allocator) char *myalloc();
+; void g();
+; void foo() {
+;   g();
+;   myalloc()
+;   g();
+; }
+
+; ModuleID = 'heapallocsite.c'
+source_filename = "heapallocsite.c"
+target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-windows-msvc"
+
+; Function Attrs: noinline nounwind optnone
+define dso_local void @f() #0 !dbg !7 {
+entry:
+  call void @g(), !dbg !11
+  %call = call i8* @myalloc(), !dbg !12, !heapallocsite !13
+  call void @g(), !dbg !14
+  ret void, !dbg !15
+}
+
+; CHECK-LABEL: f: # @f
+; CHECK: callq g
+; CHECK: .Lheapallocsite0:
+; CHECK: callq myalloc
+; CHECK: .Lheapallocsite1:
+; CHECK: retq
+
+; CHECK-LABEL: .short  4423# Record kind: S_GPROC32_ID
+; CHECK:   .short  4446# Record kind: S_HEAPALLOCSITE
+; CHECK-NEXT:  .secrel32   .Lheapallocsite0
+; CHECK-NEXT:  .secidx .Lheapallocsite0
+; CHECK-NEXT:  .short  .Lheapallocsite1-.Lheapallocsite0
+; CHECK-NEXT:  .long 112
+; CHECK-NEXT:  .p2align 2
+
+; CHECK-LABEL: .short  4431# Record kind: S_PROC_ID_END
+
+declare dso_local void @g() #1
+
+declare dso_local i8* @myalloc() #1
+
+attributes #0 = { noinline nounwind optnone "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #1 = { "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 9.0.0 (https://github.com/llvm/llvm-project.git 4eff3de99423a62fd6e833e29c71c1e62ba6140b)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, nameTableKind: None)
+!1 = !DIFile(filename: "", directory: "C:\5Csrc\5Ctest", checksumkind: CSK_MD5, checksum: "6d758cfa3834154a04ce8a55102772a9")
+!2 = !{}
+!3 = !{i32 2, !"CodeView", i32 1}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 2}
+!6 = !{!"clang version 9.0.0 (https://github.com/llvm/llvm-project.git 4eff3de99423a62fd6e833e29c71c1e62ba6140b)"}
+!7 = distinct !DISubprogram(name: "f", scope: !8, file: !8, line: 4, type: !9, scopeLine: 4, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !2)
+!8 = !DIFile(filename: "heapallocsite.c", directory: "C:\5Csrc\5Ctest", checksumkind: CSK_MD5, checksum: "6d758cfa3834154a04ce8a55102772a9")
+!9 = !DISubroutineType(types: !10)
+!10 = !{null}
+!11 = !DILocation(line: 5, scope: !7)
+!12 = !DILocation(line: 6, scope: !7)
+!13 = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char)
+!14 = !DILocation(line: 7, scope: !7)
+!15 = !DILocation(line: 8, scope: !7)
+
Index: llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
===
--- llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
+++ llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
@@ -1234,6 +1234,11 @@
   if (CLI.NumResultRegs && CLI.CS)
 updateValueMap(CLI.CS->getInstruction(), CLI.ResultReg, CLI.NumResultRegs);
 
+  // Set labels for heapallocsite call.
+  if (CLI.CS && CLI.CS->getInstruction()->getMetadata("heapallocsite"))
+MF->addCodeViewHeapAllocSite(CLI.Call, 

r358538 - [NFC] Remove unused function (Sema::pushExternalDeclIntoScope)

[Leonard Chan via cfe-commits]

Author: leonardchan
Date: Tue Apr 16 15:59:39 2019
New Revision: 358538

URL: http://llvm.org/viewvc/llvm-project?rev=358538=rev
Log:
[NFC] Remove unused function (Sema::pushExternalDeclIntoScope)

Modified:
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/SemaDecl.cpp

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=358538=358537=358538=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Tue Apr 16 15:59:39 2019
@@ -2487,14 +2487,6 @@ public:
   /// Add this decl to the scope shadowed decl chains.
   void PushOnScopeChains(NamedDecl *D, Scope *S, bool AddToContext = true);
 
-  /// Make the given externally-produced declaration visible at the
-  /// top level scope.
-  ///
-  /// \param D The externally-produced declaration to push.
-  ///
-  /// \param Name The name of the externally-produced declaration.
-  void pushExternalDeclIntoScope(NamedDecl *D, DeclarationName Name);
-
   /// isDeclInScope - If 'Ctx' is a function/method, isDeclInScope returns true
   /// if 'D' is in Scope 'S', otherwise 'S' is ignored and isDeclInScope 
returns
   /// true if 'D' belongs to the given declaration context.

Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=358538=358537=358538=diff
==
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Tue Apr 16 15:59:39 2019
@@ -1404,11 +1404,6 @@ void Sema::PushOnScopeChains(NamedDecl *
   }
 }
 
-void Sema::pushExternalDeclIntoScope(NamedDecl *D, DeclarationName Name) {
-  if (IdResolver.tryAddTopLevelDecl(D, Name) && TUScope)
-TUScope->AddDecl(D);
-}
-
 bool Sema::isDeclInScope(NamedDecl *D, DeclContext *Ctx, Scope *S,
  bool AllowInlineNamespace) {
   return IdResolver.isDeclInScope(D, Ctx, S, AllowInlineNamespace);


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

[PATCH] D60233: [clang-scan-deps] initial outline of the tool that runs preprocessor to find dependencies over a JSON compilation database

[Jan Korous via Phabricator via cfe-commits]

jkorous added inline comments.



Comment at: tools/clang-scan-deps/ClangScanDeps.cpp:39
+  bool runInvocation(std::shared_ptr Invocation,
+ FileManager *Files,
+ std::shared_ptr PCHContainerOps,

`Files` -> `FileMgr` might be more readable.



Comment at: tools/clang-scan-deps/ClangScanDeps.cpp:56
+auto Action = llvm::make_unique();
+const bool Success = Compiler.ExecuteAction(*Action);
+Files->clearStatCache();

`Success` -> `Result`?



Comment at: tools/clang-scan-deps/ClangScanDeps.cpp:91
+public:
+  DependencyScanningTool(const tooling::CompilationDatabase )
+  : Compilations(Compilations) {

Maybe it's obvious but maybe we could add some mention of expected lifetime of 
`Compilations` argument.



Comment at: tools/clang-scan-deps/ClangScanDeps.cpp:112
+  std::shared_ptr PCHContainerOps;
+  /// The real filesystem used a base for all the operations performed by the
+  /// tool.

Missing "as"?



Comment at: tools/clang-scan-deps/ClangScanDeps.cpp:124
+NumThreads("j", llvm::cl::Optional,
+   llvm::cl::desc("Number of worker threads to use"),
+   llvm::cl::init(0));

We might mention the default value (`llvm::hardware_concurrency()`).



Comment at: tools/clang-scan-deps/ClangScanDeps.cpp:203
+// Run the tool on it.
+HadErrors = WorkerTools[I]->runOnFile(Input, CWD);
+  }

Shouldn't this be `|=`? This seems like we're returning only the result of the 
last run.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60233/new/

https://reviews.llvm.org/D60233



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

r358537 - Modify test to use -S instead of -c so that it works when an external assembler is used that is not present.

[Douglas Yung via cfe-commits]

Author: dyung
Date: Tue Apr 16 15:52:05 2019
New Revision: 358537

URL: http://llvm.org/viewvc/llvm-project?rev=358537=rev
Log:
Modify test to use -S instead of -c so that it works when an external assembler 
is used that is not present.

Modified:
cfe/trunk/test/Driver/modules.cpp

Modified: cfe/trunk/test/Driver/modules.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/modules.cpp?rev=358537=358536=358537=diff
==
--- cfe/trunk/test/Driver/modules.cpp (original)
+++ cfe/trunk/test/Driver/modules.cpp Tue Apr 16 15:52:05 2019
@@ -12,7 +12,7 @@
 
 // Check compiling a .pcm file to a .o file.
 //
-// RUN: %clang -std=c++2a %t/module.pcm -c -o %t/module.pcm.o -v 2>&1 | 
FileCheck %s --check-prefix=CHECK-COMPILE
+// RUN: %clang -std=c++2a %t/module.pcm -S -o %t/module.pcm.o -v 2>&1 | 
FileCheck %s --check-prefix=CHECK-COMPILE
 //
 // CHECK-COMPILE: -cc1 {{.*}} {{-emit-obj|-S}}
 // CHECK-COMPILE-SAME: -o {{.*}}.{{pcm.o|s}}
@@ -21,7 +21,7 @@
 
 // Check use of a .pcm file in another compilation.
 //
-// RUN: %clang -std=c++2a -fmodule-file=%t/module.pcm -Dexport= %s -c -o 
%t/module.o -v 2>&1 | FileCheck %s --check-prefix=CHECK-USE
+// RUN: %clang -std=c++2a -fmodule-file=%t/module.pcm -Dexport= %s -S -o 
%t/module.o -v 2>&1 | FileCheck %s --check-prefix=CHECK-USE
 //
 // CHECK-USE: -cc1
 // CHECK-USE-SAME: {{-emit-obj|-S}}
@@ -31,7 +31,7 @@
 
 // Check combining precompile and compile steps works.
 //
-// RUN: %clang -std=c++2a -x c++-module %s -c -o %t/module2.pcm.o -v 2>&1 | 
FileCheck %s --check-prefix=CHECK-PRECOMPILE --check-prefix=CHECK-COMPILE
+// RUN: %clang -std=c++2a -x c++-module %s -S -o %t/module2.pcm.o -v 2>&1 | 
FileCheck %s --check-prefix=CHECK-PRECOMPILE --check-prefix=CHECK-COMPILE
 
 // Check that .cppm is treated as a module implicitly.
 //


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

[PATCH] D60800: [MS] Emit S_HEAPALLOCSITE debug info

[Amy Huang via Phabricator via cfe-commits]

akhuang updated this revision to Diff 195478.
akhuang added a comment.

more typos


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60800/new/

https://reviews.llvm.org/D60800

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGen/debug-info-codeview-heapallocsite.c
  llvm/include/llvm/CodeGen/MachineFunction.h
  llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
  llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h
  llvm/lib/CodeGen/MachineFunction.cpp
  llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
  llvm/test/CodeGen/X86/label-heapallocsite.ll

Index: llvm/test/CodeGen/X86/label-heapallocsite.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/label-heapallocsite.ll
@@ -0,0 +1,73 @@
+; RUN: llc -O0 < %s | FileCheck %s
+; FIXME: Add test for llc with optimizations once it is implemented.
+
+; Source to regenerate:
+; $ clang --target=x86_64-windows-msvc -S heapallocsite.c  -g -gcodeview -o t.ll \
+;  -emit-llvm -O0 -Xclang -disable-llvm-passes -fms-extensions
+; __declspec(allocator) char *myalloc();
+; void g();
+; void foo() {
+;   g();
+;   myalloc()
+;   g();
+; }
+
+; ModuleID = 'heapallocsite.c'
+source_filename = "heapallocsite.c"
+target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-windows-msvc"
+
+; Function Attrs: noinline nounwind optnone
+define dso_local void @f() #0 !dbg !7 {
+entry:
+  call void @g(), !dbg !11
+  %call = call i8* @myalloc(), !dbg !12, !heapallocsite !13
+  call void @g(), !dbg !14
+  ret void, !dbg !15
+}
+
+; CHECK-LABEL: f: # @f
+; CHECK: callq g
+; CHECK: .Lheapallocsite0:
+; CHECK: callq myalloc
+; CHECK: .Lheapallocsite1:
+; CHECK: retq
+
+; CHECK-LABEL: .short  4423# Record kind: S_GPROC32_ID
+; CHECK:   .short  4446# Record kind: S_HEAPALLOCSITE
+; CHECK-NEXT:  .secrel32   .Lheapallocsite0
+; CHECK-NEXT:  .secidx .Lheapallocsite0
+; CHECK-NEXT:  .short  .Lheapallocsite1-.Lheapallocsite0
+; CHECK-NEXT:  .long 1539
+; CHECK-NEXT:  .p2align 2
+
+; CHECK-LABEL: .short  4431# Record kind: S_PROC_ID_END
+
+declare dso_local void @g() #1
+
+declare dso_local i8* @myalloc() #1
+
+attributes #0 = { noinline nounwind optnone "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #1 = { "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 9.0.0 (https://github.com/llvm/llvm-project.git 4eff3de99423a62fd6e833e29c71c1e62ba6140b)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, nameTableKind: None)
+!1 = !DIFile(filename: "", directory: "C:\5Csrc\5Ctest", checksumkind: CSK_MD5, checksum: "6d758cfa3834154a04ce8a55102772a9")
+!2 = !{}
+!3 = !{i32 2, !"CodeView", i32 1}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 2}
+!6 = !{!"clang version 9.0.0 (https://github.com/llvm/llvm-project.git 4eff3de99423a62fd6e833e29c71c1e62ba6140b)"}
+!7 = distinct !DISubprogram(name: "f", scope: !8, file: !8, line: 4, type: !9, scopeLine: 4, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !2)
+!8 = !DIFile(filename: "heapallocsite.c", directory: "C:\5Csrc\5Ctest", checksumkind: CSK_MD5, checksum: "6d758cfa3834154a04ce8a55102772a9")
+!9 = !DISubroutineType(types: !10)
+!10 = !{null}
+!11 = !DILocation(line: 5, scope: !7)
+!12 = !DILocation(line: 6, scope: !7)
+!13 = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char)
+!14 = !DILocation(line: 7, scope: !7)
+!15 = !DILocation(line: 8, scope: !7)
+
Index: llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
===
--- llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
+++ llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
@@ -1234,6 +1234,11 @@
   if (CLI.NumResultRegs && CLI.CS)
 updateValueMap(CLI.CS->getInstruction(), CLI.ResultReg, CLI.NumResultRegs);
 
+  // Set labels for heapallocsite call.
+  if (CLI.CS && CLI.CS->getInstruction()->getMetadata("heapallocsite"))
+MF->addCodeViewHeapAllocSite(CLI.Call, 

[PATCH] D58094: Fix -Wnonportable-include-path suppression for header maps with absolute paths.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Ping.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58094/new/

https://reviews.llvm.org/D58094



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

[PATCH] D60800: [MS] Emit S_HEAPALLOCSITE debug info

[Amy Huang via Phabricator via cfe-commits]

akhuang updated this revision to Diff 195475.
akhuang added a comment.

remove comment


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60800/new/

https://reviews.llvm.org/D60800

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGen/debug-info-codeview-heapallocsite.c
  llvm/include/llvm/CodeGen/MachineFunction.h
  llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
  llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h
  llvm/lib/CodeGen/MachineFunction.cpp
  llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
  llvm/test/CodeGen/X86/label-heapallocsite.ll

Index: llvm/test/CodeGen/X86/label-heapallocsite.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/label-heapallocsite.ll
@@ -0,0 +1,73 @@
+; RUN: llc -O0 < %s | FileCheck %s
+; FIXME: Add test for llc with optimizations once it is implemented.
+
+; Source to regenerate:
+; $ clang --target=x86_64-windows-msvc -S heapallocsite.c  -g -gcodeview -o t.ll \
+;  -emit-llvm -O0 -Xclang -disable-llvm-passes -fms-extensions
+; __declspec(allocator) char *myalloc();
+; void g();
+; void foo() {
+;   g();
+;   myalloc()
+;   g();
+; }
+
+; ModuleID = 'heapallocsite.c'
+source_filename = "heapallocsite.c"
+target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-windows-msvc"
+
+; Function Attrs: noinline nounwind optnone
+define dso_local void @f() #0 !dbg !7 {
+entry:
+  call void @g(), !dbg !11
+  %call = call i8* @myalloc(), !dbg !12, !heapallocsite !13
+  call void @g(), !dbg !14
+  ret void, !dbg !15
+}
+
+; CHECK-LABEL: f: # @f
+; CHECK: callq g
+; CHECK: .Lheapallocsite0:
+; CHECK: callq myalloc
+; CHECK: .Lheapallocsite1:
+; CHECK: retq
+
+; CHECK-LABEL: .short  4423# Record kind: S_GPROC32_ID
+; CHECK:   .short  4446# Record kind: S_HEAPALLOCSITE
+; CHECK-NEXT:  .secrel32   .Lheapallocsite0
+; CHECK-NEXT:  .secidx .Lheapallocsite0
+; CHECK-NEXT:  .short  .Lheapallocsite1-.Lheapallocsite0
+; CHECK-NEXT:  .long 1539
+; CHECK-NEXT:  .p2align 2
+
+; CHECK-LABEL: .short  4431# Record kind: S_PROC_ID_END
+
+declare dso_local void @g() #1
+
+declare dso_local i8* @myalloc() #1
+
+attributes #0 = { noinline nounwind optnone "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #1 = { "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 9.0.0 (https://github.com/llvm/llvm-project.git 4eff3de99423a62fd6e833e29c71c1e62ba6140b)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, nameTableKind: None)
+!1 = !DIFile(filename: "", directory: "C:\5Csrc\5Ctest", checksumkind: CSK_MD5, checksum: "6d758cfa3834154a04ce8a55102772a9")
+!2 = !{}
+!3 = !{i32 2, !"CodeView", i32 1}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 2}
+!6 = !{!"clang version 9.0.0 (https://github.com/llvm/llvm-project.git 4eff3de99423a62fd6e833e29c71c1e62ba6140b)"}
+!7 = distinct !DISubprogram(name: "f", scope: !8, file: !8, line: 4, type: !9, scopeLine: 4, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !2)
+!8 = !DIFile(filename: "heapallocsite.c", directory: "C:\5Csrc\5Ctest", checksumkind: CSK_MD5, checksum: "6d758cfa3834154a04ce8a55102772a9")
+!9 = !DISubroutineType(types: !10)
+!10 = !{null}
+!11 = !DILocation(line: 5, scope: !7)
+!12 = !DILocation(line: 6, scope: !7)
+!13 = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char)
+!14 = !DILocation(line: 7, scope: !7)
+!15 = !DILocation(line: 8, scope: !7)
+
Index: llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
===
--- llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
+++ llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
@@ -1234,6 +1234,11 @@
   if (CLI.NumResultRegs && CLI.CS)
 updateValueMap(CLI.CS->getInstruction(), CLI.ResultReg, CLI.NumResultRegs);
 
+  // Set labels for heapallocsite call.
+  if (CLI.CS && CLI.CS->getInstruction()->getMetadata("heapallocsite"))
+MF->addCodeViewHeapAllocSite(CLI.Call, 

[PATCH] D60800: [MS] Emit S_HEAPALLOCSITE debug info

[Amy Huang via Phabricator via cfe-commits]

akhuang created this revision.
akhuang added reviewers: hans, rnk.
Herald added subscribers: llvm-commits, cfe-commits, hiraditya, aprantl.
Herald added projects: clang, LLVM.

This emits labels around heapallocsite calls and S_HEAPALLOCSITE debug
info in codeview. Currently only changes FastISel, so emitting labels still
needs to be implemented in SelectionDAG.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D60800

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGen/debug-info-codeview-heapallocsite.c
  llvm/include/llvm/CodeGen/MachineFunction.h
  llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
  llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h
  llvm/lib/CodeGen/MachineFunction.cpp
  llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
  llvm/test/CodeGen/X86/label-heapallocsite.ll

Index: llvm/test/CodeGen/X86/label-heapallocsite.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/label-heapallocsite.ll
@@ -0,0 +1,73 @@
+; RUN: llc -O0 < %s | FileCheck %s
+; FIXME: Add test for llc with optimizations once it is implemented.
+
+; Source to regenerate:
+; $ clang --target=x86_64-windows-msvc -S heapallocsite.c  -g -gcodeview -o t.ll \
+;  -emit-llvm -O0 -Xclang -disable-llvm-passes -fms-extensions
+; __declspec(allocator) char *myalloc();
+; void g();
+; void foo() {
+;   g();
+;   myalloc()
+;   g();
+; }
+
+; ModuleID = 'heapallocsite.c'
+source_filename = "heapallocsite.c"
+target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-windows-msvc"
+
+; Function Attrs: noinline nounwind optnone
+define dso_local void @f() #0 !dbg !7 {
+entry:
+  call void @g(), !dbg !11
+  %call = call i8* @myalloc(), !dbg !12, !heapallocsite !13
+  call void @g(), !dbg !14
+  ret void, !dbg !15
+}
+
+; CHECK-LABEL: f: # @f
+; CHECK: callq g
+; CHECK: .Lheapallocsite0:
+; CHECK: callq myalloc
+; CHECK: .Lheapallocsite1:
+; CHECK: retq
+
+; CHECK-LABEL: .short  4423# Record kind: S_GPROC32_ID
+; CHECK:   .short  4446# Record kind: S_HEAPALLOCSITE
+; CHECK-NEXT:  .secrel32   .Lheapallocsite0
+; CHECK-NEXT:  .secidx .Lheapallocsite0
+; CHECK-NEXT:  .short  .Lheapallocsite1-.Lheapallocsite0
+; CHECK-NEXT:  .long 1539
+; CHECK-NEXT:  .p2align 2
+
+; CHECK-LABEL: .short  4431# Record kind: S_PROC_ID_END
+
+declare dso_local void @g() #1
+
+declare dso_local i8* @myalloc() #1
+
+attributes #0 = { noinline nounwind optnone "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #1 = { "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 9.0.0 (https://github.com/llvm/llvm-project.git 4eff3de99423a62fd6e833e29c71c1e62ba6140b)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, nameTableKind: None)
+!1 = !DIFile(filename: "", directory: "C:\5Csrc\5Ctest", checksumkind: CSK_MD5, checksum: "6d758cfa3834154a04ce8a55102772a9")
+!2 = !{}
+!3 = !{i32 2, !"CodeView", i32 1}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 2}
+!6 = !{!"clang version 9.0.0 (https://github.com/llvm/llvm-project.git 4eff3de99423a62fd6e833e29c71c1e62ba6140b)"}
+!7 = distinct !DISubprogram(name: "f", scope: !8, file: !8, line: 4, type: !9, scopeLine: 4, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !2)
+!8 = !DIFile(filename: "heapallocsite.c", directory: "C:\5Csrc\5Ctest", checksumkind: CSK_MD5, checksum: "6d758cfa3834154a04ce8a55102772a9")
+!9 = !DISubroutineType(types: !10)
+!10 = !{null}
+!11 = !DILocation(line: 5, scope: !7)
+!12 = !DILocation(line: 6, scope: !7)
+!13 = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char)
+!14 = !DILocation(line: 7, scope: !7)
+!15 = !DILocation(line: 8, scope: !7)
+
Index: llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
===
--- llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
+++ llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
@@ -1234,6 +1234,19 @@
   if (CLI.NumResultRegs && CLI.CS)
 

[PATCH] D60335: Use -fomit-frame-pointer when optimizing PowerPC code

[George Koehler via Phabricator via cfe-commits]

kernigh added a comment.

I'm stuck. I didn't put a test in this patch because I can't run the tests yet. 
So far, I can build a clang executable but can't build the rest of the project. 
I won't run the tests without a complete build, and I won't edit the tests 
without running them.

I had difficulty because llvm is a large C++ project that takes a long time to 
build. I tried to save time by building it on my fastest machine, an x86 
desktop with 2 cores and 4G RAM running OpenBSD 6.4 amd64. I tried a parallel 
build in my usual way, `cmake -GNinja ...` then `ninja`, but it got stuck near 
the end when it tried to run 3 linkers in parallel. The linkers used too much 
RAM, so my machine got stuck in swap. (Do other people have more RAM?) I 
learned to save RAM by building the project with clang -Oz and without -g (so 
there is no debug info), and by using `ninja -j2 clang` to link only the clang 
executable and nothing else.

My "running OpenBSD 6.4" is a problem. Code in 
llvm/lib/Support/Unix/Threading.inc for OpenBSD tries to call 
pthread_get_name_np(), which will appear in OpenBSD 6.5. I needed to edit that 
file. Then I built a clang executable, but it didn't run, because of an "Out of 
memory" error. The kernel of OpenBSD 6.4 seems to refuse to run such a large 
executable. I worked around it by hacking clang into a large shared library 
named almost_clang, then building a tiny clang executable that just calls the 
shared library (by editing clang/tools/driver/{CMakeLists.txt,driver.cpp}). 
That caused another error, because llvm tried to set the shared library's 
version to "9svn", which isn't legal. I edited llvm/cmake/modules/AddLLVM.cmake 
to remove the shared library's version. Now I can run clang.

If I try to build the rest of the project, like `ninja -j1` (because I don't 
want parallel linkers), then I get a linker error. It seems confused about the 
shared library version. The trick that I used to build clang might have broken 
something else. Before I try to fix my build, I want to upgrade this machine to 
OpenBSD 6.5 amd64. (So I am waiting for 6.5 to release; I already have 6.5-beta 
on my slow PowerPC machine.) That should at least let me undo the 
pthread_get_name_np() removal, so I don't have too many local changes. If I can 
build the rest of llvm and clang on OpenBSD 6.5, then I can try to run and edit 
the tests.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60335/new/

https://reviews.llvm.org/D60335



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

[PATCH] D59712: [APSInt][OpenMP] Fix isNegative, etc. for unsigned types

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D59712#1469358 , @jdenny wrote:

> In D59712#1469301 , @lebedev.ri 
> wrote:
>
> > In D59712#1469295 , @craig.topper 
> > wrote:
> >
> > > Wondering if it would be better to assert for asking for the sign of an 
> > > unsigned APSInt. I could see a caller just wanting to get the msb for 
> > > some reason and not knowing that isNegative won’t work.
> >
> >
> > Yes, i, too, would think an assert is much better solution (since i 
> > literally just tripped over this in this review.)
>


Does this pass `check-all`? `check-all` of stage-2? test-suite?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59712/new/

https://reviews.llvm.org/D59712



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

[PATCH] D59712: [APSInt][OpenMP] Fix isNegative, etc. for unsigned types

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

In D59712#1469301 , @lebedev.ri wrote:

> In D59712#1469295 , @craig.topper 
> wrote:
>
> > Wondering if it would be better to assert for asking for the sign of an 
> > unsigned APSInt. I could see a caller just wanting to get the msb for some 
> > reason and not knowing that isNegative won’t work.
>
>
> Yes, i, too, would think an assert is much better solution (since i literally 
> just tripped over this in this review.)


Imagine a caller wants to check for a negative value for an APSInt from an 
arbitrary expression.  Whether the value is negative is important, but not why. 
 Given that APSInt is documented as knowing its signedness, it seems 
unintuitive that the caller is responsible for first checking for an unsigned 
type to avoid an internal compiler error.

If the fear is that developers are too used to calling isNegative to check the 
high bit, maybe isNegative should always (regardless of signedness) fail an 
assert for APSInt, and we should find different function names that won't cause 
such confusion.

Whatever we do for isNegative, we should probably do something similar for 
isNonNegative as it has the same issues.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59712/new/

https://reviews.llvm.org/D59712



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

[PATCH] D59712: [APSInt][OpenMP] Fix isNegative, etc. for unsigned types

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D59712#1469295 , @craig.topper 
wrote:

> Wondering if it would be better to assert for asking for the sign of an 
> unsigned APSInt. I could see a caller just wanting to get the msb for some 
> reason and not knowing that isNegative won’t work.


Yes, i, too, would think an assert is much better solution (since i literally 
just tripped over this in this review.)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59712/new/

https://reviews.llvm.org/D59712



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

[PATCH] D59712: [APSInt][OpenMP] Fix isNegative, etc. for unsigned types

[Joel E. Denny via Phabricator via cfe-commits]

jdenny updated this revision to Diff 195452.
jdenny added a comment.

Duplicate new APSInt test but for signed type.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59712/new/

https://reviews.llvm.org/D59712

Files:
  clang/test/OpenMP/distribute_collapse_messages.cpp
  clang/test/OpenMP/distribute_parallel_for_collapse_messages.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_collapse_messages.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_safelen_messages.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_simdlen_messages.cpp
  clang/test/OpenMP/distribute_simd_collapse_messages.cpp
  clang/test/OpenMP/distribute_simd_safelen_messages.cpp
  clang/test/OpenMP/distribute_simd_simdlen_messages.cpp
  clang/test/OpenMP/for_collapse_messages.cpp
  clang/test/OpenMP/for_ordered_clause.cpp
  clang/test/OpenMP/for_simd_collapse_messages.cpp
  clang/test/OpenMP/for_simd_safelen_messages.cpp
  clang/test/OpenMP/for_simd_simdlen_messages.cpp
  clang/test/OpenMP/parallel_for_collapse_messages.cpp
  clang/test/OpenMP/parallel_for_ordered_messages.cpp
  clang/test/OpenMP/parallel_for_simd_collapse_messages.cpp
  clang/test/OpenMP/parallel_for_simd_safelen_messages.cpp
  clang/test/OpenMP/parallel_for_simd_simdlen_messages.cpp
  clang/test/OpenMP/simd_collapse_messages.cpp
  clang/test/OpenMP/simd_safelen_messages.cpp
  clang/test/OpenMP/simd_simdlen_messages.cpp
  clang/test/OpenMP/target_parallel_for_collapse_messages.cpp
  clang/test/OpenMP/target_parallel_for_ordered_messages.cpp
  clang/test/OpenMP/target_parallel_for_simd_collapse_messages.cpp
  clang/test/OpenMP/target_parallel_for_simd_ordered_messages.cpp
  clang/test/OpenMP/target_parallel_for_simd_safelen_messages.cpp
  clang/test/OpenMP/target_parallel_for_simd_simdlen_messages.cpp
  clang/test/OpenMP/target_simd_collapse_messages.cpp
  clang/test/OpenMP/target_simd_safelen_messages.cpp
  clang/test/OpenMP/target_simd_simdlen_messages.cpp
  clang/test/OpenMP/target_teams_distribute_collapse_messages.cpp
  clang/test/OpenMP/target_teams_distribute_parallel_for_collapse_messages.cpp
  
clang/test/OpenMP/target_teams_distribute_parallel_for_simd_collapse_messages.cpp
  
clang/test/OpenMP/target_teams_distribute_parallel_for_simd_safelen_messages.cpp
  
clang/test/OpenMP/target_teams_distribute_parallel_for_simd_simdlen_messages.cpp
  clang/test/OpenMP/target_teams_distribute_simd_collapse_messages.cpp
  clang/test/OpenMP/target_teams_distribute_simd_safelen_messages.cpp
  clang/test/OpenMP/target_teams_distribute_simd_simdlen_messages.cpp
  clang/test/OpenMP/taskloop_collapse_messages.cpp
  clang/test/OpenMP/taskloop_simd_collapse_messages.cpp
  clang/test/OpenMP/taskloop_simd_safelen_messages.cpp
  clang/test/OpenMP/taskloop_simd_simdlen_messages.cpp
  clang/test/OpenMP/teams_distribute_collapse_messages.cpp
  clang/test/OpenMP/teams_distribute_parallel_for_collapse_messages.cpp
  clang/test/OpenMP/teams_distribute_parallel_for_simd_collapse_messages.cpp
  clang/test/OpenMP/teams_distribute_parallel_for_simd_safelen_messages.cpp
  clang/test/OpenMP/teams_distribute_parallel_for_simd_simdlen_messages.cpp
  clang/test/OpenMP/teams_distribute_simd_collapse_messages.cpp
  clang/test/OpenMP/teams_distribute_simd_safelen_messages.cpp
  clang/test/OpenMP/teams_distribute_simd_simdlen_messages.cpp
  llvm/include/llvm/ADT/APSInt.h
  llvm/unittests/ADT/APSIntTest.cpp

Index: llvm/unittests/ADT/APSIntTest.cpp
===
--- llvm/unittests/ADT/APSIntTest.cpp
+++ llvm/unittests/ADT/APSIntTest.cpp
@@ -159,4 +159,90 @@
 
 #endif
 
+TEST(APSIntTest, SignedHighBit) {
+  APSInt False(APInt(1, 0), false);
+  APSInt True(APInt(1, 1), false);
+  APSInt CharMin(APInt(8, 0), false);
+  APSInt CharSmall(APInt(8, 0x13), false);
+  APSInt CharBoundaryUnder(APInt(8, 0x7f), false);
+  APSInt CharBoundaryOver(APInt(8, 0x80), false);
+  APSInt CharLarge(APInt(8, 0xd9), false);
+  APSInt CharMax(APInt(8, 0xff), false);
+
+  EXPECT_FALSE(False.isNegative());
+  EXPECT_TRUE(False.isNonNegative());
+  EXPECT_FALSE(False.isStrictlyPositive());
+
+  EXPECT_TRUE(True.isNegative());
+  EXPECT_FALSE(True.isNonNegative());
+  EXPECT_FALSE(True.isStrictlyPositive());
+
+  EXPECT_FALSE(CharMin.isNegative());
+  EXPECT_TRUE(CharMin.isNonNegative());
+  EXPECT_FALSE(CharMin.isStrictlyPositive());
+
+  EXPECT_FALSE(CharSmall.isNegative());
+  EXPECT_TRUE(CharSmall.isNonNegative());
+  EXPECT_TRUE(CharSmall.isStrictlyPositive());
+
+  EXPECT_FALSE(CharBoundaryUnder.isNegative());
+  EXPECT_TRUE(CharBoundaryUnder.isNonNegative());
+  EXPECT_TRUE(CharBoundaryUnder.isStrictlyPositive());
+
+  EXPECT_TRUE(CharBoundaryOver.isNegative());
+  EXPECT_FALSE(CharBoundaryOver.isNonNegative());
+  EXPECT_FALSE(CharBoundaryOver.isStrictlyPositive());
+
+  EXPECT_TRUE(CharLarge.isNegative());
+  EXPECT_FALSE(CharLarge.isNonNegative());
+  EXPECT_FALSE(CharLarge.isStrictlyPositive());
+
+  EXPECT_TRUE(CharMax.isNegative());
+  

[PATCH] D59712: [APSInt][OpenMP] Fix isNegative, etc. for unsigned types

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

Wondering if it would be better to assert for asking for the sign of an 
unsigned APSInt. I could see a caller just wanting to get the msb for some 
reason and not knowing that isNegative won’t work.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59712/new/

https://reviews.llvm.org/D59712



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

[PATCH] D60629: [clang-tidy] Change the namespace for llvm checkers from 'llvm' to 'llvm_check'

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added inline comments.



Comment at: clang-tools-extra/clang-tidy/add_new_check.py:381
+  # Don't allow 'clang' or 'llvm' namespaces
+  if module == 'llvm' or module == 'clang':
+namespace = module + '_check'

We don't have the `clang` module. No need to check for it here. If we ever add 
one (which I doubt), we can modify this script again. But for now let's remove 
this to avoid confusion.



Comment at: clang-tools-extra/clang-tidy/rename_check.py:268
+  if old_module != new_module or new_module == 'llvm' or new_module == 'clang':
+if new_module == 'llvm' or new_module -- 'clang':
+  new_namespace = new_module + '_check'

`--`?

Anyways, we don't have (or plan to have) a module named `clang`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60629/new/

https://reviews.llvm.org/D60629



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

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM! You can either land this now and refactor after the AST matcher lands, or 
you can wait until the AST matcher lands and land this patch after -- your call.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59806/new/

https://reviews.llvm.org/D59806



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

[PATCH] D59712: [APSInt][OpenMP] Fix isNegative, etc. for unsigned types

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri marked an inline comment as done.
lebedev.ri added inline comments.



Comment at: llvm/unittests/ADT/APSIntTest.cpp:162
 
+TEST(APSIntTest, UnsignedHighBit) {
+  APSInt False(APInt(1, 0));

Can you please duplicate this whole test but for signed `APSInt`?



Comment at: llvm/unittests/ADT/APSIntTest.cpp:188-194
+  EXPECT_FALSE(CharBoundaryUnder.isNegative());
+  EXPECT_TRUE(CharBoundaryUnder.isNonNegative());
+  EXPECT_TRUE(CharBoundaryUnder.isStrictlyPositive());
+
+  EXPECT_FALSE(CharBoundaryOver.isNegative());
+  EXPECT_TRUE(CharBoundaryOver.isNonNegative());
+  EXPECT_TRUE(CharBoundaryOver.isStrictlyPositive());

jdenny wrote:
> lebedev.ri wrote:
> > I do not understand.
> > `0x7f` is 127, it is obviously a maximal positive 8-bit value.
> > but `0x80` is 128 aka -128, is it not minimal negative 8-bit value?
> > Is the test correct?
> This test checks that unsigned types are never seen as negative, so it's 
> checking values that would be negative if the type were signed.
Ooh, i see. I was looking at the wrong constructor.
```
  explicit APSInt(APInt I, bool isUnsigned = true)
   : APInt(std::move(I)), IsUnsigned(isUnsigned) {}
```
was the one being called.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59712/new/

https://reviews.llvm.org/D59712



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

[PATCH] D59712: [APSInt][OpenMP] Fix isNegative, etc. for unsigned types

[Joel E. Denny via Phabricator via cfe-commits]

jdenny marked an inline comment as done.
jdenny added inline comments.



Comment at: llvm/unittests/ADT/APSIntTest.cpp:188-194
+  EXPECT_FALSE(CharBoundaryUnder.isNegative());
+  EXPECT_TRUE(CharBoundaryUnder.isNonNegative());
+  EXPECT_TRUE(CharBoundaryUnder.isStrictlyPositive());
+
+  EXPECT_FALSE(CharBoundaryOver.isNegative());
+  EXPECT_TRUE(CharBoundaryOver.isNonNegative());
+  EXPECT_TRUE(CharBoundaryOver.isStrictlyPositive());

lebedev.ri wrote:
> I do not understand.
> `0x7f` is 127, it is obviously a maximal positive 8-bit value.
> but `0x80` is 128 aka -128, is it not minimal negative 8-bit value?
> Is the test correct?
This test checks that unsigned types are never seen as negative, so it's 
checking values that would be negative if the type were signed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59712/new/

https://reviews.llvm.org/D59712



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

[PATCH] D59168: [runtimes] Move libunwind, libc++abi and libc++ to lib/clang/ and include/

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

Is there anything I can do to help this patch make progress?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59168/new/

https://reviews.llvm.org/D59168



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

[PATCH] D60776: [clang-tidy] Add test support for the fix description.

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh requested changes to this revision.
alexfh added inline comments.
This revision now requires changes to proceed.



Comment at: clang-tools-extra/test/clang-tidy/check_clang_tidy.py:109
 check_notes_prefix = 'CHECK-NOTES' + file_check_suffix
+check_fix_descriptions_prefix = 'CHECK-FIX-DESCRIPTIONS' + 
file_check_suffix
 

Wait, how do "check descriptions" differ from notes?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60776/new/

https://reviews.llvm.org/D60776



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

[PATCH] D59712: [APSInt][OpenMP] Fix isNegative, etc. for unsigned types

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri requested changes to this revision.
lebedev.ri added inline comments.
This revision now requires changes to proceed.



Comment at: llvm/unittests/ADT/APSIntTest.cpp:188-194
+  EXPECT_FALSE(CharBoundaryUnder.isNegative());
+  EXPECT_TRUE(CharBoundaryUnder.isNonNegative());
+  EXPECT_TRUE(CharBoundaryUnder.isStrictlyPositive());
+
+  EXPECT_FALSE(CharBoundaryOver.isNegative());
+  EXPECT_TRUE(CharBoundaryOver.isNonNegative());
+  EXPECT_TRUE(CharBoundaryOver.isStrictlyPositive());

I do not understand.
`0x7f` is 127, it is obviously a maximal positive 8-bit value.
but `0x80` is 128 aka -128, is it not minimal negative 8-bit value?
Is the test correct?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59712/new/

https://reviews.llvm.org/D59712



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

[PATCH] D59923: [Driver] Simplify -g level computation and its interaction with -gsplit-dwarf

[David Blaikie via Phabricator via cfe-commits]

dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

Looks ok - thanks!


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59923/new/

https://reviews.llvm.org/D59923



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

[PATCH] D60455: [SYCL] Add support for SYCL device attributes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D60455#1468714 , @bader wrote:

> In D60455#1468386 , @Fznamznon wrote:
>
> > > Ok, my question is whether you are planning to duplicate the same logic 
> > > as for OpenCL kernel which doesn't really seem like an ideal design 
> > > choice. Is this the only difference then we can simply add an extra check 
> > > for SYCL compilation mode in this template handling case. The overall 
> > > interaction between OpenCL and SYCL implementation is still a very big 
> > > unknown to me so it's not very easy to judge about the implementations 
> > > details...
> >
> > Of course, if nothing prevents us to re-use OpenCL kernel attribute for 
> > SYCL I assume it would be good idea. 
> >  But I'm thinking about the situation with https://reviews.llvm.org/D60454 
> > . If we re-use OpenCL kernel attributes - we affected by OpenCL-related 
> > changes and OpenCL-related changes shouldn't violate SYCL semantics. Will 
> > it be usable for SYCL/OpenCL clang developers? @bader , what do you think 
> > about it?
>
>
> I also think it's worth trying. We should be able to cover "SYCL semantics" 
> with LIT test to avoid regressions by OpenCL related changes. E.g. add a test 
> case checking that -fsycl-is-device option disables restriction on applying 
> `__kernel` to template functions.
>  I want to confirm that everyone is okay to enable `__kernel` keyword for 
> SYCL extension and cover SYCL use cases with additional regression tests. 
> IIRC, on yesterday call, @keryell, said that having SYCL specific attributes 
> useful for separation of concerns.


I'm not comfortable with that decision unless the attribute semantics are 
sufficiently related to justify it. If we're just going to have a lot of 
`KernelAttr->isSYCL()` vs `KernelAttr->isOpenCL()` accessor calls, it may make 
more sense to use separate semantic attributes (even if they share spellings), 
though then I'd be curious how a user combines OpenCL and SYCL attributes.

In D60455#1468779 , @bader wrote:

> @aaron.ballman sorry for confusion.
>  SYCL specification doesn't require user to annotate "device functions" with 
> an attribute - it says following (from section 6.9.1 SYCL functions and 
> methods linkage, https://www.khronos.org/registry/SYCL/specs/sycl-1.2.1.pdf, 
> page 251):
>
> > The default behavior in SYCL applications is that all the definitions and 
> > declarations of the functions and methods
> >  are available to the SYCL compiler, in the same translation unit. When 
> > this is not the case, all the symbols that
> >  need to be exported to a SYCL library or from a C++ library to a SYCL 
> > application need to be defined using the
> >  macro: SYCL_EXTERNAL.
> >  The SYCL_EXTERNAL macro will only be defined if the implementation 
> > supports offline linking. The macro is
> >  implementation-defined, but the following restrictions apply:
> > 
> > • SYCL_EXTERNAL can only be used on functions;
> >  • the function cannot use raw pointers as parameter or return types. 
> > Explicit pointer classes must be used instead;
> >  • externally defined functions cannot call a 
> > cl::sycl::parallel_for_work_item method;
> >  • externally defined functions cannot be called from a 
> > cl::sycl::parallel_for_work_group scope.
> > 
> > The SYCL linkage mechanism is optional and implementation defined.
>
> The idea I had is that to define `SYCL_EXTERNAL` macro as `sycl_device` 
> attribute.


That seems sensible, but then we're also missing extra semantic checks in this 
patch, such as prohibiting raw pointers, etc. Those should get added along with 
tests.

> BTW, I noticed that `SYCL_EXTERNAL` puts additional requirements 
> `sycl_device` doesn't meet:
> 
>> • SYCL_EXTERNAL can only be used on functions;
> 
> I think our implementation doesn't have such limitations and able to support 
> more use cases.

Does it make sense to deviate from the SYCL spec though?

> Anyway, we can make `sycl_device` attribute implicit for now and return to 
> the implementation of cross translation unit dependencies later.

That might be an easier first-pass for the attribute. I'm not at all opposed to 
giving it a name, I was just trying to keep the threads of discussion straight. 
:-)

In D60455#1468544 , @Fznamznon wrote:

> In D60455#1467279 , @aaron.ballman 
> wrote:
>
> >
>
>
>
>
> > I'm still wondering what the actual semantics are for the attribute. Right 
> > now, these are being parsed and ignored -- are there future plans here?
>
> Yes, we are going to teach the compiler differ SYCL device code from host 
> code and ignore functions without device attributes when SYCL device mode is 
> enabled. I've described some possible implementation details in this comment 
> .


Ah, thank you for 

[PATCH] D59712: [APSInt][OpenMP] Fix isNegative, etc. for unsigned types

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59712/new/

https://reviews.llvm.org/D59712



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

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8975
+CGM.Error(Clause->getBeginLoc(),
+  "Target region emitted before requires directive.");
+  HasRequiresUnifiedSharedMemory = true;

The message speaks about target region but points to the clause. You need to 
correct the message.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:9065
+// contain at least 1 target region.
+if (HasRequiresUnifiedSharedMemory && hasEmittedTargetRegion)
+  Flags = OMP_REQ_UNIFIED_SHARED_MEMORY;

Hmm, according to the standard `A requires directive with any of the following 
clauses must appear in all compilation units of a program that contain device 
constructs or device routines or in none of them`. You can not detect use of 
the device routines in the code, can you?



Comment at: lib/CodeGen/CGOpenMPRuntime.h:644
+  /// Flag for keeping track of weather a target region has been emitted.
+  bool hasEmittedTargetRegion = false;
+

ABataev wrote:
> gtbercea wrote:
> > ABataev wrote:
> > > gtbercea wrote:
> > > > gtbercea wrote:
> > > > > ABataev wrote:
> > > > > > gtbercea wrote:
> > > > > > > ABataev wrote:
> > > > > > > > Why do you need this? I think your function should be called 
> > > > > > > > without any conditions. It does not depend on the presence of 
> > > > > > > > the target regions or not. Plus, your check is not consistent 
> > > > > > > > if you're calling the function from another module, which has 
> > > > > > > > target region inside.
> > > > > > > This does not determine if the function is called or not. This 
> > > > > > > helps determine the flags with which the function is called.
> > > > > > It does not matter, it still does not work correctly if the target 
> > > > > > region is called in the function from another module
> > > > > If the target is in another compilation unit, that unit will need to 
> > > > > have a requires directive.
> > > > If you can come up with an example which you think doesn't work I'm 
> > > > happy to try it.
> > > If the module without target directives was compiled with unified memory 
> > > and the module with target directives compiled without unfied memory 
> > > support? Is this a problem? Shall we diagnose it?
> > The requires directives in a module without targets are just not taken into 
> > consideration. In general, a target region is needed before the requires 
> > flags are checked for compatibility with flags in other modules.
> You're saying that we're going to ignore the directive completely if the 
> module does not have target regions. I'd suggest to discuss it with Alex if 
> this is ok per standard.
Must start from capital letter


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60568/new/

https://reviews.llvm.org/D60568



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

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea marked an inline comment as done.
gtbercea added inline comments.



Comment at: lib/CodeGen/CGOpenMPRuntime.h:644
+  /// Flag for keeping track of weather a target region has been emitted.
+  bool hasEmittedTargetRegion = false;
+

ABataev wrote:
> gtbercea wrote:
> > ABataev wrote:
> > > gtbercea wrote:
> > > > gtbercea wrote:
> > > > > ABataev wrote:
> > > > > > gtbercea wrote:
> > > > > > > ABataev wrote:
> > > > > > > > Why do you need this? I think your function should be called 
> > > > > > > > without any conditions. It does not depend on the presence of 
> > > > > > > > the target regions or not. Plus, your check is not consistent 
> > > > > > > > if you're calling the function from another module, which has 
> > > > > > > > target region inside.
> > > > > > > This does not determine if the function is called or not. This 
> > > > > > > helps determine the flags with which the function is called.
> > > > > > It does not matter, it still does not work correctly if the target 
> > > > > > region is called in the function from another module
> > > > > If the target is in another compilation unit, that unit will need to 
> > > > > have a requires directive.
> > > > If you can come up with an example which you think doesn't work I'm 
> > > > happy to try it.
> > > If the module without target directives was compiled with unified memory 
> > > and the module with target directives compiled without unfied memory 
> > > support? Is this a problem? Shall we diagnose it?
> > The requires directives in a module without targets are just not taken into 
> > consideration. In general, a target region is needed before the requires 
> > flags are checked for compatibility with flags in other modules.
> You're saying that we're going to ignore the directive completely if the 
> module does not have target regions. I'd suggest to discuss it with Alex if 
> this is ok per standard.
I discussed with Alex and we agreed on this being the most practical solution. 
This will enable users to ensure consistency of requires flags across relevant 
compilation units only.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60568/new/

https://reviews.llvm.org/D60568



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

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

[Alexandre Eichenberger via Phabricator via cfe-commits]

AlexEichenberger accepted this revision.
AlexEichenberger added a comment.
This revision is now accepted and ready to land.

fantastic


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60568/new/

https://reviews.llvm.org/D60568



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

[PATCH] D60539: Add -std=c++14 language standard option to tests that require C++14 default

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D60539#1469012 , @nemanjai wrote:

> > Do you need to build clangd? We explicitly don't aim to support building 
> > everywhere clang can be built, maybe we should just disable in this case?
>
> Our environment includes various OS levels running on PowerPC. We certainly 
> wouldn't want to disable building/testing `clangd` on all our PowerPC 
> machines. Is there a way to disable it only on certain OS levels?
>
> Furthermore, it seems a little too intrusive to disable an otherwise 
> functional component simply because some test cases rely on a specific 
> language standard default.
>
> Would it be an acceptable solution to add another `StringRef` parameter to 
> `ShouldCollectSymbolTest::build()` - let's call it `ExtraArgs`, to which we 
> can add options such as `-std=c++14` if the test being built relies on that 
> option?


My concern is

- a large fraction of our tests, not just those in this file. rely on the 
default std version (I suspect setting it to c++98 will reveal that). I don't 
think maintaining this information alongside each test and plumbing it through 
every test helper is reasonable. If we want to be robust to changes in this 
flag, I think need to make at least TestTU do the right thing by default. 
Unfortunately the most obvious way to do that (adding `-std-default`) won't 
work.
- there's no buildbot coverage of these configurations, so I'm not sure how 
we'll keep them clean.

Given there's no obvious alternative and the patch is small, it seems OK to 
land this (with a suitable comment), but it's hard for us to commit to 
maintaining it e.g.

- if we add non-C++ testcases in SymbolCollectorTests and need to remove the arg
- for other values of `CLANG_DEFAULT_STD_CXX`
- as more tests are added in the future


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60539/new/

https://reviews.llvm.org/D60539



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

[PATCH] D59887: [Syntax] Introduce TokenBuffer, start clangToolingSyntax library

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 195425.
ilya-biryukov added a comment.

- Simplify rawByExpanded by using a helper function.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59887/new/

https://reviews.llvm.org/D59887

Files:
  clang/include/clang/Tooling/Syntax/Tokens.h
  clang/lib/Tooling/CMakeLists.txt
  clang/lib/Tooling/Syntax/CMakeLists.txt
  clang/lib/Tooling/Syntax/Tokens.cpp
  clang/unittests/Tooling/CMakeLists.txt
  clang/unittests/Tooling/Syntax/CMakeLists.txt
  clang/unittests/Tooling/Syntax/TokensTest.cpp

Index: clang/unittests/Tooling/Syntax/TokensTest.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/Syntax/TokensTest.cpp
@@ -0,0 +1,629 @@
+//===- TokensTest.cpp -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Tooling/Syntax/Tokens.h"
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/Expr.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/FileSystemOptions.h"
+#include "clang/Basic/LLVM.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TokenKinds.def"
+#include "clang/Basic/TokenKinds.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Frontend/Utils.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Lex/PreprocessorOptions.h"
+#include "clang/Lex/Token.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Debug.h"
+#include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/VirtualFileSystem.h"
+#include "llvm/Support/raw_os_ostream.h"
+#include "llvm/Support/raw_ostream.h"
+#include "llvm/Testing/Support/Annotations.h"
+#include "gmock/gmock-more-matchers.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+using namespace clang;
+using namespace clang::syntax;
+
+using ::testing::AllOf;
+using ::testing::Contains;
+using ::testing::ElementsAre;
+using ::testing::Matcher;
+using ::testing::Pointwise;
+
+namespace {
+// Matchers for syntax::Token.
+MATCHER_P(Kind, K, "") { return arg.kind() == K; }
+MATCHER_P2(HasText, Text, SourceMgr, "") {
+  return arg.text(*SourceMgr) == Text;
+}
+MATCHER_P2(IsIdent, Text, SourceMgr, "") {
+  return arg.kind() == tok::identifier && arg.text(*SourceMgr) == Text;
+}
+/// Checks the start and end location of a token are equal to SourceRng.
+MATCHER_P(RangeIs, SourceRng, "") {
+  return arg.location() == SourceRng.first &&
+ arg.endLocation() == SourceRng.second;
+}
+/// Checks the passed tuple has two similar tokens, i.e. both are of the same
+/// kind and have the same text if they are identifiers.
+/// Ignores differences in kind between the raw and non-raw mode.
+MATCHER_P(IsSameToken, SourceMgr, "") {
+  auto ToEquivalenceClass = [](tok::TokenKind Kind) {
+if (Kind == tok::identifier || Kind == tok::raw_identifier ||
+tok::getKeywordSpelling(Kind) != nullptr)
+  return tok::identifier;
+if (Kind == tok::string_literal || Kind == tok::header_name)
+  return tok::string_literal;
+return Kind;
+  };
+
+  auto  = std::get<0>(arg);
+  auto  = std::get<1>(arg);
+  if (ToEquivalenceClass(L.kind()) != ToEquivalenceClass(R.kind()))
+return false;
+  return L.text(*SourceMgr) == L.text(*SourceMgr);
+}
+} // namespace
+
+// Actual test fixture lives in the syntax namespace as it's a friend of
+// TokenBuffer.
+class syntax::TokensTest : public ::testing::Test {
+public:
+  /// Run the clang frontend, collect the preprocessed tokens from the frontend
+  /// invocation and store them in this->Buffer.
+  /// This also clears SourceManager before running the compiler.
+  void recordTokens(llvm::StringRef Code) {
+class RecordTokens : public ASTFrontendAction {
+public:
+  explicit RecordTokens(TokenBuffer ) : Result(Result) {}
+
+  bool BeginSourceFileAction(CompilerInstance ) override {
+assert(!Collector && "expected only a single call to BeginSourceFile");
+Collector.emplace(CI.getPreprocessor());
+return true;
+  }
+  void EndSourceFileAction() override {
+assert(Collector && "BeginSourceFileAction was never called");
+Result = std::move(*Collector).consume();
+  }
+
+  std::unique_ptr
+  

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: lib/CodeGen/CGOpenMPRuntime.h:644
+  /// Flag for keeping track of weather a target region has been emitted.
+  bool hasEmittedTargetRegion = false;
+

gtbercea wrote:
> ABataev wrote:
> > gtbercea wrote:
> > > gtbercea wrote:
> > > > ABataev wrote:
> > > > > gtbercea wrote:
> > > > > > ABataev wrote:
> > > > > > > Why do you need this? I think your function should be called 
> > > > > > > without any conditions. It does not depend on the presence of the 
> > > > > > > target regions or not. Plus, your check is not consistent if 
> > > > > > > you're calling the function from another module, which has target 
> > > > > > > region inside.
> > > > > > This does not determine if the function is called or not. This 
> > > > > > helps determine the flags with which the function is called.
> > > > > It does not matter, it still does not work correctly if the target 
> > > > > region is called in the function from another module
> > > > If the target is in another compilation unit, that unit will need to 
> > > > have a requires directive.
> > > If you can come up with an example which you think doesn't work I'm happy 
> > > to try it.
> > If the module without target directives was compiled with unified memory 
> > and the module with target directives compiled without unfied memory 
> > support? Is this a problem? Shall we diagnose it?
> The requires directives in a module without targets are just not taken into 
> consideration. In general, a target region is needed before the requires 
> flags are checked for compatibility with flags in other modules.
You're saying that we're going to ignore the directive completely if the module 
does not have target regions. I'd suggest to discuss it with Alex if this is ok 
per standard.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60568/new/

https://reviews.llvm.org/D60568



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

[PATCH] D60732: [analyzer] NFC: Use -verify=... in MoveChecker tests.

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

In D60732#1469026 , @jdenny wrote:

> Sometimes you can avoid an explosion of FileCheck prefixes using -D.  I'm not 
> aware of anything like -D for -verify.


I mean a -D that expands a variable within an expected diagnostic.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60732/new/

https://reviews.llvm.org/D60732



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

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea marked an inline comment as done.
gtbercea added inline comments.



Comment at: lib/CodeGen/CGOpenMPRuntime.h:644
+  /// Flag for keeping track of weather a target region has been emitted.
+  bool hasEmittedTargetRegion = false;
+

ABataev wrote:
> gtbercea wrote:
> > gtbercea wrote:
> > > ABataev wrote:
> > > > gtbercea wrote:
> > > > > ABataev wrote:
> > > > > > Why do you need this? I think your function should be called 
> > > > > > without any conditions. It does not depend on the presence of the 
> > > > > > target regions or not. Plus, your check is not consistent if you're 
> > > > > > calling the function from another module, which has target region 
> > > > > > inside.
> > > > > This does not determine if the function is called or not. This helps 
> > > > > determine the flags with which the function is called.
> > > > It does not matter, it still does not work correctly if the target 
> > > > region is called in the function from another module
> > > If the target is in another compilation unit, that unit will need to have 
> > > a requires directive.
> > If you can come up with an example which you think doesn't work I'm happy 
> > to try it.
> If the module without target directives was compiled with unified memory and 
> the module with target directives compiled without unfied memory support? Is 
> this a problem? Shall we diagnose it?
The requires directives in a module without targets are just not taken into 
consideration. In general, a target region is needed before the requires flags 
are checked for compatibility with flags in other modules.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60568/new/

https://reviews.llvm.org/D60568



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

[PATCH] D60732: [analyzer] NFC: Use -verify=... in MoveChecker tests.

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

> Just wanted to give a bit more visibility to the underrated technology of 
> writing -verify=a,b,c instead of the #ifdefclutter.

It's great to see this getting more use.

> It's also a bit wonky because it seems that you have to write an exponential 
> number of flags to verify in order to have full flexibility (which is why i 
> didn't update the #ifdef DFS test), but it still seems to be much better in 
> most cases.

Sometimes you can avoid an explosion of FileCheck prefixes using -D.  I'm not 
aware of anything like -D for -verify.

In D60732#1467504 , @Charusso wrote:

> It is a cool reveal, could you provide a documentation?


The original patch added documentation to `-cc1 -help`.  Doxygen for 
VerifyDiagnosticConsumer is another possibility except it currently doesn't 
mention command-line usage at all.  Maybe it should?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60732/new/

https://reviews.llvm.org/D60732



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

[PATCH] D60539: Add -std=c++14 language standard option to tests that require C++14 default

[Nemanja Ivanovic via Phabricator via cfe-commits]

nemanjai added a comment.

> Do you need to build clangd? We explicitly don't aim to support building 
> everywhere clang can be built, maybe we should just disable in this case?

Our environment includes various OS levels running on PowerPC. We certainly 
wouldn't want to disable building/testing `clangd` on all our PowerPC machines. 
Is there a way to disable it only on certain OS levels?

Furthermore, it seems a little too intrusive to disable an otherwise functional 
component simply because some test cases rely on a specific language standard 
default.

Would it be an acceptable solution to add another `StringRef` parameter to 
`ShouldCollectSymbolTest::build()` - let's call it `ExtraArgs`, to which we can 
add options such as `-std=c++14` if the test being built relies on that option?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60539/new/

https://reviews.llvm.org/D60539



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

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: lib/CodeGen/CGOpenMPRuntime.h:644
+  /// Flag for keeping track of weather a target region has been emitted.
+  bool hasEmittedTargetRegion = false;
+

gtbercea wrote:
> gtbercea wrote:
> > ABataev wrote:
> > > gtbercea wrote:
> > > > ABataev wrote:
> > > > > Why do you need this? I think your function should be called without 
> > > > > any conditions. It does not depend on the presence of the target 
> > > > > regions or not. Plus, your check is not consistent if you're calling 
> > > > > the function from another module, which has target region inside.
> > > > This does not determine if the function is called or not. This helps 
> > > > determine the flags with which the function is called.
> > > It does not matter, it still does not work correctly if the target region 
> > > is called in the function from another module
> > If the target is in another compilation unit, that unit will need to have a 
> > requires directive.
> If you can come up with an example which you think doesn't work I'm happy to 
> try it.
If the module without target directives was compiled with unified memory and 
the module with target directives compiled without unfied memory support? Is 
this a problem? Shall we diagnose it?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60568/new/

https://reviews.llvm.org/D60568



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

[PATCH] D60786: [FileSystemStatCache] Update test for new FileSystemStatCache API

[Harlan Haskins via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC358511: [FileSystemStatCache] Update test for new 
FileSystemStatCache API (authored by harlanhaskins, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D60786?vs=195416=195417#toc

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60786/new/

https://reviews.llvm.org/D60786

Files:
  unittests/Basic/FileManagerTest.cpp


Index: unittests/Basic/FileManagerTest.cpp
===
--- unittests/Basic/FileManagerTest.cpp
+++ unittests/Basic/FileManagerTest.cpp
@@ -57,9 +57,10 @@
   }
 
   // Implement FileSystemStatCache::getStat().
-  LookupResult getStat(StringRef Path, llvm::vfs::Status , bool isFile,
-   std::unique_ptr *F,
-   llvm::vfs::FileSystem ) override {
+  std::error_code getStat(StringRef Path, llvm::vfs::Status ,
+  bool isFile,
+  std::unique_ptr *F,
+  llvm::vfs::FileSystem ) override {
 #ifndef _WIN32
 SmallString<128> NormalizedPath(Path);
 llvm::sys::path::native(NormalizedPath);
@@ -68,10 +69,10 @@
 
 if (StatCalls.count(Path) != 0) {
   Status = StatCalls[Path];
-  return CacheExists;
+  return std::error_code();
 }
 
-return CacheMissing;  // This means the file/directory doesn't exist.
+return std::make_error_code(std::errc::no_such_file_or_directory);
   }
 };
 


Index: unittests/Basic/FileManagerTest.cpp
===
--- unittests/Basic/FileManagerTest.cpp
+++ unittests/Basic/FileManagerTest.cpp
@@ -57,9 +57,10 @@
   }
 
   // Implement FileSystemStatCache::getStat().
-  LookupResult getStat(StringRef Path, llvm::vfs::Status , bool isFile,
-   std::unique_ptr *F,
-   llvm::vfs::FileSystem ) override {
+  std::error_code getStat(StringRef Path, llvm::vfs::Status ,
+  bool isFile,
+  std::unique_ptr *F,
+  llvm::vfs::FileSystem ) override {
 #ifndef _WIN32
 SmallString<128> NormalizedPath(Path);
 llvm::sys::path::native(NormalizedPath);
@@ -68,10 +69,10 @@
 
 if (StatCalls.count(Path) != 0) {
   Status = StatCalls[Path];
-  return CacheExists;
+  return std::error_code();
 }
 
-return CacheMissing;  // This means the file/directory doesn't exist.
+return std::make_error_code(std::errc::no_such_file_or_directory);
   }
 };
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r358511 - [FileSystemStatCache] Update test for new FileSystemStatCache API

[Harlan Haskins via cfe-commits]

Author: harlanhaskins
Date: Tue Apr 16 11:00:43 2019
New Revision: 358511

URL: http://llvm.org/viewvc/llvm-project?rev=358511=rev
Log:
[FileSystemStatCache] Update test for new FileSystemStatCache API

Summary: Update this test to return std::error_code instead of LookupResult.

Reviewers: arphaman

Subscribers: dexonsmith, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/unittests/Basic/FileManagerTest.cpp

Modified: cfe/trunk/unittests/Basic/FileManagerTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Basic/FileManagerTest.cpp?rev=358511=358510=358511=diff
==
--- cfe/trunk/unittests/Basic/FileManagerTest.cpp (original)
+++ cfe/trunk/unittests/Basic/FileManagerTest.cpp Tue Apr 16 11:00:43 2019
@@ -57,9 +57,10 @@ public:
   }
 
   // Implement FileSystemStatCache::getStat().
-  LookupResult getStat(StringRef Path, llvm::vfs::Status , bool isFile,
-   std::unique_ptr *F,
-   llvm::vfs::FileSystem ) override {
+  std::error_code getStat(StringRef Path, llvm::vfs::Status ,
+  bool isFile,
+  std::unique_ptr *F,
+  llvm::vfs::FileSystem ) override {
 #ifndef _WIN32
 SmallString<128> NormalizedPath(Path);
 llvm::sys::path::native(NormalizedPath);
@@ -68,10 +69,10 @@ public:
 
 if (StatCalls.count(Path) != 0) {
   Status = StatCalls[Path];
-  return CacheExists;
+  return std::error_code();
 }
 
-return CacheMissing;  // This means the file/directory doesn't exist.
+return std::make_error_code(std::errc::no_such_file_or_directory);
   }
 };
 


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

[PATCH] D60786: [FileSystemStatCache] Update test for new FileSystemStatCache API

[Alex Lorenz via Phabricator via cfe-commits]

arphaman accepted this revision.
arphaman added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: ormris.

LG


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60786/new/

https://reviews.llvm.org/D60786



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

[PATCH] D60786: [FileSystemStatCache] Update test for new FileSystemStatCache API

[Harlan Haskins via Phabricator via cfe-commits]

harlanhaskins created this revision.
harlanhaskins added a reviewer: arphaman.
Herald added subscribers: cfe-commits, dexonsmith.
Herald added a project: clang.
arphaman accepted this revision.
arphaman added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: ormris.

LG


Update this test to return std::error_code instead of LookupResult.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D60786

Files:
  clang/unittests/Basic/FileManagerTest.cpp


Index: clang/unittests/Basic/FileManagerTest.cpp
===
--- clang/unittests/Basic/FileManagerTest.cpp
+++ clang/unittests/Basic/FileManagerTest.cpp
@@ -57,9 +57,10 @@
   }
 
   // Implement FileSystemStatCache::getStat().
-  LookupResult getStat(StringRef Path, llvm::vfs::Status , bool isFile,
-   std::unique_ptr *F,
-   llvm::vfs::FileSystem ) override {
+  std::error_code getStat(StringRef Path, llvm::vfs::Status ,
+  bool isFile,
+  std::unique_ptr *F,
+  llvm::vfs::FileSystem ) override {
 #ifndef _WIN32
 SmallString<128> NormalizedPath(Path);
 llvm::sys::path::native(NormalizedPath);
@@ -68,10 +69,10 @@
 
 if (StatCalls.count(Path) != 0) {
   Status = StatCalls[Path];
-  return CacheExists;
+  return std::error_code();
 }
 
-return CacheMissing;  // This means the file/directory doesn't exist.
+return std::make_error_code(std::errc::no_such_file_or_directory);
   }
 };
 


Index: clang/unittests/Basic/FileManagerTest.cpp
===
--- clang/unittests/Basic/FileManagerTest.cpp
+++ clang/unittests/Basic/FileManagerTest.cpp
@@ -57,9 +57,10 @@
   }
 
   // Implement FileSystemStatCache::getStat().
-  LookupResult getStat(StringRef Path, llvm::vfs::Status , bool isFile,
-   std::unique_ptr *F,
-   llvm::vfs::FileSystem ) override {
+  std::error_code getStat(StringRef Path, llvm::vfs::Status ,
+  bool isFile,
+  std::unique_ptr *F,
+  llvm::vfs::FileSystem ) override {
 #ifndef _WIN32
 SmallString<128> NormalizedPath(Path);
 llvm::sys::path::native(NormalizedPath);
@@ -68,10 +69,10 @@
 
 if (StatCalls.count(Path) != 0) {
   Status = StatCalls[Path];
-  return CacheExists;
+  return std::error_code();
 }
 
-return CacheMissing;  // This means the file/directory doesn't exist.
+return std::make_error_code(std::errc::no_such_file_or_directory);
   }
 };
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60735: [FileSystemStatCache] Return std::error_code from stat cache methods

[Harlan Haskins via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC358509: [FileSystemStatCache] Return std::error_code from 
stat cache methods (authored by harlanhaskins, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D60735?vs=195264=195411#toc

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60735/new/

https://reviews.llvm.org/D60735

Files:
  include/clang/Basic/FileSystemStatCache.h
  lib/Basic/FileManager.cpp
  lib/Basic/FileSystemStatCache.cpp

Index: include/clang/Basic/FileSystemStatCache.h
===
--- include/clang/Basic/FileSystemStatCache.h
+++ include/clang/Basic/FileSystemStatCache.h
@@ -37,14 +37,6 @@
 public:
   virtual ~FileSystemStatCache() = default;
 
-  enum LookupResult {
-/// We know the file exists and its cached stat data.
-CacheExists,
-
-/// We know that the file doesn't exist.
-CacheMissing
-  };
-
   /// Get the 'stat' information for the specified path, using the cache
   /// to accelerate it if possible.
   ///
@@ -55,18 +47,19 @@
   /// success for directories (not files).  On a successful file lookup, the
   /// implementation can optionally fill in \p F with a valid \p File object and
   /// the client guarantees that it will close it.
-  static bool get(StringRef Path, llvm::vfs::Status , bool isFile,
-  std::unique_ptr *F,
-  FileSystemStatCache *Cache, llvm::vfs::FileSystem );
+  static std::error_code
+  get(StringRef Path, llvm::vfs::Status , bool isFile,
+  std::unique_ptr *F,
+  FileSystemStatCache *Cache, llvm::vfs::FileSystem );
 
 protected:
   // FIXME: The pointer here is a non-owning/optional reference to the
   // unique_ptr. Optional&> might be nicer, but
   // Optional needs some work to support references so this isn't possible yet.
-  virtual LookupResult getStat(StringRef Path, llvm::vfs::Status ,
-   bool isFile,
-   std::unique_ptr *F,
-   llvm::vfs::FileSystem ) = 0;
+  virtual std::error_code getStat(StringRef Path, llvm::vfs::Status ,
+  bool isFile,
+  std::unique_ptr *F,
+  llvm::vfs::FileSystem ) = 0;
 };
 
 /// A stat "cache" that can be used by FileManager to keep
@@ -84,9 +77,10 @@
   iterator begin() const { return StatCalls.begin(); }
   iterator end() const { return StatCalls.end(); }
 
-  LookupResult getStat(StringRef Path, llvm::vfs::Status , bool isFile,
-   std::unique_ptr *F,
-   llvm::vfs::FileSystem ) override;
+  std::error_code getStat(StringRef Path, llvm::vfs::Status ,
+  bool isFile,
+  std::unique_ptr *F,
+  llvm::vfs::FileSystem ) override;
 };
 
 } // namespace clang
Index: lib/Basic/FileManager.cpp
===
--- lib/Basic/FileManager.cpp
+++ lib/Basic/FileManager.cpp
@@ -429,13 +429,14 @@
   // FIXME: FileSystemOpts shouldn't be passed in here, all paths should be
   // absolute!
   if (FileSystemOpts.WorkingDir.empty())
-return FileSystemStatCache::get(Path, Status, isFile, F,StatCache.get(), *FS);
+return bool(FileSystemStatCache::get(Path, Status, isFile, F,
+ StatCache.get(), *FS));
 
   SmallString<128> FilePath(Path);
   FixupRelativePath(FilePath);
 
-  return FileSystemStatCache::get(FilePath.c_str(), Status, isFile, F,
-  StatCache.get(), *FS);
+  return bool(FileSystemStatCache::get(FilePath.c_str(), Status, isFile, F,
+   StatCache.get(), *FS));
 }
 
 bool FileManager::getNoncachedStatValue(StringRef Path,
Index: lib/Basic/FileSystemStatCache.cpp
===
--- lib/Basic/FileSystemStatCache.cpp
+++ lib/Basic/FileSystemStatCache.cpp
@@ -30,25 +30,24 @@
 /// success for directories (not files).  On a successful file lookup, the
 /// implementation can optionally fill in FileDescriptor with a valid
 /// descriptor and the client guarantees that it will close it.
-bool FileSystemStatCache::get(StringRef Path, llvm::vfs::Status ,
-  bool isFile,
-  std::unique_ptr *F,
-  FileSystemStatCache *Cache,
-  llvm::vfs::FileSystem ) {
-  LookupResult R;
+std::error_code
+FileSystemStatCache::get(StringRef Path, llvm::vfs::Status ,
+ bool isFile, std::unique_ptr *F,
+ FileSystemStatCache *Cache,
+ llvm::vfs::FileSystem ) {
   bool isForDir = !isFile;
+  std::error_code RetCode;
 
   // If we have a cache, use it to resolve the stat query.
  

r358509 - [FileSystemStatCache] Return std::error_code from stat cache methods

[Harlan Haskins via cfe-commits]

Author: harlanhaskins
Date: Tue Apr 16 10:34:26 2019
New Revision: 358509

URL: http://llvm.org/viewvc/llvm-project?rev=358509=rev
Log:
[FileSystemStatCache] Return std::error_code from stat cache methods

Summary:
Previously, we would return true/false signifying if the cache/lookup
succeeded or failed. Instead, provide clients with the underlying error
that was thrown while attempting to look up in the cache.

Since clang::FileManager doesn't make use of this information, it discards the
error that's received and casts away to bool.

This change is NFC.

Reviewers: benlangmuir, arphaman

Subscribers: dexonsmith, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/include/clang/Basic/FileSystemStatCache.h
cfe/trunk/lib/Basic/FileManager.cpp
cfe/trunk/lib/Basic/FileSystemStatCache.cpp

Modified: cfe/trunk/include/clang/Basic/FileSystemStatCache.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/FileSystemStatCache.h?rev=358509=358508=358509=diff
==
--- cfe/trunk/include/clang/Basic/FileSystemStatCache.h (original)
+++ cfe/trunk/include/clang/Basic/FileSystemStatCache.h Tue Apr 16 10:34:26 2019
@@ -37,14 +37,6 @@ class FileSystemStatCache {
 public:
   virtual ~FileSystemStatCache() = default;
 
-  enum LookupResult {
-/// We know the file exists and its cached stat data.
-CacheExists,
-
-/// We know that the file doesn't exist.
-CacheMissing
-  };
-
   /// Get the 'stat' information for the specified path, using the cache
   /// to accelerate it if possible.
   ///
@@ -55,18 +47,19 @@ public:
   /// success for directories (not files).  On a successful file lookup, the
   /// implementation can optionally fill in \p F with a valid \p File object 
and
   /// the client guarantees that it will close it.
-  static bool get(StringRef Path, llvm::vfs::Status , bool isFile,
-  std::unique_ptr *F,
-  FileSystemStatCache *Cache, llvm::vfs::FileSystem );
+  static std::error_code
+  get(StringRef Path, llvm::vfs::Status , bool isFile,
+  std::unique_ptr *F,
+  FileSystemStatCache *Cache, llvm::vfs::FileSystem );
 
 protected:
   // FIXME: The pointer here is a non-owning/optional reference to the
   // unique_ptr. Optional&> might be nicer, but
   // Optional needs some work to support references so this isn't possible yet.
-  virtual LookupResult getStat(StringRef Path, llvm::vfs::Status ,
-   bool isFile,
-   std::unique_ptr *F,
-   llvm::vfs::FileSystem ) = 0;
+  virtual std::error_code getStat(StringRef Path, llvm::vfs::Status ,
+  bool isFile,
+  std::unique_ptr *F,
+  llvm::vfs::FileSystem ) = 0;
 };
 
 /// A stat "cache" that can be used by FileManager to keep
@@ -84,9 +77,10 @@ public:
   iterator begin() const { return StatCalls.begin(); }
   iterator end() const { return StatCalls.end(); }
 
-  LookupResult getStat(StringRef Path, llvm::vfs::Status , bool isFile,
-   std::unique_ptr *F,
-   llvm::vfs::FileSystem ) override;
+  std::error_code getStat(StringRef Path, llvm::vfs::Status ,
+  bool isFile,
+  std::unique_ptr *F,
+  llvm::vfs::FileSystem ) override;
 };
 
 } // namespace clang

Modified: cfe/trunk/lib/Basic/FileManager.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/FileManager.cpp?rev=358509=358508=358509=diff
==
--- cfe/trunk/lib/Basic/FileManager.cpp (original)
+++ cfe/trunk/lib/Basic/FileManager.cpp Tue Apr 16 10:34:26 2019
@@ -429,13 +429,14 @@ bool FileManager::getStatValue(StringRef
   // FIXME: FileSystemOpts shouldn't be passed in here, all paths should be
   // absolute!
   if (FileSystemOpts.WorkingDir.empty())
-return FileSystemStatCache::get(Path, Status, isFile, F,StatCache.get(), 
*FS);
+return bool(FileSystemStatCache::get(Path, Status, isFile, F,
+ StatCache.get(), *FS));
 
   SmallString<128> FilePath(Path);
   FixupRelativePath(FilePath);
 
-  return FileSystemStatCache::get(FilePath.c_str(), Status, isFile, F,
-  StatCache.get(), *FS);
+  return bool(FileSystemStatCache::get(FilePath.c_str(), Status, isFile, F,
+   StatCache.get(), *FS));
 }
 
 bool FileManager::getNoncachedStatValue(StringRef Path,

Modified: cfe/trunk/lib/Basic/FileSystemStatCache.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/FileSystemStatCache.cpp?rev=358509=358508=358509=diff
==
--- 

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev marked 10 inline comments as done.
anton-afanasyev added a comment.

Patch by @thakis for BE passes tracing: https://reviews.llvm.org/D60782


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58675/new/

https://reviews.llvm.org/D58675



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

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea marked an inline comment as done.
gtbercea added inline comments.



Comment at: lib/CodeGen/CGOpenMPRuntime.h:644
+  /// Flag for keeping track of weather a target region has been emitted.
+  bool hasEmittedTargetRegion = false;
+

gtbercea wrote:
> ABataev wrote:
> > gtbercea wrote:
> > > ABataev wrote:
> > > > Why do you need this? I think your function should be called without 
> > > > any conditions. It does not depend on the presence of the target 
> > > > regions or not. Plus, your check is not consistent if you're calling 
> > > > the function from another module, which has target region inside.
> > > This does not determine if the function is called or not. This helps 
> > > determine the flags with which the function is called.
> > It does not matter, it still does not work correctly if the target region 
> > is called in the function from another module
> If the target is in another compilation unit, that unit will need to have a 
> requires directive.
If you can come up with an example which you think doesn't work I'm happy to 
try it.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60568/new/

https://reviews.llvm.org/D60568



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

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea marked an inline comment as done.
gtbercea added inline comments.



Comment at: lib/CodeGen/CGOpenMPRuntime.h:644
+  /// Flag for keeping track of weather a target region has been emitted.
+  bool hasEmittedTargetRegion = false;
+

ABataev wrote:
> gtbercea wrote:
> > ABataev wrote:
> > > Why do you need this? I think your function should be called without any 
> > > conditions. It does not depend on the presence of the target regions or 
> > > not. Plus, your check is not consistent if you're calling the function 
> > > from another module, which has target region inside.
> > This does not determine if the function is called or not. This helps 
> > determine the flags with which the function is called.
> It does not matter, it still does not work correctly if the target region is 
> called in the function from another module
If the target is in another compilation unit, that unit will need to have a 
requires directive.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60568/new/

https://reviews.llvm.org/D60568



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

[PATCH] D55793: [clang-tidy] Add duplicated access specifier readability check (PR25403)

[Mateusz Maćkowski via Phabricator via cfe-commits]

m4tx updated this revision to Diff 195409.
m4tx added a comment.

Updated the backticks in the documentation.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55793/new/

https://reviews.llvm.org/D55793

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/clang-tidy/readability/RedundantAccessSpecifiersCheck.cpp
  clang-tools-extra/clang-tidy/readability/RedundantAccessSpecifiersCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/readability-redundant-access-specifiers.rst
  
clang-tools-extra/test/clang-tidy/readability-redundant-access-specifiers-check-first-declaration.cpp
  clang-tools-extra/test/clang-tidy/readability-redundant-access-specifiers.cpp

Index: clang-tools-extra/test/clang-tidy/readability-redundant-access-specifiers.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/readability-redundant-access-specifiers.cpp
@@ -0,0 +1,116 @@
+// RUN: %check_clang_tidy %s readability-redundant-access-specifiers %t
+
+class FooPublic {
+public:
+  int a;
+public: // comment-0
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: duplicated access specifier [readability-redundant-access-specifiers]
+  // CHECK-MESSAGES: :[[@LINE-4]]:1: note: previously declared here
+  // CHECK-FIXES: {{^}}// comment-0{{$}}
+  int b;
+private:
+  int c;
+};
+
+struct StructPublic {
+public:
+  int a;
+public: // comment-1
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: duplicated access specifier [readability-redundant-access-specifiers]
+  // CHECK-MESSAGES: :[[@LINE-4]]:1: note: previously declared here
+  // CHECK-FIXES: {{^}}// comment-1{{$}}
+  int b;
+private:
+  int c;
+};
+
+union UnionPublic {
+public:
+  int a;
+public: // comment-2
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: duplicated access specifier [readability-redundant-access-specifiers]
+  // CHECK-MESSAGES: :[[@LINE-4]]:1: note: previously declared here
+  // CHECK-FIXES: {{^}}// comment-2{{$}}
+  int b;
+private:
+  int c;
+};
+
+class FooProtected {
+protected:
+  int a;
+protected: // comment-3
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: duplicated access specifier [readability-redundant-access-specifiers]
+  // CHECK-MESSAGES: :[[@LINE-4]]:1: note: previously declared here
+  // CHECK-FIXES: {{^}}// comment-3{{$}}
+  int b;
+private:
+  int c;
+};
+
+class FooPrivate {
+private:
+  int a;
+private: // comment-4
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: duplicated access specifier [readability-redundant-access-specifiers]
+  // CHECK-MESSAGES: :[[@LINE-4]]:1: note: previously declared here
+  // CHECK-FIXES: {{^}}// comment-4{{$}}
+  int b;
+public:
+  int c;
+};
+
+class FooMacro {
+private:
+  int a;
+#if defined(ZZ)
+  public:
+  int b;
+#endif
+private: // comment-5
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: duplicated access specifier [readability-redundant-access-specifiers]
+  // CHECK-MESSAGES: :[[@LINE-8]]:1: note: previously declared here
+  // CHECK-FIXES: {{^}}// comment-5{{$}}
+  int c;
+protected:
+  int d;
+public:
+  int e;
+};
+
+class Valid {
+private:
+  int a;
+public:
+  int b;
+private:
+  int c;
+protected:
+  int d;
+public:
+  int e;
+};
+
+class ValidInnerClass {
+public:
+  int a;
+
+  class Inner {
+  public:
+int b;
+  };
+};
+
+#define MIXIN private: int b;
+
+class ValidMacro {
+private:
+  int a;
+MIXIN
+private:
+  int c;
+protected:
+  int d;
+public:
+  int e;
+};
Index: clang-tools-extra/test/clang-tidy/readability-redundant-access-specifiers-check-first-declaration.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/readability-redundant-access-specifiers-check-first-declaration.cpp
@@ -0,0 +1,43 @@
+// RUN: %check_clang_tidy %s readability-redundant-access-specifiers %t -- \
+// RUN:   -config="{CheckOptions: [{key: readability-redundant-access-specifiers.CheckFirstDeclaration, value: 1}]}" --
+
+class FooPublic {
+private: // comment-0
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant access specifier [readability-redundant-access-specifiers]
+  // CHECK-FIXES: {{^}}// comment-0{{$}}
+  int a;
+};
+
+struct StructPublic {
+public: // comment-1
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant access specifier [readability-redundant-access-specifiers]
+  // CHECK-FIXES: {{^}}// comment-1{{$}}
+  int a;
+};
+
+union UnionPublic {
+public: // comment-2
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant access specifier [readability-redundant-access-specifiers]
+  // CHECK-FIXES: {{^}}// comment-2{{$}}
+  int a;
+};
+
+class FooMacro {
+#if defined(ZZ)
+private:
+#endif
+  int a;
+};
+
+class ValidInnerStruct {
+  struct Inner {
+  private:
+int b;
+  };
+};
+
+#define MIXIN private: int b;
+
+class 

[PATCH] D60728: [clang] [test] Add a (xfailing) test for PR41027

[Michał Górny via Phabricator via cfe-commits]

mgorny added a comment.

In D60728#1468486 , @hans wrote:

> What's the value in checking in this xfail'ed test without an actual fix for 
> the problem?




1. It may help whoever tries to address it in the future, to have a known-good 
reproducer.
2. If someone addresses this independently and doesn't notice the bug, it will 
help us get informed that the issue was fixed.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60728/new/

https://reviews.llvm.org/D60728



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

[PATCH] D55793: [clang-tidy] Add duplicated access specifier readability check (PR25403)

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability-redundant-access-specifiers.rst:34
+   If set to non-zero, the check will also give warning if the first access
+   specifier declaration is redundant (e.g. `private` inside `class`). Default
+   is `0`.

Please use double back-ticks for language keywords, like private anc class.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability-redundant-access-specifiers.rst:47
+
+If ``CheckFirstDeclaration`` option is enabled, a warning about redundant
+access specifier will be emitted, since ``public`` is the default member access

Please use single back-ticks for option names.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55793/new/

https://reviews.llvm.org/D55793



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

[PATCH] D60735: [FileSystemStatCache] Return std::error_code from stat cache methods

[Alex Lorenz via Phabricator via cfe-commits]

arphaman accepted this revision.
arphaman added a comment.
This revision is now accepted and ready to land.

LGTM, please mark it as NFC


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60735/new/

https://reviews.llvm.org/D60735



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

[PATCH] D55793: [clang-tidy] Add duplicated access specifier readability check (PR25403)

[Mateusz Maćkowski via Phabricator via cfe-commits]

m4tx updated this revision to Diff 195408.
m4tx added a comment.
Herald added a project: clang.

Updated the diff for the latest (master) version, used GitHub monorepo for the 
changes


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55793/new/

https://reviews.llvm.org/D55793

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/clang-tidy/readability/RedundantAccessSpecifiersCheck.cpp
  clang-tools-extra/clang-tidy/readability/RedundantAccessSpecifiersCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/readability-redundant-access-specifiers.rst
  
clang-tools-extra/test/clang-tidy/readability-redundant-access-specifiers-check-first-declaration.cpp
  clang-tools-extra/test/clang-tidy/readability-redundant-access-specifiers.cpp

Index: clang-tools-extra/test/clang-tidy/readability-redundant-access-specifiers.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/readability-redundant-access-specifiers.cpp
@@ -0,0 +1,116 @@
+// RUN: %check_clang_tidy %s readability-redundant-access-specifiers %t
+
+class FooPublic {
+public:
+  int a;
+public: // comment-0
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: duplicated access specifier [readability-redundant-access-specifiers]
+  // CHECK-MESSAGES: :[[@LINE-4]]:1: note: previously declared here
+  // CHECK-FIXES: {{^}}// comment-0{{$}}
+  int b;
+private:
+  int c;
+};
+
+struct StructPublic {
+public:
+  int a;
+public: // comment-1
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: duplicated access specifier [readability-redundant-access-specifiers]
+  // CHECK-MESSAGES: :[[@LINE-4]]:1: note: previously declared here
+  // CHECK-FIXES: {{^}}// comment-1{{$}}
+  int b;
+private:
+  int c;
+};
+
+union UnionPublic {
+public:
+  int a;
+public: // comment-2
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: duplicated access specifier [readability-redundant-access-specifiers]
+  // CHECK-MESSAGES: :[[@LINE-4]]:1: note: previously declared here
+  // CHECK-FIXES: {{^}}// comment-2{{$}}
+  int b;
+private:
+  int c;
+};
+
+class FooProtected {
+protected:
+  int a;
+protected: // comment-3
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: duplicated access specifier [readability-redundant-access-specifiers]
+  // CHECK-MESSAGES: :[[@LINE-4]]:1: note: previously declared here
+  // CHECK-FIXES: {{^}}// comment-3{{$}}
+  int b;
+private:
+  int c;
+};
+
+class FooPrivate {
+private:
+  int a;
+private: // comment-4
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: duplicated access specifier [readability-redundant-access-specifiers]
+  // CHECK-MESSAGES: :[[@LINE-4]]:1: note: previously declared here
+  // CHECK-FIXES: {{^}}// comment-4{{$}}
+  int b;
+public:
+  int c;
+};
+
+class FooMacro {
+private:
+  int a;
+#if defined(ZZ)
+  public:
+  int b;
+#endif
+private: // comment-5
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: duplicated access specifier [readability-redundant-access-specifiers]
+  // CHECK-MESSAGES: :[[@LINE-8]]:1: note: previously declared here
+  // CHECK-FIXES: {{^}}// comment-5{{$}}
+  int c;
+protected:
+  int d;
+public:
+  int e;
+};
+
+class Valid {
+private:
+  int a;
+public:
+  int b;
+private:
+  int c;
+protected:
+  int d;
+public:
+  int e;
+};
+
+class ValidInnerClass {
+public:
+  int a;
+
+  class Inner {
+  public:
+int b;
+  };
+};
+
+#define MIXIN private: int b;
+
+class ValidMacro {
+private:
+  int a;
+MIXIN
+private:
+  int c;
+protected:
+  int d;
+public:
+  int e;
+};
Index: clang-tools-extra/test/clang-tidy/readability-redundant-access-specifiers-check-first-declaration.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/readability-redundant-access-specifiers-check-first-declaration.cpp
@@ -0,0 +1,43 @@
+// RUN: %check_clang_tidy %s readability-redundant-access-specifiers %t -- \
+// RUN:   -config="{CheckOptions: [{key: readability-redundant-access-specifiers.CheckFirstDeclaration, value: 1}]}" --
+
+class FooPublic {
+private: // comment-0
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant access specifier [readability-redundant-access-specifiers]
+  // CHECK-FIXES: {{^}}// comment-0{{$}}
+  int a;
+};
+
+struct StructPublic {
+public: // comment-1
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant access specifier [readability-redundant-access-specifiers]
+  // CHECK-FIXES: {{^}}// comment-1{{$}}
+  int a;
+};
+
+union UnionPublic {
+public: // comment-2
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant access specifier [readability-redundant-access-specifiers]
+  // CHECK-FIXES: {{^}}// comment-2{{$}}
+  int a;
+};
+
+class FooMacro {
+#if defined(ZZ)
+private:
+#endif
+  int a;
+};
+
+class ValidInnerStruct {
+  struct Inner {
+  

[PATCH] D60749: [Test] Remove obsolete test.

[Michael Kruse via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC358507: [Test] Remove obsolete test. (authored by 
Meinersbur, committed by ).

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60749/new/

https://reviews.llvm.org/D60749

Files:
  test/AST/ast-print-pragmas-xfail.cpp


Index: test/AST/ast-print-pragmas-xfail.cpp
===
--- test/AST/ast-print-pragmas-xfail.cpp
+++ test/AST/ast-print-pragmas-xfail.cpp
@@ -1,21 +0,0 @@
-// RUN: %clang_cc1 %s -ast-print -o - | FileCheck %s
-
-// FIXME: Test fails because attribute order is reversed by ParsedAttributes.
-// XFAIL: *
-
-void run1(int *List, int Length) {
-  int i = 0;
-// CHECK: #pragma loop vectorize(4)
-// CHECK-NEXT: #pragma loop interleave(8)
-// CHECK-NEXT: #pragma loop vectorize(enable)
-// CHECK-NEXT: #pragma loop interleave(enable)
-#pragma loop vectorize(4)
-#pragma loop interleave(8)
-#pragma loop vectorize(enable)
-#pragma loop interleave(enable)
-// CHECK-NEXT: while (i < Length)
-  while (i < Length) {
-List[i] = i;
-i++;
-  }
-}


Index: test/AST/ast-print-pragmas-xfail.cpp
===
--- test/AST/ast-print-pragmas-xfail.cpp
+++ test/AST/ast-print-pragmas-xfail.cpp
@@ -1,21 +0,0 @@
-// RUN: %clang_cc1 %s -ast-print -o - | FileCheck %s
-
-// FIXME: Test fails because attribute order is reversed by ParsedAttributes.
-// XFAIL: *
-
-void run1(int *List, int Length) {
-  int i = 0;
-// CHECK: #pragma loop vectorize(4)
-// CHECK-NEXT: #pragma loop interleave(8)
-// CHECK-NEXT: #pragma loop vectorize(enable)
-// CHECK-NEXT: #pragma loop interleave(enable)
-#pragma loop vectorize(4)
-#pragma loop interleave(8)
-#pragma loop vectorize(enable)
-#pragma loop interleave(enable)
-// CHECK-NEXT: while (i < Length)
-  while (i < Length) {
-List[i] = i;
-i++;
-  }
-}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r358507 - [Test] Remove obsolete test.

[Michael Kruse via cfe-commits]

Author: meinersbur
Date: Tue Apr 16 09:44:45 2019
New Revision: 358507

URL: http://llvm.org/viewvc/llvm-project?rev=358507=rev
Log:
[Test] Remove obsolete test.

The FIXME of this test case has been addressed in r335084/r338800. Its
execution still does not succeed because of multiple syntax errors.

First, the "clang" namespace is missing on each of the 4 pragmas.
Second, the pragma for defining the vector width is "vectorize_width(4)"
instead of "vectorize(4)". Third, the pragma for defining the interleave
factor is "interleave_count(8)" instead of "interleave(8)".

The file was already using the wrong syntax when added in
r210925 2014-06-13. The file ast-print-pragmas.cpp already checks for
the correct pragma order, making this test redundant even if fixed.

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

Removed:
cfe/trunk/test/AST/ast-print-pragmas-xfail.cpp

Removed: cfe/trunk/test/AST/ast-print-pragmas-xfail.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/AST/ast-print-pragmas-xfail.cpp?rev=358506=auto
==
--- cfe/trunk/test/AST/ast-print-pragmas-xfail.cpp (original)
+++ cfe/trunk/test/AST/ast-print-pragmas-xfail.cpp (removed)
@@ -1,21 +0,0 @@
-// RUN: %clang_cc1 %s -ast-print -o - | FileCheck %s
-
-// FIXME: Test fails because attribute order is reversed by ParsedAttributes.
-// XFAIL: *
-
-void run1(int *List, int Length) {
-  int i = 0;
-// CHECK: #pragma loop vectorize(4)
-// CHECK-NEXT: #pragma loop interleave(8)
-// CHECK-NEXT: #pragma loop vectorize(enable)
-// CHECK-NEXT: #pragma loop interleave(enable)
-#pragma loop vectorize(4)
-#pragma loop interleave(8)
-#pragma loop vectorize(enable)
-#pragma loop interleave(enable)
-// CHECK-NEXT: while (i < Length)
-  while (i < Length) {
-List[i] = i;
-i++;
-  }
-}


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

[PATCH] D60778: Make precompiled headers reproducible by switching OpenCL extension to std::map

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

> Not sure how to test this though? I guess we can trust that std::map is 
> always sorted just as it says in its description.

You could add a test that contains several entries in `OpenCLTypeExtMap` and 
several entries in `OpenCLDeclExtMap`.
In that test, write te PCH, and then apply `llvm-bcanalyzer` to that PCH. It 
should dump it the bitcode in textual dump.
And then simply apply FileCheck to that textual dump, with CHECK lines 
requiring the specific order of these entries.
If the order is not deterministic in PCH, then that test would (should!) fail 
sporadically.
At least that is my guess.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60778/new/

https://reviews.llvm.org/D60778



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

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: lib/CodeGen/CGOpenMPRuntime.h:644
+  /// Flag for keeping track of weather a target region has been emitted.
+  bool hasEmittedTargetRegion = false;
+

gtbercea wrote:
> ABataev wrote:
> > Why do you need this? I think your function should be called without any 
> > conditions. It does not depend on the presence of the target regions or 
> > not. Plus, your check is not consistent if you're calling the function from 
> > another module, which has target region inside.
> This does not determine if the function is called or not. This helps 
> determine the flags with which the function is called.
It does not matter, it still does not work correctly if the target region is 
called in the function from another module


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60568/new/

https://reviews.llvm.org/D60568



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

[PATCH] D60455: [SYCL] Add support for SYCL device attributes

[Alexey Bader via Phabricator via cfe-commits]

bader added a comment.

In D60455#1467279 , @aaron.ballman 
wrote:

> In D60455#1464324 , @Fznamznon wrote:
>
> > Applied comments from @aaron.ballman and @keryell
> >
> > - Introduced a C++11 and C2x style spelling in the clang namespace. I 
> > didn't find path to add two namespaces to attribute (like 
> > [[clang::sycl::device]]) so [[clang::sycl_device]] spelling is added.
> > - Since both attributes have spellings and possible can be used as some 
> > "standard" outlining in Clang/LLVM I added documetation.
> > - Added more test cases.
> > - As @bader mentioned sycl_device can be used to mark functions, which are 
> > called from the different translation units so I added simple handling of 
> > this attribute in Sema.
>
>
> I'm confused -- I thought @bader also said "...SYCL is not supposed to expose 
> any non-standard extensions to a user." -- these attributes are not standards 
> based (WG21 and WG14 have nothing to say about them), so are these attributes 
> considered "non-standard extensions" or not?


@aaron.ballman sorry for confusion.
SYCL specification doesn't require user to annotate "device functions" with an 
attribute - it says following (from section 6.9.1 SYCL functions and methods 
linkage, https://www.khronos.org/registry/SYCL/specs/sycl-1.2.1.pdf, page 251):

> The default behavior in SYCL applications is that all the definitions and 
> declarations of the functions and methods
>  are available to the SYCL compiler, in the same translation unit. When this 
> is not the case, all the symbols that
>  need to be exported to a SYCL library or from a C++ library to a SYCL 
> application need to be defined using the
>  macro: SYCL_EXTERNAL.
>  The SYCL_EXTERNAL macro will only be defined if the implementation supports 
> offline linking. The macro is
>  implementation-defined, but the following restrictions apply:
> 
> • SYCL_EXTERNAL can only be used on functions;
>  • the function cannot use raw pointers as parameter or return types. 
> Explicit pointer classes must be used instead;
>  • externally defined functions cannot call a 
> cl::sycl::parallel_for_work_item method;
>  • externally defined functions cannot be called from a 
> cl::sycl::parallel_for_work_group scope.
> 
> The SYCL linkage mechanism is optional and implementation defined.

The idea I had is that to define `SYCL_EXTERNAL` macro as `sycl_device` 
attribute.

BTW, I noticed that `SYCL_EXTERNAL` puts additional requirements `sycl_device` 
doesn't meet:

> • SYCL_EXTERNAL can only be used on functions;

I think our implementation doesn't have such limitations and able to support 
more use cases.
Anyway, we can make `sycl_device` attribute implicit for now and return to the 
implementation of cross translation unit dependencies later.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60455/new/

https://reviews.llvm.org/D60455



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

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea marked an inline comment as done.
gtbercea added inline comments.



Comment at: lib/CodeGen/CGOpenMPRuntime.h:644
+  /// Flag for keeping track of weather a target region has been emitted.
+  bool hasEmittedTargetRegion = false;
+

ABataev wrote:
> Why do you need this? I think your function should be called without any 
> conditions. It does not depend on the presence of the target regions or not. 
> Plus, your check is not consistent if you're calling the function from 
> another module, which has target region inside.
This does not determine if the function is called or not. This helps determine 
the flags with which the function is called.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60568/new/

https://reviews.llvm.org/D60568



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

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: lib/CodeGen/CGOpenMPRuntime.h:644
+  /// Flag for keeping track of weather a target region has been emitted.
+  bool hasEmittedTargetRegion = false;
+

Why do you need this? I think your function should be called without any 
conditions. It does not depend on the presence of the target regions or not. 
Plus, your check is not consistent if you're calling the function from another 
module, which has target region inside.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60568/new/

https://reviews.llvm.org/D60568



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

[PATCH] D60379: Make precompiled headers reproducible

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

FYI, I switched to `std::map` in this review:
https://reviews.llvm.org/D60778

@sylvestre.ledru  do you think it's possible to apply this patch and see if it 
work for your bug?

Btw I am still confused about the reproducibility - does clang produce the same 
PCH at least for the runs on the same revision?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60379/new/

https://reviews.llvm.org/D60379



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

[PATCH] D60778: Make precompiled headers reproducible by switching OpenCL extension to std::map

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia created this revision.
Anastasia added reviewers: lebedev.ri, sylvestre.ledru, riccibruno.
Herald added subscribers: ebevhan, JDevlieghere, yaxunl.

As discussed in https://reviews.llvm.org/D60379 let's use std::map to store 
OpenCL extensions data structure.

Not sure how to test this though? I guess we can trust that `std::map` is 
always sorted just as it says in its description.


https://reviews.llvm.org/D60778

Files:
  include/clang/Sema/Sema.h
  include/clang/Serialization/ASTReader.h


Index: include/clang/Serialization/ASTReader.h
===
--- include/clang/Serialization/ASTReader.h
+++ include/clang/Serialization/ASTReader.h
@@ -879,10 +879,10 @@
   OpenCLOptions OpenCLExtensions;
 
   /// Extensions required by an OpenCL type.
-  llvm::DenseMap> OpenCLTypeExtMap;
+  std::map> OpenCLTypeExtMap;
 
   /// Extensions required by an OpenCL declaration.
-  llvm::DenseMap> OpenCLDeclExtMap;
+  std::map> OpenCLDeclExtMap;
 
   /// A list of the namespaces we've seen.
   SmallVector KnownNamespaces;
Index: include/clang/Sema/Sema.h
===
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -8730,9 +8730,9 @@
 private:
   std::string CurrOpenCLExtension;
   /// Extensions required by an OpenCL type.
-  llvm::DenseMap> OpenCLTypeExtMap;
+  std::map> OpenCLTypeExtMap;
   /// Extensions required by an OpenCL declaration.
-  llvm::DenseMap> OpenCLDeclExtMap;
+  std::map> OpenCLDeclExtMap;
 public:
   llvm::StringRef getCurrentOpenCLExtension() const {
 return CurrOpenCLExtension;


Index: include/clang/Serialization/ASTReader.h
===
--- include/clang/Serialization/ASTReader.h
+++ include/clang/Serialization/ASTReader.h
@@ -879,10 +879,10 @@
   OpenCLOptions OpenCLExtensions;
 
   /// Extensions required by an OpenCL type.
-  llvm::DenseMap> OpenCLTypeExtMap;
+  std::map> OpenCLTypeExtMap;
 
   /// Extensions required by an OpenCL declaration.
-  llvm::DenseMap> OpenCLDeclExtMap;
+  std::map> OpenCLDeclExtMap;
 
   /// A list of the namespaces we've seen.
   SmallVector KnownNamespaces;
Index: include/clang/Sema/Sema.h
===
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -8730,9 +8730,9 @@
 private:
   std::string CurrOpenCLExtension;
   /// Extensions required by an OpenCL type.
-  llvm::DenseMap> OpenCLTypeExtMap;
+  std::map> OpenCLTypeExtMap;
   /// Extensions required by an OpenCL declaration.
-  llvm::DenseMap> OpenCLDeclExtMap;
+  std::map> OpenCLDeclExtMap;
 public:
   llvm::StringRef getCurrentOpenCLExtension() const {
 return CurrOpenCLExtension;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea updated this revision to Diff 195394.
gtbercea added a comment.

- Fix checks for use of requires directive.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60568/new/

https://reviews.llvm.org/D60568

Files:
  lib/CodeGen/CGOpenMPRuntime.cpp
  lib/CodeGen/CGOpenMPRuntime.h
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  lib/CodeGen/CGOpenMPRuntimeNVPTX.h
  lib/CodeGen/CodeGenModule.cpp
  test/OpenMP/openmp_offload_registration.cpp

Index: test/OpenMP/openmp_offload_registration.cpp
===
--- test/OpenMP/openmp_offload_registration.cpp
+++ test/OpenMP/openmp_offload_registration.cpp
@@ -26,7 +26,7 @@
 // CHECK: [[DESC:@.+]] = internal constant [[DSCTY]] { i32 2, [[DEVTY]]* getelementptr inbounds ([2 x [[DEVTY]]], [2 x [[DEVTY]]]* [[IMAGES]], i32 0, i32 0), [[ENTTY]]* [[ENTBEGIN]], [[ENTTY]]* [[ENTEND]] }, comdat($[[REGFN]])
 
 // Check target registration is registered as a Ctor.
-// CHECK: appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
+// CHECK: appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @.omp_offloading.requires_reg, i8* null }, { i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
 
 // Check presence of foo() and the outlined target region
 // CHECK: define void [[FOO:@.+]]()
@@ -34,6 +34,11 @@
 
 // Check registration and unregistration code.
 
+// CHECK: define internal void @.omp_offloading.requires_reg()
+// CHECK: call void @__tgt_register_requires(i64 0)
+// CHECK: ret void
+// CHECK: declare void @__tgt_register_requires(i64)
+
 // CHECK: define internal void @[[UNREGFN:.+]](i8*)
 // CHECK-SAME: comdat($[[REGFN]]) {
 // CHECK: call i32 @__tgt_unregister_lib([[DSCTY]]* [[DESC]])
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -410,6 +410,10 @@
   AddGlobalCtor(CudaCtorFunction);
   }
   if (OpenMPRuntime) {
+if (llvm::Function *OpenMPRequiresDirectiveRegFun =
+OpenMPRuntime->emitRequiresDirectiveRegFun()) {
+  AddGlobalCtor(OpenMPRequiresDirectiveRegFun, 0);
+}
 if (llvm::Function *OpenMPRegistrationFunction =
 OpenMPRuntime->emitRegistrationFunction()) {
   auto ComdatKey = OpenMPRegistrationFunction->hasComdat() ?
Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.h
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.h
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.h
@@ -383,7 +383,7 @@
 
   /// Perform check on requires decl to ensure that target architecture
   /// supports unified addressing
-  void checkArchForUnifiedAddressing(const OMPRequiresDecl *D) const override;
+  void checkArchForUnifiedAddressing(const OMPRequiresDecl *D) override;
 
   /// Returns default address space for the constant firstprivates, __constant__
   /// address space by default.
Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -4942,7 +4942,7 @@
 /// Check to see if target architecture supports unified addressing which is
 /// a restriction for OpenMP requires clause "unified_shared_memory".
 void CGOpenMPRuntimeNVPTX::checkArchForUnifiedAddressing(
-const OMPRequiresDecl *D) const {
+const OMPRequiresDecl *D) {
   for (const OMPClause *Clause : D->clauselists()) {
 if (Clause->getClauseKind() == OMPC_unified_shared_memory) {
   switch (getCudaArch(CGM)) {
@@ -4987,6 +4987,7 @@
   }
 }
   }
+  CGOpenMPRuntime::checkArchForUnifiedAddressing(D);
 }
 
 /// Get number of SMs and number of blocks per SM.
Index: lib/CodeGen/CGOpenMPRuntime.h
===
--- lib/CodeGen/CGOpenMPRuntime.h
+++ lib/CodeGen/CGOpenMPRuntime.h
@@ -636,6 +636,13 @@
   /// must be emitted.
   llvm::SmallDenseSet DeferredGlobalVariables;
 
+  /// Flag for keeping track of weather a requires unified_shared_memory
+  /// directive is present.
+  bool HasRequiresUnifiedSharedMemory = false;
+
+  /// Flag for keeping track of weather a target region has been emitted.
+  bool hasEmittedTargetRegion = false;
+
   /// Creates and registers offloading binary descriptor for the current
   /// compilation unit. The function that does the registration is returned.
   llvm::Function *createOffloadingBinaryDescriptorRegistration();
@@ -1429,6 +1436,10 @@
   /// \param GD Global to scan.
   virtual bool emitTargetGlobal(GlobalDecl GD);
 
+  /// Creates and returns a registration function for when at least one
+  /// requires directives was used in the current module.
+  llvm::Function *emitRequiresDirectiveRegFun();
+
   /// 

[PATCH] D58033: Add option for emitting dbg info for call sites

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

I guess I'm not clear what your final goal is for the option.  Keep it, even 
though GCC doesn't have one like it?  Eliminate it?  Please clearly state what 
you intend to have in the end, and what you might have in the short term in 
case that is different.




Comment at: include/clang/Driver/CC1Options.td:362
 HelpText<"Use public LTO visibility for classes in std and stdext 
namespaces">;
+def emit_param_entry_values_cc1:
+Flag<["-"], "emit-param-entry-values-cc1">,

I think this is not necessary, see comment on Options.td.



Comment at: include/clang/Driver/Options.td:919
   HelpText<"Do not use jump tables for lowering switches">;
+def emit_param_entry_values : Joined<["-"], "femit-param-entry-values">,
+   Group,

djtodoro wrote:
> aprantl wrote:
> > I assume that this is the same -f option that GCC uses?
> Actually in GCC production of call_site and call_site_parameters debug info 
> is enabled by default.
> Production of entry_values is implemented on top of variable's value tracking 
> system and there is no particular option just for entry_values.
> 
> Basically, we introduce this option as experimental one. As soon as we test 
> everything we should get rid of this or turn it ON by default (and maybe add 
> '-fno-emit-param-entry-values' in order to have an option for disabling the 
> functionality). That will be ideal scenario.
By convention, the name of the option should start with 'f' and match the 
option spelling (with hyphens changed to underscore).



Comment at: include/clang/Driver/Options.td:921
+   Group,
+   Flags<[CC1Option]>,
+   HelpText<"Enables debug info about call site and call 
site parameters">;

I believe that by marking this with `[CC1Option]` cc1 will automatically 
understand it and you won't need to define a separate option in CC1Options.td.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58033/new/

https://reviews.llvm.org/D58033



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

[PATCH] D60455: [SYCL] Add support for SYCL device attributes

[Alexey Bader via Phabricator via cfe-commits]

bader added a comment.

In D60455#1468386 , @Fznamznon wrote:

> > Ok, my question is whether you are planning to duplicate the same logic as 
> > for OpenCL kernel which doesn't really seem like an ideal design choice. Is 
> > this the only difference then we can simply add an extra check for SYCL 
> > compilation mode in this template handling case. The overall interaction 
> > between OpenCL and SYCL implementation is still a very big unknown to me so 
> > it's not very easy to judge about the implementations details...
>
> Of course, if nothing prevents us to re-use OpenCL kernel attribute for SYCL 
> I assume it would be good idea. 
>  But I'm thinking about the situation with https://reviews.llvm.org/D60454 . 
> If we re-use OpenCL kernel attributes - we affected by OpenCL-related changes 
> and OpenCL-related changes shouldn't violate SYCL semantics. Will it be 
> usable for SYCL/OpenCL clang developers? @bader , what do you think about it?


I also think it's worth trying. We should be able to cover "SYCL semantics" 
with LIT test to avoid regressions by OpenCL related changes. E.g. add a test 
case checking that -fsycl-is-device option disables restriction on applying 
`__kernel` to template functions.
I want to confirm that everyone is okay to enable `__kernel` keyword for SYCL 
extension and cover SYCL use cases with additional regression tests. IIRC, on 
yesterday call, @keryell, said that having SYCL specific attributes useful for 
separation of concerns.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60455/new/

https://reviews.llvm.org/D60455



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

r358503 - [OPENMP][NVPTX]Run combined constructs with if clause in SPMD mode.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Tue Apr 16 08:39:12 2019
New Revision: 358503

URL: http://llvm.org/viewvc/llvm-project?rev=358503=rev
Log:
[OPENMP][NVPTX]Run combined constructs with if clause in SPMD mode.

Combined constructs with parallel and if clauses without modifiers may
be executed in SPMD mode since if the condition is true for the target
region, it is also true for parallel region and the threads must be run
in parallel.

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
cfe/trunk/test/OpenMP/nvptx_SPMD_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp?rev=358503=358502=358503=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp Tue Apr 16 08:39:12 2019
@@ -717,10 +717,12 @@ getDataSharingMode(CodeGenModule ) {
 /// Check if the parallel directive has an 'if' clause with non-constant or
 /// false condition.
 static bool hasParallelIfClause(ASTContext ,
-const OMPExecutableDirective ) {
+const OMPExecutableDirective ,
+bool StandaloneParallel) {
   for (const auto *C : D.getClausesOfKind()) {
 OpenMPDirectiveKind NameModifier = C->getNameModifier();
-if (NameModifier != OMPD_parallel && NameModifier != OMPD_unknown)
+if (NameModifier != OMPD_parallel &&
+(!StandaloneParallel || NameModifier != OMPD_unknown))
   continue;
 const Expr *Cond = C->getCondition();
 bool Result;
@@ -744,7 +746,7 @@ static bool hasNestedSPMDDirective(ASTCo
 switch (D.getDirectiveKind()) {
 case OMPD_target:
   if (isOpenMPParallelDirective(DKind) &&
-  !hasParallelIfClause(Ctx, *NestedDir))
+  !hasParallelIfClause(Ctx, *NestedDir, /*StandaloneParallel=*/true))
 return true;
   if (DKind == OMPD_teams) {
 Body = NestedDir->getInnermostCapturedStmt()->IgnoreContainers(
@@ -756,14 +758,14 @@ static bool hasNestedSPMDDirective(ASTCo
 dyn_cast_or_null(ChildStmt)) {
   DKind = NND->getDirectiveKind();
   if (isOpenMPParallelDirective(DKind) &&
-  !hasParallelIfClause(Ctx, *NND))
+  !hasParallelIfClause(Ctx, *NND, /*StandaloneParallel=*/true))
 return true;
 }
   }
   return false;
 case OMPD_target_teams:
   return isOpenMPParallelDirective(DKind) &&
- !hasParallelIfClause(Ctx, *NestedDir);
+ !hasParallelIfClause(Ctx, *NestedDir, 
/*StandaloneParallel=*/true);
 case OMPD_target_simd:
 case OMPD_target_parallel:
 case OMPD_target_parallel_for:
@@ -837,7 +839,7 @@ static bool supportsSPMDExecutionMode(AS
   case OMPD_target_parallel_for_simd:
   case OMPD_target_teams_distribute_parallel_for:
   case OMPD_target_teams_distribute_parallel_for_simd:
-return !hasParallelIfClause(Ctx, D);
+return !hasParallelIfClause(Ctx, D, /*StandaloneParallel=*/false);
   case OMPD_target_simd:
   case OMPD_target_teams_distribute:
   case OMPD_target_teams_distribute_simd:

Modified: cfe/trunk/test/OpenMP/nvptx_SPMD_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/nvptx_SPMD_codegen.cpp?rev=358503=358502=358503=diff
==
--- cfe/trunk/test/OpenMP/nvptx_SPMD_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/nvptx_SPMD_codegen.cpp Tue Apr 16 08:39:12 2019
@@ -8,6 +8,8 @@
 #ifndef HEADER
 #define HEADER
 
+int a;
+
 // CHECK-NOT: @__omp_offloading_{{.+}}_exec_mode = weak constant i8 1
 // CHECK-DAG: [[DISTR_LIGHT:@.+]] = private unnamed_addr constant 
%struct.ident_t { i32 0, i32 2050, i32 3, i32 0, i8* getelementptr inbounds
 // CHECK-DAG: [[FOR_LIGHT:@.+]] = private unnamed_addr constant 
%struct.ident_t { i32 0, i32 514, i32 3, i32 0, i8* getelementptr inbounds
@@ -43,7 +45,7 @@ void foo() {
 // CHECK: call void @__kmpc_spmd_kernel_init(i32 {{.+}}, i16 1, i16 {{.+}})
 // CHECK-DAG: [[DISTR_FULL]]
 // CHECK-DAG: [[FULL]]
-#pragma omp target teams distribute parallel for simd
+#pragma omp target teams distribute parallel for simd if(a)
   for (int i = 0; i < 10; ++i)
 ;
 #pragma omp target teams distribute parallel for simd schedule(static)
@@ -301,7 +303,7 @@ int a;
 // CHECK-DAG: [[FULL]]
 // CHECK: call void @__kmpc_spmd_kernel_init(i32 {{.+}}, i16 1, i16 {{.+}})
 // CHECK-DAG: [[FULL]]
-#pragma omp target parallel for
+#pragma omp target parallel for if(a)
   for (int i = 0; i < 10; ++i)
 ;
 #pragma omp target parallel for schedule(static)
@@ -346,7 +348,7 @@ int a;
 // CHECK: call void @__kmpc_spmd_kernel_init(i32 {{.+}}, i16 1, i16 {{.+}})
 // CHECK-DAG: [[FULL]]
 // CHECK-DAG: [[BAR_FULL]]
-#pragma omp target parallel
+#pragma omp target parallel if(a)
 #pragma omp for 

[PATCH] D60728: [clang] [test] Add a (xfailing) test for PR41027

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

In D60728#1468486 , @hans wrote:

> What's the value in checking in this xfail'ed test without an actual fix for 
> the problem?


Raise awareness about the problem.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60728/new/

https://reviews.llvm.org/D60728



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

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

[Tamás Zolnai via Phabricator via cfe-commits]

ztamas marked 2 inline comments as done.
ztamas added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/bugprone-unhandled-self-assignment.cpp:351
+  int *p;
+};

aaron.ballman wrote:
> ztamas wrote:
> > JonasToth wrote:
> > > Please add tests with templated classes and self-assignment.
> > I tested with template classes, but TemplateCopyAndSwap test case, for 
> > example, was wrongly caught by the check. So I just changed the code to 
> > ignore template classes. I did not find any template class related catch 
> > either in LibreOffice or LLVM when I run the first version of this check, 
> > so we don't lose too much with ignoring template classes, I think.
> I am not keen on ignoring template classes for the check; that seems like a 
> bug in the check that should be addressed. At a minimum, the test cases 
> should be present with FIXME comments about the unwanted diagnostics.
I don't think it's a good idea to change the check now to catch template 
classes and produce false positives.

First of all the check does not work with template classes because the AST is 
different. Check TemplateCopyAndSwap test case for example. It's expected that 
the definition of operator= contains a CXXConstructExpr, but something is 
different for templates. It might be a lower level problem, how to detect a 
constructor call in a template class or templates just need different matcher. 
So implementing this check with correct template handling might be tricky and 
it might make the checker more complex. I'm not sure it worth the time, because 
as I mentioned I did not see any template related catch in the tested code 
bases. However, it might be a good idea to mention this limitation in the 
documentation.

About the false positives. This template related false positives are different 
from other false positives. In general, check doesn't warn, if the code uses 
one of the three patterns (self-check, copy-and-move, copy-and-swap). However, 
TemplateCopyAndSwap test case was wrongly caught by the check even though it 
uses copy-and-swap method. I would not introduce this kind of false positives. 
So the user of the check can expect that if he / she uses one of the three 
patterns, then there will be no warning in his / her code.

I already added five template related test cases. I think the comments are 
clear about which test case should be ignored by the check and which test cases 
are incorrectly ignored by now.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60507/new/

https://reviews.llvm.org/D60507



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

[PATCH] D59887: [Syntax] Introduce TokenBuffer, start clangToolingSyntax library

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang/include/clang/Tooling/Syntax/TokenBuffer.h:146
+  llvm::Optional>
+  toOffsetRange(const Token *Begin, const Token *End,
+const SourceManager ) const;

ilya-biryukov wrote:
> ilya-biryukov wrote:
> > sammccall wrote:
> > > I'd think this would map a character range onto a character range, or a 
> > > token range onto a token range - is there some reason otherwise?
> > No particular reason, this was driven by its usage in function computing 
> > replacements (not in this patch, see [[ 
> > https://github.com/ilya-biryukov/llvm-project/blob/syntax/clang/lib/Tooling/Syntax/ComputeReplacements.cpp
> >  | github prototype ]], if interested, but keep in mind the prototype is 
> > not ready for review yet).
> > 
> > Mapping to a range of "raw" tokens seems more consistent with our model, 
> > will update accordingly, obtaining a range is easy after one has tokens 
> > from the file.
> Added a method to map from an expanded token range to a raw token range.
> Kept the method that maps an expanded token range to a character range too.
> 
> Note that we don't currently have a way to map from raw token range to 
> expanded, that could be added later, we don't need it for the initial 
> use-case (map the ranges coming from AST nodes into raw source ranges), so I 
> left it out for now.
> Note that we don't currently have a way to map from raw token range to 
> expanded, that could be added later, we don't need it for the initial 
> use-case (map the ranges coming from AST nodes into raw source ranges), so I 
> left it out for now.

That seems fine, can we add a comment somewhere (class header)?
Not exactly a FIXME, but it would clarify that this is in principle a 
bidirectional mapping, just missing some implementation.



Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:48
+namespace syntax {
+class TokenBuffer;
+

not needed



Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:59
+  explicit Token(const clang::Token );
+
+  tok::TokenKind kind() const { return Kind; }

Token should be copyable I think?



Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:62
+  SourceLocation location() const { return Location; }
+  SourceLocation endLocation() const {
+return Location.getLocWithOffset(Length);

sadly, this would need documentation - it's the one-past-the-end location, not 
the last character in the token, not the location of the "next" token, or the 
location of the "next" character...

I do wonder whether this is actually the right function to expose...
 Do you ever care about end but not start? (The reverse seems likelier). Having 
two independent source location accessors obscures the invariant that they have 
the same file ID.

I think exposing a `FileRange` accessor instead might be better, but for now 
you could also make callers use `Tok.location().getLocWithOffset(Tok.length())` 
until we know it's the right pattern to encourage



Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:78
+  /// For debugging purposes.
+  std::string str() const;
+

(do we need to have two names for this version?)



Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:95
+  /// End offset (exclusive) in a corresponding file.
+  unsigned End = 0;
+};

may want to offer `length()` and `StringRef text(const SourceManager&)`



Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:111
+///   replacements,
+///2. Raw tokens: corresponding directly to the source code of a file 
before
+///   any macro replacements occurred.

I think "Spelled" would be a better name than "Raw" here. (Despite being longer 
and grammatically awkward)

- The spelled/expanded dichotomy is well-established in clang-land, and this 
will help understand how they relate to each other and why the difference is 
important. It's true we'd be extending the meaning a bit (all PP activity, not 
just macros), but that applies to Expanded too. I think consistency is valuable 
here.
- "Raw" regarding tokens has an established meaning (raw lexer mode, 
raw_identifier etc) that AIUI you're not using here, but plausibly could be. So 
I think this may cause some confusion.

There's like 100 occurrences of "raw" in this patch, happy to discuss first to 
avoid churn.



Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:133
+  /// point to one of these tokens.
+  /// FIXME: the notable exception is '>>' being split into two '>'. figure out
+  ///how to deal with it.

It's not clear from this comment what the current behavior *is*, and how it's 
exceptional.
(It's a FIXME, but these sometimes last a while...)



Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:133
+  /// 

[PATCH] D60776: [clang-tidy] Add test support for the fix description.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 195385.
hokein added a comment.

cleanup


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60776/new/

https://reviews.llvm.org/D60776

Files:
  clang-tools-extra/test/clang-tidy/check_clang_tidy.py
  clang-tools-extra/test/clang-tidy/misc-unused-using-decls.cpp


Index: clang-tools-extra/test/clang-tidy/misc-unused-using-decls.cpp
===
--- clang-tools-extra/test/clang-tidy/misc-unused-using-decls.cpp
+++ clang-tools-extra/test/clang-tidy/misc-unused-using-decls.cpp
@@ -80,6 +80,7 @@
 using n::A; // A
 // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: using decl 'A' is unused
 // CHECK-FIXES: {{^}}// A
+// CHECK-FIX-DESCRIPTIONS: :[[@LINE-3]]:10: note: remove the using
 using n::B;
 using n::C;
 using n::D;
Index: clang-tools-extra/test/clang-tidy/check_clang_tidy.py
===
--- clang-tools-extra/test/clang-tidy/check_clang_tidy.py
+++ clang-tools-extra/test/clang-tidy/check_clang_tidy.py
@@ -90,10 +90,12 @@
   check_fixes_prefixes = []
   check_messages_prefixes = []
   check_notes_prefixes = []
+  check_fix_descriptions_prefixes = []
 
   has_check_fixes = False
   has_check_messages = False
   has_check_notes = False
+  has_check_fix_descriptions = False
 
   for check in args.check_suffix:
 if check and not re.match('^[A-Z0-9\-]+$', check):
@@ -104,26 +106,33 @@
 check_fixes_prefix = 'CHECK-FIXES' + file_check_suffix
 check_messages_prefix = 'CHECK-MESSAGES' + file_check_suffix
 check_notes_prefix = 'CHECK-NOTES' + file_check_suffix
+check_fix_descriptions_prefix = 'CHECK-FIX-DESCRIPTIONS' + 
file_check_suffix
 
 has_check_fix = check_fixes_prefix in input_text
 has_check_message = check_messages_prefix in input_text
 has_check_note = check_notes_prefix in input_text
+has_check_fix_description = check_fix_descriptions_prefix in input_text
 
 if has_check_note and has_check_message:
   sys.exit('Please use either %s or %s but not both' %
 (check_notes_prefix, check_messages_prefix))
 
-if not has_check_fix and not has_check_message and not has_check_note:
-  sys.exit('%s, %s or %s not found in the input' %
-(check_fixes_prefix, check_messages_prefix, check_notes_prefix))
+if not has_check_fix and not has_check_message and not has_check_note and \
+  not has_check_fix_description:
+  sys.exit('%s, %s, %s or %s not found in the input' %
+(check_fixes_prefix, check_messages_prefix, check_notes_prefix,
+  check_fix_descriptions_prefix))
 
 has_check_fixes = has_check_fixes or has_check_fix
 has_check_messages = has_check_messages or has_check_message
 has_check_notes = has_check_notes or has_check_note
+has_check_fix_descriptions = has_check_fix_descriptions or \
+  has_check_fix_description
 
 check_fixes_prefixes.append(check_fixes_prefix)
 check_messages_prefixes.append(check_messages_prefix)
 check_notes_prefixes.append(check_notes_prefix)
+check_fix_descriptions_prefixes.append(check_fix_descriptions_prefix)
 
   assert has_check_fixes or has_check_messages or has_check_notes
   # Remove the contents of the CHECK lines to avoid CHECKs matching on
@@ -175,6 +184,18 @@
   print('FileCheck failed:\n' + e.output.decode())
   raise
 
+  if has_check_fix_descriptions:
+try:
+  fix_descriptions_file = temp_file_name + '.fix-descriptions'
+  write_file(fix_descriptions_file, clang_tidy_output)
+  subprocess.check_output(
+  ['FileCheck', '-input-file=' + fix_descriptions_file, 
input_file_name,
+   '-check-prefixes=' + ','.join(check_fix_descriptions_prefixes)],
+  stderr=subprocess.STDOUT)
+except subprocess.CalledProcessError as e:
+  print('FileCheck failed:\n' + e.output.decode())
+  raise
+
   if has_check_messages:
 messages_file = temp_file_name + '.msg'
 write_file(messages_file, clang_tidy_output)


Index: clang-tools-extra/test/clang-tidy/misc-unused-using-decls.cpp
===
--- clang-tools-extra/test/clang-tidy/misc-unused-using-decls.cpp
+++ clang-tools-extra/test/clang-tidy/misc-unused-using-decls.cpp
@@ -80,6 +80,7 @@
 using n::A; // A
 // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: using decl 'A' is unused
 // CHECK-FIXES: {{^}}// A
+// CHECK-FIX-DESCRIPTIONS: :[[@LINE-3]]:10: note: remove the using
 using n::B;
 using n::C;
 using n::D;
Index: clang-tools-extra/test/clang-tidy/check_clang_tidy.py
===
--- clang-tools-extra/test/clang-tidy/check_clang_tidy.py
+++ clang-tools-extra/test/clang-tidy/check_clang_tidy.py
@@ -90,10 +90,12 @@
   check_fixes_prefixes = []
   check_messages_prefixes = []
   check_notes_prefixes = []
+  check_fix_descriptions_prefixes = []
 
   has_check_fixes 

[PATCH] D60776: [clang-tidy] Add test support for the fix description.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: alexfh.
Herald added a subscriber: xazax.hun.
Herald added a project: clang.

Add a new annotation "CHECK-FIX-DESCRIPTIONS" to lit test to
verify the fix description provided by checks.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D60776

Files:
  clang-tools-extra/test/clang-tidy/check_clang_tidy.py
  clang-tools-extra/test/clang-tidy/misc-unused-using-decls.cpp


Index: clang-tools-extra/test/clang-tidy/misc-unused-using-decls.cpp
===
--- clang-tools-extra/test/clang-tidy/misc-unused-using-decls.cpp
+++ clang-tools-extra/test/clang-tidy/misc-unused-using-decls.cpp
@@ -80,6 +80,7 @@
 using n::A; // A
 // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: using decl 'A' is unused
 // CHECK-FIXES: {{^}}// A
+// CHECK-FIX-DESCRIPTIONS: :[[@LINE-3]]:10: note: remove the using
 using n::B;
 using n::C;
 using n::D;
Index: clang-tools-extra/test/clang-tidy/check_clang_tidy.py
===
--- clang-tools-extra/test/clang-tidy/check_clang_tidy.py
+++ clang-tools-extra/test/clang-tidy/check_clang_tidy.py
@@ -90,10 +90,12 @@
   check_fixes_prefixes = []
   check_messages_prefixes = []
   check_notes_prefixes = []
+  check_fix_descriptions_prefixes = []
 
   has_check_fixes = False
   has_check_messages = False
   has_check_notes = False
+  has_check_fix_descriptions = False
 
   for check in args.check_suffix:
 if check and not re.match('^[A-Z0-9\-]+$', check):
@@ -104,26 +106,33 @@
 check_fixes_prefix = 'CHECK-FIXES' + file_check_suffix
 check_messages_prefix = 'CHECK-MESSAGES' + file_check_suffix
 check_notes_prefix = 'CHECK-NOTES' + file_check_suffix
+check_fix_descriptions_prefix = 'CHECK-FIX-DESCRIPTIONS' + 
file_check_suffix
 
 has_check_fix = check_fixes_prefix in input_text
 has_check_message = check_messages_prefix in input_text
 has_check_note = check_notes_prefix in input_text
+has_check_fix_description = check_fix_descriptions_prefix in input_text
 
 if has_check_note and has_check_message:
   sys.exit('Please use either %s or %s but not both' %
 (check_notes_prefix, check_messages_prefix))
 
-if not has_check_fix and not has_check_message and not has_check_note:
-  sys.exit('%s, %s or %s not found in the input' %
-(check_fixes_prefix, check_messages_prefix, check_notes_prefix))
+if not has_check_fix and not has_check_message and not has_check_note and \
+  not has_check_fix_description:
+  sys.exit('%s, %s, %s or %s not found in the input' %
+(check_fixes_prefix, check_messages_prefix, check_notes_prefix,
+  check_fix_descriptions_prefix))
 
 has_check_fixes = has_check_fixes or has_check_fix
 has_check_messages = has_check_messages or has_check_message
 has_check_notes = has_check_notes or has_check_note
+has_check_fix_descriptions = has_check_fix_descriptions or \
+  has_check_fix_description
 
 check_fixes_prefixes.append(check_fixes_prefix)
 check_messages_prefixes.append(check_messages_prefix)
 check_notes_prefixes.append(check_notes_prefix)
+check_fix_descriptions_prefixes.append(check_fix_descriptions_prefix)
 
   assert has_check_fixes or has_check_messages or has_check_notes
   # Remove the contents of the CHECK lines to avoid CHECKs matching on
@@ -175,6 +184,21 @@
   print('FileCheck failed:\n' + e.output.decode())
   raise
 
+  if has_check_fix_descriptions:
+try:
+  fix_descriptions_file = temp_file_name + '.fix-descriptions'
+  # filtered_output = [line for line in clang_tidy_output.splitlines()
+  #if not "note: FIX-IT applied" in line]
+  #write_file(notes_file, '\n'.join(filtered_output))
+  write_file(fix_descriptions_file, clang_tidy_output)
+  subprocess.check_output(
+  ['FileCheck', '-input-file=' + fix_descriptions_file, 
input_file_name,
+   '-check-prefixes=' + ','.join(check_fix_descriptions_prefixes)],
+  stderr=subprocess.STDOUT)
+except subprocess.CalledProcessError as e:
+  print('FileCheck failed:\n' + e.output.decode())
+  raise
+
   if has_check_messages:
 messages_file = temp_file_name + '.msg'
 write_file(messages_file, clang_tidy_output)


Index: clang-tools-extra/test/clang-tidy/misc-unused-using-decls.cpp
===
--- clang-tools-extra/test/clang-tidy/misc-unused-using-decls.cpp
+++ clang-tools-extra/test/clang-tidy/misc-unused-using-decls.cpp
@@ -80,6 +80,7 @@
 using n::A; // A
 // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: using decl 'A' is unused
 // CHECK-FIXES: {{^}}// A
+// CHECK-FIX-DESCRIPTIONS: :[[@LINE-3]]:10: note: remove the using
 using n::B;
 using n::C;
 using n::D;
Index: clang-tools-extra/test/clang-tidy/check_clang_tidy.py

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

[Tamás Zolnai via Phabricator via cfe-commits]

ztamas marked 4 inline comments as done.
ztamas added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst:6
+
+`cert-oop54-cpp` redirects here as an alias for this check.
+

aaron.ballman wrote:
> You should add a link to CERT's documentation somewhere around this text.
In an earlier version of this patch, there was a link. I removed it because of 
a reviewer comment. Now I add it back, I hope now are OK.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60507/new/

https://reviews.llvm.org/D60507



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

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

[Tamás Zolnai via Phabricator via cfe-commits]

ztamas updated this revision to Diff 195382.
ztamas added a comment.

Update patch based on reviewer comments.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60507/new/

https://reviews.llvm.org/D60507

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst
  clang-tools-extra/docs/clang-tidy/checks/cert-oop54-cpp.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/bugprone-unhandled-self-assignment.cpp

Index: clang-tools-extra/test/clang-tidy/bugprone-unhandled-self-assignment.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/bugprone-unhandled-self-assignment.cpp
@@ -0,0 +1,434 @@
+// RUN: %check_clang_tidy %s bugprone-unhandled-self-assignment %t
+
+namespace std {
+
+template 
+void swap(T x, T y) {
+}
+
+template 
+T &(T x) {
+}
+
+template 
+class unique_ptr {
+};
+
+template 
+class shared_ptr {
+};
+
+template 
+class weak_ptr {
+};
+
+template 
+class auto_ptr {
+};
+
+} // namespace std
+
+void assert(int expression){};
+
+///
+/// Test cases correctly caught by the check.
+
+class PtrField {
+public:
+  PtrField =(const PtrField );
+
+private:
+  int *p;
+};
+
+PtrField ::operator=(const PtrField ) {
+  // CHECK-MESSAGES: [[@LINE-1]]:21: warning: operator=() might not handle self-assignment properly [bugprone-unhandled-self-assignment]
+  // ...
+  return *this;
+}
+
+// Class with an inline operator definition.
+class InlineDefinition {
+public:
+  InlineDefinition =(const InlineDefinition ) {
+// CHECK-MESSAGES: [[@LINE-1]]:21: warning: operator=() might not handle self-assignment properly [bugprone-unhandled-self-assignment]
+// ...
+return *this;
+  }
+
+private:
+  int *p;
+};
+
+class UniquePtrField {
+public:
+  UniquePtrField =(const UniquePtrField ) {
+// CHECK-MESSAGES: [[@LINE-1]]:19: warning: operator=() might not handle self-assignment properly [bugprone-unhandled-self-assignment]
+// ...
+return *this;
+  }
+
+private:
+  std::unique_ptr p;
+};
+
+class SharedPtrField {
+public:
+  SharedPtrField =(const SharedPtrField ) {
+// CHECK-MESSAGES: [[@LINE-1]]:19: warning: operator=() might not handle self-assignment properly [bugprone-unhandled-self-assignment]
+// ...
+return *this;
+  }
+
+private:
+  std::shared_ptr p;
+};
+
+class WeakPtrField {
+public:
+  WeakPtrField =(const WeakPtrField ) {
+// CHECK-MESSAGES: [[@LINE-1]]:17: warning: operator=() might not handle self-assignment properly [bugprone-unhandled-self-assignment]
+// ...
+return *this;
+  }
+
+private:
+  std::weak_ptr p;
+};
+
+class AutoPtrField {
+public:
+  AutoPtrField =(const AutoPtrField ) {
+// CHECK-MESSAGES: [[@LINE-1]]:17: warning: operator=() might not handle self-assignment properly [bugprone-unhandled-self-assignment]
+// ...
+return *this;
+  }
+
+private:
+  std::auto_ptr p;
+};
+
+// Class with C array field.
+class CArrayField {
+public:
+  CArrayField =(const CArrayField ) {
+// CHECK-MESSAGES: [[@LINE-1]]:16: warning: operator=() might not handle self-assignment properly [bugprone-unhandled-self-assignment]
+// ...
+return *this;
+  }
+
+private:
+  int array[256];
+};
+
+// Make sure to not ignore cases when the operator definition calls
+// a copy constructor of another class.
+class CopyConstruct {
+public:
+  CopyConstruct =(const CopyConstruct ) {
+// CHECK-MESSAGES: [[@LINE-1]]:18: warning: operator=() might not handle self-assignment properly [bugprone-unhandled-self-assignment]
+WeakPtrField a;
+WeakPtrField b(a);
+// ...
+return *this;
+  }
+
+private:
+  int *p;
+};
+
+// Make sure to not ignore cases when the operator definition calls
+// a copy assignment operator of another class.
+class AssignOperator {
+public:
+  AssignOperator =(const AssignOperator ) {
+// CHECK-MESSAGES: [[@LINE-1]]:19: warning: operator=() might not handle self-assignment properly [bugprone-unhandled-self-assignment]
+a.operator=(object.a);
+// ...
+return *this;
+  }
+
+private:
+  int *p;
+  WeakPtrField a;
+};
+
+///
+/// Test cases correctly ignored by the check.
+
+// Self-assignment is checked using the equality operator.
+class SelfCheck1 {
+public:
+  SelfCheck1 =(const SelfCheck1 ) {
+if (this == )
+  return *this;
+// ...
+return *this;
+  }
+
+private:
+  int *p;
+};
+
+class SelfCheck2 {
+public:
+  SelfCheck2 =(const SelfCheck2 ) {
+if ( == this)
+  

[PATCH] D60775: [libclang] Expose ext_vector_type

[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!


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60775/new/

https://reviews.llvm.org/D60775



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

[PATCH] D60687: [clangd] Check file path of declaring header when deciding whether to insert include.

[Eric Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL358496: [clangd] Check file path of declaring header when 
deciding whether to insert… (authored by ioeric, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60687/new/

https://reviews.llvm.org/D60687

Files:
  clang-tools-extra/trunk/clangd/CodeComplete.cpp
  clang-tools-extra/trunk/clangd/Headers.cpp
  clang-tools-extra/trunk/clangd/Headers.h
  clang-tools-extra/trunk/clangd/IncludeFixer.cpp
  clang-tools-extra/trunk/unittests/clangd/HeadersTests.cpp

Index: clang-tools-extra/trunk/unittests/clangd/HeadersTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/HeadersTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/HeadersTests.cpp
@@ -93,11 +93,10 @@
  >getPreprocessor().getHeaderSearchInfo());
 for (const auto  : Inclusions)
   Inserter.addExisting(Inc);
-auto Declaring = ToHeaderFile(Original);
 auto Inserted = ToHeaderFile(Preferred);
-if (!Inserter.shouldInsertInclude(Declaring, Inserted))
+if (!Inserter.shouldInsertInclude(Original, Inserted))
   return "";
-std::string Path = Inserter.calculateIncludePath(Declaring, Inserted);
+std::string Path = Inserter.calculateIncludePath(Inserted);
 Action.EndSourceFile();
 return Path;
   }
@@ -258,16 +257,15 @@
/*BuildDir=*/"", /*HeaderSearchInfo=*/nullptr);
 
   auto HeaderPath = testPath("sub/bar.h");
-  auto Declaring = HeaderFile{HeaderPath, /*Verbatim=*/false};
   auto Inserting = HeaderFile{HeaderPath, /*Verbatim=*/false};
   auto Verbatim = HeaderFile{"", /*Verbatim=*/true};
 
-  EXPECT_EQ(Inserter.calculateIncludePath(Declaring, Inserting),
+  EXPECT_EQ(Inserter.calculateIncludePath(Inserting),
 "\"" + HeaderPath + "\"");
-  EXPECT_EQ(Inserter.shouldInsertInclude(Declaring, Inserting), false);
+  EXPECT_EQ(Inserter.shouldInsertInclude(HeaderPath, Inserting), false);
 
-  EXPECT_EQ(Inserter.calculateIncludePath(Declaring, Verbatim), "");
-  EXPECT_EQ(Inserter.shouldInsertInclude(Declaring, Verbatim), true);
+  EXPECT_EQ(Inserter.calculateIncludePath(Verbatim), "");
+  EXPECT_EQ(Inserter.shouldInsertInclude(HeaderPath, Verbatim), true);
 }
 
 } // namespace
Index: clang-tools-extra/trunk/clangd/Headers.cpp
===
--- clang-tools-extra/trunk/clangd/Headers.cpp
+++ clang-tools-extra/trunk/clangd/Headers.cpp
@@ -173,22 +173,21 @@
 /// FIXME(ioeric): we might not want to insert an absolute include path if the
 /// path is not shortened.
 bool IncludeInserter::shouldInsertInclude(
-const HeaderFile , const HeaderFile ) const {
-  assert(DeclaringHeader.valid() && InsertedHeader.valid());
+PathRef DeclaringHeader, const HeaderFile ) const {
+  assert(InsertedHeader.valid());
   if (!HeaderSearchInfo && !InsertedHeader.Verbatim)
 return false;
-  if (FileName == DeclaringHeader.File || FileName == InsertedHeader.File)
+  if (FileName == DeclaringHeader || FileName == InsertedHeader.File)
 return false;
   auto Included = [&](llvm::StringRef Header) {
 return IncludedHeaders.find(Header) != IncludedHeaders.end();
   };
-  return !Included(DeclaringHeader.File) && !Included(InsertedHeader.File);
+  return !Included(DeclaringHeader) && !Included(InsertedHeader.File);
 }
 
 std::string
-IncludeInserter::calculateIncludePath(const HeaderFile ,
-  const HeaderFile ) const {
-  assert(DeclaringHeader.valid() && InsertedHeader.valid());
+IncludeInserter::calculateIncludePath(const HeaderFile ) const {
+  assert(InsertedHeader.valid());
   if (InsertedHeader.Verbatim)
 return InsertedHeader.File;
   bool IsSystem = false;
Index: clang-tools-extra/trunk/clangd/CodeComplete.cpp
===
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp
@@ -349,15 +349,18 @@
 // Turn absolute path into a literal string that can be #included.
 auto Inserted = [&](llvm::StringRef Header)
 -> llvm::Expected> {
-  auto ResolvedDeclaring =
-  toHeaderFile(C.IndexResult->CanonicalDeclaration.FileURI, FileName);
+  auto DeclaringURI =
+  URI::parse(C.IndexResult->CanonicalDeclaration.FileURI);
+  if (!DeclaringURI)
+return DeclaringURI.takeError();
+  auto ResolvedDeclaring = URI::resolve(*DeclaringURI, FileName);
   if (!ResolvedDeclaring)
 return ResolvedDeclaring.takeError();
   auto ResolvedInserted = toHeaderFile(Header, FileName);
   if (!ResolvedInserted)
 return ResolvedInserted.takeError();
   return std::make_pair(
-  Includes.calculateIncludePath(*ResolvedDeclaring, 

[clang-tools-extra] r358496 - [clangd] Check file path of declaring header when deciding whether to insert include.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Tue Apr 16 07:35:49 2019
New Revision: 358496

URL: http://llvm.org/viewvc/llvm-project?rev=358496=rev
Log:
[clangd] Check file path of declaring header when deciding whether to insert 
include.

Summary:
Previously, we would use include spelling of the declaring header to check
whether the inserted header is the same as the main file. This doesn't help 
because
we only have file path of the main file.

Reviewers: sammccall

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/clangd/Headers.cpp
clang-tools-extra/trunk/clangd/Headers.h
clang-tools-extra/trunk/clangd/IncludeFixer.cpp
clang-tools-extra/trunk/unittests/clangd/HeadersTests.cpp

Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=358496=358495=358496=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Tue Apr 16 07:35:49 2019
@@ -349,15 +349,18 @@ struct CodeCompletionBuilder {
 // Turn absolute path into a literal string that can be #included.
 auto Inserted = [&](llvm::StringRef Header)
 -> llvm::Expected> {
-  auto ResolvedDeclaring =
-  toHeaderFile(C.IndexResult->CanonicalDeclaration.FileURI, FileName);
+  auto DeclaringURI =
+  URI::parse(C.IndexResult->CanonicalDeclaration.FileURI);
+  if (!DeclaringURI)
+return DeclaringURI.takeError();
+  auto ResolvedDeclaring = URI::resolve(*DeclaringURI, FileName);
   if (!ResolvedDeclaring)
 return ResolvedDeclaring.takeError();
   auto ResolvedInserted = toHeaderFile(Header, FileName);
   if (!ResolvedInserted)
 return ResolvedInserted.takeError();
   return std::make_pair(
-  Includes.calculateIncludePath(*ResolvedDeclaring, *ResolvedInserted),
+  Includes.calculateIncludePath(*ResolvedInserted),
   Includes.shouldInsertInclude(*ResolvedDeclaring, *ResolvedInserted));
 };
 bool ShouldInsert = C.headerToInsertIfAllowed(Opts).hasValue();

Modified: clang-tools-extra/trunk/clangd/Headers.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Headers.cpp?rev=358496=358495=358496=diff
==
--- clang-tools-extra/trunk/clangd/Headers.cpp (original)
+++ clang-tools-extra/trunk/clangd/Headers.cpp Tue Apr 16 07:35:49 2019
@@ -173,22 +173,21 @@ void IncludeInserter::addExisting(const
 /// FIXME(ioeric): we might not want to insert an absolute include path if the
 /// path is not shortened.
 bool IncludeInserter::shouldInsertInclude(
-const HeaderFile , const HeaderFile ) const 
{
-  assert(DeclaringHeader.valid() && InsertedHeader.valid());
+PathRef DeclaringHeader, const HeaderFile ) const {
+  assert(InsertedHeader.valid());
   if (!HeaderSearchInfo && !InsertedHeader.Verbatim)
 return false;
-  if (FileName == DeclaringHeader.File || FileName == InsertedHeader.File)
+  if (FileName == DeclaringHeader || FileName == InsertedHeader.File)
 return false;
   auto Included = [&](llvm::StringRef Header) {
 return IncludedHeaders.find(Header) != IncludedHeaders.end();
   };
-  return !Included(DeclaringHeader.File) && !Included(InsertedHeader.File);
+  return !Included(DeclaringHeader) && !Included(InsertedHeader.File);
 }
 
 std::string
-IncludeInserter::calculateIncludePath(const HeaderFile ,
-  const HeaderFile ) const {
-  assert(DeclaringHeader.valid() && InsertedHeader.valid());
+IncludeInserter::calculateIncludePath(const HeaderFile ) const {
+  assert(InsertedHeader.valid());
   if (InsertedHeader.Verbatim)
 return InsertedHeader.File;
   bool IsSystem = false;

Modified: clang-tools-extra/trunk/clangd/Headers.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Headers.h?rev=358496=358495=358496=diff
==
--- clang-tools-extra/trunk/clangd/Headers.h (original)
+++ clang-tools-extra/trunk/clangd/Headers.h Tue Apr 16 07:35:49 2019
@@ -137,25 +137,22 @@ public:
   ///   in \p Inclusions (including those included via different paths).
   ///   - \p DeclaringHeader or \p InsertedHeader is the same as \p File.
   ///
-  /// \param DeclaringHeader is the original header corresponding to \p
+  /// \param DeclaringHeader is path of the original header corresponding to \p
   /// InsertedHeader e.g. the header that declares a symbol.
   /// \param InsertedHeader The preferred header to be inserted. This could be
   /// the same as DeclaringHeader but must be provided.
-  bool 

[PATCH] D60764: Add clang cc1 option to generate OpenCL builtin functions

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: clang/include/clang/Driver/CC1Options.td:755
   HelpText<"Include the default header file for OpenCL">;
+def fgenerate_opencl_builtin: Flag<["-"], "fgenerate-opencl-builtin">,
+  HelpText<"Generate OpenCL builtin functions using Tablegen">;

I feel may be it should be `-fadd-opencl-builtins` just to hide implementation 
details from the clang users.

Also the description should be something like:

"Add OpenCL builtin function definitions automatically..."


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60764/new/

https://reviews.llvm.org/D60764



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

r358493 - [OPENMP]Require aarch arch for the tests, NFC.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Tue Apr 16 07:26:10 2019
New Revision: 358493

URL: http://llvm.org/viewvc/llvm-project?rev=358493=rev
Log:
[OPENMP]Require aarch arch for the tests, NFC.

Modified:
cfe/trunk/test/OpenMP/declare_simd_aarch64.c
cfe/trunk/test/OpenMP/declare_simd_aarch64.cpp
cfe/trunk/test/OpenMP/declare_simd_aarch64_complex.c
cfe/trunk/test/OpenMP/declare_simd_aarch64_fix.c
cfe/trunk/test/OpenMP/declare_simd_aarch64_sve.c
cfe/trunk/test/OpenMP/declare_simd_aarch64_warning_advsimd.c
cfe/trunk/test/OpenMP/declare_simd_aarch64_warning_sve.c

Modified: cfe/trunk/test/OpenMP/declare_simd_aarch64.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/declare_simd_aarch64.c?rev=358493=358492=358493=diff
==
--- cfe/trunk/test/OpenMP/declare_simd_aarch64.c (original)
+++ cfe/trunk/test/OpenMP/declare_simd_aarch64.c Tue Apr 16 07:26:10 2019
@@ -1,3 +1,4 @@
+// REQUIRES: aarch64-registered-target
 // -fopemp and -fopenmp-simd behavior are expected to be the same.
 
 // RUN: %clang_cc1 -triple aarch64-linux-gnu -target-feature +neon -fopenmp -x 
c -emit-llvm %s -o - -femit-all-decls | FileCheck %s --check-prefix=AARCH64

Modified: cfe/trunk/test/OpenMP/declare_simd_aarch64.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/declare_simd_aarch64.cpp?rev=358493=358492=358493=diff
==
--- cfe/trunk/test/OpenMP/declare_simd_aarch64.cpp (original)
+++ cfe/trunk/test/OpenMP/declare_simd_aarch64.cpp Tue Apr 16 07:26:10 2019
@@ -1,3 +1,4 @@
+// REQUIRES: aarch64-registered-target
 // -fopemp and -fopenmp-simd behavior are expected to be the same.
 
 // RUN: %clang_cc1 -verify -triple aarch64-linux-gnu -target-feature +neon 
-fopenmp -x c++ -emit-llvm %s -o - -femit-all-decls -verify| FileCheck %s 
--check-prefix=ADVSIMD

Modified: cfe/trunk/test/OpenMP/declare_simd_aarch64_complex.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/declare_simd_aarch64_complex.c?rev=358493=358492=358493=diff
==
--- cfe/trunk/test/OpenMP/declare_simd_aarch64_complex.c (original)
+++ cfe/trunk/test/OpenMP/declare_simd_aarch64_complex.c Tue Apr 16 07:26:10 
2019
@@ -1,3 +1,4 @@
+// REQUIRES: aarch64-registered-target
 // RUN: %clang_cc1 -triple aarch64-linux-gnu -target-feature +neon -fopenmp -x 
c -std=c11 -emit-llvm %s -o - -femit-all-decls | FileCheck %s
 
 // RUN: %clang_cc1 -triple aarch64-linux-gnu -target-feature +sve -fopenmp -x 
c -std=c11 -emit-llvm %s -o - -femit-all-decls | FileCheck %s --check-prefix=SVE

Modified: cfe/trunk/test/OpenMP/declare_simd_aarch64_fix.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/declare_simd_aarch64_fix.c?rev=358493=358492=358493=diff
==
--- cfe/trunk/test/OpenMP/declare_simd_aarch64_fix.c (original)
+++ cfe/trunk/test/OpenMP/declare_simd_aarch64_fix.c Tue Apr 16 07:26:10 2019
@@ -1,3 +1,4 @@
+// REQUIRES: aarch64-registered-target
 // This test is making sure that no crash happens.
 
 // RUN: %clang -o - -fno-fast-math -S -target aarch64-linux-gnu \

Modified: cfe/trunk/test/OpenMP/declare_simd_aarch64_sve.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/declare_simd_aarch64_sve.c?rev=358493=358492=358493=diff
==
--- cfe/trunk/test/OpenMP/declare_simd_aarch64_sve.c (original)
+++ cfe/trunk/test/OpenMP/declare_simd_aarch64_sve.c Tue Apr 16 07:26:10 2019
@@ -1,3 +1,4 @@
+// REQUIRES: aarch64-registered-target
 // -fopemp and -fopenmp-simd behavior are expected to be the same
 
 // RUN: %clang_cc1 -triple aarch64-linux-gnu -target-feature +sve \

Modified: cfe/trunk/test/OpenMP/declare_simd_aarch64_warning_advsimd.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/declare_simd_aarch64_warning_advsimd.c?rev=358493=358492=358493=diff
==
--- cfe/trunk/test/OpenMP/declare_simd_aarch64_warning_advsimd.c (original)
+++ cfe/trunk/test/OpenMP/declare_simd_aarch64_warning_advsimd.c Tue Apr 16 
07:26:10 2019
@@ -1,3 +1,4 @@
+// REQUIRES: aarch64-registered-target
 // RUN: %clang_cc1 -triple aarch64-linux-gnu -target-feature +neon  -fopenmp  
%s -S  -o %t -verify
 // RUN: %clang_cc1 -triple aarch64-linux-gnu -target-feature +neon  
-fopenmp-simd  %s -S  -o %t -verify
 

Modified: cfe/trunk/test/OpenMP/declare_simd_aarch64_warning_sve.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/declare_simd_aarch64_warning_sve.c?rev=358493=358492=358493=diff
==
--- cfe/trunk/test/OpenMP/declare_simd_aarch64_warning_sve.c (original)
+++ 

[PATCH] D60763: Prototype OpenCL BIFs using Tablegen

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a subscriber: joey.
Anastasia added a comment.

Not related to this patch but it might be good to start thinking about testing 
this functionality properly. In the past we haven't tested the header because 
it would take a lot of testing time. So I would suggest we keep a light minimal 
basic testing in regular clang tests as is, but then we either add special 
build target to tests extra header functionality or add special cmake option 
(i.e. `LLVM_INCLUDE_OPENCL_BIFS_TESTS`) that would enable such testing with 
regular `check-clang`. I assume the latter can even be used to avoid generating 
the extra tables from TableGen minimizing the impact on the clang binary size 
for the situations OpenCL isn't required in the installation.

Any thoughts?




Comment at: clang/include/clang/Basic/OpenCLBuiltins.td:10
+//===--===//
+
+class Type {

I would add a comment describing each section:
- Defining classes
- Instantiating types
- Defining buintin functions
- etc...

Also I am a bit lost in the structure here. I think we should group 
functionality in sections + document each (as requested above).



Comment at: clang/include/clang/Basic/OpenCLBuiltins.td:15
+  int isPointer = 0;
+  string as = "clang::LangAS::Default";
+  int has_rounding = 0;

can be renamed to `addrSpace`



Comment at: clang/include/clang/Basic/OpenCLBuiltins.td:16
+  string as = "clang::LangAS::Default";
+  int has_rounding = 0;
+  int is_integer = 0;

why is the naming here follows different convention?



Comment at: clang/include/clang/Basic/OpenCLBuiltins.td:50
+
+let is_integer = 1 in {
+  def char_t   : Type<"char">;

may be this can go below or above?



Comment at: clang/include/clang/Basic/OpenCLBuiltins.td:65
+  def float_t  : Type<"float">;
+  def double_t : Type<"double">;
+}

half?



Comment at: clang/include/clang/Basic/OpenCLBuiltins.td:82
+// Creates builtins for one argument BIFs, taking and returning the same type.
+multiclass bi_vec {
+  def: Builtin;

again the name is not coherent with the rest.



Comment at: clang/include/clang/Basic/OpenCLBuiltins.td:125
+// example 'foo', to show using 'version'
+def: Builtin<"foo_version", int_t, [PointerType]>;
+let version = CL20 in {

I think we need a real BIF here or it has to be removed from the commit for now?



Comment at: clang/lib/Sema/SemaLookup.cpp:675
 
+// TODO: Auto-generate this from tablegen
+static QualType OCL2Qual(ASTContext , OpenCLType Ty) {

Does this mean we have to produce this in `ClangOpenCLBuiltinEmitter.cpp`?



Comment at: clang/lib/Sema/SemaLookup.cpp:762
+
+// TODO: This part is taken from Sema::LazilyCreateBuiltin, maybe refactor 
it.
+DeclContext *Parent = Context.getTranslationUnitDecl();

@joey are you suggesting to use a common helper function?



Comment at: clang/lib/Sema/SemaLookup.cpp:777
+  for (unsigned i = 0, e = FT->getNumParams(); i != e; ++i) {
+ParmVarDecl *parm =
+ParmVarDecl::Create(Context, New, SourceLocation(), 
SourceLocation(),

Variable naming convention is not followed!



Comment at: clang/test/SemaOpenCL/builtin-new.cl:15
+kernel void test2(global int* bar) {
+  bar[0] = foo_version(bar);
+}

I guess we need to find an actual function here. How about `ctz`?

Although, it doesn't have to be CL2.0 actually.



Comment at: clang/test/SemaOpenCL/builtin-new.cl:30
+kernel void test5(write_only image2d_t img, int2 coord, float4 colour) {
+  write_imagef(img, coord, colour);
+}

I think different overloads can be available in different CL version. @svenvh 
can we already handle this?



Comment at: clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp:12
+//
+// This tablegen backend emits Clang OpenCL Builtin checking code.
+//

Builtin -> builtin function



Comment at: clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp:34
+namespace {
+class BuiltinNameEmitter {
+public:

I think this file deserves more documentation...


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60763/new/

https://reviews.llvm.org/D60763



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

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

[Don Hinton via Phabricator via cfe-commits]

hintonda marked an inline comment as done.
hintonda added inline comments.



Comment at: clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp:27-28
 CheckFactories.registerCheck("llvm-include-order");
+CheckFactories.registerCheck(
+"llvm-prefer-isa-or-dyn-cast-in-conditionals");
 CheckFactories.registerCheck(

aaron.ballman wrote:
> hintonda wrote:
> > aaron.ballman wrote:
> > > Please keep this list sorted alphabetically.
> > I think this is where add_new_check.py put it, but I'll alphabetize.
> Huh, I thought that script kept everything alphabetized? That may be worth 
> investigating (don't feel obligated though).
I'm sure it's something I did inadvertently, but can't really remember the 
details. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59802/new/

https://reviews.llvm.org/D59802



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

[PATCH] D59302: [clangd] Surface diagnostics from headers inside main file

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: unittests/clangd/DiagnosticsTests.cpp:620
 
+ParsedAST build(const std::string ,
+const llvm::StringMap ) {

ilya-biryukov wrote:
> We were discussing adding the extra files to `TestTU` instead.
> Any reason this did not work out?
Yes, my LRU cache evicted the discussion :D


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59302/new/

https://reviews.llvm.org/D59302



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

[PATCH] D60775: [libclang] Expose ext_vector_type

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh created this revision.
svenvh added a reviewer: Anastasia.
Herald added subscribers: cfe-commits, arphaman.
Herald added a project: clang.

Expose `ext_vector_type` through the C API.


Repository:
  rC Clang

https://reviews.llvm.org/D60775

Files:
  bindings/python/clang/cindex.py
  include/clang-c/Index.h
  test/Index/opencl-types.cl
  tools/libclang/CXType.cpp


Index: tools/libclang/CXType.cpp
===
--- tools/libclang/CXType.cpp
+++ tools/libclang/CXType.cpp
@@ -109,6 +109,7 @@
 TKCASE(VariableArray);
 TKCASE(DependentSizedArray);
 TKCASE(Vector);
+TKCASE(ExtVector);
 TKCASE(MemberPointer);
 TKCASE(Auto);
 TKCASE(Elaborated);
@@ -600,6 +601,7 @@
 TKIND(VariableArray);
 TKIND(DependentSizedArray);
 TKIND(Vector);
+TKIND(ExtVector);
 TKIND(MemberPointer);
 TKIND(Auto);
 TKIND(Elaborated);
@@ -804,6 +806,9 @@
 case Type::Vector:
   ET = cast (TP)->getElementType();
   break;
+case Type::ExtVector:
+  ET = cast(TP)->getElementType();
+  break;
 case Type::Complex:
   ET = cast (TP)->getElementType();
   break;
@@ -827,6 +832,9 @@
 case Type::Vector:
   result = cast (TP)->getNumElements();
   break;
+case Type::ExtVector:
+  result = cast(TP)->getNumElements();
+  break;
 default:
   break;
 }
Index: test/Index/opencl-types.cl
===
--- test/Index/opencl-types.cl
+++ test/Index/opencl-types.cl
@@ -17,11 +17,11 @@
 }
 
 // CHECK: VarDecl=scalarHalf:11:8 (Definition){{( \(invalid\))?}} [type=half] 
[typekind=Half] [isPOD=1]
-// CHECK: VarDecl=vectorHalf:12:9 (Definition) [type=half4] [typekind=Typedef] 
[canonicaltype=half __attribute__((ext_vector_type(4)))] 
[canonicaltypekind=Unexposed] [isPOD=1]
+// CHECK: VarDecl=vectorHalf:12:9 (Definition) [type=half4] [typekind=Typedef] 
[canonicaltype=half __attribute__((ext_vector_type(4)))] 
[canonicaltypekind=ExtVector] [isPOD=1]
 // CHECK: VarDecl=scalarFloat:13:9 (Definition) [type=float] [typekind=Float] 
[isPOD=1]
-// CHECK: VarDecl=vectorFloat:14:10 (Definition) [type=float4] 
[typekind=Typedef] [canonicaltype=float __attribute__((ext_vector_type(4)))] 
[canonicaltypekind=Unexposed] [isPOD=1]
+// CHECK: VarDecl=vectorFloat:14:10 (Definition) [type=float4] 
[typekind=Typedef] [canonicaltype=float __attribute__((ext_vector_type(4)))] 
[canonicaltypekind=ExtVector] [isPOD=1]
 // CHECK: VarDecl=scalarDouble:15:10 (Definition){{( \(invalid\))?}} 
[type=double] [typekind=Double] [isPOD=1]
-// CHECK: VarDecl=vectorDouble:16:11 (Definition){{( \(invalid\))?}} 
[type=double4] [typekind=Typedef] [canonicaltype=double 
__attribute__((ext_vector_type(4)))] [canonicaltypekind=Unexposed] [isPOD=1]
+// CHECK: VarDecl=vectorDouble:16:11 (Definition){{( \(invalid\))?}} 
[type=double4] [typekind=Typedef] [canonicaltype=double 
__attribute__((ext_vector_type(4)))] [canonicaltypekind=ExtVector] [isPOD=1]
 
 #pragma OPENCL EXTENSION cl_khr_gl_msaa_sharing : enable
 
Index: include/clang-c/Index.h
===
--- include/clang-c/Index.h
+++ include/clang-c/Index.h
@@ -32,7 +32,7 @@
  * compatible, thus CINDEX_VERSION_MAJOR is expected to remain stable.
  */
 #define CINDEX_VERSION_MAJOR 0
-#define CINDEX_VERSION_MINOR 54
+#define CINDEX_VERSION_MINOR 55
 
 #define CINDEX_VERSION_ENCODE(major, minor) ( \
   ((major) * 1)   \
@@ -3315,7 +3315,9 @@
   CXType_OCLIntelSubgroupAVCImeResultDualRefStreamout = 173,
   CXType_OCLIntelSubgroupAVCImeSingleRefStreamin = 174,
 
-  CXType_OCLIntelSubgroupAVCImeDualRefStreamin = 175
+  CXType_OCLIntelSubgroupAVCImeDualRefStreamin = 175,
+
+  CXType_ExtVector = 176
 };
 
 /**
Index: bindings/python/clang/cindex.py
===
--- bindings/python/clang/cindex.py
+++ bindings/python/clang/cindex.py
@@ -2121,6 +2121,8 @@
 TypeKind.OCLQUEUE = TypeKind(159)
 TypeKind.OCLRESERVEID = TypeKind(160)
 
+TypeKind.EXTVECTOR = TypeKind(176)
+
 class RefQualifierKind(BaseEnumeration):
 """Describes a specific ref-qualifier of a type."""
 


Index: tools/libclang/CXType.cpp
===
--- tools/libclang/CXType.cpp
+++ tools/libclang/CXType.cpp
@@ -109,6 +109,7 @@
 TKCASE(VariableArray);
 TKCASE(DependentSizedArray);
 TKCASE(Vector);
+TKCASE(ExtVector);
 TKCASE(MemberPointer);
 TKCASE(Auto);
 TKCASE(Elaborated);
@@ -600,6 +601,7 @@
 TKIND(VariableArray);
 TKIND(DependentSizedArray);
 TKIND(Vector);
+TKIND(ExtVector);
 TKIND(MemberPointer);
 TKIND(Auto);
 TKIND(Elaborated);
@@ -804,6 +806,9 @@
 case Type::Vector:
   ET = cast (TP)->getElementType();
   break;
+case Type::ExtVector:
+  ET = cast(TP)->getElementType();
+  break;
 

[PATCH] D59302: [clangd] Surface diagnostics from headers inside main file

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 195375.
kadircet marked 14 inline comments as done.
kadircet added a comment.

- Address comments


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59302/new/

https://reviews.llvm.org/D59302

Files:
  clangd/Diagnostics.cpp
  clangd/Diagnostics.h
  unittests/clangd/DiagnosticsTests.cpp
  unittests/clangd/TestTU.cpp
  unittests/clangd/TestTU.h

Index: unittests/clangd/TestTU.h
===
--- unittests/clangd/TestTU.h
+++ unittests/clangd/TestTU.h
@@ -18,8 +18,11 @@
 #define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTTU_H
 
 #include "ClangdUnit.h"
+#include "Path.h"
 #include "index/Index.h"
 #include "gtest/gtest.h"
+#include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -45,6 +48,9 @@
   std::string HeaderCode;
   std::string HeaderFilename = "TestTU.h";
 
+  // Absolute path and contents of each file.
+  std::vector> AdditionalFiles;
+
   // Extra arguments for the compiler invocation.
   std::vector ExtraArgs;
 
Index: unittests/clangd/TestTU.cpp
===
--- unittests/clangd/TestTU.cpp
+++ unittests/clangd/TestTU.cpp
@@ -20,6 +20,13 @@
 ParsedAST TestTU::build() const {
   std::string FullFilename = testPath(Filename),
   FullHeaderName = testPath(HeaderFilename);
+
+  llvm::StringMap Files;
+  Files[FullFilename] = Code;
+  Files[FullHeaderName] = HeaderCode;
+  for (const auto  : AdditionalFiles)
+Files[Entry.first] = Entry.second;
+
   std::vector Cmd = {"clang", FullFilename.c_str()};
   // FIXME: this shouldn't need to be conditional, but it breaks a
   // GoToDefinition test for some reason (getMacroArgExpandedLocation fails).
@@ -33,7 +40,7 @@
   Inputs.CompileCommand.CommandLine = {Cmd.begin(), Cmd.end()};
   Inputs.CompileCommand.Directory = testRoot();
   Inputs.Contents = Code;
-  Inputs.FS = buildTestFS({{FullFilename, Code}, {FullHeaderName, HeaderCode}});
+  Inputs.FS = buildTestFS(Files);
   Inputs.Opts = ParseOptions();
   Inputs.Opts.ClangTidyOpts.Checks = ClangTidyChecks;
   Inputs.Index = ExternalIndex;
Index: unittests/clangd/DiagnosticsTests.cpp
===
--- unittests/clangd/DiagnosticsTests.cpp
+++ unittests/clangd/DiagnosticsTests.cpp
@@ -8,13 +8,17 @@
 
 #include "Annotations.h"
 #include "ClangdUnit.h"
+#include "Path.h"
+#include "Protocol.h"
 #include "SourceCode.h"
+#include "TestFS.h"
 #include "TestIndex.h"
 #include "TestTU.h"
 #include "index/MemIndex.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
 
 namespace clang {
 namespace clangd {
@@ -45,6 +49,15 @@
   return arg.Range == Range && arg.Message == Message;
 }
 
+MATCHER_P3(Diag, Range, Message, IncludeStack,
+   "Diag at " + llvm::to_string(Range) + " = [" + Message + "]") {
+  if (arg.Range != Range || arg.Message != Message ||
+  arg.IncludeStack.size() != IncludeStack.size())
+return false;
+  return std::equal(IncludeStack.begin(), IncludeStack.end(),
+arg.IncludeStack.begin());
+}
+
 MATCHER_P3(Fix, Range, Replacement, Message,
"Fix " + llvm::to_string(Range) + " => " +
testing::PrintToString(Replacement) + " = [" + Message + "]") {
@@ -73,7 +86,6 @@
   return true;
 }
 
-
 // Helper function to make tests shorter.
 Position pos(int line, int character) {
   Position Res;
@@ -251,6 +263,8 @@
   D.InsideMainFile = true;
   D.Severity = DiagnosticsEngine::Error;
   D.File = "foo/bar/main.cpp";
+  D.IncludeStack.push_back({"a/b.h", pos(0, 1)});
+  D.IncludeStack.push_back({"a/c.h", pos(1, 1)});
 
   clangd::Note NoteInMain;
   NoteInMain.Message = "declared somewhere in the main file";
@@ -281,8 +295,9 @@
   };
 
   // Diagnostics should turn into these:
-  clangd::Diagnostic MainLSP =
-  MatchingLSP(D, R"(Something terrible happened (fix available)
+  clangd::Diagnostic MainLSP = MatchingLSP(
+  D, R"(In file a/c.h:2:2: something terrible happened (fix available)
+In file included from: a/b.h:1:2
 
 main.cpp:6:7: remark: declared somewhere in the main file
 
@@ -603,7 +618,144 @@
   "Add include \"x.h\" for symbol a::X");
 }
 
+TEST(DiagsInHeaders, DiagInsideHeader) {
+  Annotations Main(R"cpp(
+#include [["a.h"]]
+void foo() {})cpp");
+  TestTU TU = TestTU::withCode(Main.code());
+  TU.AdditionalFiles = {{testPath("a.h"), "no_type_spec;"}};
+  EXPECT_THAT(TU.build().getDiagnostics(),
+  UnorderedElementsAre(
+  Diag(Main.range(),
+   "C++ requires a type specifier for all declarations",
+   std::vector>{
+   {"/clangd-test/a.h", pos(0, 0)}})));
+}
+
+TEST(DiagsInHeaders, DiagInTransitiveInclude) {
+  Annotations Main(R"cpp(
+#include [["a.h"]]
+void foo() 

[PATCH] D60455: [SYCL] Add support for SYCL device attributes

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

In D60455#1467279 , @aaron.ballman 
wrote:

>




> I'm still wondering what the actual semantics are for the attribute. Right 
> now, these are being parsed and ignored -- are there future plans here?

Yes, we are going to teach the compiler differ SYCL device code from host code 
and ignore functions without device attributes when SYCL device mode is 
enabled. I've described some possible implementation details in this comment 
.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60455/new/

https://reviews.llvm.org/D60455



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

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang/include/clang/Lex/Preprocessor.h:120
+/// callback that records tokens.
+enum class TokenSource {
+  File, // a token coming directly from a file that is not a macro directive,

This is supposed to provide enough information to distinguish token from an 
intermediate macro expansion from a token of a final expanded stream.

It's not very well thought-through, though, the final set of enum values will 
definitely end up being different.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59885/new/

https://reviews.llvm.org/D59885



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

[PATCH] D60583: [AArch64] Implement Vector Funtion ABI name mangling.

[Alexey Bataev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC358490: [AArch64] Implement Vector Funtion ABI name 
mangling. (authored by ABataev, committed by ).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D60583?vs=195231=195371#toc

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60583/new/

https://reviews.llvm.org/D60583

Files:
  lib/CodeGen/CGOpenMPRuntime.cpp
  test/OpenMP/Inputs/declare-simd-fix.h
  test/OpenMP/declare_simd_aarch64.c
  test/OpenMP/declare_simd_aarch64.cpp
  test/OpenMP/declare_simd_aarch64_complex.c
  test/OpenMP/declare_simd_aarch64_fix.c
  test/OpenMP/declare_simd_aarch64_sve.c
  test/OpenMP/declare_simd_aarch64_warning_advsimd.c
  test/OpenMP/declare_simd_aarch64_warning_sve.c

Index: lib/CodeGen/CGOpenMPRuntime.cpp
===
--- lib/CodeGen/CGOpenMPRuntime.cpp
+++ lib/CodeGen/CGOpenMPRuntime.cpp
@@ -9648,6 +9648,307 @@
   }
 }
 
+// This are the Functions that are needed to mangle the name of the
+// vector functions generated by the compiler, according to the rules
+// defined in the "Vector Function ABI specifications for AArch64",
+// available at
+// https://developer.arm.com/products/software-development-tools/hpc/arm-compiler-for-hpc/vector-function-abi.
+
+/// Maps To Vector (MTV), as defined in 3.1.1 of the AAVFABI.
+///
+/// TODO: Need to implement the behavior for reference marked with a
+/// var or no linear modifiers (1.b in the section). For this, we
+/// need to extend ParamKindTy to support the linear modifiers.
+static bool getAArch64MTV(QualType QT, ParamKindTy Kind) {
+  QT = QT.getCanonicalType();
+
+  if (QT->isVoidType())
+return false;
+
+  if (Kind == ParamKindTy::Uniform)
+return false;
+
+  if (Kind == ParamKindTy::Linear)
+return false;
+
+  // TODO: Handle linear references with modifiers
+
+  if (Kind == ParamKindTy::LinearWithVarStride)
+return false;
+
+  return true;
+}
+
+/// Pass By Value (PBV), as defined in 3.1.2 of the AAVFABI.
+static bool getAArch64PBV(QualType QT, ASTContext ) {
+  QT = QT.getCanonicalType();
+  unsigned Size = C.getTypeSize(QT);
+
+  // Only scalars and complex within 16 bytes wide set PVB to true.
+  if (Size != 8 && Size != 16 && Size != 32 && Size != 64 && Size != 128)
+return false;
+
+  if (QT->isFloatingType())
+return true;
+
+  if (QT->isIntegerType())
+return true;
+
+  if (QT->isPointerType())
+return true;
+
+  // TODO: Add support for complex types (section 3.1.2, item 2).
+
+  return false;
+}
+
+/// Computes the lane size (LS) of a return type or of an input parameter,
+/// as defined by `LS(P)` in 3.2.1 of the AAVFABI.
+/// TODO: Add support for references, section 3.2.1, item 1.
+static unsigned getAArch64LS(QualType QT, ParamKindTy Kind, ASTContext ) {
+  if (getAArch64MTV(QT, Kind) && QT.getCanonicalType()->isPointerType()) {
+QualType PTy = QT.getCanonicalType()->getPointeeType();
+if (getAArch64PBV(PTy, C))
+  return C.getTypeSize(PTy);
+  }
+  if (getAArch64PBV(QT, C))
+return C.getTypeSize(QT);
+
+  return C.getTypeSize(C.getUIntPtrType());
+}
+
+// Get Narrowest Data Size (NDS) and Widest Data Size (WDS) from the
+// signature of the scalar function, as defined in 3.2.2 of the
+// AAVFABI.
+static std::tuple
+getNDSWDS(const FunctionDecl *FD, ArrayRef ParamAttrs) {
+  QualType RetType = FD->getReturnType().getCanonicalType();
+
+  ASTContext  = FD->getASTContext();
+
+  bool OutputBecomesInput = false;
+
+  llvm::SmallVector Sizes;
+  if (!RetType->isVoidType()) {
+Sizes.push_back(getAArch64LS(RetType, ParamKindTy::Vector, C));
+if (!getAArch64PBV(RetType, C) && getAArch64MTV(RetType, {}))
+  OutputBecomesInput = true;
+  }
+  for (unsigned I = 0, E = FD->getNumParams(); I < E; ++I) {
+QualType QT = FD->getParamDecl(I)->getType().getCanonicalType();
+Sizes.push_back(getAArch64LS(QT, ParamAttrs[I].Kind, C));
+  }
+
+  assert(!Sizes.empty() && "Unable to determine NDS and WDS.");
+  // The LS of a function parameter / return value can only be a power
+  // of 2, starting from 8 bits, up to 128.
+  assert(std::all_of(Sizes.begin(), Sizes.end(),
+ [](unsigned Size) {
+   return Size == 8 || Size == 16 || Size == 32 ||
+  Size == 64 || Size == 128;
+ }) &&
+ "Invalid size");
+
+  return std::make_tuple(*std::min_element(std::begin(Sizes), std::end(Sizes)),
+ *std::max_element(std::begin(Sizes), std::end(Sizes)),
+ OutputBecomesInput);
+}
+
+/// Mangle the parameter part of the vector function name according to
+/// their OpenMP classification. The mangling function is defined in
+/// section 3.5 of the AAVFABI.
+static std::string mangleVectorParameters(ArrayRef ParamAttrs) {
+  SmallString<256>