[PATCH] D80109: [AST][RecoveryExpr] Preserve type for broken member call expr.

2020-11-29 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 308267.
hokein added a comment.
Herald added a subscriber: lxfind.

rebase and address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80109

Files:
  clang/lib/Sema/SemaCoroutine.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/AST/ast-dump-recovery.cpp
  clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p12.cpp

Index: clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p12.cpp
===
--- clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p12.cpp
+++ clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p12.cpp
@@ -9,7 +9,7 @@
 //   parameter types in a base class (rather than conflicting).
 
 template  struct Opaque {};
-template  void expect(Opaque _) {}
+template  void expect(Opaque _) {} // expected-note 4 {{candidate function template not viable}}
 
 // PR5727
 // This just shouldn't crash.
@@ -134,14 +134,14 @@
   void test() {
 expect<0>(Base().foo());
 expect<1>(Base().foo<0>());
-expect<0>(Derived1().foo()); // expected-error {{no matching member function for call to 'foo'}}
+expect<0>(Derived1().foo()); // expected-error {{no matching member function for call to 'foo'}} expected-error {{no matching function for call to 'expect'}}
 expect<2>(Derived1().foo<0>());
-expect<0>(Derived2().foo()); // expected-error {{no matching member function for call to 'foo'}}
+expect<0>(Derived2().foo()); // expected-error {{no matching member function for call to 'foo'}} expected-error {{no matching function for call to 'expect'}}
 expect<2>(Derived2().foo<0>());
 expect<3>(Derived3().foo());
-expect<1>(Derived3().foo<0>()); // expected-error {{no matching member function for call to 'foo'}}
+expect<1>(Derived3().foo<0>()); // expected-error {{no matching member function for call to 'foo'}} expected-error {{no matching function for call to 'expect'}}
 expect<3>(Derived4().foo());
-expect<1>(Derived4().foo<0>()); // expected-error {{no matching member function for call to 'foo'}}
+expect<1>(Derived4().foo<0>()); // expected-error {{no matching member function for call to 'foo'}} expected-error {{no matching function for call to 'expect'}}
   }
 }
 
Index: clang/test/AST/ast-dump-recovery.cpp
===
--- clang/test/AST/ast-dump-recovery.cpp
+++ clang/test/AST/ast-dump-recovery.cpp
@@ -121,6 +121,21 @@
   foo->func(x);
 }
 
+// CHECK:  FunctionDecl {{.*}} test2
+// CHECK-NEXT: |-ParmVarDecl {{.*}} f
+// CHECK-NEXT: `-CompoundStmt
+// CHECK-NEXT:-RecoveryExpr {{.*}} 'int' contains-errors
+// CHECK-NEXT:|-UnresolvedMemberExpr
+// CHECK-NEXT:   `-DeclRefExpr {{.*}} 'Foo2'
+// CHECK-NEXT:`-IntegerLiteral {{.*}} 'int' 1
+struct Foo2 {
+  int overload();
+  int overload(int, int);
+};
+void test2(Foo2 f) {
+  f.overload(1); // verify "int" is preserved
+}
+
 // CHECK: |-AlignedAttr {{.*}} alignas
 // CHECK-NEXT:| `-RecoveryExpr {{.*}} contains-errors
 // CHECK-NEXT:|   `-UnresolvedLookupExpr {{.*}} 'invalid'
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -14239,6 +14239,7 @@
 UnbridgedCasts.restore();
 
 OverloadCandidateSet::iterator Best;
+bool Succeeded = false;
 switch (CandidateSet.BestViableFunction(*this, UnresExpr->getBeginLoc(),
 Best)) {
 case OR_Success:
@@ -14246,7 +14247,7 @@
   FoundDecl = Best->FoundDecl;
   CheckUnresolvedMemberAccess(UnresExpr, Best->FoundDecl);
   if (DiagnoseUseOfDecl(Best->FoundDecl, UnresExpr->getNameLoc()))
-return ExprError();
+break;
   // If FoundDecl is different from Method (such as if one is a template
   // and the other a specialization), make sure DiagnoseUseOfDecl is
   // called on both.
@@ -14255,7 +14256,8 @@
   // being used.
   if (Method != FoundDecl.getDecl() &&
   DiagnoseUseOfDecl(Method, UnresExpr->getNameLoc()))
-return ExprError();
+break;
+  Succeeded = true;
   break;
 
 case OR_No_Viable_Function:
@@ -14265,8 +14267,7 @@
   PDiag(diag::err_ovl_no_viable_member_function_in_call)
   << DeclName << MemExprE->getSourceRange()),
   *this, OCD_AllCandidates, Args);
-  // FIXME: Leaking incoming expressions!
-  return ExprError();
+  break;
 
 case OR_Ambiguous:
   CandidateSet.NoteCandidates(
@@ -14274,8 +14275,7 @@
   PDiag(diag::err_ovl_ambiguous_member_call)
   << DeclName << MemExprE->getSourceRange()),
   *this, OCD_AmbiguousCandidates, Args);
-  // FIXME: Leaking incoming expressions!
-  return 

[PATCH] D90871: [Sema] Fold VLAs to constant arrays in a few more contexts

2020-11-29 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

Thanks, that's very clean.




Comment at: clang/lib/Sema/SemaDecl.cpp:6027
+/// Attempt to fold a variable-sized type to a constant-sized type, returning
+/// true if it we were successful.
+static bool tryToFixVariablyModifiedVarType(Sema , TypeSourceInfo *,

Typo "it we"



Comment at: clang/lib/Sema/SemaDecl.cpp:6899-6901
+if (D.hasInitializer() && R->isVariablyModifiedType())
+  tryToFixVariablyModifiedVarType(*this, TInfo, R, D.getIdentifierLoc(),
+  /*DiagID=*/0);

Is there a reason to make this C-specific? We support VLAs in C++ as an 
extension; it'd seem reasonable to do this folding as an extension too, if it's 
as simple as moving the new code out of the `if (!C++)`.



Comment at: clang/test/Sema/vla.c:104
+
+void test_fold_to_constant_array() {
+  const int ksize = 4;

Can you also add a positive test? Eg, a `goto` over one of the 
folded-to-constant cases, to show that we can jump over those. (As-is, this 
test ensures that we produce the warning, but not that we actually treat the 
array variable as not being a VLA.)


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

https://reviews.llvm.org/D90871

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


[PATCH] D92298: [AST][RecoveryAST] Preserve type for member call expr if argments are not matched.

2020-11-29 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision.
hokein added a reviewer: sammccall.
Herald added a project: clang.
hokein requested review of this revision.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92298

Files:
  clang/lib/Sema/SemaOverload.cpp
  clang/test/AST/ast-dump-recovery.cpp
  clang/test/SemaCXX/ms-property-error.cpp


Index: clang/test/SemaCXX/ms-property-error.cpp
===
--- clang/test/SemaCXX/ms-property-error.cpp
+++ clang/test/SemaCXX/ms-property-error.cpp
@@ -13,7 +13,7 @@
 public:
   __declspec(property(get=GetX,put=PutX)) T x[];
   T GetX(T i, T j) { return i+j; } // expected-note 3 {{'GetX' declared here}}
-  T PutX(T i, T j, T k) { return j = i = k; }  // expected-note 2 {{'PutX' 
declared here}}
+  T PutX(T i, T j, T k) { return j = i = k; }  // expected-note 3 {{'PutX' 
declared here}}
   ~St() {
 x[1] = 0; // expected-error {{too few arguments to function call, expected 
3, have 2}}
 x[2][3] = 4;
@@ -33,5 +33,6 @@
   float j1 = (p2->x); // expected-error {{too few arguments to function call, 
expected 2, have 0}}
   ((p2->x)[23])[1][2] = *argv; // expected-error {{too many arguments to 
function call, expected 3, have 4}}
   argv = p2->x[11][22] = argc; // expected-error {{assigning to 'char **' from 
incompatible type 'float'}}
-  return ++(((p2->x)[23])); // expected-error {{too few arguments to function 
call, expected 2, have 1}}
+  return ++(((p2->x)[23])); // expected-error {{too few arguments to function 
call, expected 2, have 1}} \
+   expected-error {{too few arguments to function 
call, expected 3, have 2}} 
 }
Index: clang/test/AST/ast-dump-recovery.cpp
===
--- clang/test/AST/ast-dump-recovery.cpp
+++ clang/test/AST/ast-dump-recovery.cpp
@@ -121,6 +121,20 @@
   foo->func(x);
 }
 
+// CHECK:  FunctionDecl {{.*}} test2
+// CHECK-NEXT: |-ParmVarDecl {{.*}} f
+// CHECK-NEXT: `-CompoundStmt
+// CHECK-NEXT:   `-RecoveryExpr {{.*}} 'double'
+// CHECK-NEXT: |-MemberExpr {{.*}} ''
+// CHECK-NEXT: | `-DeclRefExpr {{.*}}
+// CHECK-NEXT:   `-IntegerLiteral {{.*}} 'int' 1
+struct Foo2 {
+  double func();
+};
+void test2(Foo2 f) {
+  f.func(1);
+}
+
 // CHECK: |-AlignedAttr {{.*}} alignas
 // CHECK-NEXT:| `-RecoveryExpr {{.*}} contains-errors
 // CHECK-NEXT:|   `-UnresolvedLookupExpr {{.*}} 'invalid'
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -14328,8 +14328,12 @@
 
   // Convert the rest of the arguments
   if (ConvertArgumentsForCall(TheCall, MemExpr, Method, Proto, Args,
-  RParenLoc))
-return ExprError();
+  RParenLoc)) {
+std::vector SubExprs = {MemExprE};
+llvm::for_each(Args, [](Expr *E) { SubExprs.push_back(E); });
+return CreateRecoveryExpr(MemExprE->getBeginLoc(), RParenLoc, SubExprs,
+  ResultType);
+  }
 
   DiagnoseSentinelCalls(Method, LParenLoc, Args);
 


Index: clang/test/SemaCXX/ms-property-error.cpp
===
--- clang/test/SemaCXX/ms-property-error.cpp
+++ clang/test/SemaCXX/ms-property-error.cpp
@@ -13,7 +13,7 @@
 public:
   __declspec(property(get=GetX,put=PutX)) T x[];
   T GetX(T i, T j) { return i+j; } // expected-note 3 {{'GetX' declared here}}
-  T PutX(T i, T j, T k) { return j = i = k; }  // expected-note 2 {{'PutX' declared here}}
+  T PutX(T i, T j, T k) { return j = i = k; }  // expected-note 3 {{'PutX' declared here}}
   ~St() {
 x[1] = 0; // expected-error {{too few arguments to function call, expected 3, have 2}}
 x[2][3] = 4;
@@ -33,5 +33,6 @@
   float j1 = (p2->x); // expected-error {{too few arguments to function call, expected 2, have 0}}
   ((p2->x)[23])[1][2] = *argv; // expected-error {{too many arguments to function call, expected 3, have 4}}
   argv = p2->x[11][22] = argc; // expected-error {{assigning to 'char **' from incompatible type 'float'}}
-  return ++(((p2->x)[23])); // expected-error {{too few arguments to function call, expected 2, have 1}}
+  return ++(((p2->x)[23])); // expected-error {{too few arguments to function call, expected 2, have 1}} \
+   expected-error {{too few arguments to function call, expected 3, have 2}} 
 }
Index: clang/test/AST/ast-dump-recovery.cpp
===
--- clang/test/AST/ast-dump-recovery.cpp
+++ clang/test/AST/ast-dump-recovery.cpp
@@ -121,6 +121,20 @@
   foo->func(x);
 }
 
+// CHECK:  FunctionDecl {{.*}} test2
+// CHECK-NEXT: |-ParmVarDecl {{.*}} f
+// CHECK-NEXT: `-CompoundStmt
+// CHECK-NEXT:   `-RecoveryExpr {{.*}} 'double'
+// CHECK-NEXT: |-MemberExpr {{.*}} ''
+// CHECK-NEXT: | `-DeclRefExpr {{.*}}
+// CHECK-NEXT:   

[PATCH] D92297: [CodeGen] -fno-delete-null-pointer-checks: change dereferenceable to dereferenceable_or_null

2020-11-29 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment.

This was discussed on the original patch 
(https://reviews.llvm.org/D17993#inline-830995) and @jdoerfert argued that 
`dereferenceable` is correct even in `NullPointerIsValid` mode. Does this 
indicate a bug in the middle-end? If so, we should add a FIXME here to remove 
the workaround once the underlying bug is fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92297

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


[PATCH] D92297: [CodeGen] -fno-delete-null-pointer-checks: change dereferenceable to dereferenceable_or_null

2020-11-29 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision.
MaskRay added reviewers: bkramer, CJ-Johnson, jdoerfert, rjmccall, rsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
MaskRay requested review of this revision.

After D17993 , with 
-fno-delete-null-pointer-checks we add the dereferenceable attribute.

We have observed that one target which worked before fails with 
-fno-delete-null-pointer-checks.
Switching to dereferenceable_or_null fixes the problem.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92297

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/test/CodeGenCXX/this-nonnull.cpp


Index: clang/test/CodeGenCXX/this-nonnull.cpp
===
--- clang/test/CodeGenCXX/this-nonnull.cpp
+++ clang/test/CodeGenCXX/this-nonnull.cpp
@@ -12,8 +12,8 @@
   s.ReturnsVoid();
 
   // CHECK-YES: call void @_ZN6Struct11ReturnsVoidEv(%struct.Struct* nonnull 
dereferenceable(12) %0)
-  // CHECK-NO: call void @_ZN6Struct11ReturnsVoidEv(%struct.Struct* 
dereferenceable(12) %0)
+  // CHECK-NO: call void @_ZN6Struct11ReturnsVoidEv(%struct.Struct* 
dereferenceable_or_null(12) %0)
 }
 
 // CHECK-YES: declare void @_ZN6Struct11ReturnsVoidEv(%struct.Struct* nonnull 
