[PATCH] D137458: [clang] Add __decay as a builtin template

[Troy Johnson via Phabricator via cfe-commits]

troyj added a comment.

> Also, it would be nice to have some numbers for the 'measurably faster' claim 
> :)

Sure. Here's an example of a library change that started using builtins for 
`__make_integer_seq` and `__type_pack_element` 
https://github.com/facebook/fatal/commit/58102a3f7e66ad122d7d3335c446399b09d5085e
 where there was a 1.8% speedup for a file that spent 70 seconds in the front 
end. It also reduced the peak memory usage of the front end by 0.5%. We have 
reason to believe that a `__decay` builtin would produce similar benefit. 
Unfortunately, all of the intrinsics in https://reviews.llvm.org/D116203 were 
implemented with parentheses syntax instead of as builtin templates, which 
makes it more difficult. Ideally, we'd like to see all of these implemented as 
builtin templates, but `__decay` is the first one that we're proposing.

I'm not familiar with the mangling issue that you mentioned. I'll look into it 
more.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137458

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

[PATCH] D137458: [clang] Add __decay as a builtin template

[Troy Johnson via Phabricator via cfe-commits]

troyj created this revision.
troyj added a reviewer: bruno.
Herald added a subscriber: martong.
Herald added a reviewer: shafik.
Herald added a project: All.
troyj requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

`__decay(T)` exists as a builtin, but does not use template syntax like the 
other builtin templates `__make_integer_seq` and `__type_pack_element`. With 
template syntax, users can write portable code that uses `__decay` when it's 
available but falls back to a `__decay` in a local namespace when it's not 
available. Template aliasing can provide the same behavior, but at the cost of 
longer compile time. It is faster to compile a direct use of a builtin than a 
use via an alias. This compile time difference is measurable in large TUs that 
make heavy use of templates. Currently the only performant alternative is to 
use preprocessor macros in the user's code.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137458

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Basic/Builtins.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/DeclTemplate.cpp
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/SemaCXX/libstdcxx_transform_type_traits_hack.cpp
  clang/test/SemaCXX/type-traits.cpp

Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -3414,6 +3414,50 @@
   static_assert(__is_same(decay_t, int S::*), "");
 }
 
+void check_decay_template() {
+  static_assert(__is_same(__decay, void), "");
+  static_assert(__is_same(__decay, void), "");
+  static_assert(__is_same(__decay, int), "");
+  static_assert(__is_same(__decay, int), "");
+  static_assert(__is_same(__decay, int), "");
+  static_assert(__is_same(__decay, int), "");
+  static_assert(__is_same(__decay, int *), "");
+  static_assert(__is_same(__decay, int *), "");
+  static_assert(__is_same(__decay, int *), "");
+  static_assert(__is_same(__decay, int const *), "");
+  static_assert(__is_same(__decay, int const *), "");
+  static_assert(__is_same(__decay, int), "");
+  static_assert(__is_same(__decay, int), "");
+  static_assert(__is_same(__decay, int), "");
+  static_assert(__is_same(__decay, int), "");
+  static_assert(__is_same(__decay, int (*)()), "");
+  static_assert(__is_same(__decay, int (*)()), "");
+  static_assert(__is_same(__decay, int (*)()), "");
+  static_assert(__is_same(__decay, int (*)()), "");
+  static_assert(__is_same(__decay, int (*)()), "");
+  static_assert(__is_same(__decay, int (*)()), "");
+  static_assert(__is_same(__decay, int *), "");
+  static_assert(__is_same(__decay, int *), "");
+
+  static_assert(__is_same(__decay, S), "");
+  static_assert(__is_same(__decay, S), "");
+  static_assert(__is_same(__decay, S), "");
+  static_assert(__is_same(__decay, S), "");
+  static_assert(__is_same(__decay, S), "");
+  static_assert(__is_same(__decay, S), "");
+  static_assert(__is_same(__decay, S), "");
+  static_assert(__is_same(__decay, S), "");
+  static_assert(__is_same(__decay, S), "");
+  static_assert(__is_same(__decay, S), "");
+  static_assert(__is_same(__decay, S), "");
+  static_assert(__is_same(__decay, S), "");
+  static_assert(__is_same(__decay, int S::*), "");
+  static_assert(__is_same(__decay, int(S::*)()), "");
+  static_assert(__is_same(__decay, int S::*), "");
+  static_assert(__is_same(__decay, int(S::*)()), "");
+  static_assert(__is_same(__decay, int S::*), "");
+}
+
 template  struct CheckAbominableFunction {};
 template 
 struct CheckAbominableFunction {
Index: clang/test/SemaCXX/libstdcxx_transform_type_traits_hack.cpp
===
--- clang/test/SemaCXX/libstdcxx_transform_type_traits_hack.cpp
+++ clang/test/SemaCXX/libstdcxx_transform_type_traits_hack.cpp
@@ -58,11 +58,6 @@
 template 
 using J = Same<__remove_cvref, __remove_cvref>;
 
-template 
-using __decay = int; // expected-warning{{keyword '__decay' will be made available as an identifier here}}
-template 
-using K = Same<__decay, __decay>;
-
 template 
 using __make_signed = int; // expected-warning{{keyword '__make_signed' will be made available as an identifier here}}
 template 
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -4621,6 +4621,7 @@
  PREDEF_DECL_CF_CONSTANT_STRING_TAG_ID);
   RegisterPredefDecl(Context.TypePackElementDecl,
  PREDEF_DECL_TYPE_PACK_ELEMENT_ID);
+  RegisterPredefDecl(Context.DecayDecl, PREDEF_DECL_DECAY_ID);
 
   // Build 

[PATCH] D137179: [clang][Lex] Header map search case insensitivity

[Troy Johnson 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 rG00022c5613a7: [clang][Lex] Header map search case 
insensitivity (authored by troyj).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137179

Files:
  clang/lib/Lex/HeaderSearch.cpp


Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -388,7 +388,9 @@
   break;
 
 // Give earlier keys precedence over identical later keys.
-auto Callback = [&](StringRef Filename) { Index.try_emplace(Filename, i); 
};
+auto Callback = [&](StringRef Filename) {
+  Index.try_emplace(Filename.lower(), i);
+};
 Dir.getHeaderMap()->forEachKey(Callback);
   }
 
@@ -1021,7 +1023,7 @@
 // Handle cold misses of user includes in the presence of many header
 // maps.  We avoid searching perhaps thousands of header maps by
 // jumping directly to the correct one or jumping beyond all of them.
-auto Iter = SearchDirHeaderMapIndex.find(Filename);
+auto Iter = SearchDirHeaderMapIndex.find(Filename.lower());
 if (Iter == SearchDirHeaderMapIndex.end())
   // Not in index => Skip to first SearchDir after initial header maps
   It = search_dir_nth(FirstNonHeaderMapSearchDirIdx);


Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -388,7 +388,9 @@
   break;
 
 // Give earlier keys precedence over identical later keys.
-auto Callback = [&](StringRef Filename) { Index.try_emplace(Filename, i); };
+auto Callback = [&](StringRef Filename) {
+  Index.try_emplace(Filename.lower(), i);
+};
 Dir.getHeaderMap()->forEachKey(Callback);
   }
 
@@ -1021,7 +1023,7 @@
 // Handle cold misses of user includes in the presence of many header
 // maps.  We avoid searching perhaps thousands of header maps by
 // jumping directly to the correct one or jumping beyond all of them.
-auto Iter = SearchDirHeaderMapIndex.find(Filename);
+auto Iter = SearchDirHeaderMapIndex.find(Filename.lower());
 if (Iter == SearchDirHeaderMapIndex.end())
   // Not in index => Skip to first SearchDir after initial header maps
   It = search_dir_nth(FirstNonHeaderMapSearchDirIdx);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D137179: [clang][Lex] Header search case insensitivity

[Troy Johnson via Phabricator via cfe-commits]

troyj created this revision.
troyj added a reviewer: bruno.
Herald added a project: All.
troyj requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Correct D135801  to be case insensitive.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137179

Files:
  clang/lib/Lex/HeaderSearch.cpp


Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -388,7 +388,9 @@
   break;
 
 // Give earlier keys precedence over identical later keys.
-auto Callback = [&](StringRef Filename) { Index.try_emplace(Filename, i); 
};
+auto Callback = [&](StringRef Filename) {
+  Index.try_emplace(Filename.lower(), i);
+};
 Dir.getHeaderMap()->forEachKey(Callback);
   }
 
@@ -1021,7 +1023,7 @@
 // Handle cold misses of user includes in the presence of many header
 // maps.  We avoid searching perhaps thousands of header maps by
 // jumping directly to the correct one or jumping beyond all of them.
-auto Iter = SearchDirHeaderMapIndex.find(Filename);
+auto Iter = SearchDirHeaderMapIndex.find(Filename.lower());
 if (Iter == SearchDirHeaderMapIndex.end())
   // Not in index => Skip to first SearchDir after initial header maps
   It = search_dir_nth(FirstNonHeaderMapSearchDirIdx);


Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -388,7 +388,9 @@
   break;
 
 // Give earlier keys precedence over identical later keys.
-auto Callback = [&](StringRef Filename) { Index.try_emplace(Filename, i); };
+auto Callback = [&](StringRef Filename) {
+  Index.try_emplace(Filename.lower(), i);
+};
 Dir.getHeaderMap()->forEachKey(Callback);
   }
 
@@ -1021,7 +1023,7 @@
 // Handle cold misses of user includes in the presence of many header
 // maps.  We avoid searching perhaps thousands of header maps by
 // jumping directly to the correct one or jumping beyond all of them.
-auto Iter = SearchDirHeaderMapIndex.find(Filename);
+auto Iter = SearchDirHeaderMapIndex.find(Filename.lower());
 if (Iter == SearchDirHeaderMapIndex.end())
   // Not in index => Skip to first SearchDir after initial header maps
   It = search_dir_nth(FirstNonHeaderMapSearchDirIdx);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136190: [clang][Sema][NFC] Remove redundant isTypeValid

[Troy Johnson 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 rG488cf1038c88: [clang][Sema][NFC] Remove redundant 
isTypeValid (authored by troyj).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136190

Files:
  clang/lib/Sema/SemaOverload.cpp


Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -4606,15 +4606,6 @@
   return ImplicitConversionSequence::Indistinguishable;
 }
 