dereferenceable(12))
-// CHECK-NO: declare void @_ZN6Struct11ReturnsVoidEv(%struct.Struct* 
dereferenceable(12))
+// CHECK-NO: declare void @_ZN6Struct11ReturnsVoidEv(%struct.Struct* 
dereferenceable_or_null(12))
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -2168,13 +2168,17 @@
 if (!CodeGenOpts.NullPointerIsValid &&
 getContext().getTargetAddressSpace(FI.arg_begin()->type) == 0) {
   Attrs.addAttribute(llvm::Attribute::NonNull);
+  Attrs.addDereferenceableAttr(
+  getMinimumObjectSize(
+  FI.arg_begin()->type.castAs()->getPointeeType())
+  .getQuantity());
+} else {
+  Attrs.addDereferenceableOrNullAttr(
+  getMinimumObjectSize(
+  FI.arg_begin()->type.castAs()->getPointeeType())
+  .getQuantity());
 }
 
-Attrs.addDereferenceableAttr(
-getMinimumObjectSize(
-FI.arg_begin()->type.castAs()->getPointeeType())
-.getQuantity());
-
 ArgAttrs[IRArgs.first] = llvm::AttributeSet::get(getLLVMContext(), Attrs);
   }
 


Index: clang/test/CodeGenCXX/this-nonnull.cpp
===
--- clang/test/CodeGenCXX/this-nonnull.cpp
+++ clang/test/CodeGenCXX/this-nonnull.cpp
@@ -12,8 +12,8 @@
   s.ReturnsVoid();
 
   // CHECK-YES: call void @_ZN6Struct11ReturnsVoidEv(%struct.Struct* nonnull dereferenceable(12) %0)
-  // CHECK-NO: call void @_ZN6Struct11ReturnsVoidEv(%struct.Struct* dereferenceable(12) %0)
+  // CHECK-NO: call void @_ZN6Struct11ReturnsVoidEv(%struct.Struct* dereferenceable_or_null(12) %0)
 }
 
 // CHECK-YES: declare void @_ZN6Struct11ReturnsVoidEv(%struct.Struct* nonnull dereferenceable(12))
-// CHECK-NO: declare void @_ZN6Struct11ReturnsVoidEv(%struct.Struct* dereferenceable(12))
+// CHECK-NO: declare void @_ZN6Struct11ReturnsVoidEv(%struct.Struct* dereferenceable_or_null(12))
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -2168,13 +2168,17 @@
 if (!CodeGenOpts.NullPointerIsValid &&
 getContext().getTargetAddressSpace(FI.arg_begin()->type) == 0) {
   Attrs.addAttribute(llvm::Attribute::NonNull);
+  Attrs.addDereferenceableAttr(
+  getMinimumObjectSize(
+  FI.arg_begin()->type.castAs()->getPointeeType())
+  .getQuantity());
+} else {
+  Attrs.addDereferenceableOrNullAttr(
+  getMinimumObjectSize(
+  FI.arg_begin()->type.castAs()->getPointeeType())
+  .getQuantity());
 }
 
-Attrs.addDereferenceableAttr(
-getMinimumObjectSize(
-FI.arg_begin()->type.castAs()->getPointeeType())
-.getQuantity());
-
 ArgAttrs[IRArgs.first] = llvm::AttributeSet::get(getLLVMContext(), Attrs);
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 1db60c1 - Remove redundant check for access in the conversion from the naming

2020-11-29 Thread Richard Smith via cfe-commits

Author: Richard Smith
Date: 2020-11-29T19:21:59-08:00
New Revision: 1db60c1307ac2e24796047c39d09bf400c22e531

URL: 
https://github.com/llvm/llvm-project/commit/1db60c1307ac2e24796047c39d09bf400c22e531
DIFF: 
https://github.com/llvm/llvm-project/commit/1db60c1307ac2e24796047c39d09bf400c22e531.diff

LOG: Remove redundant check for access in the conversion from the naming
class to the declaring class in a class member access.

This check does not appear to be backed by any rule in the standard (the
rule in question was likely removed over the years), and only ever
produces duplicate diagnostics. (It's also not meaningful because there
isn't a unique declaring class after the resolution of core issue 39.)

Added: 


Modified: 
clang/lib/Sema/SemaExpr.cpp
clang/test/CXX/class.access/class.access.base/p1.cpp
clang/test/CXX/class.access/class.access.base/p5.cpp
clang/test/CXX/class.access/class.friend/p1.cpp
clang/test/CXX/class.access/class.protected/p1.cpp
clang/test/CXX/class.access/p4.cpp
clang/test/CXX/drs/dr0xx.cpp
clang/test/CXX/drs/dr1xx.cpp
clang/test/CXX/drs/dr2xx.cpp
clang/test/CXX/drs/dr3xx.cpp
clang/test/SemaCXX/anonymous-union.cpp
clang/test/SemaCXX/conversion-function.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index d25d91223826..88dab26f2e3b 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -2818,21 +2818,24 @@ Sema::LookupInObjCMethod(LookupResult , Scope *S,
 
 /// Cast a base object to a member's actual type.
 ///
-/// Logically this happens in three phases:
+/// There are two relevant checks:
 ///
-/// * First we cast from the base type to the naming class.
-///   The naming class is the class into which we were looking
-///   when we found the member;  it's the qualifier type if a
-///   qualifier was provided, and otherwise it's the base type.
+/// C++ [class.access.base]p7:
 ///
-/// * Next we cast from the naming class to the declaring class.
-///   If the member we found was brought into a class's scope by
-///   a using declaration, this is that class;  otherwise it's
-///   the class declaring the member.
+///   If a class member access operator [...] is used to access a non-static
+///   data member or non-static member function, the reference is ill-formed if
+///   the left operand [...] cannot be implicitly converted to a pointer to the
+///   naming class of the right operand.
 ///
-/// * Finally we cast from the declaring class to the "true"
-///   declaring class of the member.  This conversion does not
-///   obey access control.
+/// C++ [expr.ref]p7:
+///
+///   If E2 is a non-static data member or a non-static member function, the
+///   program is ill-formed if the class of which E2 is directly a member is an
+///   ambiguous base (11.8) of the naming class (11.9.3) of E2.
+///
+/// Note that the latter check does not consider access; the access of the
+/// "real" base class is checked as appropriate when checking the access of the
+/// member name.
 ExprResult
 Sema::PerformObjectMemberConversion(Expr *From,
 NestedNameSpecifier *Qualifier,
@@ -2956,45 +2959,10 @@ Sema::PerformObjectMemberConversion(Expr *From,
 }
   }
 
-  bool IgnoreAccess = false;
-
-  // If we actually found the member through a using declaration, cast
-  // down to the using declaration's type.
-  //
-  // Pointer equality is fine here because only one declaration of a
-  // class ever has member declarations.
-  if (FoundDecl->getDeclContext() != Member->getDeclContext()) {
-assert(isa(FoundDecl));
-QualType URecordType = Context.getTypeDeclType(
-   cast(FoundDecl->getDeclContext()));
-
-// We only need to do this if the naming-class to declaring-class
-// conversion is non-trivial.
-if (!Context.hasSameUnqualifiedType(FromRecordType, URecordType)) {
-  assert(IsDerivedFrom(FromLoc, FromRecordType, URecordType));
-  CXXCastPath BasePath;
-  if (CheckDerivedToBaseConversion(FromRecordType, URecordType,
-   FromLoc, FromRange, ))
-return ExprError();
-
-  QualType UType = URecordType;
-  if (PointerConversions)
-UType = Context.getPointerType(UType);
-  From = ImpCastExprToType(From, UType, CK_UncheckedDerivedToBase,
-   VK, ).get();
-  FromType = UType;
-  FromRecordType = URecordType;
-}
-
-// We don't do access control for the conversion from the
-// declaring class to the true declaring class.
-IgnoreAccess = true;
-  }
-
   CXXCastPath BasePath;
   if (CheckDerivedToBaseConversion(FromRecordType, DestRecordType,
FromLoc, FromRange, ,
-   IgnoreAccess))
+   /*IgnoreAccess=*/true))
  

[PATCH] D86844: [LoopDeletion] Allows deletion of possibly infinite side-effect free loops

2020-11-29 Thread Atmn Patel via Phabricator via cfe-commits
atmnpatel updated this revision to Diff 308249.
atmnpatel added a comment.

I believe this happened becase when I removed the loop, I did not update 
MemorySSA. The exact error was from GVN, but this update seems to fix the stage 
2 build compile time error locally (I checked by running the build bot script).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86844

Files:
  clang/test/Misc/loop-opt-setup.c
  llvm/include/llvm/Transforms/Utils/LoopUtils.h
  llvm/lib/Transforms/Scalar/LoopDeletion.cpp
  llvm/lib/Transforms/Utils/LoopUtils.cpp
  llvm/test/Other/loop-deletion-printer.ll
  llvm/test/Other/loop-pm-invalidation.ll
  llvm/test/Transforms/LICM/2003-02-27-PreheaderProblem.ll
  llvm/test/Transforms/LoopDeletion/2017-07-11-incremental-dt.ll
  llvm/test/Transforms/LoopDeletion/basic-remark.ll
  llvm/test/Transforms/LoopDeletion/diundef.ll
  llvm/test/Transforms/LoopDeletion/invalidation.ll
  llvm/test/Transforms/LoopDeletion/multiple-exit-conditions.ll
  llvm/test/Transforms/LoopDeletion/multiple-exits.ll
  llvm/test/Transforms/LoopDeletion/mustprogress.ll
  llvm/test/Transforms/LoopDeletion/no-exit-blocks.ll
  llvm/test/Transforms/LoopDeletion/unreachable-loops.ll
  llvm/test/Transforms/LoopDeletion/use-in-unreachable.ll
  llvm/test/Transforms/SCCP/calltest.ll
  llvm/test/Transforms/SimpleLoopUnswitch/pr37888.ll

Index: llvm/test/Transforms/SimpleLoopUnswitch/pr37888.ll
===
--- llvm/test/Transforms/SimpleLoopUnswitch/pr37888.ll
+++ llvm/test/Transforms/SimpleLoopUnswitch/pr37888.ll
@@ -7,7 +7,7 @@
 
 target triple = "x86_64-unknown-linux-gnu"
 