-/// Determine whether the given type is valid, e.g., it is not an invalid
-/// C++ class.
-static bool isTypeValid(QualType T) {
-  if (CXXRecordDecl *Record = T->getAsCXXRecordDecl())
-return !Record->isInvalidDecl();
-
-  return true;
-}
-
 static QualType withoutUnaligned(ASTContext , QualType T) {
   if (!T.getQualifiers().hasUnaligned())
 return T;
@@ -4664,7 +4655,6 @@
   if (UnqualT1 == UnqualT2) {
 // Nothing to do.
   } else if (isCompleteType(Loc, OrigT2) &&
- isTypeValid(UnqualT1) && isTypeValid(UnqualT2) &&
  IsDerivedFrom(Loc, UnqualT2, UnqualT1))
 Conv |= ReferenceConversions::DerivedToBase;
   else if (UnqualT1->isObjCObjectOrInterfaceType() &&


Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -4606,15 +4606,6 @@
   return ImplicitConversionSequence::Indistinguishable;
 }
 
-/// Determine whether the given type is valid, e.g., it is not an invalid
-/// C++ class.
-static bool isTypeValid(QualType T) {
-  if (CXXRecordDecl *Record = T->getAsCXXRecordDecl())
-return !Record->isInvalidDecl();
-
-  return true;
-}
-
 static QualType withoutUnaligned(ASTContext , QualType T) {
   if (!T.getQualifiers().hasUnaligned())
 return T;
@@ -4664,7 +4655,6 @@
   if (UnqualT1 == UnqualT2) {
 // Nothing to do.
   } else if (isCompleteType(Loc, OrigT2) &&
- isTypeValid(UnqualT1) && isTypeValid(UnqualT2) &&
  IsDerivedFrom(Loc, UnqualT2, UnqualT1))
 Conv |= ReferenceConversions::DerivedToBase;
   else if (UnqualT1->isObjCObjectOrInterfaceType() &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136779: [clang][AST] make FunctionProtoType::Profile consistent for FoldingSet

[Troy Johnson via Phabricator via cfe-commits]

troyj created this revision.
troyj added a reviewer: bruno.
Herald added a project: All.
troyj requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch changes the Profile function to match the FunctionProtoType 
constructor, which uses epi.ExceptionSpec.SourceDecl and not 
epi.ExceptionSpec.SourceDecl->getCanonicalDecl(). This change eliminates an 
inconsistency that was observed within the FunctionProtoTypes set by adding a 
verification function to FoldingSet to assert that all items in the set still 
hash to their current bucket. Asserting this verification showed that some 
FunctionProtoTypes no longer hashed to their original bucket. This 
inconsistency was not catastrophic because of self-correction -- FoldingSet 
does not store hashes and recomputes them whenever it rebuckets. Thus, it's not 
possible to observe the issue immediately after rebucketing. Comparing the 
before and after profiles of a FunctionProtoType that was in the wrong bucket 
showed that the only part of the profile that changed was 
epi.ExceptionSpec.SourceDecl->getCanonicalDecl(). After changing the profile 
function to match the constructor, no incorrectly bucketed FunctionProtoTypes 
were observed in a TU with ~2.6M prototypes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136779

Files:
  clang/lib/AST/Type.cpp


Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -3385,7 +3385,7 @@
 epi.ExceptionSpec.NoexceptExpr->Profile(ID, Context, Canonical);
   } else if (epi.ExceptionSpec.Type == EST_Uninstantiated ||
  epi.ExceptionSpec.Type == EST_Unevaluated) {
-ID.AddPointer(epi.ExceptionSpec.SourceDecl->getCanonicalDecl());
+ID.AddPointer(epi.ExceptionSpec.SourceDecl);
   }
   if (epi.ExtParameterInfos) {
 for (unsigned i = 0; i != NumParams; ++i)


Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -3385,7 +3385,7 @@
 epi.ExceptionSpec.NoexceptExpr->Profile(ID, Context, Canonical);
   } else if (epi.ExceptionSpec.Type == EST_Uninstantiated ||
  epi.ExceptionSpec.Type == EST_Unevaluated) {
-ID.AddPointer(epi.ExceptionSpec.SourceDecl->getCanonicalDecl());
+ID.AddPointer(epi.ExceptionSpec.SourceDecl);
   }
   if (epi.ExtParameterInfos) {
 for (unsigned i = 0; i != NumParams; ++i)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136633: [clang][AST] avoid unnecessary FunctionProtoTypes.FindNodeOrInsertPos call

[Troy Johnson via Phabricator via cfe-commits]

troyj created this revision.
troyj added a reviewer: bruno.
Herald added a project: All.
troyj requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

getFunctionTypeInternal forms a FoldingSetNodeID and then calls 
FunctionProtoTypes.FindNodeOrInsertPos(). Later, upon returning from recursion, 
it calls FunctionProtoTypes.FindNodeOrInsertPos() again with the same ID. This 
second call will yield the same InsertPos unless the recursion caused 
FunctionProtoTypes to rebucket. The rebucketing can be detected by comparing 
the capacity of the set before and after the recursion, which allows us to skip 
the redundant call.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136633

Files:
  clang/lib/AST/ASTContext.cpp


Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -4514,13 +4514,17 @@
 
 // Adjust the canonical function result type.
 CanQualType CanResultTy = getCanonicalFunctionResultType(ResultTy);
+const auto OrigCapacity = FunctionProtoTypes.capacity();
 Canonical =
 getFunctionTypeInternal(CanResultTy, CanonicalArgs, CanonicalEPI, 
true);
 
-// Get the new insert position for the node we care about.
-FunctionProtoType *NewIP =
-  FunctionProtoTypes.FindNodeOrInsertPos(ID, InsertPos);
-assert(!NewIP && "Shouldn't be in the map!"); (void)NewIP;
+// Get the new insert position for the node we care about, if the recursive
+// call invalidated InsertPos.
+if (FunctionProtoTypes.capacity() != OrigCapacity) {
+  FunctionProtoType *NewIP =
+FunctionProtoTypes.FindNodeOrInsertPos(ID, InsertPos);
+  assert(!NewIP && "Shouldn't be in the map!"); (void)NewIP;
+}
   }
 
   // Compute the needed size to hold this FunctionProtoType and the


Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -4514,13 +4514,17 @@
 
 // Adjust the canonical function result type.
 CanQualType CanResultTy = getCanonicalFunctionResultType(ResultTy);
+const auto OrigCapacity = FunctionProtoTypes.capacity();
 Canonical =
 getFunctionTypeInternal(CanResultTy, CanonicalArgs, CanonicalEPI, true);
 
-// Get the new insert position for the node we care about.
-FunctionProtoType *NewIP =
-  FunctionProtoTypes.FindNodeOrInsertPos(ID, InsertPos);
-assert(!NewIP && "Shouldn't be in the map!"); (void)NewIP;
+// Get the new insert position for the node we care about, if the recursive
+// call invalidated InsertPos.
+if (FunctionProtoTypes.capacity() != OrigCapacity) {
+  FunctionProtoType *NewIP =
+FunctionProtoTypes.FindNodeOrInsertPos(ID, InsertPos);
+  assert(!NewIP && "Shouldn't be in the map!"); (void)NewIP;
+}
   }
 
   // Compute the needed size to hold this FunctionProtoType and the
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136190: [clang][Sema] remove redundant isTypeValid

[Troy Johnson via Phabricator via cfe-commits]

troyj created this revision.
troyj added a reviewer: bruno.
Herald added a project: All.
troyj requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

These isTypeValid calls are redundant because the isDerivedFrom call performs 
the same checks.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136190

Files:
  clang/lib/Sema/SemaOverload.cpp


Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -4590,15 +4590,6 @@
   return ImplicitConversionSequence::Indistinguishable;
 }
 
-/// Determine whether the given type is valid, e.g., it is not an invalid
-/// C++ class.
-static bool isTypeValid(QualType T) {
-  if (CXXRecordDecl *Record = T->getAsCXXRecordDecl())
-return !Record->isInvalidDecl();
-
-  return true;
-}
-
 static QualType withoutUnaligned(ASTContext , QualType T) {
   if (!T.getQualifiers().hasUnaligned())
 return T;
@@ -4648,7 +4639,6 @@
   if (UnqualT1 == UnqualT2) {
 // Nothing to do.
   } else if (isCompleteType(Loc, OrigT2) &&
- isTypeValid(UnqualT1) && isTypeValid(UnqualT2) &&
  IsDerivedFrom(Loc, UnqualT2, UnqualT1))
 Conv |= ReferenceConversions::DerivedToBase;
   else if (UnqualT1->isObjCObjectOrInterfaceType() &&


Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -4590,15 +4590,6 @@
   return ImplicitConversionSequence::Indistinguishable;
 }
 
-/// Determine whether the given type is valid, e.g., it is not an invalid
-/// C++ class.
-static bool isTypeValid(QualType T) {
-  if (CXXRecordDecl *Record = T->getAsCXXRecordDecl())
-return !Record->isInvalidDecl();
-
-  return true;
-}
-
 static QualType withoutUnaligned(ASTContext , QualType T) {
   if (!T.getQualifiers().hasUnaligned())
 return T;
@@ -4648,7 +4639,6 @@
   if (UnqualT1 == UnqualT2) {
 // Nothing to do.
   } else if (isCompleteType(Loc, OrigT2) &&
- isTypeValid(UnqualT1) && isTypeValid(UnqualT2) &&
  IsDerivedFrom(Loc, UnqualT2, UnqualT1))
 Conv |= ReferenceConversions::DerivedToBase;
   else if (UnqualT1->isObjCObjectOrInterfaceType() &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135801: [clang][Lexer] Speedup HeaderSearch when there are many HeaderMaps

[Troy Johnson via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG10c0eca25523: [clang][Lexer] Speed up HeaderSearch when 
there are many HeaderMaps (authored by troyj).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135801

Files:
  clang/include/clang/Lex/HeaderMap.h
  clang/include/clang/Lex/HeaderSearch.h
  clang/lib/Lex/HeaderSearch.cpp

Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -116,6 +116,7 @@
   NoCurDirSearch = noCurDirSearch;
   SearchDirToHSEntry = std::move(searchDirToHSEntry);
   //LookupFileCache.clear();
+  indexInitialHeaderMaps();
 }
 
 void HeaderSearch::AddSearchPath(const DirectoryLookup , bool isAngled) {
@@ -372,6 +373,29 @@
   return Module;
 }
 
+void HeaderSearch::indexInitialHeaderMaps() {
+  llvm::StringMap Index(SearchDirs.size());
+
+  // Iterate over all filename keys and associate them with the index i.
+  unsigned i = 0;
+  for (; i != SearchDirs.size(); ++i) {
+auto  = SearchDirs[i];
+
+// We're concerned with only the initial contiguous run of header
+// maps within SearchDirs, which can be 99% of SearchDirs when
+// SearchDirs.size() is ~1.
+if (!Dir.isHeaderMap())
+  break;
+
+// Give earlier keys precedence over identical later keys.
+auto Callback = [&](StringRef Filename) { Index.try_emplace(Filename, i); };
+Dir.getHeaderMap()->forEachKey(Callback);
+  }
+
+  SearchDirHeaderMapIndex = std::move(Index);
+  FirstNonHeaderMapSearchDirIdx = i;
+}
+
 //===--===//
 // File lookup within a DirectoryLookup scope
 //===--===//
@@ -977,24 +1001,37 @@
 
   ConstSearchDirIterator NextIt = std::next(It);
 
-  // If the entry has been previously looked up, the first value will be
-  // non-zero.  If the value is equal to i (the start point of our search), then
-  // this is a matching hit.
-  if (!SkipCache && CacheLookup.StartIt == NextIt) {
-// Skip querying potentially lots of directories for this lookup.
-if (CacheLookup.HitIt)
-  It = CacheLookup.HitIt;
-if (CacheLookup.MappedName) {
-  Filename = CacheLookup.MappedName;
-  if (IsMapped)
-*IsMapped = true;
+  if (!SkipCache) {
+if (CacheLookup.StartIt == NextIt) {
+  // HIT: Skip querying potentially lots of directories for this lookup.
+  if (CacheLookup.HitIt)
+It = CacheLookup.HitIt;
+  if (CacheLookup.MappedName) {
+Filename = CacheLookup.MappedName;
+if (IsMapped)
+  *IsMapped = true;
+  }
+} else {
+  // MISS: This is the first query, or the previous query didn't match
+  // our search start.  We will fill in our found location below, so prime
+  // the start point value.
+  CacheLookup.reset(/*NewStartIt=*/NextIt);
+
+  if (It == search_dir_begin() && FirstNonHeaderMapSearchDirIdx > 0) {
+// Handle cold misses of user includes in the presence of many header
+// maps.  We avoid searching perhaps thousands of header maps by
+// jumping directly to the correct one or jumping beyond all of them.
+auto Iter = SearchDirHeaderMapIndex.find(Filename);
+if (Iter == SearchDirHeaderMapIndex.end())
+  // Not in index => Skip to first SearchDir after initial header maps
+  It = search_dir_nth(FirstNonHeaderMapSearchDirIdx);
+else
+  // In index => Start with a specific header map
+  It = search_dir_nth(Iter->second);
+  }
 }
-  } else {
-// Otherwise, this is the first query, or the previous query didn't match
-// our search start.  We will fill in our found location below, so prime the
-// start point value.
+  } else
 CacheLookup.reset(/*NewStartIt=*/NextIt);
-  }
 
   SmallString<64> MappedName;
 
Index: clang/include/clang/Lex/HeaderSearch.h
===
--- clang/include/clang/Lex/HeaderSearch.h
+++ clang/include/clang/Lex/HeaderSearch.h
@@ -249,6 +249,14 @@
   unsigned SystemDirIdx = 0;
   bool NoCurDirSearch = false;
 
+  /// Maps HeaderMap keys to SearchDir indices. When HeaderMaps are used
+  /// heavily, SearchDirs can start with thousands of HeaderMaps, so this Index
+  /// lets us avoid scanning them all to find a match.
+  llvm::StringMap SearchDirHeaderMapIndex;
+
+  /// The index of the first SearchDir that isn't a header map.
+  unsigned FirstNonHeaderMapSearchDirIdx = 0;
+
   /// \#include prefixes for which the 'system header' property is
   /// overridden.
   ///
@@ -330,6 +338,10 @@
   /// Entity used to look up stored header file information.
   ExternalHeaderFileInfoSource *ExternalSource = nullptr;
 
+  /// Scan all of the 

[PATCH] D135801: [clang][Lexer] Speedup HeaderSearch when there are many HeaderMaps

[Troy Johnson via Phabricator via cfe-commits]

troyj updated this revision to Diff 467496.
troyj added a comment.

Addressed Bruno's comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135801

Files:
  clang/include/clang/Lex/HeaderMap.h
  clang/include/clang/Lex/HeaderSearch.h
  clang/lib/Lex/HeaderSearch.cpp

Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -116,6 +116,7 @@
   NoCurDirSearch = noCurDirSearch;
   SearchDirToHSEntry = std::move(searchDirToHSEntry);
   //LookupFileCache.clear();
+  indexInitialHeaderMaps();
 }
 
 void HeaderSearch::AddSearchPath(const DirectoryLookup , bool isAngled) {
@@ -372,6 +373,29 @@
   return Module;
 }
 
+void HeaderSearch::indexInitialHeaderMaps() {
+  llvm::StringMap Index(SearchDirs.size());
+
+  // Iterate over all filename keys and associate them with the index i.
+  unsigned i = 0;
+  for (; i != SearchDirs.size(); ++i) {
+auto  = SearchDirs[i];
+
+// We're concerned with only the initial contiguous run of header
+// maps within SearchDirs, which can be 99% of SearchDirs when
+// SearchDirs.size() is ~1.
+if (!Dir.isHeaderMap())
+  break;
+
+// Give earlier keys precedence over identical later keys.
+auto Callback = [&](StringRef Filename) { Index.try_emplace(Filename, i); };
+Dir.getHeaderMap()->forEachKey(Callback);
+  }
+
+  SearchDirHeaderMapIndex = std::move(Index);
+  FirstNonHeaderMapSearchDirIdx = i;
+}
+
 //===--===//
 // File lookup within a DirectoryLookup scope
 //===--===//
@@ -977,24 +1001,37 @@
 
   ConstSearchDirIterator NextIt = std::next(It);
 
-  // If the entry has been previously looked up, the first value will be
-  // non-zero.  If the value is equal to i (the start point of our search), then
-  // this is a matching hit.
-  if (!SkipCache && CacheLookup.StartIt == NextIt) {
-// Skip querying potentially lots of directories for this lookup.
-if (CacheLookup.HitIt)
-  It = CacheLookup.HitIt;
-if (CacheLookup.MappedName) {
-  Filename = CacheLookup.MappedName;
-  if (IsMapped)
-*IsMapped = true;
+  if (!SkipCache) {
+if (CacheLookup.StartIt == NextIt) {
+  // HIT: Skip querying potentially lots of directories for this lookup.
+  if (CacheLookup.HitIt)
+It = CacheLookup.HitIt;
+  if (CacheLookup.MappedName) {
+Filename = CacheLookup.MappedName;
+if (IsMapped)
+  *IsMapped = true;
+  }
+} else {
+  // MISS: This is the first query, or the previous query didn't match
+  // our search start.  We will fill in our found location below, so prime
+  // the start point value.
+  CacheLookup.reset(/*NewStartIt=*/NextIt);
+
+  if (It == search_dir_begin() && FirstNonHeaderMapSearchDirIdx > 0) {
+// Handle cold misses of user includes in the presence of many header
+// maps.  We avoid searching perhaps thousands of header maps by
+// jumping directly to the correct one or jumping beyond all of them.
+auto Iter = SearchDirHeaderMapIndex.find(Filename);
+if (Iter == SearchDirHeaderMapIndex.end())
+  // Not in index => Skip to first SearchDir after initial header maps
+  It = search_dir_nth(FirstNonHeaderMapSearchDirIdx);
+else
+  // In index => Start with a specific header map
+  It = search_dir_nth(Iter->second);
+  }
 }
-  } else {
-// Otherwise, this is the first query, or the previous query didn't match
-// our search start.  We will fill in our found location below, so prime the
-// start point value.
+  } else
 CacheLookup.reset(/*NewStartIt=*/NextIt);
-  }
 
   SmallString<64> MappedName;
 
Index: clang/include/clang/Lex/HeaderSearch.h
===
--- clang/include/clang/Lex/HeaderSearch.h
+++ clang/include/clang/Lex/HeaderSearch.h
@@ -249,6 +249,14 @@
   unsigned SystemDirIdx = 0;
   bool NoCurDirSearch = false;
 
+  /// Maps HeaderMap keys to SearchDir indices. When HeaderMaps are used
+  /// heavily, SearchDirs can start with thousands of HeaderMaps, so this Index
+  /// lets us avoid scanning them all to find a match.
+  llvm::StringMap SearchDirHeaderMapIndex;
+
+  /// The index of the first SearchDir that isn't a header map.
+  unsigned FirstNonHeaderMapSearchDirIdx = 0;
+
   /// \#include prefixes for which the 'system header' property is
   /// overridden.
   ///
@@ -330,6 +338,10 @@
   /// Entity used to look up stored header file information.
   ExternalHeaderFileInfoSource *ExternalSource = nullptr;
 
+  /// Scan all of the header maps at the beginning of SearchDirs and
+  /// map their keys to the SearchDir index of their 

[PATCH] D135801: [clang][Lexer] Speedup HeaderSearch when there are many HeaderMaps

[Troy Johnson via Phabricator via cfe-commits]

troyj added inline comments.



Comment at: clang/include/clang/Lex/HeaderSearch.h:255
+  /// lets us avoid scanning them all to find a match.
+  llvm::StringMap SearchDirHeaderMapIndex;
+

bruno wrote:
> Did you end up getting data on the memory footprint of this map? It seems to 
> me that it should be fine if header maps aren't much populated.
Right, with ~15000 header maps, this map has ~92000 entries, so each map 
contributes about 6 unique keys. The patch does not increase the peak memory 
usage of Clang.



Comment at: clang/lib/Lex/HeaderSearch.cpp:392
+
+Dir.getHeaderMap()->forEachKey(Callback);
+  }

bruno wrote:
> What happens if different header maps contain the same header name?
> 
The first one sticks because try_emplace doesn't insert if the key already 
exists. I'll add a comment.



Comment at: clang/lib/Lex/HeaderSearch.cpp:1035
   } else {
-// Otherwise, this is the first query, or the previous query didn't match
-// our search start.  We will fill in our found location below, so prime 
the
-// start point value.
 CacheLookup.reset(/*NewStartIt=*/NextIt);
   }

bruno wrote:
> Looks like we could have only one call to `CacheLookup.reset` instead?
The reset call has a side effect of setting CacheLookup.MappedName to nullptr, 
so we can't call it unconditionally because the Hit case should not call it. 
The unpatched code combined the !SkipCache check and the Hit check, so it still 
called reset when SkipCache == true. I preserved that behavior because I think 
it's necessary. SkipCache seems to imply "don't consult the cache for this 
call" instead of "don't cache anything" so it still caches the result for a 
future LookupFile call that passes SkipCache == false.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135801

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

[PATCH] D135801: [clang][Lexer] Speedup HeaderSearch when there are many HeaderMaps

[Troy Johnson via Phabricator via cfe-commits]

troyj created this revision.
troyj added a reviewer: bruno.
Herald added a project: All.
troyj requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

HeaderSearch already uses a caching system to avoid duplicate searches, but the 
initial cold miss can take a long time if a build system has supplied thousands 
of HeaderMaps. For this case, the SearchDirs vector begins with those 
HeaderMaps, so a cache miss requires testing if the sought filename is present 
in each of those maps. Instead, we can consolidate the keys of those HeaderMaps 
into one StringMap and then each cache miss can skip directly to the correct 
HeaderMap or continue its search beyond the initial sequence of HeaderMaps. In 
testing on TUs with ~15000 SearchDirs where the initial 99% are HeaderMaps, 
time spent in Clang was reduced by 15%. This patch is expected to be neutral 
for SearchDir vectors that do not begin with HeaderMaps.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135801

Files:
  clang/include/clang/Lex/HeaderMap.h
  clang/include/clang/Lex/HeaderSearch.h
  clang/lib/Lex/HeaderSearch.cpp

Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -116,6 +116,7 @@
   NoCurDirSearch = noCurDirSearch;
   SearchDirToHSEntry = std::move(searchDirToHSEntry);
   //LookupFileCache.clear();
+  indexInitialHeaderMaps();
 }
 
 void HeaderSearch::AddSearchPath(const DirectoryLookup , bool isAngled) {
@@ -372,6 +373,29 @@
   return Module;
 }
 
+void HeaderSearch::indexInitialHeaderMaps() {
+  llvm::StringMap Index(SearchDirs.size());
+
+  // Iterate over all filename keys and associate them with the index i.
+  unsigned i = 0;
+  for (; i != SearchDirs.size(); ++i) {
+auto  = SearchDirs[i];
+
+// We're concerned with only the initial contiguous run of header
+// maps within SearchDirs, which can be 99% of SearchDirs when
+// SearchDirs.size() is ~1.
+if (!Dir.isHeaderMap())
+  break;
+
+auto Callback = [&](StringRef Filename) { Index.try_emplace(Filename, i); };
+
+Dir.getHeaderMap()->forEachKey(Callback);
+  }
+
+  SearchDirHeaderMapIndex = std::move(Index);
+  FirstNonHeaderMapSearchDirIdx = i;
+}
+
 //===--===//
 // File lookup within a DirectoryLookup scope
 //===--===//
@@ -977,22 +1001,37 @@
 
   ConstSearchDirIterator NextIt = std::next(It);
 
-  // If the entry has been previously looked up, the first value will be
-  // non-zero.  If the value is equal to i (the start point of our search), then
-  // this is a matching hit.
-  if (!SkipCache && CacheLookup.StartIt == NextIt) {
-// Skip querying potentially lots of directories for this lookup.
-if (CacheLookup.HitIt)
-  It = CacheLookup.HitIt;
-if (CacheLookup.MappedName) {
-  Filename = CacheLookup.MappedName;
-  if (IsMapped)
-*IsMapped = true;
+  if (!SkipCache) {
+if (CacheLookup.StartIt == NextIt) {
+  // HIT: Skip querying potentially lots of directories for this lookup.
+  if (CacheLookup.HitIt)
+It = CacheLookup.HitIt;
+  if (CacheLookup.MappedName) {
+Filename = CacheLookup.MappedName;
+if (IsMapped)
+  *IsMapped = true;
+  }
+} else {
+  // MISS: This is the first query, or the previous query didn't match
+  // our search start.  We will fill in our found location below, so prime
+  // the start point value.
+  CacheLookup.reset(/*NewStartIt=*/NextIt);
+
+  if (It == search_dir_begin() && FirstNonHeaderMapSearchDirIdx > 0) {
+// Handle cold misses of user includes in the presence of many header
+// maps.  We avoid searching perhaps thousands of header maps by
+// jumping directly to the correct one or jumping beyond all of them.
+auto Iter = SearchDirHeaderMapIndex.find(Filename);
+if (Iter == SearchDirHeaderMapIndex.end()) {
+  // Not in index => Skip to first SearchDir after initial header maps
+  It = search_dir_nth(FirstNonHeaderMapSearchDirIdx);
+} else {
+  // In index => Start with a specific header map
+  It = search_dir_nth(Iter->second);
+}
+  }
 }
   } else {
-// Otherwise, this is the first query, or the previous query didn't match
-// our search start.  We will fill in our found location below, so prime the
-// start point value.
 CacheLookup.reset(/*NewStartIt=*/NextIt);
   }
 
Index: clang/include/clang/Lex/HeaderSearch.h
===
--- clang/include/clang/Lex/HeaderSearch.h
+++ clang/include/clang/Lex/HeaderSearch.h
@@ -249,6 +249,14 @@
   unsigned SystemDirIdx = 0;
   bool NoCurDirSearch = false;
 
+  /// Maps HeaderMap 

[PATCH] D135370: Narrow inline namespace filtration for unqualified friend declarations

[Troy Johnson via Phabricator via cfe-commits]

troyj added a comment.

I agree. I wouldn't want to mislead people into thinking that they'll see this 
improvement on everything.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135370

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

[PATCH] D135370: Narrow inline namespace filtration for unqualified friend declarations

[Troy Johnson 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 rGb62f5fb64ede: [Clang][Sema] Narrow inline namespace 
filtration for unqualified friend… (authored by troyj).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135370

Files:
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp


Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -2273,9 +2273,11 @@
 // Filter out previous declarations that don't match the scope. The only
 // effect this has is to remove declarations found in inline namespaces
 // for friend declarations with unqualified names.
-SemaRef.FilterLookupForScope(Previous, DC, /*Scope*/ nullptr,
- /*ConsiderLinkage*/ true,
- QualifierLoc.hasQualifier());
+if (isFriend && !QualifierLoc && !FunctionTemplate) {
+  SemaRef.FilterLookupForScope(Previous, DC, /*Scope=*/ nullptr,
+   /*ConsiderLinkage=*/ true,
+   QualifierLoc.hasQualifier());
+}
   }
 
   // Per [temp.inst], default arguments in function declarations at local scope


Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -2273,9 +2273,11 @@
 // Filter out previous declarations that don't match the scope. The only
 // effect this has is to remove declarations found in inline namespaces
 // for friend declarations with unqualified names.
-SemaRef.FilterLookupForScope(Previous, DC, /*Scope*/ nullptr,
- /*ConsiderLinkage*/ true,
- QualifierLoc.hasQualifier());
+if (isFriend && !QualifierLoc && !FunctionTemplate) {
+  SemaRef.FilterLookupForScope(Previous, DC, /*Scope=*/ nullptr,
+   /*ConsiderLinkage=*/ true,
+   QualifierLoc.hasQualifier());
+}
   }
 
   // Per [temp.inst], default arguments in function declarations at local scope
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135370: Narrow inline namespace filtration for unqualified friend declarations

[Troy Johnson via Phabricator via cfe-commits]

troyj updated this revision to Diff 466072.
troyj added a comment.

Added more diff context as requested.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135370

Files:
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp


Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -2273,9 +2273,11 @@
 // Filter out previous declarations that don't match the scope. The only
 // effect this has is to remove declarations found in inline namespaces
 // for friend declarations with unqualified names.
-SemaRef.FilterLookupForScope(Previous, DC, /*Scope*/ nullptr,
- /*ConsiderLinkage*/ true,
- QualifierLoc.hasQualifier());
+if (isFriend && !QualifierLoc && !FunctionTemplate) {
+  SemaRef.FilterLookupForScope(Previous, DC, /*Scope=*/ nullptr,
+   /*ConsiderLinkage=*/ true,
+   QualifierLoc.hasQualifier());
+}
   }
 
   // Per [temp.inst], default arguments in function declarations at local scope


Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -2273,9 +2273,11 @@
 // Filter out previous declarations that don't match the scope. The only
 // effect this has is to remove declarations found in inline namespaces
 // for friend declarations with unqualified names.
-SemaRef.FilterLookupForScope(Previous, DC, /*Scope*/ nullptr,
- /*ConsiderLinkage*/ true,
- QualifierLoc.hasQualifier());
+if (isFriend && !QualifierLoc && !FunctionTemplate) {
+  SemaRef.FilterLookupForScope(Previous, DC, /*Scope=*/ nullptr,
+   /*ConsiderLinkage=*/ true,
+   QualifierLoc.hasQualifier());
+}
   }
 
   // Per [temp.inst], default arguments in function declarations at local scope
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135370: Narrow inline namespace filtration for unqualified friend declarations