-define void @pr37888() {
+define void @pr37888() willreturn {
 ; CHECK-LABEL: define void @pr37888()
 entry:
   %tobool = icmp ne i16 undef, 0
Index: llvm/test/Transforms/SCCP/calltest.ll
===
--- llvm/test/Transforms/SCCP/calltest.ll
+++ llvm/test/Transforms/SCCP/calltest.ll
@@ -4,7 +4,7 @@
 %empty = type {}
 declare %empty @has_side_effects()
 
-define double @test_0(i32 %param) {
+define double @test_0(i32 %param) willreturn {
 ; CHECK-LABEL: @test_0(
 ; CHECK-NOT: br
 entry:
Index: llvm/test/Transforms/LoopDeletion/use-in-unreachable.ll
===
--- llvm/test/Transforms/LoopDeletion/use-in-unreachable.ll
+++ llvm/test/Transforms/LoopDeletion/use-in-unreachable.ll
@@ -3,7 +3,7 @@
 ; Checking that possible users of instruction from the loop in
 ; unreachable blocks are handled.
 
-define i64 @foo() {
+define i64 @foo() willreturn {
 entry:
   br label %invloop
 ; CHECK-LABEL-NOT: invloop
Index: llvm/test/Transforms/LoopDeletion/unreachable-loops.ll
===
--- llvm/test/Transforms/LoopDeletion/unreachable-loops.ll
+++ llvm/test/Transforms/LoopDeletion/unreachable-loops.ll
@@ -216,7 +216,7 @@
 ; Show recursive deletion of loops. Since we start with subloops and progress outward 
 ; to parent loop, we first delete the loop L2. Now loop L1 becomes a non-loop since it's backedge
 ; from L2's preheader to L1's exit block is never taken. So, L1 gets deleted as well.
-define void @test8(i64 %n) {
+define void @test8(i64 %n) #0 {
 ; CHECK-LABEL: test8
 ; CHECK-LABEL: entry:
 ; CHECK-NEXT: br label %exit
@@ -318,7 +318,7 @@
 ; deleted.
 ; In the next iteration, since L2 is never executed and has no subloops, we delete
 ; L2 as well. Finally, the outermost loop L1 is deleted.
-define void @test11(i64 %n) {
+define void @test11(i64 %n) #0 {
 ; CHECK-LABEL: test11
 ; CHECK-LABEL: entry:
 ; CHECK-NEXT: br label %exit
@@ -355,7 +355,7 @@
 
 
 ; 2 edges from a single exiting block to the exit block.
-define i64 @test12(i64 %n){
+define i64 @test12(i64 %n) #0 {
 ;CHECK-LABEL: @test12
 ; CHECK-NOT: L1:
 ; CHECK-NOT: L1Latch:
@@ -392,7 +392,7 @@
 }
 
 ; multiple edges to exit block from the same exiting blocks
-define i64 @test13(i64 %n) {
+define i64 @test13(i64 %n) #0 {
 ; CHECK-LABEL: @test13
 ; CHECK-NOT: L1:
 ; CHECK-NOT: L1Latch:
@@ -433,3 +433,5 @@
   %y.phi = phi i64 [ 10, %L1Block ], [ 10, %L1Block ], [ %y.next, %L1 ], [ 30, %L1Latch ], [ 30, %L1Latch ]
   ret i64 %y.phi
 }
+
+attributes #0 = { willreturn }
Index: llvm/test/Transforms/LoopDeletion/no-exit-blocks.ll
===
--- llvm/test/Transforms/LoopDeletion/no-exit-blocks.ll
+++ llvm/test/Transforms/LoopDeletion/no-exit-blocks.ll
@@ -5,14 +5,7 @@
 ; CHECK: Function Attrs: mustprogress
 ; CHECK-LABEL: define {{[^@]+}}@f
 ; CHECK-SAME: () [[ATTR0:#.*]] {
-; CHECK-NEXT:br label [[TMP1:%.*]]
-; CHECK:   1:
-; CHECK-NEXT:[[DOT01:%.*]] = phi i32 [ 1, [[TMP0:%.*]] ], [ [[TMP3:%.*]], [[TMP2:%.*]] ]
-; CHECK-NEXT:[[DOT0:%.*]] = phi i32 [ 1, [[TMP0]] ], [ [[TMP3]], [[TMP2]] ]
-; CHECK-NEXT:br label [[TMP2]]
-; CHECK:  

[PATCH] D86844: [LoopDeletion] Allows deletion of possibly infinite side-effect free loops

2020-11-29 Thread Atmn Patel via Phabricator via cfe-commits
atmnpatel reopened this revision.
atmnpatel added a comment.
This revision is now accepted and ready to land.

This introduced a compile-time error that showed up during a stage 2 build.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86844

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


[PATCH] D92291: clang/test: Remove platform-linker feature

2020-11-29 Thread Nico Weber via Phabricator via cfe-commits
thakis created this revision.
thakis added reviewers: serge-sans-paille, leonardchan, vvereschaka.
Herald added subscribers: llvm-commits, frasercrmck, luismarques, apazos, 
sameer.abuasal, pzheng, s.egerton, lenary, Jim, jocewei, PkmX, the_o, 
brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, niosHD, sabuasal, 
simoncook, johnrusso, rbar, asb.
Herald added a project: LLVM.
thakis requested review of this revision.

By explicitly requesting the system linker with `-fuse-ld=`, the
tests are able to CHECK for the system linker even with
CLANG_DEFAULT_LINKER=lld.

Alternative to D74704 .


https://reviews.llvm.org/D92291

Files:
  clang/test/Driver/riscv32-toolchain-extra.c
  clang/test/Driver/riscv32-toolchain.c
  clang/test/Driver/riscv64-toolchain-extra.c
  clang/test/Driver/riscv64-toolchain.c
  clang/test/lit.site.cfg.py.in
  llvm/utils/gn/secondary/clang/test/BUILD.gn

Index: llvm/utils/gn/secondary/clang/test/BUILD.gn
===
--- llvm/utils/gn/secondary/clang/test/BUILD.gn
+++ llvm/utils/gn/secondary/clang/test/BUILD.gn
@@ -46,7 +46,6 @@
 "LLVM_WITH_Z3=",  # Must be empty, not 0.
 "CLANG_BUILD_EXAMPLES=0",
 "CLANG_DEFAULT_CXX_STDLIB=",  # Empty string means "default value" here.
-"CLANG_DEFAULT_LINKER=",
 "CLANG_TOOLS_DIR=" + rebase_path("$root_out_dir/bin", dir),
 "CLANG_VENDOR_UTI=org.llvm.clang",
 
Index: clang/test/lit.site.cfg.py.in
===
--- clang/test/lit.site.cfg.py.in
+++ clang/test/lit.site.cfg.py.in
@@ -47,9 +47,6 @@
 import lit.llvm
 lit.llvm.initialize(lit_config, config)
 
-if not "@CLANG_DEFAULT_LINKER@":
-config.available_features.add('platform-linker')
-
 # Let the main config do the real work.
 lit_config.load_config(
 config, os.path.join(config.clang_src_dir, "test/lit.cfg.py"))
Index: clang/test/Driver/riscv64-toolchain.c
===
--- clang/test/Driver/riscv64-toolchain.c
+++ clang/test/Driver/riscv64-toolchain.c
@@ -1,5 +1,4 @@
 // A basic clang -cc1 command-line, and simple environment check.
-// REQUIRES: platform-linker
 
 // RUN: %clang %s -### -no-canonical-prefixes -target riscv64 \
 // RUN:   --gcc-toolchain=%S/Inputs/basic_riscv64_tree 2>&1 \
@@ -15,7 +14,7 @@
 // In the below tests, --rtlib=platform is used so that the driver ignores
 // the configure-time CLANG_DEFAULT_RTLIB option when choosing the runtime lib
 
-// RUN: %clang %s -### -no-canonical-prefixes \
+// RUN: %clang %s -### -no-canonical-prefixes -fuse-ld= \
 // RUN:   -target riscv64-unknown-elf --rtlib=platform \
 // RUN:   --gcc-toolchain=%S/Inputs/basic_riscv64_tree \
 // RUN:   --sysroot=%S/Inputs/basic_riscv64_tree/riscv64-unknown-elf 2>&1 \
@@ -30,7 +29,7 @@
 // C-RV64-BAREMETAL-LP64: "--start-group" "-lc" "-lgloss" "--end-group" "-lgcc"
 // C-RV64-BAREMETAL-LP64: "{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtend.o"
 
-// RUN: %clang %s -### -no-canonical-prefixes \
+// RUN: %clang %s -### -no-canonical-prefixes -fuse-ld= \
 // RUN:   -target riscv64-unknown-elf --rtlib=platform \
 // RUN:   --sysroot= \
 // RUN:   --gcc-toolchain=%S/Inputs/basic_riscv64_tree 2>&1 \
@@ -44,7 +43,7 @@
 // C-RV64-BAREMETAL-NOSYSROOT-LP64: "--start-group" "-lc" "-lgloss" "--end-group" "-lgcc"
 // C-RV64-BAREMETAL-NOSYSROOT-LP64: "{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtend.o"
 
-// RUN: %clangxx %s -### -no-canonical-prefixes \
+// RUN: %clangxx %s -### -no-canonical-prefixes -fuse-ld= \
 // RUN:   -target riscv64-unknown-elf -stdlib=libstdc++ --rtlib=platform \
 // RUN:   --gcc-toolchain=%S/Inputs/basic_riscv64_tree \
 // RUN:   --sysroot=%S/Inputs/basic_riscv64_tree/riscv64-unknown-elf 2>&1 \
@@ -60,7 +59,7 @@
 // CXX-RV64-BAREMETAL-LP64: "-lstdc++" "--start-group" "-lc" "-lgloss" "--end-group" "-lgcc"
 // CXX-RV64-BAREMETAL-LP64: "{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtend.o"
 
-// RUN: %clangxx %s -### -no-canonical-prefixes \
+// RUN: %clangxx %s -### -no-canonical-prefixes -fuse-ld= \
 // RUN:   -target riscv64-unknown-elf -stdlib=libstdc++ --rtlib=platform \
 // RUN:   --sysroot= \
 // RUN:   --gcc-toolchain=%S/Inputs/basic_riscv64_tree 2>&1 \
@@ -75,7 +74,7 @@
 // CXX-RV64-BAREMETAL-NOSYSROOT-LP64: "-lstdc++" "--start-group" "-lc" "-lgloss" "--end-group" "-lgcc"
 // CXX-RV64-BAREMETAL-NOSYSROOT-LP64: "{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtend.o"
 
-// RUN: %clang %s -### -no-canonical-prefixes -fuse-ld=ld \
+// RUN: %clang %s -### -no-canonical-prefixes -fuse-ld=ld -fuse-ld= \
 // RUN:   -target riscv64-unknown-linux-gnu --rtlib=platform -mabi=lp64 \
 // RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_linux_sdk \
 // RUN:   --sysroot=%S/Inputs/multilib_riscv_linux_sdk/sysroot 2>&1 \
Index: 

[PATCH] D92221: Don't delete default constructor of PathDiagnosticConsumerOptions

2020-11-29 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision.
NoQ added a reviewer: vsavchenko.
NoQ added a comment.
This revision is now accepted and ready to land.

Aha, ok then! Thanks for cleaning this up.

Maybe we should provide default initializers for `bool` flags then, so that 
they weren't undefined by default.




Comment at: clang/include/clang/Analysis/PathDiagnostic.h:71
   /// without re-compiling the program under analysis.
   bool ShouldDisplayMacroExpansions;
 





Comment at: clang/include/clang/Analysis/PathDiagnostic.h:75
   /// Useful for profiling the tool on large real-world codebases.
   bool ShouldSerializeStats;
 





Comment at: clang/include/clang/Analysis/PathDiagnostic.h:85
   /// off by default.
   bool ShouldWriteStableReportFilename;
 





Comment at: clang/include/clang/Analysis/PathDiagnostic.h:89
   /// Useful for breaking your build when issues are found.
   bool ShouldDisplayWarningsAsErrors;
 





Comment at: clang/include/clang/Analysis/PathDiagnostic.h:93
   /// with fix-it hints attached to the diagnostics it consumes.
   bool ShouldApplyFixIts;
 





Comment at: clang/include/clang/Analysis/PathDiagnostic.h:97
   /// the diagnostic (eg., a checker) so that the user knew how to disable it.
   bool ShouldDisplayDiagnosticName;
 };




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92221

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


[PATCH] D92272: [clang-tidy] [clangd] Avoid multi-line diagnostic range for else-after-return diagnostic

2020-11-29 Thread Nathan Ridge via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf15b7869e5af: [clang-tidy] [clangd] Avoid multi-line 
diagnostic range for else-after-return… (authored by nridge).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92272

Files:
  clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -474,6 +474,24 @@
   EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre()); // no-crash
 }
 
+TEST(DiagnosticTest, ElseAfterReturnRange) {
+  Annotations Main(R"cpp(
+int foo(int cond) {
+if (cond == 1) {
+  return 42;
+} [[else]] if (cond == 2) {
+  return 43;
+}
+return 44;
+}
+  )cpp");
+  TestTU TU = TestTU::withCode(Main.code());
+  TU.ClangTidyProvider = addTidyChecks("llvm-else-after-return");
+  EXPECT_THAT(
+  TU.build().getDiagnostics(),
+  ElementsAre(Diag(Main.range(), "do not use 'else' after 'return'")));
+}
+
 TEST(DiagnosticsTest, Preprocessor) {
   // This looks like a preamble, but there's an #else in the middle!
   // Check that:
Index: clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
@@ -267,7 +267,8 @@
   // If the if statement is the last statement its enclosing statements
   // scope, we can pull the decl out of the if statement.
   DiagnosticBuilder Diag = diag(ElseLoc, WarningMessage)
-   << ControlFlowInterruptor;
+   << ControlFlowInterruptor
+   << SourceRange(ElseLoc);
   if (checkInitDeclUsageInElse(If) != nullptr) {
 Diag << tooling::fixit::createReplacement(
 SourceRange(If->getIfLoc()),
@@ -302,7 +303,8 @@
   // If the if statement is the last statement its enclosing statements
   // scope, we can pull the decl out of the if statement.
   DiagnosticBuilder Diag = diag(ElseLoc, WarningMessage)
-   << ControlFlowInterruptor;
+   << ControlFlowInterruptor
+   << SourceRange(ElseLoc);
   Diag << tooling::fixit::createReplacement(
   SourceRange(If->getIfLoc()),
   (tooling::fixit::getText(*If->getInit(), *Result.Context) +
@@ -319,7 +321,7 @@
   }
 
   DiagnosticBuilder Diag = diag(ElseLoc, WarningMessage)
-   << ControlFlowInterruptor;
+   << ControlFlowInterruptor << SourceRange(ElseLoc);
   removeElseAndBrackets(Diag, *Result.Context, Else, ElseLoc);
 }
 


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -474,6 +474,24 @@
   EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre()); // no-crash
 }
 
+TEST(DiagnosticTest, ElseAfterReturnRange) {
+  Annotations Main(R"cpp(
+int foo(int cond) {
+if (cond == 1) {
+  return 42;
+} [[else]] if (cond == 2) {
+  return 43;
+}
+return 44;
+}
+  )cpp");
+  TestTU TU = TestTU::withCode(Main.code());
+  TU.ClangTidyProvider = addTidyChecks("llvm-else-after-return");
+  EXPECT_THAT(
+  TU.build().getDiagnostics(),
+  ElementsAre(Diag(Main.range(), "do not use 'else' after 'return'")));
+}
+
 TEST(DiagnosticsTest, Preprocessor) {
   // This looks like a preamble, but there's an #else in the middle!
   // Check that:
Index: clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
@@ -267,7 +267,8 @@
   // If the if statement is the last statement its enclosing statements
   // scope, we can pull the decl out of the if statement.
   DiagnosticBuilder Diag = diag(ElseLoc, WarningMessage)
-   << ControlFlowInterruptor;
+   << ControlFlowInterruptor
+   << SourceRange(ElseLoc);
   if (checkInitDeclUsageInElse(If) != nullptr) {
 Diag << tooling::fixit::createReplacement(
 SourceRange(If->getIfLoc()),
@@ -302,7 +303,8 @@
   // If the if statement is the last 

[clang-tools-extra] f15b786 - [clang-tidy] [clangd] Avoid multi-line diagnostic range for else-after-return diagnostic

2020-11-29 Thread Nathan Ridge via cfe-commits

Author: Nathan Ridge
Date: 2020-11-29T18:32:23-05:00
New Revision: f15b7869e5afbd6c24ef440b0b62593e80fbd24f

URL: 
https://github.com/llvm/llvm-project/commit/f15b7869e5afbd6c24ef440b0b62593e80fbd24f
DIFF: 
https://github.com/llvm/llvm-project/commit/f15b7869e5afbd6c24ef440b0b62593e80fbd24f.diff

LOG: [clang-tidy] [clangd] Avoid multi-line diagnostic range for 
else-after-return diagnostic

Fixes https://bugs.llvm.org/show_bug.cgi?id=47809

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
index 79e3cded45bc..89bb02e78cc6 100644
--- a/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
@@ -267,7 +267,8 @@ void ElseAfterReturnCheck::check(const 
MatchFinder::MatchResult ) {
   // If the if statement is the last statement its enclosing statements
   // scope, we can pull the decl out of the if statement.
   DiagnosticBuilder Diag = diag(ElseLoc, WarningMessage)
-   << ControlFlowInterruptor;
+   << ControlFlowInterruptor
+   << SourceRange(ElseLoc);
   if (checkInitDeclUsageInElse(If) != nullptr) {
 Diag << tooling::fixit::createReplacement(
 SourceRange(If->getIfLoc()),
@@ -302,7 +303,8 @@ void ElseAfterReturnCheck::check(const 
MatchFinder::MatchResult ) {
   // If the if statement is the last statement its enclosing statements
   // scope, we can pull the decl out of the if statement.
   DiagnosticBuilder Diag = diag(ElseLoc, WarningMessage)
-   << ControlFlowInterruptor;
+   << ControlFlowInterruptor
+   << SourceRange(ElseLoc);
   Diag << tooling::fixit::createReplacement(
   SourceRange(If->getIfLoc()),
   (tooling::fixit::getText(*If->getInit(), *Result.Context) +
@@ -319,7 +321,7 @@ void ElseAfterReturnCheck::check(const 
MatchFinder::MatchResult ) {
   }
 
   DiagnosticBuilder Diag = diag(ElseLoc, WarningMessage)
-   << ControlFlowInterruptor;
+   << ControlFlowInterruptor << SourceRange(ElseLoc);
   removeElseAndBrackets(Diag, *Result.Context, Else, ElseLoc);
 }
 

diff  --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp 
b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
index ba7029e54dbb..c6a14aeeb469 100644
--- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -474,6 +474,24 @@ TEST(DiagnosticTest, ClangTidyNoLiteralDataInMacroToken) {
   EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre()); // no-crash
 }
 
+TEST(DiagnosticTest, ElseAfterReturnRange) {
+  Annotations Main(R"cpp(
+int foo(int cond) {
+if (cond == 1) {
+  return 42;
+} [[else]] if (cond == 2) {
+  return 43;
+}
+return 44;
+}
+  )cpp");
+  TestTU TU = TestTU::withCode(Main.code());
+  TU.ClangTidyProvider = addTidyChecks("llvm-else-after-return");
+  EXPECT_THAT(
+  TU.build().getDiagnostics(),
+  ElementsAre(Diag(Main.range(), "do not use 'else' after 'return'")));
+}
+
 TEST(DiagnosticsTest, Preprocessor) {
   // This looks like a preamble, but there's an #else in the middle!
   // Check that:



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


[PATCH] D92272: [clang-tidy] [clangd] Avoid multi-line diagnostic range for else-after-return diagnostic

2020-11-29 Thread Nathan James via Phabricator via cfe-commits
njames93 accepted this revision.
njames93 added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92272

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


[PATCH] D91025: [clangd] Fix locateMacroAt() for macro definition outside preamble

2020-11-29 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment.

Review ping :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91025

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


[PATCH] D92290: [clangd] Use clangd's Context mechanism to make the ASTContext of the AST being operated on available everywhere

2020-11-29 Thread Nathan Ridge via Phabricator via cfe-commits
nridge created this revision.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, 
javed.absar.
Herald added a project: clang.
nridge requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.

Use it to remove the NameFactory hack from 
getMembersReferencedViaDependentName().


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92290

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/ParsedAST.h
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/unittests/TestTU.cpp
  clang-tools-extra/clangd/unittests/TestTU.h

Index: clang-tools-extra/clangd/unittests/TestTU.h
===
--- clang-tools-extra/clangd/unittests/TestTU.h
+++ clang-tools-extra/clangd/unittests/TestTU.h
@@ -76,6 +76,10 @@
   // to eliminate this option some day.
   bool OverlayRealFileSystemForModules = false;
 
+  // This is used to add the ASTContext of the AST produced by build() to the
+  // clangd Context.
+  mutable std::unique_ptr ASTCtx;
+
   // By default, build() will report Error diagnostics as GTest errors.
   // Suppress this behavior by adding an 'error-ok' comment to the code.
   ParsedAST build() const;
Index: clang-tools-extra/clangd/unittests/TestTU.cpp
===
--- clang-tools-extra/clangd/unittests/TestTU.cpp
+++ clang-tools-extra/clangd/unittests/TestTU.cpp
@@ -139,6 +139,8 @@
 break; // Just report first error for simplicity.
   }
   }
+  ASTCtx = std::make_unique(ParsedAST::CurrentASTContext,
+  >getASTContext());
   return std::move(*AST);
 }
 
Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -721,6 +721,8 @@
   return Action(error(llvm::errc::invalid_argument, "invalid AST"));
 vlog("ASTWorker running {0} on version {2} of {1}", Name, FileName,
  FileInputs.Version);
+WithContextValue Ctx(ParsedAST::CurrentASTContext,
+ &(*AST)->getASTContext());
 Action(InputsAndAST{FileInputs, **AST});
   };
   startTask(Name, std::move(Task), /*UpdateType=*/None, Invalidation);
Index: clang-tools-extra/clangd/ParsedAST.h
===
--- clang-tools-extra/clangd/ParsedAST.h
+++ clang-tools-extra/clangd/ParsedAST.h
@@ -26,6 +26,7 @@
 #include "Headers.h"
 #include "Preamble.h"
 #include "index/CanonicalIncludes.h"
+#include "support/Context.h"
 #include "support/Path.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Frontend/PrecompiledPreamble.h"
@@ -109,6 +110,10 @@
   /// AST. Might be None if no Preamble is used.
   llvm::Optional preambleVersion() const;
 
+  /// This context variable makes available the ASTContext of the AST being
+  /// operated on currently.
+  static Key CurrentASTContext;
+
 private:
   ParsedAST(llvm::StringRef Version,
 std::shared_ptr Preamble,
Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -237,6 +237,8 @@
 
 } // namespace
 
+Key ParsedAST::CurrentASTContext;
+
 llvm::Optional
 ParsedAST::build(llvm::StringRef Filename, const ParseInputs ,
  std::unique_ptr CI,
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -8,6 +8,8 @@
 
 #include "FindTarget.h"
 #include "AST.h"
+#include "ParsedAST.h"
+#include "support/Context.h"
 #include "support/Logger.h"
 #include "clang/AST/ASTTypeTraits.h"
 #include "clang/AST/Decl.h"
@@ -98,21 +100,18 @@
 // name (e.g. if it's an operator name), but the caller may not have
 // access to an ASTContext.
 std::vector getMembersReferencedViaDependentName(
-const Type *T,
-llvm::function_ref NameFactory,
+const Type *T, DeclarationName Name,
 llvm::function_ref Filter) {
   if (!T)
 return {};
   if (auto *ET = T->getAs()) {
-auto Result =
-ET->getDecl()->lookup(NameFactory(ET->getDecl()->getASTContext()));
+auto Result = ET->getDecl()->lookup(Name);
 return {Result.begin(), Result.end()};
   }
   if (auto *RD = resolveTypeToRecordDecl(T)) {
 if (!RD->hasDefinition())
   return {};
 RD = RD->getDefinition();
-DeclarationName Name = NameFactory(RD->getASTContext());
 return RD->lookupDependentName(Name, Filter);
   }
   return {};
@@ -147,9 +146,9 @@
   // smart pointer type.
   auto ArrowOps = getMembersReferencedViaDependentName(
   T,
-  [](ASTContext ) {
-return 

[PATCH] D92272: [clang-tidy] [clangd] Avoid multi-line diagnostic range for else-after-return diagnostic

2020-11-29 Thread Nathan Ridge via Phabricator via cfe-commits
nridge updated this revision to Diff 308235.
nridge added a comment.

Rebase and address review comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92272

Files:
  clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -474,6 +474,24 @@
   EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre()); // no-crash
 }
 
+TEST(DiagnosticTest, ElseAfterReturnRange) {
+  Annotations Main(R"cpp(
+int foo(int cond) {
+if (cond == 1) {
+  return 42;
+} [[else]] if (cond == 2) {
+  return 43;
+}
+return 44;
+}
+  )cpp");
+  TestTU TU = TestTU::withCode(Main.code());
+  TU.ClangTidyProvider = addTidyChecks("llvm-else-after-return");
+  EXPECT_THAT(
+  TU.build().getDiagnostics(),
+  ElementsAre(Diag(Main.range(), "do not use 'else' after 'return'")));
+}
+
 TEST(DiagnosticsTest, Preprocessor) {
   // This looks like a preamble, but there's an #else in the middle!
   // Check that:
Index: clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
@@ -267,7 +267,8 @@
   // If the if statement is the last statement its enclosing statements
   // scope, we can pull the decl out of the if statement.
   DiagnosticBuilder Diag = diag(ElseLoc, WarningMessage)
-   << ControlFlowInterruptor;
+   << ControlFlowInterruptor
+   << SourceRange(ElseLoc);
   if (checkInitDeclUsageInElse(If) != nullptr) {
 Diag << tooling::fixit::createReplacement(
 SourceRange(If->getIfLoc()),
@@ -302,7 +303,8 @@
   // If the if statement is the last statement its enclosing statements
   // scope, we can pull the decl out of the if statement.
   DiagnosticBuilder Diag = diag(ElseLoc, WarningMessage)
-   << ControlFlowInterruptor;
+   << ControlFlowInterruptor
+   << SourceRange(ElseLoc);
   Diag << tooling::fixit::createReplacement(
   SourceRange(If->getIfLoc()),
   (tooling::fixit::getText(*If->getInit(), *Result.Context) +
@@ -319,7 +321,7 @@
   }
 
   DiagnosticBuilder Diag = diag(ElseLoc, WarningMessage)
-   << ControlFlowInterruptor;
+   << ControlFlowInterruptor << SourceRange(ElseLoc);
   removeElseAndBrackets(Diag, *Result.Context, Else, ElseLoc);
 }
 


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -474,6 +474,24 @@
   EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre()); // no-crash
 }
 
+TEST(DiagnosticTest, ElseAfterReturnRange) {
+  Annotations Main(R"cpp(
+int foo(int cond) {
+if (cond == 1) {
+  return 42;
+} [[else]] if (cond == 2) {
+  return 43;
+}
+return 44;
+}
+  )cpp");
+  TestTU TU = TestTU::withCode(Main.code());
+  TU.ClangTidyProvider = addTidyChecks("llvm-else-after-return");
+  EXPECT_THAT(
+  TU.build().getDiagnostics(),
+  ElementsAre(Diag(Main.range(), "do not use 'else' after 'return'")));
+}
+
 TEST(DiagnosticsTest, Preprocessor) {
   // This looks like a preamble, but there's an #else in the middle!
   // Check that:
Index: clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
@@ -267,7 +267,8 @@
   // If the if statement is the last statement its enclosing statements
   // scope, we can pull the decl out of the if statement.
   DiagnosticBuilder Diag = diag(ElseLoc, WarningMessage)
-   << ControlFlowInterruptor;
+   << ControlFlowInterruptor
+   << SourceRange(ElseLoc);
   if (checkInitDeclUsageInElse(If) != nullptr) {
 Diag << tooling::fixit::createReplacement(
 SourceRange(If->getIfLoc()),
@@ -302,7 +303,8 @@
   // If the if statement is the last statement its enclosing statements
   // scope, we can pull the decl out of the if statement.
   

[PATCH] D89869: [OpenCL] Define OpenCL feature macros for all versions

2020-11-29 Thread Anton Zabaznov via Phabricator via cfe-commits
azabaznov added a comment.

> I didn't want to end up with a big refactoring from this patch but now I 
> start to think that considering the situation it might be unavoidable. :(

Well, it was expected. However, I didn't want to mess this refactoring with 
OpenCL C 3.0 support at first.

I already do have some progress in this direction: 
https://reviews.llvm.org/D92277. We can differentiate between core and optional 
features by the defining them appropriately in `OpenCLExtensions.def`. Note 
that this also removes duplication of `OpenCLOptions` so we can query all 
features/extensions from `TargetInfo` directly. This will also allow 
simplifying `OpenCLOptions` in future


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

https://reviews.llvm.org/D89869

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


[PATCH] D91789: [clang-tidy] find/fix unneeded trailing semicolons in macros

2020-11-29 Thread Tom Rix via Phabricator via cfe-commits
trixirt updated this revision to Diff 308214.
trixirt edited the summary of this revision.
trixirt added a comment.

Refactor to combine switch and trailing macro into one fixer


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91789

Files:
  clang-tools-extra/clang-tidy/linuxkernel/CMakeLists.txt
  clang-tools-extra/clang-tidy/linuxkernel/ExtraSemiCheck.cpp
  clang-tools-extra/clang-tidy/linuxkernel/ExtraSemiCheck.h
  clang-tools-extra/clang-tidy/linuxkernel/LinuxKernelTidyModule.cpp
  clang-tools-extra/test/clang-tidy/checkers/linuxkernel-macro-trailing-semi.c
  clang-tools-extra/test/clang-tidy/checkers/linuxkernel-switch-semi.c

Index: clang-tools-extra/test/clang-tidy/checkers/linuxkernel-switch-semi.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/linuxkernel-switch-semi.c
@@ -0,0 +1,41 @@
+// RUN: %check_clang_tidy %s linuxkernel-extra-semi %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: linuxkernel-extra-semi.Fixer, \
+// RUN:   value: 'Switch'}, \
+// RUN: ]}"
+
+int f(int a) {
+  switch (a) {
+  case 1:
+return 0;
+  default:
+break;
+  };
+  // CHECK-MESSAGES: :[[@LINE-1]]:4: warning: unneeded semicolon
+  // CHECK-FIXES:   }{{$}}
+  return 0;
+}
+
+// A normal switch, should not produce a warning
+int p1(int a) {
+  switch (a) {
+  case 1:
+return 0;
+  default:
+break;
+  }
+  return 0;
+}
+
+#define EMPTY_MACRO()
+// A corner case, do not fix an empty macro
+int p2(int a) {
+  switch (a) {
+  case 1:
+return 0;
+  default:
+break;
+  }
+  EMPTY_MACRO();
+  return 0;
+}
Index: clang-tools-extra/test/clang-tidy/checkers/linuxkernel-macro-trailing-semi.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/linuxkernel-macro-trailing-semi.c
@@ -0,0 +1,32 @@
+// RUN: %check_clang_tidy %s linuxkernel-extra-semi %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: linuxkernel-extra-semi.Fixer, \
+// RUN:   value: 'TrailingMacro'}, \
+// RUN: ]}"
+
+#define M(a) a++;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: unneeded semicolon
+// CHECK-FIXES: #define M(a) a++{{$}}
+
+int f() {
+  int v = 0;
+  M(v);
+  return v;
+}
+
+// A corner case
+// An empty macro could conditionally contain another path so
+// do not warn or fix.
+#ifdef UNSET_CONDITION
+#define E() ;
+#else
+#define E()
+#endif
+#define N(a) a++;
+
+int g() {
+  int v = 0;
+  N(v)
+  E();
+  return v;
+}
Index: clang-tools-extra/clang-tidy/linuxkernel/LinuxKernelTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/linuxkernel/LinuxKernelTidyModule.cpp
+++ clang-tools-extra/clang-tidy/linuxkernel/LinuxKernelTidyModule.cpp
@@ -9,6 +9,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "ExtraSemiCheck.h"
 #include "MustCheckErrsCheck.h"
 
 namespace clang {
@@ -19,6 +20,7 @@
 class LinuxKernelModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories ) override {
+CheckFactories.registerCheck("linuxkernel-extra-semi");
 CheckFactories.registerCheck(
 "linuxkernel-must-check-errs");
   }
Index: clang-tools-extra/clang-tidy/linuxkernel/ExtraSemiCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/linuxkernel/ExtraSemiCheck.h
@@ -0,0 +1,43 @@
+//===--- ExtraSemiCheck.h - clang-tidy --*- C++ -*-===//
+//
+// 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
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LINUXKERNEL_EXTRASEMICHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LINUXKERNEL_EXTRASEMICHECK_H
+
+#include "../ClangTidyCheck.h"
+#include "clang/Lex/MacroInfo.h"
+#include 
+
+namespace clang {
+namespace tidy {
+namespace linuxkernel {
+
+enum ExtraSemiFixerKind { ESFK_None, ESFK_Switch, ESFK_TrailingMacro };
+class ExtraSemiCheck : public ClangTidyCheck {
+public:
+  ExtraSemiCheck(StringRef Name, ClangTidyContext *Context);
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void registerPPCallbacks(const SourceManager , Preprocessor *PP,
+   Preprocessor *ModuleExpanderPP) override final;
+
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+  void checkMacro(SourceManager , const Token ,
+  const MacroInfo *MI);
+  void storeOptions(ClangTidyOptions::OptionMap ) override;
+
+private:
+  std::vector SuspectMacros;
+  enum ExtraSemiFixerKind FixerKind;
+  

[PATCH] D71124: [RISCV] support clang driver to select cpu

2020-11-29 Thread Kuan Hsu Chen (Zakk) via Phabricator via cfe-commits
khchen added a comment.

> How does the CLI options correspond with the back-end capabilities?

`clang -print-supported-cpus`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71124

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


[PATCH] D92269: [TableGen] Eliminate the 'code' type

2020-11-29 Thread Paul C. Anagnostopoulos via Phabricator via cfe-commits
Paul-C-Anagnostopoulos added a comment.

The TypeOf_xxx field is only used by the searchable tables backend. My trek 
through all the backends revealed it as the only one that needs to distinguish 
string and code, because it can emit the C++ initializer for any type of field. 
All the other backends know what is in each field they process. You can see 
from the various patches that there is nothing special about code fields in the 
other backends.

I will fix the lint issues. I will let this stew here for a few days in case 
there are other comments.

(A new !replace operator, along with !lookup, are on my to-do list.)




Comment at: llvm/test/TableGen/generic-tables.td:138
+  string TypeOf_Kind = "CEnum";
+  GenericEnum TypeOf_Kind = CEnum;
 

I will get rid of this line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92269

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


[PATCH] D92269: [TableGen] Eliminate the 'code' type

2020-11-29 Thread Chris Lattner via Phabricator via cfe-commits
lattner added a comment.

Oh ok, well I have no problem keeping it around in searchable tables, I misread 
this as making that more general.

jrtc27, we had a long conversation about this.  We'd like to eliminate the Code 
type as a way to simplify the internals of tblgen.  It is a distinction without 
a useful difference for many things, and this is one step towards making tblgen 
simpler and more consistent.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92269

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


[PATCH] D92269: [TableGen] Eliminate the 'code' type

2020-11-29 Thread Jessica Clarke via Phabricator via cfe-commits
jrtc27 added a comment.

In D92269#2421399 , 
@Paul-C-Anagnostopoulos wrote:

> Yes, we could create !codeconcat instead, along with !codeinterleave, 
> !codeeq, etc. We could also just extend the existing bang operators to work 
> on the code type. I thought it was cleaner just to get rid of the distinction.

You wouldn't need a new !eq, you can just add another overloading. Really there 
should probably be an overloaded !concat that works on string, bits, list and 
code; !strconcat is the unusual case where it's qualified with the type in the 
name.

> I agree that !subst is a mess, even without this change. My plan is 
> eventually to implement a saner !replace. But I'm not sure why doing string 
> replacement in code is sad. There is even a utility to do it: 
> GlobalISel\CodeExpander.

I was confused and thought !subst was substring not substitute, which is 
normally a meaningless operation to perform on code. Substitution is less 
weird, thought normally the useful/meaningful representation for code is 
something like a list of tokens which would make !subst a bit more useful as 
currently you just get raw string replacement which risks all manner of false 
positives for matches.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92269

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


[PATCH] D92269: [TableGen] Eliminate the 'code' type

2020-11-29 Thread Paul C. Anagnostopoulos via Phabricator via cfe-commits
Paul-C-Anagnostopoulos added a comment.

Yes, we could create !codeconcat instead, along with !codeinterleave, !codeeq, 
etc. We could also just extend the existing bang operators to work on the code 
type. I thought it was cleaner just to get rid of the distinction.

I agree that !subst is a mess, even without this change. My plan is eventually 
to implement a saner !replace. But I'm not sure why doing string replacement in 
code is sad.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92269

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


[PATCH] D92269: [TableGen] Eliminate the 'code' type

2020-11-29 Thread Paul C. Anagnostopoulos via Phabricator via cfe-commits
Paul-C-Anagnostopoulos added a comment.

The TypeOf_xxx feature of searchable tables already exists. I just co-opted it 
as the way to tell the searchable table backend to emit the code without 
quotes. If we go with this revision, we will always need it in certain cases, 
when the code is constructed as a string and the backend cannot tell that it is 
a string.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92269

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


[PATCH] D92269: [TableGen] Eliminate the 'code' type

2020-11-29 Thread Jessica Clarke via Phabricator via cfe-commits
jrtc27 added a comment.

Losing types and permitting nonsensical operators like `!subst` on code seems a 
bit sad. Why not just define a `!codeconcat`, like how we have a separate 
`!listconcat`? I agree that the TypeOf_xxx stuff is really ugly and so having 
_greater_ expressivity like `!codeconcat` would go a long way to help that. 
Also, `cast(str)` would be the natural way to express the cases where you 
still really do want to construct code as a string without needing TypeOf_xxx.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92269

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


[PATCH] D92269: [TableGen] Eliminate the 'code' type

2020-11-29 Thread Chris Lattner via Phabricator via cfe-commits
lattner added a comment.

Ah, I thought TypeOf_xxx was a transitionary thing that we'd drop over time (to 
reduce complexity).  If that's the case, I'd recommend not documenting it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92269

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


[PATCH] D91495: [clang-tidy] false-positive for bugprone-redundant-branch-condition in case of passed-by-ref params

2020-11-29 Thread Zinovy Nis via Phabricator via cfe-commits
zinovy.nis added a comment.

What do you think of it?


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

https://reviews.llvm.org/D91495

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


[clang-tools-extra] d99da80 - [clangd] Fix path edge-case condition.

2020-11-29 Thread Sam McCall via cfe-commits

Author: Sam McCall
Date: 2020-11-29T13:40:29+01:00
New Revision: d99da80841cb4d9734db4a48cd49e37b623176bc

URL: 
https://github.com/llvm/llvm-project/commit/d99da80841cb4d9734db4a48cd49e37b623176bc
DIFF: 
https://github.com/llvm/llvm-project/commit/d99da80841cb4d9734db4a48cd49e37b623176bc.diff

LOG: [clangd] Fix path edge-case condition.

Added: 


Modified: 
clang-tools-extra/clangd/ConfigProvider.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/ConfigProvider.cpp 
b/clang-tools-extra/clangd/ConfigProvider.cpp
index e020ec04d1d2..8a3991ed1c1e 100644
--- a/clang-tools-extra/clangd/ConfigProvider.cpp
+++ b/clang-tools-extra/clangd/ConfigProvider.cpp
@@ -103,7 +103,7 @@ Provider::fromAncestorRelativeYAMLFiles(llvm::StringRef 
RelPath,
I != E; ++I) {
 // Avoid weird non-substring cases like phantom "." components.
 // In practice, Component is a substring for all "normal" ancestors.
-if (I->end() < Parent.begin() && I->end() > Parent.end())
+if (I->end() < Parent.begin() || I->end() > Parent.end())
   continue;
 Ancestors.emplace_back(Parent.begin(), I->end() - Parent.begin());
   }



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


[PATCH] D92133: [clangd] Cache .clang-tidy files again.

2020-11-29 Thread Sam McCall via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
sammccall marked an inline comment as done.
Closed by commit rG67d16b6da4be: [clangd] Cache .clang-tidy files again. 
(authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D92133?vs=308004=308207#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92133

Files:
  clang-tools-extra/clangd/TidyProvider.cpp

Index: clang-tools-extra/clangd/TidyProvider.cpp
===
--- clang-tools-extra/clangd/TidyProvider.cpp
+++ clang-tools-extra/clangd/TidyProvider.cpp
@@ -8,7 +8,9 @@
 
 #include "TidyProvider.h"
 #include "Config.h"
+#include "support/FileCache.h"
 #include "support/Logger.h"
+#include "support/ThreadsafeFS.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
@@ -19,53 +21,115 @@
 
 namespace clang {
 namespace clangd {
+namespace {
 
-static void mergeCheckList(llvm::Optional ,
-   llvm::StringRef List) {
-  if (List.empty())
-return;
-  if (!Checks || Checks->empty()) {
-Checks.emplace(List);
-return;
+// Access to config from a .clang-tidy file, caching IO and parsing.
+class DotClangTidyCache : private FileCache {
+  // We cache and expose shared_ptr to avoid copying the value on every lookup
+  // when we're ultimately just going to pass it to mergeWith.
+  mutable std::shared_ptr Value;
+
+public:
+  DotClangTidyCache(PathRef Path) : FileCache(Path) {}
+
+  std::shared_ptr
+  get(const ThreadsafeFS ,
+  std::chrono::steady_clock::time_point FreshTime) const {
+std::shared_ptr Result;
+read(
+TFS, FreshTime,
+[this](llvm::Optional Data) {
+  Value.reset();
+  if (Data && !Data->empty()) {
+if (auto Parsed = tidy::parseConfiguration(*Data))
+  Value = std::make_shared(
+  std::move(*Parsed));
+else
+  elog("Error parsing clang-tidy configuration in {0}: {1}", path(),
+   Parsed.getError().message());
+  }
+},
+[&]() { Result = Value; });
+return Result;
   }
-  *Checks = llvm::join_items(",", *Checks, List);
-}
+};
 
-static llvm::Optional
-tryReadConfigFile(llvm::vfs::FileSystem *FS, llvm::StringRef Directory) {
-  assert(!Directory.empty());
-  // We guaranteed that child directories of Directory exist, so this assert
-  // should hopefully never fail.
-  assert(FS->exists(Directory));
+// Access to combined config from .clang-tidy files governing a source file.
+// Each config file is cached and the caches are shared for affected sources.
+//
+// FIXME: largely duplicates config::Provider::fromAncestorRelativeYAMLFiles.
+// Potentially useful for compile_commands.json too. Extract?
+class DotClangTidyTree {
+  const ThreadsafeFS 
+  std::string RelPath;
+  std::chrono::steady_clock::duration MaxStaleness;
 
-  llvm::SmallString<128> ConfigFile(Directory);
-  llvm::sys::path::append(ConfigFile, ".clang-tidy");
+  mutable std::mutex Mu;
+  // Keys are the ancestor directory, not the actual config path within it.
+  // We only insert into this map, so pointers to values are stable forever.
+  // Mutex guards the map itself, not the values (which are threadsafe).
+  mutable llvm::StringMap Cache;
 
-  llvm::ErrorOr FileStatus = FS->status(ConfigFile);
+public:
+  DotClangTidyTree(const ThreadsafeFS )
+  : FS(FS), RelPath(".clang-tidy"), MaxStaleness(std::chrono::seconds(5)) {}
 
-  if (!FileStatus || !FileStatus->isRegularFile())
-return llvm::None;
+  void apply(tidy::ClangTidyOptions , PathRef AbsPath) {
+namespace path = llvm::sys::path;
+assert(path::is_absolute(AbsPath));
+
+// Compute absolute paths to all ancestors (substrings of P.Path).
+// Ensure cache entries for each ancestor exist in the map.
+llvm::StringRef Parent = path::parent_path(AbsPath);
+llvm::SmallVector Caches;
+{
+  std::lock_guard Lock(Mu);
+  for (auto I = path::begin(Parent, path::Style::posix),
+E = path::end(Parent);
+   I != E; ++I) {
+assert(I->end() >= Parent.begin() && I->end() <= Parent.end() &&
+   "Canonical path components should be substrings");
+llvm::StringRef Ancestor(Parent.begin(), I->end() - Parent.begin());
+auto It = Cache.find(Ancestor);
 
-  llvm::ErrorOr> Text =
-  FS->getBufferForFile(ConfigFile);
-  if (std::error_code EC = Text.getError()) {
-elog("Can't read '{0}': {1}", ConfigFile, EC.message());
-return llvm::None;
+// Assemble the actual config file path only if needed.
+if (It == Cache.end()) {
+  llvm::SmallString<256> ConfigPath = Ancestor;
+  path::append(ConfigPath, RelPath);
+  It = 

[clang-tools-extra] 67d16b6 - [clangd] Cache .clang-tidy files again.

2020-11-29 Thread Sam McCall via cfe-commits

Author: Sam McCall
Date: 2020-11-29T13:28:53+01:00
New Revision: 67d16b6da4bef1ee174514148854e77151a62605

URL: 
https://github.com/llvm/llvm-project/commit/67d16b6da4bef1ee174514148854e77151a62605
DIFF: 
https://github.com/llvm/llvm-project/commit/67d16b6da4bef1ee174514148854e77151a62605.diff

LOG: [clangd] Cache .clang-tidy files again.

This cache went away in 73fdd998701cce3aa6c4d8d2a73ab97351a0313b

This time, the cache is periodically validated against disk, so config
is still mostly "live".

The per-file cache reuses FileCache, but the tree-of-file-caches is
duplicated from ConfigProvider. .clangd, .clang-tidy, .clang-format, and
compile_commands.json all have this pattern, we should extract it at some point.
TODO for now though.

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

Added: 


Modified: 
clang-tools-extra/clangd/TidyProvider.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/TidyProvider.cpp 
b/clang-tools-extra/clangd/TidyProvider.cpp
index 1fb79f3abbce3..687ae6501d25f 100644
--- a/clang-tools-extra/clangd/TidyProvider.cpp
+++ b/clang-tools-extra/clangd/TidyProvider.cpp
@@ -8,7 +8,9 @@
 
 #include "TidyProvider.h"
 #include "Config.h"
+#include "support/FileCache.h"
 #include "support/Logger.h"
+#include "support/ThreadsafeFS.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
@@ -19,53 +21,115 @@
 
 namespace clang {
 namespace clangd {
+namespace {
 
-static void mergeCheckList(llvm::Optional ,
-   llvm::StringRef List) {
-  if (List.empty())
-return;
-  if (!Checks || Checks->empty()) {
-Checks.emplace(List);
-return;
+// Access to config from a .clang-tidy file, caching IO and parsing.
+class DotClangTidyCache : private FileCache {
+  // We cache and expose shared_ptr to avoid copying the value on every lookup
+  // when we're ultimately just going to pass it to mergeWith.
+  mutable std::shared_ptr Value;
+
+public:
+  DotClangTidyCache(PathRef Path) : FileCache(Path) {}
+
+  std::shared_ptr
+  get(const ThreadsafeFS ,
+  std::chrono::steady_clock::time_point FreshTime) const {
+std::shared_ptr Result;
+read(
+TFS, FreshTime,
+[this](llvm::Optional Data) {
+  Value.reset();
+  if (Data && !Data->empty()) {
+if (auto Parsed = tidy::parseConfiguration(*Data))
+  Value = std::make_shared(
+  std::move(*Parsed));
+else
+  elog("Error parsing clang-tidy configuration in {0}: {1}", 
path(),
+   Parsed.getError().message());
+  }
+},
+[&]() { Result = Value; });
+return Result;
   }
-  *Checks = llvm::join_items(",", *Checks, List);
-}
+};
 
-static llvm::Optional
-tryReadConfigFile(llvm::vfs::FileSystem *FS, llvm::StringRef Directory) {
-  assert(!Directory.empty());
-  // We guaranteed that child directories of Directory exist, so this assert
-  // should hopefully never fail.
-  assert(FS->exists(Directory));
+// Access to combined config from .clang-tidy files governing a source file.
+// Each config file is cached and the caches are shared for affected sources.
+//
+// FIXME: largely duplicates config::Provider::fromAncestorRelativeYAMLFiles.
+// Potentially useful for compile_commands.json too. Extract?
+class DotClangTidyTree {
+  const ThreadsafeFS 
+  std::string RelPath;
+  std::chrono::steady_clock::duration MaxStaleness;
 
-  llvm::SmallString<128> ConfigFile(Directory);
-  llvm::sys::path::append(ConfigFile, ".clang-tidy");
+  mutable std::mutex Mu;
+  // Keys are the ancestor directory, not the actual config path within it.
+  // We only insert into this map, so pointers to values are stable forever.
+  // Mutex guards the map itself, not the values (which are threadsafe).
+  mutable llvm::StringMap Cache;
 
-  llvm::ErrorOr FileStatus = FS->status(ConfigFile);
+public:
+  DotClangTidyTree(const ThreadsafeFS )
+  : FS(FS), RelPath(".clang-tidy"), MaxStaleness(std::chrono::seconds(5)) 
{}
 
-  if (!FileStatus || !FileStatus->isRegularFile())
-return llvm::None;
+  void apply(tidy::ClangTidyOptions , PathRef AbsPath) {
+namespace path = llvm::sys::path;
+assert(path::is_absolute(AbsPath));
+
+// Compute absolute paths to all ancestors (substrings of P.Path).
+// Ensure cache entries for each ancestor exist in the map.
+llvm::StringRef Parent = path::parent_path(AbsPath);
+llvm::SmallVector Caches;
+{
+  std::lock_guard Lock(Mu);
+  for (auto I = path::begin(Parent, path::Style::posix),
+E = path::end(Parent);
+   I != E; ++I) {
+assert(I->end() >= Parent.begin() && I->end() <= Parent.end() &&
+   "Canonical path components should be substrings");
+llvm::StringRef Ancestor(Parent.begin(), I->end() - Parent.begin());
+auto It = 

[PATCH] D92270: [ConstantFold] Fold more operations to poison

2020-11-29 Thread Juneyoung Lee via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG53040a968dc2: [ConstantFold] Fold more operations to poison 
(authored by aqjune).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92270

Files:
  clang/test/Frontend/fixed_point_unary.c
  llvm/lib/IR/ConstantFold.cpp
  llvm/test/CodeGen/AMDGPU/amdgpu-codegenprepare-fold-binop-select.ll
  llvm/test/Transforms/InstCombine/apint-shift.ll
  llvm/test/Transforms/InstCombine/canonicalize-ashr-shl-to-masking.ll
  llvm/test/Transforms/InstCombine/canonicalize-lshr-shl-to-masking.ll
  llvm/test/Transforms/InstCombine/canonicalize-shl-lshr-to-masking.ll
  llvm/test/Transforms/InstCombine/icmp.ll
  
llvm/test/Transforms/InstCombine/partally-redundant-left-shift-input-masking-after-truncation-variant-a.ll
  
llvm/test/Transforms/InstCombine/partally-redundant-left-shift-input-masking-after-truncation-variant-b.ll
  
llvm/test/Transforms/InstCombine/partally-redundant-left-shift-input-masking-after-truncation-variant-c.ll
  
llvm/test/Transforms/InstCombine/partally-redundant-left-shift-input-masking-after-truncation-variant-d.ll
  
llvm/test/Transforms/InstCombine/partally-redundant-left-shift-input-masking-after-truncation-variant-e.ll
  
llvm/test/Transforms/InstCombine/partally-redundant-left-shift-input-masking-variant-a.ll
  
llvm/test/Transforms/InstCombine/partally-redundant-left-shift-input-masking-variant-b.ll
  
llvm/test/Transforms/InstCombine/partally-redundant-left-shift-input-masking-variant-c.ll
  
llvm/test/Transforms/InstCombine/partally-redundant-left-shift-input-masking-variant-d.ll
  
llvm/test/Transforms/InstCombine/partally-redundant-left-shift-input-masking-variant-e.ll
  llvm/test/Transforms/InstCombine/select-of-bittest.ll
  llvm/test/Transforms/InstCombine/shift-add.ll
  llvm/test/Transforms/InstSimplify/ConstProp/InsertElement.ll
  llvm/test/Transforms/InstSimplify/ConstProp/cast.ll
  llvm/test/Transforms/InstSimplify/ConstProp/poison.ll
  llvm/test/Transforms/InstSimplify/ConstProp/shift.ll
  llvm/test/Transforms/InstSimplify/ConstProp/vector-undef-elts.ll
  llvm/test/Transforms/InstSimplify/ConstProp/vscale.ll
  llvm/test/Transforms/InstSimplify/div.ll
  llvm/test/Transforms/InstSimplify/rem.ll
  llvm/test/Transforms/InstSimplify/undef.ll
  llvm/test/Transforms/SROA/phi-gep.ll
  llvm/test/Transforms/SROA/select-gep.ll
  llvm/test/Transforms/VectorCombine/X86/insert-binop-with-constant.ll
  llvm/test/Transforms/VectorCombine/X86/insert-binop.ll
  llvm/unittests/IR/ConstantsTest.cpp

Index: llvm/unittests/IR/ConstantsTest.cpp
===
--- llvm/unittests/IR/ConstantsTest.cpp
+++ llvm/unittests/IR/ConstantsTest.cpp
@@ -27,7 +27,7 @@
   Constant* Zero = ConstantInt::get(Int1, 0);
   Constant* NegOne = ConstantInt::get(Int1, static_cast(-1), true);
   EXPECT_EQ(NegOne, ConstantInt::getSigned(Int1, -1));
-  Constant* Undef = UndefValue::get(Int1);
+  Constant* Poison = PoisonValue::get(Int1);
 
   // Input:  @b = constant i1 add(i1 1 , i1 1)
   // Output: @b = constant i1 false
@@ -53,21 +53,21 @@
   // @g = constant i1 false
   EXPECT_EQ(Zero, ConstantExpr::getSub(One, One));
 
-  // @h = constant i1 shl(i1 1 , i1 1)  ; undefined
-  // @h = constant i1 undef
-  EXPECT_EQ(Undef, ConstantExpr::getShl(One, One));
+  // @h = constant i1 shl(i1 1 , i1 1)  ; poison
+  // @h = constant i1 poison
+  EXPECT_EQ(Poison, ConstantExpr::getShl(One, One));
 
   // @i = constant i1 shl(i1 1 , i1 0)
   // @i = constant i1 true
   EXPECT_EQ(One, ConstantExpr::getShl(One, Zero));
 
-  // @j = constant i1 lshr(i1 1, i1 1)  ; undefined
-  // @j = constant i1 undef
-  EXPECT_EQ(Undef, ConstantExpr::getLShr(One, One));
+  // @j = constant i1 lshr(i1 1, i1 1)  ; poison
+  // @j = constant i1 poison
+  EXPECT_EQ(Poison, ConstantExpr::getLShr(One, One));
 
-  // @m = constant i1 ashr(i1 1, i1 1)  ; undefined
-  // @m = constant i1 undef
-  EXPECT_EQ(Undef, ConstantExpr::getAShr(One, One));
+  // @m = constant i1 ashr(i1 1, i1 1)  ; poison
+  // @m = constant i1 poison
+  EXPECT_EQ(Poison, ConstantExpr::getAShr(One, One));
 
   // @n = constant i1 mul(i1 -1, i1 1)
   // @n = constant i1 true
@@ -218,7 +218,6 @@
   Constant *Elt = ConstantInt::get(Int16Ty, 2015);
   Constant *Poison16 = PoisonValue::get(Int16Ty);
   Constant *Undef64  = UndefValue::get(Int64Ty);
-  Constant *UndefV16 = UndefValue::get(P6->getType());
   Constant *PoisonV16 = PoisonValue::get(P6->getType());
 
   #define P0STR "ptrtoint (i32** @dummy to i32)"
@@ -295,8 +294,8 @@
 
   EXPECT_EQ(Elt, ConstantExpr::getExtractElement(
  ConstantExpr::getInsertElement(P6, Elt, One), One));
-  EXPECT_EQ(UndefV16, ConstantExpr::getInsertElement(P6, Elt, Two));
-  EXPECT_EQ(UndefV16, ConstantExpr::getInsertElement(P6, Elt, Big));
+  EXPECT_EQ(PoisonV16, 

[clang] 53040a9 - [ConstantFold] Fold more operations to poison

2020-11-29 Thread Juneyoung Lee via cfe-commits

Author: Juneyoung Lee
Date: 2020-11-29T21:19:48+09:00
New Revision: 53040a968dc2ff20931661e55f05da2ef8b964a0

URL: 
https://github.com/llvm/llvm-project/commit/53040a968dc2ff20931661e55f05da2ef8b964a0
DIFF: 
https://github.com/llvm/llvm-project/commit/53040a968dc2ff20931661e55f05da2ef8b964a0.diff

LOG: [ConstantFold] Fold more operations to poison

This patch folds more operations to poison.

Alive2 proof: https://alive2.llvm.org/ce/z/mxcb9G (it does not contain tests 
about div/rem because they fold to poison when raising UB)

Reviewed By: nikic

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

Added: 


Modified: 
clang/test/Frontend/fixed_point_unary.c
llvm/lib/IR/ConstantFold.cpp
llvm/test/CodeGen/AMDGPU/amdgpu-codegenprepare-fold-binop-select.ll
llvm/test/Transforms/InstCombine/apint-shift.ll
llvm/test/Transforms/InstCombine/canonicalize-ashr-shl-to-masking.ll
llvm/test/Transforms/InstCombine/canonicalize-lshr-shl-to-masking.ll
llvm/test/Transforms/InstCombine/canonicalize-shl-lshr-to-masking.ll
llvm/test/Transforms/InstCombine/icmp.ll

llvm/test/Transforms/InstCombine/partally-redundant-left-shift-input-masking-after-truncation-variant-a.ll

llvm/test/Transforms/InstCombine/partally-redundant-left-shift-input-masking-after-truncation-variant-b.ll

llvm/test/Transforms/InstCombine/partally-redundant-left-shift-input-masking-after-truncation-variant-c.ll

llvm/test/Transforms/InstCombine/partally-redundant-left-shift-input-masking-after-truncation-variant-d.ll

llvm/test/Transforms/InstCombine/partally-redundant-left-shift-input-masking-after-truncation-variant-e.ll

llvm/test/Transforms/InstCombine/partally-redundant-left-shift-input-masking-variant-a.ll

llvm/test/Transforms/InstCombine/partally-redundant-left-shift-input-masking-variant-b.ll

llvm/test/Transforms/InstCombine/partally-redundant-left-shift-input-masking-variant-c.ll

llvm/test/Transforms/InstCombine/partally-redundant-left-shift-input-masking-variant-d.ll

llvm/test/Transforms/InstCombine/partally-redundant-left-shift-input-masking-variant-e.ll
llvm/test/Transforms/InstCombine/select-of-bittest.ll
llvm/test/Transforms/InstCombine/shift-add.ll
llvm/test/Transforms/InstSimplify/ConstProp/InsertElement.ll
llvm/test/Transforms/InstSimplify/ConstProp/cast.ll
llvm/test/Transforms/InstSimplify/ConstProp/poison.ll
llvm/test/Transforms/InstSimplify/ConstProp/shift.ll
llvm/test/Transforms/InstSimplify/ConstProp/vector-undef-elts.ll
llvm/test/Transforms/InstSimplify/ConstProp/vscale.ll
llvm/test/Transforms/InstSimplify/div.ll
llvm/test/Transforms/InstSimplify/rem.ll
llvm/test/Transforms/InstSimplify/undef.ll
llvm/test/Transforms/SROA/phi-gep.ll
llvm/test/Transforms/SROA/select-gep.ll
llvm/test/Transforms/VectorCombine/X86/insert-binop-with-constant.ll
llvm/test/Transforms/VectorCombine/X86/insert-binop.ll
llvm/unittests/IR/ConstantsTest.cpp

Removed: 




diff  --git a/clang/test/Frontend/fixed_point_unary.c 
b/clang/test/Frontend/fixed_point_unary.c
index 849e38a94bc4..6ce760daba11 100644
--- a/clang/test/Frontend/fixed_point_unary.c
+++ b/clang/test/Frontend/fixed_point_unary.c
@@ -90,7 +90,7 @@ void inc_usa() {
 // SIGNED-LABEL: @inc_uf(
 // SIGNED-NEXT:  entry:
 // SIGNED-NEXT:[[TMP0:%.*]] = load i16, i16* @uf, align 2
-// SIGNED-NEXT:[[TMP1:%.*]] = add i16 [[TMP0]], undef
+// SIGNED-NEXT:[[TMP1:%.*]] = add i16 [[TMP0]], poison
 // SIGNED-NEXT:store i16 [[TMP1]], i16* @uf, align 2
 // SIGNED-NEXT:ret void
 //
@@ -271,7 +271,7 @@ void dec_usa() {
 // SIGNED-LABEL: @dec_uf(
 // SIGNED-NEXT:  entry:
 // SIGNED-NEXT:[[TMP0:%.*]] = load i16, i16* @uf, align 2
-// SIGNED-NEXT:[[TMP1:%.*]] = sub i16 [[TMP0]], undef
+// SIGNED-NEXT:[[TMP1:%.*]] = sub i16 [[TMP0]], poison
 // SIGNED-NEXT:store i16 [[TMP1]], i16* @uf, align 2
 // SIGNED-NEXT:ret void
 //

diff  --git a/llvm/lib/IR/ConstantFold.cpp b/llvm/lib/IR/ConstantFold.cpp
index a52cd725d530..3243ddd604ee 100644
--- a/llvm/lib/IR/ConstantFold.cpp
+++ b/llvm/lib/IR/ConstantFold.cpp
@@ -630,7 +630,7 @@ Constant *llvm::ConstantFoldCastInstruction(unsigned opc, 
Constant *V,
   V.convertToInteger(IntVal, APFloat::rmTowardZero, )) {
 // Undefined behavior invoked - the destination type can't represent
 // the input constant.
-return UndefValue::get(DestTy);
+return PoisonValue::get(DestTy);
   }
   return ConstantInt::get(FPC->getContext(), IntVal);
 }
@@ -916,7 +916,7 @@ Constant 
*llvm::ConstantFoldInsertElementInstruction(Constant *Val,
 
   unsigned NumElts = ValTy->getNumElements();
   if (CIdx->uge(NumElts))
-return UndefValue::get(Val->getType());
+return PoisonValue::get(Val->getType());
 
   SmallVector Result;
   Result.reserve(NumElts);
@@ -1144,23 +1144,21 @@ 

[PATCH] D92272: [clang-tidy] [clangd] Avoid multi-line diagnostic range for else-after-return diagnostic

2020-11-29 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment.

This small fix is definitely worth while, the more involved patch may not ever 
land.




Comment at: 
clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp:322
   DiagnosticBuilder Diag = diag(ElseLoc, WarningMessage)
-   << ControlFlowInterruptor;
+   << ControlFlowInterruptor << SourceRange(ElseLoc);
   removeElseAndBrackets(Diag, *Result.Context, Else, ElseLoc);

There are more places where diag is called. Maybe add this to all callsites of 
diag that then emit fixes, otherwise this may miss a few cases. 



Comment at: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp:486
+  )cpp");
+  TestTU TU = TestTU::withCode(Main.code());
+  TU.ClangTidyChecks = "-*,llvm-else-after-return";

This needs rebasing, clang tidy interface for checks has changed. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92272

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


[PATCH] D92198: [clangd] Implement remote index handshake

2020-11-29 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

It'd be nice to know what problem this is trying to solve :-)
I can guess, but don't feel entirely sure here.

In D92198#2420203 , @kadircet wrote:

> Haven't checked the details but is there a specific reason for implementing a 
> custom protocol rather than making use of `NotifyOnStateChange` 
> (https://grpc.github.io/grpc/cpp/classgrpc_1_1_channel_interface.html) or 
> even `WaitForStateChange` if we really want to block the channel creation ?

Right, this is usually the way to go: we should definitely log 
NotifyOnStateChange events (note that the connection can go between 
available/unavailable many times over the life of the program). 
WaitForStateChange would be useful for blocking, but in a program where we want 
to proceed even if the connection is down (which is AFAIK the plan here) it's 
usually best not to block, and just handle fast-failure because the channel is 
down the same way as any other RPC error.

(I can imagine scenarios where we'd want to notify the user with showMessage or 
so whether the index is connected, but this requires more design)




Comment at: clang-tools-extra/clangd/index/remote/Client.cpp:166
+  const uint32_t Checksum = 42;
+  const bool OK = Idx->handshake(Checksum);
+  if (!OK) {

This causes `getClient` to become blocking.

Granted, we expect this to be used from ProjectAwareIndex which currently 
places it on a separate thread anyway. But in general, what's the advantage of 
blocking here?



Comment at: clang-tools-extra/clangd/index/remote/Client.cpp:170
+ "remote index.");
+return nullptr;
+  }

this means if we're offline or so when clangd starts up, the remote index will 
not work until restarting clangd. Do we want this behavior?



Comment at: clang-tools-extra/clangd/index/remote/Index.proto:14
+message HandshakeRequest {
+  required uint32 checksum = 1;
+}

I find this protocol confusing, e.g. the "checksum" isn't actually a checksum 
of anything.
But this wolud be easier to assess if we knew what this is for.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92198

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


[PATCH] D92201: [clangd] Make sure project-aware index is up-to-date for estimateMemoryUsage()

2020-11-29 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

In D92201#2419933 , @kbobyrev wrote:

> Hm, yes, this makes sense. I just found it awkward that memory usage estimate 
> will spike right after the first request but that's actually the reality 
> (loading the index is deferred until the first actual index request).

Exactly. The good news is: I don't think we have any particular use case for 
probing the memory usage right after startup.

> My confusion here is that I don't really understand whether deferring the 
> index loading is a good thing because we're practically moving the delay from 
> the warm-up to the working state which is suboptimal.

It's not great that we offer partial functionality without signaling this 
somehow, and we could consider making this more transparent.
However this happens in lots of places in clangd, especially w.r.t index 
(completion before preamble, background index async load, actual background 
indexing itself, the cached-build-artifacts we have internally at google) and 
in every case feedback has been positive when we introduced it. Blocking the 
user for any one feature is rarely a good trade, if we can recover even 
half-gracefully instead.

> I was trying to cause the index to load in the initialization stage for 
> D92198  but couldn't figure out where 
> exactly the config is loaded and what could be the best place for triggering 
> index loading.

Fair point, will have a look at that. Forcing an index load might be a nice 
side-effect of this patch, but I think we can find a better way to do it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92201

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


[PATCH] D92211: [clang] Improve diagnostics for auto-return-type function if the return expr contains expr.

2020-11-29 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

Great stuff, thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92211

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


[PATCH] D92270: [ConstantFold] Fold more operations to poison

2020-11-29 Thread Nikita Popov via Phabricator via cfe-commits
nikic accepted this revision.
nikic added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92270

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


[PATCH] D89490: Introduce __attribute__((darwin_abi))

2020-11-29 Thread Adrien Guinet via Phabricator via cfe-commits
aguinet updated this revision to Diff 308203.
aguinet added a comment.

Relax checks in `CodeGenCXX/darwinabi-returnthis.cpp` Clang test (to adapt to 
new attributes), and removes some useless brackets in if statements.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89490

Files:
  clang/include/clang-c/Index.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/Specifiers.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/darwin_abi.c
  clang/test/CodeGen/darwin_abi_empty_structs.cpp
  clang/test/CodeGen/darwin_abi_vaarg.c
  clang/test/CodeGen/debug-info-cc.c
  clang/test/CodeGenCXX/darwinabi-returnthis.cpp
  clang/tools/libclang/CXType.cpp
  llvm/include/llvm/BinaryFormat/Dwarf.def
  llvm/include/llvm/IR/CallingConv.h
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/Target/AArch64/AArch64CallingConvention.cpp
  llvm/lib/Target/AArch64/AArch64FastISel.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.h
  llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
  llvm/lib/Target/AArch64/AArch64Subtarget.h
  llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
  llvm/test/CodeGen/AArch64/darwin_abi.ll
  llvm/test/CodeGen/AArch64/darwin_abi_vararg.ll

Index: llvm/test/CodeGen/AArch64/darwin_abi_vararg.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/darwin_abi_vararg.ll
@@ -0,0 +1,39 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=aarch64-pc-linux | FileCheck %s
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-pc-linux"
+
+%struct.__va_list = type { i8*, i8*, i8*, i32, i32 }
+
+define dso_local aarch64_darwincc void @foo(i32 %n, ...) local_unnamed_addr #0 {
+; CHECK-LABEL: foo:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:sub sp, sp, #48 // =48
+; CHECK-NEXT:stp x29, x30, [sp, #32] // 16-byte Folded Spill
+; CHECK-NEXT:add x29, sp, #32 // =32
+; CHECK-NEXT:add x8, x29, #16 // =16
+; CHECK-NEXT:mov x1, sp
+; CHECK-NEXT:str xzr, [sp, #24]
+; CHECK-NEXT:str x8, [sp]
+; CHECK-NEXT:bl vfoo
+; CHECK-NEXT:ldp x29, x30, [sp, #32] // 16-byte Folded Reload
+; CHECK-NEXT:add sp, sp, #48 // =48
+; CHECK-NEXT:ret
+entry:
+  %va = alloca %struct.__va_list, align 8
+  %0 = bitcast %struct.__va_list* %va to i8*
+  call void @llvm.va_start(i8* nonnull %0)
+  call void @vfoo(i32 %n, %struct.__va_list* nonnull %va) #1
+  call void @llvm.va_end(i8* nonnull %0)
+  ret void
+}
+
+declare void @llvm.va_start(i8*) #1
+
+declare dso_local void @vfoo(i32, %struct.__va_list*) local_unnamed_addr #0
+
+declare void @llvm.va_end(i8*) #1
+
+attributes #0 = { nounwind "disable-tail-calls"="false" "frame-pointer"="non-leaf" "target-cpu"="generic" }
+attributes #1 = { nounwind }
Index: llvm/test/CodeGen/AArch64/darwin_abi.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/darwin_abi.ll
@@ -0,0 +1,29 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=aarch64-pc-linux | FileCheck %s
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-pc-linux"
+
+define dso_local aarch64_darwincc signext i16 @f1(i16 signext %a) local_unnamed_addr #0 {
+; CHECK-LABEL: f1:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:add w8, w0, #1 // =1
+; CHECK-NEXT:sxth w0, w8
+; CHECK-NEXT:ret
+entry:
+  %add = add i16 %a, 1
+  ret i16 %add
+}
+
+define dso_local aarch64_darwincc zeroext i16 @f2(i16 zeroext %a) local_unnamed_addr #0 {
+; CHECK-LABEL: f2:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:add w8, w0, #1 // =1
+; CHECK-NEXT:and w0, w8, #0x
+; CHECK-NEXT:ret
+entry:
+  %add = add i16 %a, 1
+  ret i16 %add
+}
+
+attributes #0 = { norecurse nounwind readnone "disable-tail-calls"="false" "frame-pointer"="non-leaf" "target-cpu"="generic" }
Index: llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
===
--- llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
+++ llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
@@ -474,8 +474,8 @@
   uint64_t StackOffset = Handler.StackUsed;
   if (F.isVarArg()) {
 auto  = MF.getSubtarget();
-if (!Subtarget.isTargetDarwin()) {
-   

[PATCH] D89490: Introduce __attribute__((darwin_abi))

2020-11-29 Thread Adrien Guinet via Phabricator via cfe-commits
aguinet added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:1481
+def DarwinABI : DeclOrTypeAttr {
+  let Spellings = [GCC<"darwin_abi">];
+//  let Subjects = [Function, ObjCMethod];

aaron.ballman wrote:
> I suspect this should be using the `Clang` spelling as I don't believe GCC 
> supports this attribute.
Changed in new patch.



Comment at: clang/include/clang/Basic/AttrDocs.td:2258
+  let Content = [{
+On Linux ARM64 targets, this attribute changes the calling convention of
+a function to match the default convention used on Apple ARM64. This

aaron.ballman wrote:
> Adding more details about what that ABI actually is, or linking to a place 
> where the user can learn more on their own, would be appreciated.
Do you know if we can we put http links into this documentation? Would this 
render correctly in the final clang documentation? 



Comment at: clang/include/clang/Basic/Specifiers.h:269
 
+  // clang-format off
   /// CallingConv - Specifies the calling convention that a function uses.

aaron.ballman wrote:
> My personal opinion is that these formatting markers are noise. However, 
> there's a fair number of enumerations this could cover and disabling the 
> format behavior may be reasonable.
> 
> I think I'd rather see the formatting comments removed from this patch. They 
> can be added in a different commit but perhaps the file can be restructured 
> so that we don't feel the need to sprinkle so many comments around.
I don't have strong opinions about this, but so we agree that `clang-format` 
won't be happy about this patch but we're fine with it?



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:70
+// Returns true if AArch64 Darwin ABI is explicitly used.
+const bool IsCtorOrDtor = (isa(GD.getDecl()) ||
+   (isa(GD.getDecl()) &&

aaron.ballman wrote:
> We don't typically go with top-level `const` on non-pointer/reference types, 
> so I'd drop the `const`.
I am not sure to understand the reasoning behind it. IMHO it's interesting when 
reading the code to know that this value will never change accross the 
function. Is there an issue I am missing?



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:78-79
+cast(GD.getDecl())->getType()->getAs();
+const auto FCC = FTy->getCallConv();
+return FCC == CallingConv::CC_AArch64Darwin;
+  }

aaron.ballman wrote:
> `return FTy->getCallConv() == CallingConv::CC_Aarch64Darwin;`
> 
> (Removes a questionable use of `auto` and a top-level `const` at the same 
> time.)
Changed in new patch!



Comment at: clang/lib/CodeGen/TargetInfo.cpp:5449
   bool isDarwinPCS() const { return Kind == DarwinPCS; }
+  bool isDarwinPCS(const unsigned CConv) const {
+return isDarwinPCS() || CConv == llvm::CallingConv::AArch64Darwin;

aaron.ballman wrote:
> Drop the top-level `const` in the parameter.
Changed in new patch!



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4640
+  case ParsedAttr::AT_DarwinABI:
+CC = Context.getTargetInfo().getTriple().isOSDarwin() ? CC_C
+  : CC_AArch64Darwin;

aaron.ballman wrote:
> Similar confusion on my part here -- if the OS is Darwin, we want to default 
> to cdecl?
Yes! The idea is to implement the C ABI of Darwin/AArch64 on foreign OSes. So, 
if we are already targeting Darwin, just use the C ABI. This is the same logic 
used just above with `AT_MSABI`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89490

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


[PATCH] D89490: Introduce __attribute__((darwin_abi))

2020-11-29 Thread Adrien Guinet via Phabricator via cfe-commits
aguinet updated this revision to Diff 308202.
aguinet marked 10 inline comments as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89490

Files:
  clang/include/clang-c/Index.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/Specifiers.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/darwin_abi.c
  clang/test/CodeGen/darwin_abi_empty_structs.cpp
  clang/test/CodeGen/darwin_abi_vaarg.c
  clang/test/CodeGen/debug-info-cc.c
  clang/test/CodeGenCXX/darwinabi-returnthis.cpp
  clang/tools/libclang/CXType.cpp
  llvm/include/llvm/BinaryFormat/Dwarf.def
  llvm/include/llvm/IR/CallingConv.h
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/Target/AArch64/AArch64CallingConvention.cpp
  llvm/lib/Target/AArch64/AArch64FastISel.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.h
  llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
  llvm/lib/Target/AArch64/AArch64Subtarget.h
  llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
  llvm/test/CodeGen/AArch64/darwin_abi.ll
  llvm/test/CodeGen/AArch64/darwin_abi_vararg.ll

Index: llvm/test/CodeGen/AArch64/darwin_abi_vararg.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/darwin_abi_vararg.ll
@@ -0,0 +1,39 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=aarch64-pc-linux | FileCheck %s
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-pc-linux"
+
+%struct.__va_list = type { i8*, i8*, i8*, i32, i32 }
+
+define dso_local aarch64_darwincc void @foo(i32 %n, ...) local_unnamed_addr #0 {
+; CHECK-LABEL: foo:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:sub sp, sp, #48 // =48
+; CHECK-NEXT:stp x29, x30, [sp, #32] // 16-byte Folded Spill
+; CHECK-NEXT:add x29, sp, #32 // =32
+; CHECK-NEXT:add x8, x29, #16 // =16
+; CHECK-NEXT:mov x1, sp
+; CHECK-NEXT:str xzr, [sp, #24]
+; CHECK-NEXT:str x8, [sp]
+; CHECK-NEXT:bl vfoo
+; CHECK-NEXT:ldp x29, x30, [sp, #32] // 16-byte Folded Reload
+; CHECK-NEXT:add sp, sp, #48 // =48
+; CHECK-NEXT:ret
+entry:
+  %va = alloca %struct.__va_list, align 8
+  %0 = bitcast %struct.__va_list* %va to i8*
+  call void @llvm.va_start(i8* nonnull %0)
+  call void @vfoo(i32 %n, %struct.__va_list* nonnull %va) #1
+  call void @llvm.va_end(i8* nonnull %0)
+  ret void
+}
+
+declare void @llvm.va_start(i8*) #1
+
+declare dso_local void @vfoo(i32, %struct.__va_list*) local_unnamed_addr #0
+
+declare void @llvm.va_end(i8*) #1
+
+attributes #0 = { nounwind "disable-tail-calls"="false" "frame-pointer"="non-leaf" "target-cpu"="generic" }
+attributes #1 = { nounwind }
Index: llvm/test/CodeGen/AArch64/darwin_abi.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/darwin_abi.ll
@@ -0,0 +1,29 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=aarch64-pc-linux | FileCheck %s
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-pc-linux"
+
+define dso_local aarch64_darwincc signext i16 @f1(i16 signext %a) local_unnamed_addr #0 {
+; CHECK-LABEL: f1:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:add w8, w0, #1 // =1
+; CHECK-NEXT:sxth w0, w8
+; CHECK-NEXT:ret
+entry:
+  %add = add i16 %a, 1
+  ret i16 %add
+}
+
+define dso_local aarch64_darwincc zeroext i16 @f2(i16 zeroext %a) local_unnamed_addr #0 {
+; CHECK-LABEL: f2:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:add w8, w0, #1 // =1
+; CHECK-NEXT:and w0, w8, #0x
+; CHECK-NEXT:ret
+entry:
+  %add = add i16 %a, 1
+  ret i16 %add
+}
+
+attributes #0 = { norecurse nounwind readnone "disable-tail-calls"="false" "frame-pointer"="non-leaf" "target-cpu"="generic" }
Index: llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
===
--- llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
+++ llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
@@ -474,8 +474,8 @@
   uint64_t StackOffset = Handler.StackUsed;
   if (F.isVarArg()) {
 auto  = MF.getSubtarget();
-if (!Subtarget.isTargetDarwin()) {
-// FIXME: we need to reimplement saveVarArgsRegisters from
+if