[Troy Johnson via Phabricator via cfe-commits]

troyj updated this revision to Diff 465759.
troyj added a comment.

Pulled and regenerated diff. I'm not sure what the problem was and it still 
looks the same.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135370

Files:
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp


Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -2273,9 +2273,11 @@
 // Filter out previous declarations that don't match the scope. The only
 // effect this has is to remove declarations found in inline namespaces
 // for friend declarations with unqualified names.
-SemaRef.FilterLookupForScope(Previous, DC, /*Scope*/ nullptr,
- /*ConsiderLinkage*/ true,
- QualifierLoc.hasQualifier());
+if (isFriend && !QualifierLoc && !FunctionTemplate) {
+  SemaRef.FilterLookupForScope(Previous, DC, /*Scope*/ nullptr,
+   /*ConsiderLinkage*/ true,
+   QualifierLoc.hasQualifier());
+}
   }
 
   // Per [temp.inst], default arguments in function declarations at local scope


Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -2273,9 +2273,11 @@
 // Filter out previous declarations that don't match the scope. The only
 // effect this has is to remove declarations found in inline namespaces
 // for friend declarations with unqualified names.
-SemaRef.FilterLookupForScope(Previous, DC, /*Scope*/ nullptr,
- /*ConsiderLinkage*/ true,
- QualifierLoc.hasQualifier());
+if (isFriend && !QualifierLoc && !FunctionTemplate) {
+  SemaRef.FilterLookupForScope(Previous, DC, /*Scope*/ nullptr,
+   /*ConsiderLinkage*/ true,
+   QualifierLoc.hasQualifier());
+}
   }
 
   // Per [temp.inst], default arguments in function declarations at local scope
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135370: Narrow inline namespace filtration for unqualified friend declarations

[Troy Johnson via Phabricator via cfe-commits]

troyj created this revision.
troyj added a reviewer: rsmith.
Herald added a project: All.
troyj requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

rG04ba1856 
 
introduced a call to FilterLookupForScope that is expensive for very large 
translation units where it was observed to cause a 6% compile time degradation. 
As the comment states, the only effect is to "remove declarations found in 
inline namespaces for friend declarations with unqualified names." This change 
limits the call to that scenario. The test that was added by rG04ba1856 
 continues 
to pass, but the observed degradation is cut in half.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135370

Files:
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp


Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -2170,9 +2170,11 @@
 // Filter out previous declarations that don't match the scope. The only
 // effect this has is to remove declarations found in inline namespaces
 // for friend declarations with unqualified names.
-SemaRef.FilterLookupForScope(Previous, DC, /*Scope*/ nullptr,
- /*ConsiderLinkage*/ true,
- QualifierLoc.hasQualifier());
+if (isFriend && !QualifierLoc && !FunctionTemplate) {
+  SemaRef.FilterLookupForScope(Previous, DC, /*Scope*/ nullptr,
+   /*ConsiderLinkage*/ true,
+   QualifierLoc.hasQualifier());
+}
   }
 
   SemaRef.CheckFunctionDeclaration(/*Scope*/ nullptr, Function, Previous,


Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -2170,9 +2170,11 @@
 // Filter out previous declarations that don't match the scope. The only
 // effect this has is to remove declarations found in inline namespaces
 // for friend declarations with unqualified names.
-SemaRef.FilterLookupForScope(Previous, DC, /*Scope*/ nullptr,
- /*ConsiderLinkage*/ true,
- QualifierLoc.hasQualifier());
+if (isFriend && !QualifierLoc && !FunctionTemplate) {
+  SemaRef.FilterLookupForScope(Previous, DC, /*Scope*/ nullptr,
+   /*ConsiderLinkage*/ true,
+   QualifierLoc.hasQualifier());
+}
   }
 
   SemaRef.CheckFunctionDeclaration(/*Scope*/ nullptr, Function, Previous,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100502: Allow lib64 in driver test

[Troy Johnson via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8628ed0310e2: [Driver] Allow both lib64 and lib in 
rocm-detect test. (authored by troyj).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100502

Files:
  clang/test/Driver/rocm-detect.hip


Index: clang/test/Driver/rocm-detect.hip
===
--- clang/test/Driver/rocm-detect.hip
+++ clang/test/Driver/rocm-detect.hip
@@ -81,7 +81,7 @@
 
 // SPACK: ROCm installation search path (Spack 4.0.0): [[DIR:.*]]
 // SPACK: ROCm installation search path: [[CLANG:.*]]
-// SPACK: ROCm installation search path: 
[[CLANG]]/{{(llvm/)?}}lib/clang/{{[0-9.]+}}
+// SPACK: ROCm installation search path: 
[[CLANG]]/{{(llvm/)?}}lib{{[0-9]*}}/clang/{{[0-9.]+}}
 // SPACK: ROCm installation search path: /opt/rocm
 // SPACK: InstalledDir: 
[[DIR]]/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/bin
 // SPACK: Found HIP installation: 
[[DIR]]/hip-4.0.0-5f63slrursbrvfe2txrrjkynbsywsob5, version 4.0.20214-a2917cd


Index: clang/test/Driver/rocm-detect.hip
===
--- clang/test/Driver/rocm-detect.hip
+++ clang/test/Driver/rocm-detect.hip
@@ -81,7 +81,7 @@
 
 // SPACK: ROCm installation search path (Spack 4.0.0): [[DIR:.*]]
 // SPACK: ROCm installation search path: [[CLANG:.*]]
-// SPACK: ROCm installation search path: [[CLANG]]/{{(llvm/)?}}lib/clang/{{[0-9.]+}}
+// SPACK: ROCm installation search path: [[CLANG]]/{{(llvm/)?}}lib{{[0-9]*}}/clang/{{[0-9.]+}}
 // SPACK: ROCm installation search path: /opt/rocm
 // SPACK: InstalledDir: [[DIR]]/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/bin
 // SPACK: Found HIP installation: [[DIR]]/hip-4.0.0-5f63slrursbrvfe2txrrjkynbsywsob5, version 4.0.20214-a2917cd
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100502: Allow lib64 in driver test

[Troy Johnson via Phabricator via cfe-commits]

troyj updated this revision to Diff 337827.
troyj added a comment.

Correct + to *.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100502

Files:
  clang/test/Driver/rocm-detect.hip


Index: clang/test/Driver/rocm-detect.hip
===
--- clang/test/Driver/rocm-detect.hip
+++ clang/test/Driver/rocm-detect.hip
@@ -81,7 +81,7 @@
 
 // SPACK: ROCm installation search path (Spack 4.0.0): [[DIR:.*]]
 // SPACK: ROCm installation search path: [[CLANG:.*]]
-// SPACK: ROCm installation search path: 
[[CLANG]]/{{(llvm/)?}}lib/clang/{{[0-9.]+}}
+// SPACK: ROCm installation search path: 
[[CLANG]]/{{(llvm/)?}}lib{{[0-9]*}}/clang/{{[0-9.]+}}
 // SPACK: ROCm installation search path: /opt/rocm
 // SPACK: InstalledDir: 
[[DIR]]/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/bin
 // SPACK: Found HIP installation: 
[[DIR]]/hip-4.0.0-5f63slrursbrvfe2txrrjkynbsywsob5, version 4.0.20214-a2917cd


Index: clang/test/Driver/rocm-detect.hip
===
--- clang/test/Driver/rocm-detect.hip
+++ clang/test/Driver/rocm-detect.hip
@@ -81,7 +81,7 @@
 
 // SPACK: ROCm installation search path (Spack 4.0.0): [[DIR:.*]]
 // SPACK: ROCm installation search path: [[CLANG:.*]]
-// SPACK: ROCm installation search path: [[CLANG]]/{{(llvm/)?}}lib/clang/{{[0-9.]+}}
+// SPACK: ROCm installation search path: [[CLANG]]/{{(llvm/)?}}lib{{[0-9]*}}/clang/{{[0-9.]+}}
 // SPACK: ROCm installation search path: /opt/rocm
 // SPACK: InstalledDir: [[DIR]]/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/bin
 // SPACK: Found HIP installation: [[DIR]]/hip-4.0.0-5f63slrursbrvfe2txrrjkynbsywsob5, version 4.0.20214-a2917cd
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100502: Allow lib64 in driver test

[Troy Johnson via Phabricator via cfe-commits]

troyj created this revision.
troyj added a reviewer: clang.
troyj added a project: clang.
troyj requested review of this revision.
Herald added a subscriber: cfe-commits.

The test clang/test/Driver/rocm-detect.hip does not consider that "lib" may be 
"lib64" instead.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D100502

Files:
  clang/test/Driver/rocm-detect.hip


Index: clang/test/Driver/rocm-detect.hip
===
--- clang/test/Driver/rocm-detect.hip
+++ clang/test/Driver/rocm-detect.hip
@@ -81,7 +81,7 @@
 
 // SPACK: ROCm installation search path (Spack 4.0.0): [[DIR:.*]]
 // SPACK: ROCm installation search path: [[CLANG:.*]]
-// SPACK: ROCm installation search path: 
[[CLANG]]/{{(llvm/)?}}lib/clang/{{[0-9.]+}}
+// SPACK: ROCm installation search path: 
[[CLANG]]/{{(llvm/)?}}lib{{[0-9]+}}/clang/{{[0-9.]+}}
 // SPACK: ROCm installation search path: /opt/rocm
 // SPACK: InstalledDir: 
[[DIR]]/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/bin
 // SPACK: Found HIP installation: 
[[DIR]]/hip-4.0.0-5f63slrursbrvfe2txrrjkynbsywsob5, version 4.0.20214-a2917cd


Index: clang/test/Driver/rocm-detect.hip
===
--- clang/test/Driver/rocm-detect.hip
+++ clang/test/Driver/rocm-detect.hip
@@ -81,7 +81,7 @@
 
 // SPACK: ROCm installation search path (Spack 4.0.0): [[DIR:.*]]
 // SPACK: ROCm installation search path: [[CLANG:.*]]
-// SPACK: ROCm installation search path: [[CLANG]]/{{(llvm/)?}}lib/clang/{{[0-9.]+}}
+// SPACK: ROCm installation search path: [[CLANG]]/{{(llvm/)?}}lib{{[0-9]+}}/clang/{{[0-9.]+}}
 // SPACK: ROCm installation search path: /opt/rocm
 // SPACK: InstalledDir: [[DIR]]/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/bin
 // SPACK: Found HIP installation: [[DIR]]/hip-4.0.0-5f63slrursbrvfe2txrrjkynbsywsob5, version 4.0.20214-a2917cd
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63976: Allow clang -Os and -Oz to work with -flto and lld

[Troy Johnson via Phabricator via cfe-commits]

troyj added a comment.

I ran into this same problem and would like to see this patch or a similar one 
land.  Note that there is also a -Og option to consider, which currently has 
the same problem.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63976



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

[PATCH] D53608: [builtins] Build float128 soft float builtins for x86_64.

[Troy Johnson via Phabricator via cfe-commits]

troyj added a comment.

Wound up here while trying to use compiler-rt for static linking with our 
downstream compiler.  It seems that compiler-rt's current approach of only 
providing these routines on certain platforms is problematic because libgcc 
always provides them, and thus compiler-rt is not a full replacement for libgcc 
in some cases.  I also encountered the same two missing entry points.

Would very much like to see an upstream change to match the libgcc entry 
points, no matter the platform.


Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D53608



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

[PATCH] D64067: [X86][PPC] Support -mlong-double-64

[Troy Johnson via Phabricator via cfe-commits]

troyj added a comment.

Hi, we just inherited this commit at Cray when we did our latest upstream merge 
and there are a few problems with it that I'd like to point out.  Sorry that I 
was not part of the initial discussion here, but I didn't know that this work 
was being done and I had already done it for x86 in our downstream compiler a 
while ago.

1. This patch adds the -m options to f_Group, which is weird.  They should be 
in m_Group since they are target-specific.  I actually implemented them as part 
of a subgroup of m_Group so that I can take the last option from that group 
that appears on the command line.
2. This patch lacks the GNU -mlong-double-80 option for x86.
3. Instead of tracking the size in clang/Basic/LangOptions.def, I track it in 
clang/Basic/TargetOptions.h.  This is a target-specific thing afterall.

We're trying to resolve merge conflicts with this change and might be able to 
submit a patch to help reduce our differences.  Would that be of interest?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64067



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

[PATCH] D54489: Implement -frecord-gcc-switches

[Troy Johnson via Phabricator via cfe-commits]

troyj added a comment.

I realize that you're probably striving for option compatibility with gcc, but 
continuing to name it -frecord-gcc-switches when it actually records Clang 
switches seems weird to me.  It almost sounds like something that would dump 
gcc equivalents of all Clang options, or maybe let you know which Clang options 
you've used that match gcc options.  Either way, by the name -- if you aren't 
familiar with the gcc option -- it doesn't read like it records Clang options.

Would it be that bad to name it -frecord-clang-switches?  Or just 
-frecord-switches?


Repository:
  rC Clang

https://reviews.llvm.org/D54489



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

[PATCH] D9403: llvm.noalias - Clang CodeGen for local restrict-qualified pointers

[Troy Johnson via Phabricator via cfe-commits]

troyj added a comment.

Hi, I got here via llvm-dev => https://reviews.llvm.org/D9375 => 
https://reviews.llvm.org/D9403 (this) and have read through everything.  I see 
that this patch has been stalled for about a year and I would like to know its 
status.  Is it waiting on a resolution in LLVM for this problem that Jeroen 
mentioned on llvm-dev?

  "One of the bigger problems is, that some optimization passes optimize away 
some (but not all) of the llvm.noalias intrinsics in a function. Because of 
that, the alias analysis can get confused and can identify two pointers as 
not-aliasing where they should be aliasing."

That's an improper optimization though, right?  If the semantics of the 
intrinsic are that all of them or none of them must be present for correctness, 
then any optimization that removes some but not all is defective.  Is the issue 
that these optimizations cannot tell that they are doing anything wrong?

I am interested in seeing this patch merged, so I would like to know what the 
obstacles are.  Thanks.


https://reviews.llvm.org/D9403



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