[PATCH] D86763: RedeclarePropertyAccessor (clang/lib/Sema/SemaObjCProperty.cpp shipped with https://reviews.llvm.org/rG2073dd2da702baca447efaf1879cb6151e8c6100) create a synthesized property accessor

[酷酷的哀殿 via Phabricator via cfe-commits]

sunbohong created this revision.
Herald added subscribers: cfe-commits, yaxunl.
Herald added a project: clang.
sunbohong requested review of this revision.

...metadata in RewriteModernObjC.cpp.

This commit will fix https://bugs.llvm.org/show_bug.cgi?id=47330 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86763

Files:
  clang/lib/Frontend/Rewrite/RewriteModernObjC.cpp


Index: clang/lib/Frontend/Rewrite/RewriteModernObjC.cpp
===
--- clang/lib/Frontend/Rewrite/RewriteModernObjC.cpp
+++ clang/lib/Frontend/Rewrite/RewriteModernObjC.cpp
@@ -7024,27 +7024,6 @@
 
   // Build _objc_method_list for class's instance methods if needed
   SmallVector InstanceMethods(IDecl->instance_methods());
-
-  // If any of our property implementations have associated getters or
-  // setters, produce metadata for them as well.
-  for (const auto *Prop : IDecl->property_impls()) {
-if (Prop->getPropertyImplementation() == ObjCPropertyImplDecl::Dynamic)
-  continue;
-if (!Prop->getPropertyIvarDecl())
-  continue;
-ObjCPropertyDecl *PD = Prop->getPropertyDecl();
-if (!PD)
-  continue;
-if (ObjCMethodDecl *Getter = Prop->getGetterMethodDecl())
-  if (mustSynthesizeSetterGetterMethod(IDecl, PD, true /*getter*/))
-InstanceMethods.push_back(Getter);
-if (PD->isReadOnly())
-  continue;
-if (ObjCMethodDecl *Setter = Prop->getSetterMethodDecl())
-  if (mustSynthesizeSetterGetterMethod(IDecl, PD, false /*setter*/))
-InstanceMethods.push_back(Setter);
-  }
-
   Write_method_list_t_initializer(*this, Context, Result, InstanceMethods,
   "_OBJC_$_INSTANCE_METHODS_",
   IDecl->getNameAsString(), true);


Index: clang/lib/Frontend/Rewrite/RewriteModernObjC.cpp
===
--- clang/lib/Frontend/Rewrite/RewriteModernObjC.cpp
+++ clang/lib/Frontend/Rewrite/RewriteModernObjC.cpp
@@ -7024,27 +7024,6 @@
 
   // Build _objc_method_list for class's instance methods if needed
   SmallVector InstanceMethods(IDecl->instance_methods());
-
-  // If any of our property implementations have associated getters or
-  // setters, produce metadata for them as well.
-  for (const auto *Prop : IDecl->property_impls()) {
-if (Prop->getPropertyImplementation() == ObjCPropertyImplDecl::Dynamic)
-  continue;
-if (!Prop->getPropertyIvarDecl())
-  continue;
-ObjCPropertyDecl *PD = Prop->getPropertyDecl();
-if (!PD)
-  continue;
-if (ObjCMethodDecl *Getter = Prop->getGetterMethodDecl())
-  if (mustSynthesizeSetterGetterMethod(IDecl, PD, true /*getter*/))
-InstanceMethods.push_back(Getter);
-if (PD->isReadOnly())
-  continue;
-if (ObjCMethodDecl *Setter = Prop->getSetterMethodDecl())
-  if (mustSynthesizeSetterGetterMethod(IDecl, PD, false /*setter*/))
-InstanceMethods.push_back(Setter);
-  }
-
   Write_method_list_t_initializer(*this, Context, Result, InstanceMethods,
   "_OBJC_$_INSTANCE_METHODS_",
   IDecl->getNameAsString(), true);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D86671: [clang-tidy] Add new case type to check variables with Hungarian notation

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

It'll be good idea to add test case.




Comment at: clang-tools-extra/docs/ReleaseNotes.rst:70
 
+* Added: Add IdentifierNamingCheck::CaseType, CT_HungarianNotation, supporting 
naming check with Hungarian notation.
+

Please move to `Changes in existing checks` section (in alphabetical order).

I think statement should describe user-facing changes.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:76
 
-- Improved :doc:`readability-identifier-naming
+- Improved: doc:`readability-identifier-naming
   ` check.

Unrelated change.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D86671

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

[PATCH] D86000: Add an unsigned shift base sanitizer

[JF Bastien via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG82d29b397bb2: Add an unsigned shift base sanitizer (authored 
by jfb).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86000

Files:
  clang/docs/UndefinedBehaviorSanitizer.rst
  clang/include/clang/Basic/Sanitizers.def
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/test/CodeGen/unsigned-shift-base.c
  clang/test/Driver/fsanitize.c
  compiler-rt/test/ubsan/TestCases/Integer/unsigned-shift.cpp
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -157,6 +157,14 @@
 Changes to LLDB
 -
 
+Changes to Sanitizers
+-
+
+The integer sanitizer `-fsanitize=integer` now has a new sanitizer:
+`-fsanitize=unsigned-shift-base`. It's not undefined behavior for an unsigned
+left shift to overflow (i.e. to shift bits out), but it has been the source of
+bugs and exploits in certain codebases in the past.
+
 External Open Source Projects Using LLVM 12
 ===
 
Index: compiler-rt/test/ubsan/TestCases/Integer/unsigned-shift.cpp
===
--- /dev/null
+++ compiler-rt/test/ubsan/TestCases/Integer/unsigned-shift.cpp
@@ -0,0 +1,54 @@
+// RUN: %clangxx -fsanitize=unsigned-shift-base %s -o %t1 && not %run %t1 2>&1 | FileCheck %s
+// RUN: %clangxx -fsanitize=unsigned-shift-base,shift-exponent %s -o %t1 && not %run %t1 2>&1 | FileCheck %s
+
+#define shift(val, amount) ({  \
+  volatile unsigned _v = (val);\
+  volatile unsigned _a = (amount); \
+  unsigned res = _v << _a; \
+  res; \
+})
+
+int main() {
+
+  shift(0b''', 31);
+  shift(0b'''0001, 31);
+  shift(0b'''0010, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 2 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b'''0100, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 4 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b'''1000, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 8 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b'''0001, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 16 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b'''0010, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 32 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b'''0100, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 64 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b'''1000, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 128 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b''0001', 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 256 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b''0010', 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 512 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b''0100', 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 1024 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b''1000', 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 2048 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b''0001', 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 4096 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b''0010', 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 8192 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b''0100', 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 16384 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b''1000', 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 32768 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b'0001'', 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 65536 by 31 places cannot be 

[PATCH] D86671: [clang-tidy] Add new case type to check variables with Hungarian notation

[Douglas Chen via Phabricator via cfe-commits]

dougpuob updated this revision to Diff 288520.
dougpuob added a comment.
Herald added a subscriber: mgehre.

Improved suggestions from code review and clang-tidy.

1. Add keyword `const` to variables which checked via clang-tidy.
2. Add log to clang-tools-extra/docs/ReleaseNotes.rst. [Suggestion from 
Eugene.Zelenko]
3. Don't use `auto` with variables are not specified explicitly. [Suggestion 
from Eugene.Zelenko]


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D86671

Files:
  clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.h
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst

Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -67,10 +67,13 @@
 Improvements to clang-tidy
 --
 
+* Added: Add IdentifierNamingCheck::CaseType, CT_HungarianNotation, supporting naming check with Hungarian notation.
+
+
 Changes in existing checks
 ^^
 
-- Improved :doc:`readability-identifier-naming
+- Improved: doc:`readability-identifier-naming
   ` check.
 
   Added an option `GetConfigPerFile` to support including files which use
Index: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h
===
--- clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h
+++ clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h
@@ -32,7 +32,7 @@
 
   /// Derived classes should not implement any matching logic themselves; this
   /// class will do the matching and call the derived class'
-  /// GetDeclFailureInfo() and GetMacroFailureInfo() for determining whether a
+  /// getDeclFailureInfo() and GetMacroFailureInfo() for determining whether a
   /// given identifier passes or fails the check.
   void registerMatchers(ast_matchers::MatchFinder *Finder) override final;
   void
@@ -124,7 +124,7 @@
   /// Overridden by derived classes, returns information about if and how a Decl
   /// failed the check. A 'None' result means the Decl did not fail the check.
   virtual llvm::Optional
-  GetDeclFailureInfo(const StringRef , const NamedDecl *Decl,
+  getDeclFailureInfo(const StringRef , const NamedDecl *Decl,
  const SourceManager ) const = 0;
 
   /// Overridden by derived classes, returns information about if and how a
Index: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
===
--- clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
+++ clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
@@ -400,11 +400,11 @@
 
   // Get type text of variable declarations.
   const auto  = Decl->getASTContext().getSourceManager();
-  const char *szBegin = SrcMgr.getCharacterData(Decl->getBeginLoc());
-  const char *szCurr = SrcMgr.getCharacterData(Decl->getLocation());
-  const intptr_t iPtrLen = szCurr - szBegin;
-  if (iPtrLen > 0) {
-std::string Type(szBegin, iPtrLen);
+  const char *Begin = SrcMgr.getCharacterData(Decl->getBeginLoc());
+  const char *Curr = SrcMgr.getCharacterData(Decl->getLocation());
+  const intptr_t StrLen = Curr - Begin;
+  if (StrLen > 0) {
+std::string Type(Begin, StrLen);
 
 const static std::list Keywords = {
 // Qualifier
@@ -415,23 +415,23 @@
 "constexpr", "constinit", "const_cast", "consteval"};
 
 // Remove keywords
-for (const auto  : Keywords) {
-  for (size_t pos = 0;
-   (pos = Type.find(kw, pos)) != std::string::npos;) {
-Type.replace(pos, kw.length(), "");
+for (const auto  : Keywords) {
+  for (size_t Pos = 0;
+   (Pos = Type.find(Kw, Pos)) != std::string::npos;) {
+Type.replace(Pos, Kw.length(), "");
   }
 }
 
 // Replace spaces with single space
-for (size_t pos = 0; (pos = Type.find("  ", pos)) != std::string::npos;
- pos += strlen(" ")) {
-  Type.replace(pos, strlen("  "), " ");
+for (size_t Pos = 0; (Pos = Type.find("  ", Pos)) != std::string::npos;
+ Pos += strlen(" ")) {
+  Type.replace(Pos, strlen("  "), " ");
 }
 
 // Replace " *" with "*"
-for (size_t pos = 0; (pos = Type.find(" *", pos)) != std::string::npos;
- pos += strlen("*")) {
-  Type.replace(pos, strlen(" *"), "*");
+for (size_t Pos = 0; (Pos = Type.find(" 

[PATCH] D86757: Improved suggestions from code review and clang-tidy. 1) Add keyword `const` to variables which checked via clang-tidy. 2) Add log to clang-tools-extra/docs/ReleaseNotes.rst. [Euge

[Douglas Chen via Phabricator via cfe-commits]

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

...ot. [Eugene.Zelenko]

...specified explicitly.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86757

Files:
  clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.h
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst

Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -67,10 +67,13 @@
 Improvements to clang-tidy
 --
 
+* Added: Add IdentifierNamingCheck::CaseType, CT_HungarianNotation, supporting naming check with Hungarian notation.
+
+
 Changes in existing checks
 ^^
 
-- Improved :doc:`readability-identifier-naming
+- Improved: doc:`readability-identifier-naming
   ` check.
 
   Added an option `GetConfigPerFile` to support including files which use
Index: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h
===
--- clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h
+++ clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h
@@ -32,7 +32,7 @@
 
   /// Derived classes should not implement any matching logic themselves; this
   /// class will do the matching and call the derived class'
-  /// GetDeclFailureInfo() and GetMacroFailureInfo() for determining whether a
+  /// getDeclFailureInfo() and GetMacroFailureInfo() for determining whether a
   /// given identifier passes or fails the check.
   void registerMatchers(ast_matchers::MatchFinder *Finder) override final;
   void
@@ -124,7 +124,7 @@
   /// Overridden by derived classes, returns information about if and how a Decl
   /// failed the check. A 'None' result means the Decl did not fail the check.
   virtual llvm::Optional
-  GetDeclFailureInfo(const StringRef , const NamedDecl *Decl,
+  getDeclFailureInfo(const StringRef , const NamedDecl *Decl,
  const SourceManager ) const = 0;
 
   /// Overridden by derived classes, returns information about if and how a
Index: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
===
--- clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
+++ clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
@@ -400,11 +400,11 @@
 
   // Get type text of variable declarations.
   const auto  = Decl->getASTContext().getSourceManager();
-  const char *szBegin = SrcMgr.getCharacterData(Decl->getBeginLoc());
-  const char *szCurr = SrcMgr.getCharacterData(Decl->getLocation());
-  const intptr_t iPtrLen = szCurr - szBegin;
-  if (iPtrLen > 0) {
-std::string Type(szBegin, iPtrLen);
+  const char *Begin = SrcMgr.getCharacterData(Decl->getBeginLoc());
+  const char *Curr = SrcMgr.getCharacterData(Decl->getLocation());
+  const intptr_t StrLen = Curr - Begin;
+  if (StrLen > 0) {
+std::string Type(Begin, StrLen);
 
 const static std::list Keywords = {
 // Qualifier
@@ -415,23 +415,23 @@
 "constexpr", "constinit", "const_cast", "consteval"};
 
 // Remove keywords
-for (const auto  : Keywords) {
-  for (size_t pos = 0;
-   (pos = Type.find(kw, pos)) != std::string::npos;) {
-Type.replace(pos, kw.length(), "");
+for (const auto  : Keywords) {
+  for (size_t Pos = 0;
+   (Pos = Type.find(Kw, Pos)) != std::string::npos;) {
+Type.replace(Pos, Kw.length(), "");
   }
 }
 
 // Replace spaces with single space
-for (size_t pos = 0; (pos = Type.find("  ", pos)) != std::string::npos;
- pos += strlen(" ")) {
-  Type.replace(pos, strlen("  "), " ");
+for (size_t Pos = 0; (Pos = Type.find("  ", Pos)) != std::string::npos;
+ Pos += strlen(" ")) {
+  Type.replace(Pos, strlen("  "), " ");
 }
 
 // Replace " *" with "*"
-for (size_t pos = 0; (pos = Type.find(" *", pos)) != std::string::npos;
- pos += strlen("*")) {
-  Type.replace(pos, strlen(" *"), "*");
+for (size_t Pos = 0; (Pos = Type.find(" *", Pos)) != std::string::npos;
+ Pos += strlen("*")) {
+  Type.replace(Pos, strlen(" *"), "*");
 }
 
 Type = Type.erase(Type.find_last_not_of(" ") + 1);
@@ -460,7 +460,7 @@
   return;
 
 Optional MaybeFailure =
-

[PATCH] D86559: [Sema, CodeGen] Allow [[likely]] and [[unlikely]] on labels

[Staffan Tjernstrom via Phabricator via cfe-commits]

staffantj added a comment.

In D86559#2236931 , @Quuxplusone wrote:

> This feels like the wrong approach to me... but I admit that I don't know 
> what the "right" approach might be. (I doubt any right approach exists.)
>
>   if (ch == ' ') [[likely]] {
>   goto whitespace;  // A
>   } else if (ch == '\n' || ch == '\t') [[unlikely]] {
>   goto whitespace;  // B
>   } else {
>   foo();
>   }
>   [[likely]] whitespace: bar();  // C
>
> It seems like this patch would basically "copy" the `[[likely]]` attribute 
> from line C up to lines A and B, where it would reinforce the likelihood of 
> path A and (maybe?) "cancel out" the unlikelihood of path B, without actually 
> saying anything specifically about the likelihood of label C (which is surely 
> what the programmer intended by applying the attribute, right?). OTOH, I 
> can't think of any particular optimization that would care about the 
> likelihood of label C. I could imagine trying to align label C to a 4-byte 
> boundary or something, but that wouldn't be an //optimization// on any 
> platform as far as I know.

The only reason I could see for writing the above (and it's far more likely to 
be written with [[unlikely]] in my experience) is to control the probability 
that the call to bar() gets inlined or not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86559

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

[PATCH] D86559: [Sema, CodeGen] Allow [[likely]] and [[unlikely]] on labels

[Staffan Tjernstrom via Phabricator via cfe-commits]

staffantj added a comment.

In D86559#2242586 , @Mordante wrote:

> if(a)  // Attribute not allowed on a declaration,
>
>   [[likely]] int i = 5;  // but I can't think about a good use-case
>  // for this code.
>
> if(a) [[likely]] { // Good allowed on the compound statement
>
> int i = 5; // Now i seems to have a purpose
>   ...
>
> }

The use case for this becomes clearer when considering that the attribute that 
will be used 95% of the time is [[unlikely]]. You may have a constructor that 
you wish to ask the compiler to please, please, do not inline this, in a 
particular function, even if the rest of that function is either likely or 
neutral.

  if (a)
[[unlikely]] expensive_class q{};

This could be the case if expensive_class held large static members, for 
instance.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86559

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

[PATCH] D86559: [Sema, CodeGen] Allow [[likely]] and [[unlikely]] on labels

[Staffan Tjernstrom via Phabricator via cfe-commits]

staffantj added a comment.

In D86559#2242586 , @Mordante wrote:

>> That is exactly the behavior I am coming to believe we should follow. You 
>> can either write it on a compound statement that is guarded by a flow 
>> control decision (`if`/`else`/`while`) or you can write it on a label, 
>> otherwise the attribute is parsed and ignored (with a diagnostic). This 
>> feels like the right mixture of useful with understandable semantics so far, 
>> but perhaps we'll find other examples that change our minds.
>>
>> The fallthrough behavior question has one more case we may want to think 
>> about:
>>
>>   switch (x) {
>>   case 0:
>>   case 1:
>>   [[likely]] case 2: break;
>>   [[unlikely]] default:
>>   }
>>
>> Does this mark cases `0` and `1` as being likely or not? I could see users 
>> wanting to use shorthand rather than repeat themselves on all the cases. 
>> However, I'm also not certain whether there would be any performance impact 
>> if we marked only `case 2` as likely and left cases `0` and `1` with default 
>> likelihood. My gut feeling is that this should only mark `case 2`, but 
>> others may have different views.
>
> Not according to the standard: "A path of execution includes a label if and 
> only if it contains a jump to that label."
> However for an if statement I use 2 weights: 2000 for likely, 1 for unlikely. 
> (These can be configured by a command-line option already used for 
> `__builtin_expect`.)
> For a switch I intent to use 3 weights: 2000 for likely, (2000 + 1)/2 for no 
> likelihood, 1 for unlikely. `__builtin_expect` doesn't have a 'neutral' 
> values since in a switch you can only mark one branch as likely. Instead of 
> adding a new option I picked the midpoint.
> So the weights in your example would be:
>
>   > switch (x) {
>   case 0:  // 1000
>   case 1:  // 1000
>   [[likely]] case 2: break; // 2000
>   [[unlikely]] default: // 1
>}
>
> In this case the user could also write:
>
>   > switch (x) {
>   case 0:  // 1000
>   case 1:  // 1000
>   case 2: break; // 1000
>   [[unlikely]] default: // 1
>}
>
> where the values 0, 1, 2 still would be more likely than the default 
> statement. Obviously this will be documented when I implement the switch.
> How do you feel about this approach?

As one of the SG14 industry members driving this, I'm firmly in the camp that 
this is what we're expecting. In the first case the 1/2 case are "neutral". 
This is a very explicit, and local, marker. Anything else makes it so vague as 
to be unusable for fine tuned code.

I should also make the point that we are not talking about a feature that is 
expected, or indeed should be, used by anyone other than someone with an 
exceedingly good understanding of what is going on. This is not a "teach 
everyone about it, it's safe" feature. It's there to produce a very 
fine-grained control in those cases where it really matters, and where 
profiling-guided optimizations would produce exactly the wrong result. Using 
this feature should be an automatic "is this needed" question in a code review. 
It is needed sometimes, just rarely.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86559

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

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

[Wenlei He via Phabricator via cfe-commits]

wenlei added a comment.

In D86156#2243393 , @asbirlea wrote:

> Diff looks reasonable at this point. Thank you for the patch!
> Please wait on @nikic for compile-time impact or additional feedback.
>
> Just out of curiosity, in D65060 , it was 
> mentioned that using BFI got you ~7% improvement for a CPU related metric 
> (@wenlei). Are you seeing benefits from this patch? And which pass manager 
> are you using?

Thanks for quick review. We got the perf improvement from preventing a bad 
hoisting in a critical loop using BFI. Originally it was with legacy pass 
manager, hence the invalidation issue with new pass manager wasn't caught from 
our usage. This change was made as an internal patch for perf parity when we 
moved to new pass manager, and we've been using it for a while now.


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

https://reviews.llvm.org/D86156

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

[clang-tools-extra] 3776999 - [clang-query][NFC] Silence a few lint warnings

[Nathan James via cfe-commits]

Author: Nathan James
Date: 2020-08-28T01:06:46+01:00
New Revision: 3776999b494d05abc87a52bf8d5317fd3d68a8ab

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

LOG: [clang-query][NFC] Silence a few lint warnings

Added: 


Modified: 
clang-tools-extra/clang-query/tool/ClangQuery.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-query/tool/ClangQuery.cpp 
b/clang-tools-extra/clang-query/tool/ClangQuery.cpp
index 0c471def2e14..31c7f12251c9 100644
--- a/clang-tools-extra/clang-query/tool/ClangQuery.cpp
+++ b/clang-tools-extra/clang-query/tool/ClangQuery.cpp
@@ -110,31 +110,33 @@ int main(int argc, const char **argv) {
   ClangTool Tool(OptionsParser->getCompilations(),
  OptionsParser->getSourcePathList());
   std::vector> ASTs;
-  int Status = Tool.buildASTs(ASTs);
   int ASTStatus = 0;
-  if (Status == 1) {
-// Building ASTs failed.
+  switch (Tool.buildASTs(ASTs)) {
+  case 0:
+break;
+  case 1: // Building ASTs failed.
 return 1;
-  } else if (Status == 2) {
+  case 2:
 ASTStatus |= 1;
 llvm::errs() << "Failed to build AST for some of the files, "
  << "results may be incomplete."
  << "\n";
-  } else {
-assert(Status == 0 && "Unexpected status returned");
+break;
+  default:
+llvm_unreachable("Unexpected status returned");
   }
 
   QuerySession QS(ASTs);
 
   if (!Commands.empty()) {
-for (auto I = Commands.begin(), E = Commands.end(); I != E; ++I) {
-  QueryRef Q = QueryParser::parse(*I, QS);
+for (auto  : Commands) {
+  QueryRef Q = QueryParser::parse(Command, QS);
   if (!Q->run(llvm::outs(), QS))
 return 1;
 }
   } else if (!CommandFiles.empty()) {
-for (auto I = CommandFiles.begin(), E = CommandFiles.end(); I != E; ++I) {
-  if (runCommandsInFile(argv[0], *I, QS))
+for (auto  : CommandFiles) {
+  if (runCommandsInFile(argv[0], CommandFile, QS))
 return 1;
 }
   } else {



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

[PATCH] D86751: Add new warning for compound punctuation tokens that are split across macro expansions or split by whitespace.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

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

For example:

FOO({})

... forms a statement-expression after macro expansion. This warning
applies to '({' and '})' delimiting statement-expressions, '[[' and ']]'
delimiting attributes, and '::*' introducing a pointer-to-member.

The warning for forming these compound tokens across macro expansions
(or across files!) is enabled by default; the warning for whitespace
within the tokens is not, but is included in -Wall.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86751

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Parse/Parser.h
  clang/lib/Headers/altivec.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Parse/Parser.cpp
  clang/test/Misc/warning-wall.c
  clang/test/Parser/compound-token-split.cpp

Index: clang/test/Parser/compound-token-split.cpp
===
--- /dev/null
+++ clang/test/Parser/compound-token-split.cpp
@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 %s -verify
+// RUN: %clang_cc1 %s -verify=expected,space -Wcompound-token-split
+// RUN: %clang_cc1 %s -verify=expected,space -Wall
+
+#ifdef LSQUARE
+[ // expected-note {{second '[' token is here}}
+#else
+
+#define VAR(type, name, init) type name = (init)
+
+void f() {
+  VAR(int, x, {}); // #1
+  // expected-warning@#1 {{'(' and '{' tokens introducing statement expression appear in different macro expansion contexts}}
+  // expected-note-re@#1 ^}}'{' token is here}}
+  //
+  // FIXME: It would be nice to suppress this when we already warned about the opening '({'.
+  // expected-warning@#1 {{'}' and ')' tokens terminating statement expression appear in different macro expansion contexts}}
+  // expected-note-re@#1 ^}}')' token is here}}
+  //
+  // expected-error@#1 {{cannot initialize a variable of type 'int' with an rvalue of type 'void'}}
+}
+
+#define RPAREN )
+
+int f2() {
+  int n = ({ 1; }RPAREN; // expected-warning {{'}' and ')' tokens terminating statement expression appear in different macro expansion contexts}} expected-note {{')' token is here}}
+  return n;
+}
+
+[ // expected-warning-re ^}}'[' tokens introducing attribute appear in different source files}}
+#define LSQUARE
+#include __FILE__
+  noreturn ]]  void g();
+
+[[noreturn] ] void h(); // space-warning-re ^}}']' tokens terminating attribute are separated by whitespace}}
+
+struct X {};
+int X:: *p; // space-warning {{'::' and '*' tokens forming pointer to member type are separated by whitespace}}
+
+#endif
Index: clang/test/Misc/warning-wall.c
===
--- clang/test/Misc/warning-wall.c
+++ clang/test/Misc/warning-wall.c
@@ -94,6 +94,9 @@
 CHECK-NEXT:  -Wswitch
 CHECK-NEXT:  -Wswitch-bool
 CHECK-NEXT:  -Wmisleading-indentation
+CHECK-NEXT:  -Wcompound-token-split
+CHECK-NEXT:-Wcompound-token-split-by-macro
+CHECK-NEXT:-Wcompound-token-split-by-space
 
 
 CHECK-NOT:-W
Index: clang/lib/Parse/Parser.cpp
===
--- clang/lib/Parse/Parser.cpp
+++ clang/lib/Parse/Parser.cpp
@@ -227,6 +227,39 @@
   return true;
 }
 
+void Parser::checkCompoundToken(SourceLocation FirstTokLoc,
+tok::TokenKind FirstTokKind, CompoundToken Op) {
+  if (FirstTokLoc.isInvalid())
+return;
+  SourceLocation SecondTokLoc = Tok.getLocation();
+
+  // We expect both tokens to come from the same FileID.
+  FileID FirstID = PP.getSourceManager().getFileID(FirstTokLoc);
+  FileID SecondID = PP.getSourceManager().getFileID(SecondTokLoc);
+  if (FirstID != SecondID) {
+Diag(FirstTokLoc, (FirstTokLoc.isMacroID() || SecondTokLoc.isMacroID())
+  ? diag::warn_compound_token_split_by_macro
+  : diag::warn_compound_token_split_by_file)
+<< (FirstTokKind == Tok.getKind()) << FirstTokKind << Tok.getKind()
+<< static_cast(Op) << SourceRange(FirstTokLoc);
+Diag(SecondTokLoc, diag::note_compound_token_split_second_token_here)
+<< (FirstTokKind == Tok.getKind()) << Tok.getKind()
+<< SourceRange(SecondTokLoc);
+return;
+  }
+
+  // We expect the tokens to abut.
+  if (Tok.hasLeadingSpace()) {
+SourceLocation SpaceLoc = PP.getLocForEndOfToken(FirstTokLoc);
+if (SpaceLoc.isInvalid())
+  SpaceLoc = FirstTokLoc;
+Diag(SpaceLoc, diag::warn_compound_token_split_by_whitespace)
+<< (FirstTokKind == Tok.getKind()) << FirstTokKind << Tok.getKind()
+<< static_cast(Op) << SourceRange(FirstTokLoc, SecondTokLoc);
+return;
+  }
+}
+
 //===--===//
 // 

[PATCH] D67112: [Sema] Introduce function reference conversion, NFC

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.

Ping. Maybe this change is too silly, but I think it's better to be precise and 
not muddle qualification conversions together with casting away `noexcept`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67112

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

[PATCH] D67113: ICK_Function_Conversion is a third kind conversion

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a reviewer: aaron.ballman.
aaronpuchert added a comment.

Maybe this is to trivial for a review. The comment on 
`StandardConversionSequence::Third` in clang/Sema/Overload.h says

> The third conversion can be a qualification conversion or a function 
> conversion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67113

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

[PATCH] D68720: Support -fstack-clash-protection for x86

[Eli Friedman via Phabricator via cfe-commits]

efriedma closed this revision.
efriedma added a comment.

It was reverted, then re-landed.  It's in trunk now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68720

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

[PATCH] D68720: Support -fstack-clash-protection for x86

[Kees Cook via Phabricator via cfe-commits]

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

Sorry if I missed something here, but why is this marked as "Closed"? It seems 
like the feature has still not landed (i.e. it got reverted).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68720

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

[PATCH] D84603: Thread safety analysis: More consistent warning message

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added inline comments.



Comment at: clang/lib/Analysis/ThreadSafety.cpp:1644
 if (!LDat) {
-  Analyzer->Handler.handleMutexNotHeld("", D, POK, Cp.toString(),
-   LK_Shared, Loc);
+  Analyzer->Handler.handleMutexNotHeld("negative capability", D, POK,
+   Cp.toString(), LK_Shared, Loc);

aaron.ballman wrote:
> aaronpuchert wrote:
> > aaron.ballman wrote:
> > > aaronpuchert wrote:
> > > > aaron.ballman wrote:
> > > > > It's a bit odd that we aren't using `DiagKind` as below, I assume 
> > > > > that's because this is a negative test and the others are positive 
> > > > > tests, but doesn't this introduce a terminology difference where a 
> > > > > positive failure may call it a mutex and a negative failure may call 
> > > > > it a negative capability? Should this be hooked in to 
> > > > > `ClassifyDiagnostic()` (perhaps we need a 
> > > > > `ClassifyNegativeDiagnostic()`?
> > > > My thinking was that whatever the positive capability is called, we 
> > > > should only talk about a "negative capability" instead of a "negative 
> > > > mutex" or a "negative role". Also because not holding a capability is 
> > > > in some way its own kind of capability.
> > > I may still be confused or thinking of this differently, but I would 
> > > assume that a negative mutex would be a mutex that's explicitly not held, 
> > > which you may want to ensure on a function boundary to avoid deadlock. 
> > > From that, I'd have guessed we would want the diagnostic to read `cannot 
> > > call function 'bar' while mutex 'mu' is held` or `calling function 'bar' 
> > > requires mutex 'mu' to not be held` because that's more clear than 
> > > talking about negative capabilities (when the user is thinking in terms 
> > > of mutexes which are or aren't held).
> > Now I get it. I don't see an issue with that, but we need to distinguish 
> > between `EXCLUDES(mu)` and `REQUIRES(!mu)`. The former will produce "cannot 
> > call function 'bar' while mutex 'mu' is held" and we probably want the 
> > latter to produce a different warning message.
> > 
> > Now one argument for the existing scheme remains that with 
> > `-Wthread-safety-negative`, if you see a warning like "acquiring mutex 'mu' 
> > requires negative capability '!mu'" on a lock operation, you know you can 
> > fix that by adding `REQUIRES(!mu)` to your function.
> > 
> > If we say "warning: acquiring mutex 'mu' requires mutex 'mu' not to be 
> > held" it might not be as clear what we want.
> > Now I get it. I don't see an issue with that, but we need to distinguish 
> > between EXCLUDES(mu) and REQUIRES(!mu). The former will produce "cannot 
> > call function 'bar' while mutex 'mu' is held" and we probably want the 
> > latter to produce a different warning message.
> 
> Ahhh, that's a good point.
> 
> > Now one argument for the existing scheme remains that with 
> > -Wthread-safety-negative, if you see a warning like "acquiring mutex 'mu' 
> > requires negative capability '!mu'" on a lock operation, you know you can 
> > fix that by adding REQUIRES(!mu) to your function.
> >
> > If we say "warning: acquiring mutex 'mu' requires mutex 'mu' not to be 
> > held" it might not be as clear what we want.
> 
> Hm, that's a good point as well.
> 
> Now that I understand the situation a bit better, I will be happy with either 
> route, so I leave the decision in your capable hands. (If we get it wrong, we 
> can always change the diagnostics later.) Do you have a preferred approach?
There are basically two warnings related to negative capabilities: one is about 
acquiring a capability without holding a negative capability, the other about 
calling a function without holding a negative capability. Negative capabilities 
can't be acquired or released, nor do they protect anything.

The other warning message also says "negative capability '!mu'", but mentions 
the original capability kind earlier:

```
def warn_acquire_requires_negative_cap : Warning<
  "acquiring %0 '%1' requires negative capability '%2'">,
```

I think that makes sense because there is a relation between the "positive" 
capability of whatever kind that we want to acquire and the negative capability 
that we need. The situation here is different: I just have some `CallExpr` to a 
function with `REQUIRES(!...)`.

For that we have two different warnings, depending on whether we know the 
capability to be held or not:

```
void foo() REQUIRES(!mu);
void bar() REQUIRES(!mu) {
  mu.Lock();
  foo();// warning: cannot call function 'foo' while mutex 'mu' is held
  mu.Unlock();
}
void baz() {
  foo();// warning: calling function 'foo' requires holding [negative 
capability] '!mu'
}
```

The warning here is about the `baz` case where the analysis doesn't know 
whether I hold the capability (e.g. it could be acquired higher in the stack 
without the function being annotated), but 

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

[Alina Sbirlea via Phabricator via cfe-commits]

asbirlea accepted this revision.
asbirlea added a comment.

Diff looks reasonable at this point. Thank you for the patch!
Please wait on @nikic for compile-time impact or additional feedback.

Just out of curiosity, in D65060 , it was 
mentioned that using BFI got you ~7% improvement for a CPU related metric 
(@wenlei). Are you seeing benefits from this patch? And which pass manager are 
you using?


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

https://reviews.llvm.org/D86156

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

[PATCH] D85214: [OpenMP] Ensure testing for versions 4.5 and default - Part 3

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added a comment.

In D85214#2243167 , @saiislam wrote:

> In D85214#2243160 , @gribozavr2 
> wrote:
>
>> @saiislam This change seems to have broken a buildbot, please take a look at 
>> your earliest convenience.
>>
>> Before: 
>> http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/35354
>>
>> After: 
>> http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/35355
>
> Is it still crashing after this fix: 
> https://github.com/llvm/llvm-project/commit/a1ac047b3453f205eacc36adc787ac31b952a502
>  ?

Thanks, it is green now: 
http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85214

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

[PATCH] D86373: [analyzer] Add modeling for unique_ptr move constructor

[Artem Dergachev via Phabricator via cfe-commits]

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

Yup, that's more like it!~


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86373

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

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

[Di Mo via Phabricator via cfe-commits]

modimo added inline comments.



Comment at: llvm/include/llvm/Transforms/Scalar/LoopPassManager.h:273
   : nullptr;
+BlockFrequencyInfo *BFI = UseBlockFrequencyInfo
+  ? ((F))

asbirlea wrote:
> Add `&& F.hasProfileData()` check here, in the NPM as well?
Makes sense, added.


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

https://reviews.llvm.org/D86156

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

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

[Di Mo via Phabricator via cfe-commits]

modimo updated this revision to Diff 288477.
modimo added a comment.

Condition usage of BFI to PGO in newPM as well


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

https://reviews.llvm.org/D86156

Files:
  llvm/include/llvm/Analysis/BlockFrequencyInfo.h
  llvm/include/llvm/Analysis/BlockFrequencyInfoImpl.h
  llvm/include/llvm/Analysis/LoopAnalysisManager.h
  llvm/include/llvm/Transforms/Scalar/LoopPassManager.h
  llvm/lib/Analysis/BlockFrequencyInfo.cpp
  llvm/lib/Passes/PassBuilder.cpp
  llvm/lib/Transforms/Scalar/LICM.cpp
  llvm/lib/Transforms/Scalar/LoopDistribute.cpp
  llvm/lib/Transforms/Scalar/LoopLoadElimination.cpp
  llvm/lib/Transforms/Scalar/LoopUnswitch.cpp
  llvm/lib/Transforms/Utils/LoopVersioning.cpp
  llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
  llvm/test/Other/opt-O2-pipeline.ll
  llvm/test/Other/opt-O3-pipeline-enable-matrix.ll
  llvm/test/Other/opt-O3-pipeline.ll
  llvm/test/Other/opt-Os-pipeline.ll
  llvm/unittests/Transforms/Scalar/LoopPassManagerTest.cpp

Index: llvm/unittests/Transforms/Scalar/LoopPassManagerTest.cpp
===
--- llvm/unittests/Transforms/Scalar/LoopPassManagerTest.cpp
+++ llvm/unittests/Transforms/Scalar/LoopPassManagerTest.cpp
@@ -9,7 +9,10 @@
 #include "llvm/Transforms/Scalar/LoopPassManager.h"
 #include "llvm/Analysis/AliasAnalysis.h"
 #include "llvm/Analysis/AssumptionCache.h"
+#include "llvm/Analysis/BlockFrequencyInfo.h"
+#include "llvm/Analysis/BranchProbabilityInfo.h"
 #include "llvm/Analysis/MemorySSA.h"
+#include "llvm/Analysis/PostDominators.h"
 #include "llvm/Analysis/ScalarEvolution.h"
 #include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/Analysis/TargetTransformInfo.h"
@@ -294,6 +297,9 @@
 // those.
 FAM.registerPass([&] { return AAManager(); });
 FAM.registerPass([&] { return AssumptionAnalysis(); });
+FAM.registerPass([&] { return BlockFrequencyAnalysis(); });
+FAM.registerPass([&] { return BranchProbabilityAnalysis(); });
+FAM.registerPass([&] { return PostDominatorTreeAnalysis(); });
 FAM.registerPass([&] { return MemorySSAAnalysis(); });
 FAM.registerPass([&] { return ScalarEvolutionAnalysis(); });
 FAM.registerPass([&] { return TargetLibraryAnalysis(); });
Index: llvm/test/Other/opt-Os-pipeline.ll
===
--- llvm/test/Other/opt-Os-pipeline.ll
+++ llvm/test/Other/opt-Os-pipeline.ll
@@ -97,6 +97,8 @@
 ; CHECK-NEXT: Loop Pass Manager
 ; CHECK-NEXT:   Rotate Loops
 ; CHECK-NEXT: Memory SSA
+; CHECK-NEXT: Lazy Branch Probability Analysis
+; CHECK-NEXT: Lazy Block Frequency Analysis
 ; CHECK-NEXT: Loop Pass Manager
 ; CHECK-NEXT:   Loop Invariant Code Motion
 ; CHECK-NEXT:   Unswitch loops
@@ -154,6 +156,8 @@
 ; CHECK-NEXT: LCSSA Verifier
 ; CHECK-NEXT: Loop-Closed SSA Form Pass
 ; CHECK-NEXT: Scalar Evolution Analysis
+; CHECK-NEXT: Lazy Branch Probability Analysis
+; CHECK-NEXT: Lazy Block Frequency Analysis
 ; CHECK-NEXT: Loop Pass Manager
 ; CHECK-NEXT:   Loop Invariant Code Motion
 ; CHECK-NEXT: Post-Dominator Tree Construction
@@ -256,10 +260,10 @@
 ; CHECK-NEXT:   LCSSA Verifier
 ; CHECK-NEXT:   Loop-Closed SSA Form Pass
 ; CHECK-NEXT:   Scalar Evolution Analysis
-; CHECK-NEXT:   Loop Pass Manager
-; CHECK-NEXT: Loop Invariant Code Motion
 ; CHECK-NEXT:   Lazy Branch Probability Analysis
 ; CHECK-NEXT:   Lazy Block Frequency Analysis
+; CHECK-NEXT:   Loop Pass Manager
+; CHECK-NEXT: Loop Invariant Code Motion
 ; CHECK-NEXT:   Optimization Remark Emitter
 ; CHECK-NEXT:   Warn about non-applied transformations
 ; CHECK-NEXT:   Alignment from assumptions
Index: llvm/test/Other/opt-O3-pipeline.ll
===
--- llvm/test/Other/opt-O3-pipeline.ll
+++ llvm/test/Other/opt-O3-pipeline.ll
@@ -116,6 +116,8 @@
 ; CHECK-NEXT: Loop Pass Manager
 ; CHECK-NEXT:   Rotate Loops
 ; CHECK-NEXT: Memory SSA
+; CHECK-NEXT: Lazy Branch Probability Analysis
+; CHECK-NEXT: Lazy Block Frequency Analysis
 ; CHECK-NEXT: Loop Pass Manager
 ; CHECK-NEXT:   Loop Invariant Code Motion
 ; CHECK-NEXT:   Unswitch loops
@@ -173,6 +175,8 @@
 ; CHECK-NEXT: LCSSA Verifier
 ; CHECK-NEXT: Loop-Closed SSA Form Pass
 ; CHECK-NEXT: Scalar Evolution Analysis
+; CHECK-NEXT: Lazy Branch Probability Analysis
+; CHECK-NEXT: Lazy Block Frequency Analysis
 ; CHECK-NEXT: Loop Pass Manager
 ; CHECK-NEXT:   Loop Invariant Code Motion
 ; CHECK-NEXT: Post-Dominator Tree Construction
@@ -275,10 +279,10 @@
 ; CHECK-NEXT:   LCSSA Verifier
 ; CHECK-NEXT:   Loop-Closed SSA Form Pass
 ; CHECK-NEXT:   Scalar Evolution Analysis
-; CHECK-NEXT:   

[PATCH] D86027: [analyzer] Add bool operator modeling for unque_ptr

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.

I have no other concerns, i think this is good to go!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86027

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

[PATCH] D86743: [analyzer] Ignore VLASizeChecker case that could cause crash

[Vince Bridgers via Phabricator via cfe-commits]

vabridgers created this revision.
vabridgers added reviewers: balazske, NoQ, martong, baloghadamsoftware, 
Szelethus, gamesh411.
Herald added subscribers: cfe-commits, ASDenysPetrov, Charusso, donat.nagy, 
mikhail.ramalho, a.sidorin, rnkovacs, szepet, xazax.hun.
Herald added a project: clang.
vabridgers requested review of this revision.

See https://bugs.llvm.org/show_bug.cgi?id=47272. The checker does not
yet comprehend constraints involving multiple symbols, so it's
possible to calculate a VLA size that's causes an assert. A LIT is added to
catch regressions, and this change simply bails if a size is calculated
that is not known.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86743

Files:
  clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
  clang/test/Analysis/vla.c


Index: clang/test/Analysis/vla.c
===
--- clang/test/Analysis/vla.c
+++ clang/test/Analysis/vla.c
@@ -151,3 +151,22 @@
   foo();
   }
 } // no-crash
+
+
+// https://bugs.llvm.org/show_bug.cgi?id=47272
+// similar to the above case, just different enough to have not
+// been covered.
+// Just don't crash.
+int bb;
+int c() {
+  int d = 0;
+  int sum = 0;
+  while (bb) {
+int count = bb - d;
+int e[count];
+if (count > 4)
+  sum++;
+d++;
+  }
+  return sum;
+} // no-crash
Index: clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
@@ -298,8 +298,11 @@
 DefinedOrUnknownSVal SizeIsKnown = SVB.evalEQ(State, DynSize, 
*ArraySizeNL);
 State = State->assume(SizeIsKnown, true);
 
-// Assume should not fail at this point.
-assert(State);
+// State may not be valid since constraints do not comprehend expressions
+// used for VLAs. If State is null, just silently return.
+// See https://bugs.llvm.org/show_bug.cgi?id=47272.
+if (!State)
+  return;
   }
 
   // Remember our assumptions!


Index: clang/test/Analysis/vla.c
===
--- clang/test/Analysis/vla.c
+++ clang/test/Analysis/vla.c
@@ -151,3 +151,22 @@
   foo();
   }
 } // no-crash
+
+
+// https://bugs.llvm.org/show_bug.cgi?id=47272
+// similar to the above case, just different enough to have not
+// been covered.
+// Just don't crash.
+int bb;
+int c() {
+  int d = 0;
+  int sum = 0;
+  while (bb) {
+int count = bb - d;
+int e[count];
+if (count > 4)
+  sum++;
+d++;
+  }
+  return sum;
+} // no-crash
Index: clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
@@ -298,8 +298,11 @@
 DefinedOrUnknownSVal SizeIsKnown = SVB.evalEQ(State, DynSize, *ArraySizeNL);
 State = State->assume(SizeIsKnown, true);
 
-// Assume should not fail at this point.
-assert(State);
+// State may not be valid since constraints do not comprehend expressions
+// used for VLAs. If State is null, just silently return.
+// See https://bugs.llvm.org/show_bug.cgi?id=47272.
+if (!State)
+  return;
   }
 
   // Remember our assumptions!
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D86169: Initial support for letting plugins perform custom parsing of attribute arguments.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D86169#2242673 , 
@jonathan.protzenko wrote:

> In D86169#2241830 , @aaron.ballman 
> wrote:
>
>>> - Upon encountering `[[ attr(X) ]]` where `attr` is a plugin-handled 
>>> attribute: clang parses `X` as token soup, only caring about finding a 
>>> matching parenthesis
>>
>> Just to be clear, what you mean is to lex all of the tokens comprising `X` 
>> and store them off for later?
>
> Correct.
>
>>> Does this stand any chance of working?
>>
>> This is along the same lines of what I was thinking, but there is one 
>> potential snag. Clang layers our library components and Sema should never 
>> refer back to the Parser (it doesn't link the library in). Because you have 
>> an odd requirement where you need to do further parsing during the semantics 
>> stage, you may run into awkwardness from this. Also, you may have to worry 
>> about needing this functionality recursively. e.g., `int x [[your_attr(int y 
>> [[your_attr(int z;)]];)]];` which could be... interesting.
>
> I guess I'd have to i) call the action to create, say, a Decl, then later ii) 
> call the plugin and extend the Decl with fresh attributes, assuming the 
> attribute vector on the Decl can be grown *after* the Decl is created -- 
> hopefully this can be driven from the parser, without a back-edge in the 
> dependency graph? Or maybe I'm missing something?

I can't think of another case where we've needed to parse from within Sema, so 
I'm not certain what issues you will or won't run into. The concern I have is 
that the parser calls into sema for semantic checking, so this causes a 
dependency cycle that the components may not handle well (or they may handle it 
just fine). e.g., `int x [[whatever(int y;)]];` the parser will parse the `int 
x [[...]]` bit, then call into Sema to check the attribute, which would then 
spawn a parser to parse the `int y;` bit, which calls into Sema again for 
semantic handling. It's that last step that may be an issue (for instance, it 
may introduce a new AST node for `int y;` along with the semantic checking of 
the declaration).

> Do you have any example somewhere of how to store tokens and re-fill the lex 
> buffer with them later on? This is much more involved than my current patch 
> so may take me a while to figure out.

We don't really have examples of it quite in this way, but we do have a 
`TokenLexer` object that you would likely need to use. I think you'd squirrel 
away the tokens when parsing the attribute argument clause, and then make a new 
`TokenLexer` that is used by the parser's `Preprocessor` member for parsing the 
tokens. This gets used when expanding tokens from `_Pragma`, for instance.

>>> Bonus question: seeing `diagAppertainsToDecl`, the name of this function 
>>> led me to believe that for now, plugins could only handle attributes 
>>> attached to declarations. Is this the case?
>>
>> Correct, the plugin system does not yet handle type or statement attributes 
>> (it was focusing on declaration attributes first, which are the lion's share 
>> of attributes in Clang).
>
> Ok, I was asking because if there's no plans to handle type or statement 
> attributes via a plugin, then we're somewhat over-engineering this.

There are plans to support those, but I'm not certain if anyone is actively 
working on adding that support. I want to avoid adding anything that would make 
supporting statement and type attributes harder, and I'd be worried that 
passing in the `Declarator *` would make that project harder to get started on.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86169

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

[PATCH] D86065: [SVE] Make ElementCount members private

[Christopher Tetreault via Phabricator via cfe-commits]

ctetreau accepted this revision.
ctetreau added a comment.

I think this is good to go as is. Assuming @paulwalker-arm is satisfied with 
leaving operator/ as is, then LGTM.




Comment at: llvm/include/llvm/Support/TypeSize.h:66
+
+  ElementCount /=(unsigned RHS) {
+Min /= RHS;

paulwalker-arm wrote:
> If you add an assert that the divide is lossless (i.e. MIN % RHS == 0) then 
> asserts like:
> ```
> assert(EltCnt.isKnownEven() && "Splitting vector, but not in half!");
> ```
> are no longer required.  Plus those places which are not checking for 
> lossless division will be automatically protected.  This feels like a 
> sensible default to me.  If somebody wants a truncated result, they can do 
> the maths using getKnownMinValue().
I would prefer that this not be done. This would make this function non-total 
in an unrecoverable way, and would force everybody to write a bunch of tedious 
error handling code, even if the normal integer division behavior would have 
been fine:

```
ElementCount res = LHS.getKnownMinValue() % RHS.getKnownMinValue() == 0 ? LHS / 
RHS : SomeOtherThing;
```

Everybody knows how integer division works, so I think the lossy behavior will 
not surprise anybody. An assert might.


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

https://reviews.llvm.org/D86065

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

[PATCH] D85743: [CodeGen][AArch64] Support arm_sve_vector_bits attribute

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added a comment.

In D85743#2242931 , @leonardchan wrote:

> Hi! The `attr-arm-sve-vector-bits-call.c` test seems to be failing on our 
> clang builders:
>
> Could you take a look? Thanks.
>
> Builder: 
> https://luci-milo.appspot.com/p/fuchsia/builders/ci/clang-linux-x64/b8870800848452818112?

Sorry about that, I've reverted it (commit 2e7041f 
) whilst I 
investigate. Thanks for raising.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85743

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

[PATCH] D85214: [OpenMP] Ensure testing for versions 4.5 and default - Part 3

[Saiyedul Islam via Phabricator via cfe-commits]

saiislam added a comment.

In D85214#2243160 , @gribozavr2 wrote:

> @saiislam This change seems to have broken a buildbot, please take a look at 
> your earliest convenience.
>
> Before: 
> http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/35354
>
> After: http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/35355

Is it still crashing after this fix: 
https://github.com/llvm/llvm-project/commit/a1ac047b3453f205eacc36adc787ac31b952a502
 ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85214

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

[PATCH] D86065: [SVE] Make ElementCount members private

[Christopher Tetreault via Phabricator via cfe-commits]

ctetreau added a comment.

In D86065#2241146 , @david-arm wrote:

> Hi @ctetreau, ok for now I'm going to completely remove the operators and 
> revert the code using those operators to how it was before. ...

This is probably for the best.

In D86065#2241146 , @david-arm wrote:

> ... I'm not sure what you mean about the predicate functions ...

I'm referring to providing some built in way to std::sort a collection of 
ElementCount or have a std::set. By default, C++ wants to use 
operator< for this, which I believe was the original motivation for the 
operator being here in the first place. I think it's reasonable for 
ElementCount to provide a built-in function to establish an ordering for these 
purposes, but the function should be named such that nobody thinks the function 
is intended to be the mathematical relation.


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

https://reviews.llvm.org/D86065

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

[PATCH] D85214: [OpenMP] Ensure testing for versions 4.5 and default - Part 3

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added a comment.

@saiislam This change seems to have broken a buildbot, please take a look at 
your earliest convenience.

Before: http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/35354

After: http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/35355


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85214

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

[PATCH] D86660: Modifying ImportDeclContext(...) to ensure that we also handle the case when the FieldDecl is an ArrayType whose ElementType is a RecordDecl

[Shafik Yaghmour via Phabricator via cfe-commits]

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



Comment at: clang/lib/AST/ASTImporter.cpp:1755-1759
+  QualType FromTy = ArrayFrom->getElementType();
+  QualType ToTy = ArrayTo->getElementType();
+
+  FromRecordDecl = FromTy->getAsRecordDecl();
+  ToRecordDecl = ToTy->getAsRecordDecl();

martong wrote:
> labath wrote:
> > What about 2- or n-dimensional arrays?
> @labath, this is a very good question! And made me realize that D71378 is 
> fundamentally flawed (@shafik, please take no offense). Let me explain.
> 
> So, with D71378, we added the `if (ImportedOrErr) { ... }` block to import 
> definitions specifically of fields' Record types. But we forget to handle 
> arrays. Now we may forget to handle multidimensional arrays ... and we may 
> forget to handle other language constructs. So, we would finally end up in 
> re-implementing the logic of `ASTNodeImporter::VisitFieldDecl`.
> 
> So all this should have been handled properly by the preceding import call of 
> the FieldDecl! Here
> ```
> 1735: ExpectedDecl ImportedOrErr = import(From);
> ```
> I have a suspicion that real reason why this import call fails in case of the 
> public ASTImporter::ImportDefinition() is that we fail to drive through the 
> import kind (`IDK_Everything`) during the import process.
> Below we set IDK_Everything and start a complete import process.
> ```
>   8784   if (auto *ToRecord = dyn_cast(To)) {
>   8785 if (!ToRecord->getDefinition()) {
>   8786   return Importer.ImportDefinition(   // 
> ASTNodeImporter::ImportDefinition !
>   8787   cast(FromDC), ToRecord,
>   8788   ASTNodeImporter::IDK_Everything);
>   8789 }
>   8790   }
> ```
> However, there may be many places where we fail to channel through that we 
> want to do a complete import. E.g here
> ```
> 1957   
> ImportDefinitionIfNeeded(Base1.getType()->getAsCXXRecordDecl()))
> ```
> we do another definition import and IDK_Everything is not set. So we may have 
> a wrong kind of import since the "minimal" flag is set.
> 
> The thing is, it is really confusing and error-prone to have both the 
> `ASTImporter::Minimal` flag and the `ImportDefinitionKind`. They seem to be 
> in contradiction to each other some times.
> I think we should get rid of the Minimal flag. And Kind should be either a 
> full completion (IDK_Everythink) or just a minimal (IDK_Basic). So, I'd scrap 
> the IDK_Default too. Alternatively we could have a Kind member in 
> AST**//Node//**Importer.
> I think we should go into this direction to avoid similar problems during 
> CodeGen in the future. WDYT?
No offense, you definitely understand the Importer better than I, so I value 
your input here. I don't believe that should have other fields where we could 
have a record that effects the layout but the concern is still valid and yes I 
did miss multi-dimensional arrays.

Your theory on `IDK_Everything` not be pushed through everywhere, I did a quick 
look and it seems pretty reasonable. 

I agree that the `ASTImporter::Minimal` flag and the `ImportDefinitionKind` 
seem to inconsistent or perhaps a work-around. That seems like a bigger change 
with a lot more impact beyond this fix but worth looking into longer term. 

If pushing through `IDK_Everything` everywhere fixes the underlying issue I am 
very happy to take that approach. If not we can discuss alternatives. 



Comment at: 
lldb/test/API/commands/expression/codegen-crash-import-def-arraytype-element/main.cpp:22-27
+  union {
+struct {
+  unsigned char *_s;
+} t;
+char *tt[1];
+  } U;

labath wrote:
> What's the significance of this union?
Not sure, I was not able to discern why this is important to reproduce the 
crash. I wanted feedback on the approach so I left to further investigation 
later on. 


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

https://reviews.llvm.org/D86660

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

[clang] ba1de5f - [OPENMP]Do not crash for globals in inner regions with outer target

[Alexey Bataev via cfe-commits]

Author: Alexey Bataev
Date: 2020-08-27T17:07:53-04:00
New Revision: ba1de5f2f7b078f69d5f6b0fe3af4911f76bb8fd

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

LOG: [OPENMP]Do not crash for globals in inner regions with outer target
region.

If the global variable is used in the target region,it is always
captured, if not marked as declare target.

Added: 


Modified: 
clang/lib/Sema/SemaOpenMP.cpp
clang/test/OpenMP/target_codegen.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index cd20b6bfdb98..7b62c841b48a 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -2417,17 +2417,20 @@ bool Sema::isOpenMPGlobalCapturedDecl(ValueDecl *D, 
unsigned Level,
 
   if (const auto *VD = dyn_cast(D)) {
 if (!VD->hasLocalStorage()) {
+  if (isInOpenMPTargetExecutionDirective())
+return true;
   DSAStackTy::DSAVarData TopDVar =
   DSAStack->getTopDSA(D, /*FromParent=*/false);
   unsigned NumLevels =
   getOpenMPCaptureLevels(DSAStack->getDirective(Level));
   if (Level == 0)
 return (NumLevels == CaptureLevel + 1) && TopDVar.CKind != OMPC_shared;
-  DSAStackTy::DSAVarData DVar = DSAStack->getImplicitDSA(D, Level - 1);
-  return DVar.CKind != OMPC_shared ||
- isOpenMPGlobalCapturedDecl(
- D, Level - 1,
- getOpenMPCaptureLevels(DSAStack->getDirective(Level - 1)) - 
1);
+  do {
+--Level;
+DSAStackTy::DSAVarData DVar = DSAStack->getImplicitDSA(D, Level);
+if (DVar.CKind != OMPC_shared)
+  return true;
+  } while (Level >= 0);
 }
   }
   return true;

diff  --git a/clang/test/OpenMP/target_codegen.cpp 
b/clang/test/OpenMP/target_codegen.cpp
index 55b03aae00a5..c8570bdf6b65 100644
--- a/clang/test/OpenMP/target_codegen.cpp
+++ b/clang/test/OpenMP/target_codegen.cpp
@@ -706,6 +706,8 @@ int bar(int n){
 
 // CHECK:   [[IFEND]]
 
+// OMP45: define internal void 
@__omp_offloading_{{.+}}_{{.+}}bar{{.+}}_l838(i[[SZ]] %{{.+}})
+
 // OMP45: define {{.*}}@{{.*}}zee{{.*}}
 
 // OMP45:   [[LOCAL_THIS:%.+]] = alloca [[S2]]*
@@ -803,6 +805,7 @@ int bar(int n){
 // CHECK-DAG:   load i16, i16* [[REF_AA]]
 // CHECK-DAG:   getelementptr inbounds [10 x i32], [10 x i32]* [[REF_B]], 
i[[SZ]] 0, i[[SZ]] 2
 
+// OMP50: define internal void 
@__omp_offloading_{{.+}}_{{.+}}bar{{.+}}_l838(i[[SZ]] %{{.+}})
 
 // OMP50: define {{.*}}@{{.*}}zee{{.*}}
 
@@ -833,7 +836,11 @@ int bar(int n){
 void bar () {
 #define pragma_target _Pragma("omp target")
 pragma_target
-{}
+{
+  global = 0;
+#pragma omp parallel shared(global)
+  global = 1;
+}
 }
 
 class S2 {



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

[PATCH] D85773: [Driver][XRay][test] Update the macOS support check

[Azharuddin Mohammed via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd22985c41087: [Driver][XRay][test] Update the macOS support 
check (authored by azharudd).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85773

Files:
  clang/test/Driver/XRay/xray-instrument-os.c


Index: clang/test/Driver/XRay/xray-instrument-os.c
===
--- clang/test/Driver/XRay/xray-instrument-os.c
+++ clang/test/Driver/XRay/xray-instrument-os.c
@@ -1,4 +1,4 @@
 // RUN: not %clang -o /dev/null -v -fxray-instrument -c %s
-// XFAIL: -linux-, -freebsd, -darwin, -macos
+// XFAIL: -linux-, -freebsd, x86_64-apple-darwin, x86_64-apple-macos
 // REQUIRES: amd64 || x86_64 || x86_64h || arm || aarch64 || arm64
 typedef int a;


Index: clang/test/Driver/XRay/xray-instrument-os.c
===
--- clang/test/Driver/XRay/xray-instrument-os.c
+++ clang/test/Driver/XRay/xray-instrument-os.c
@@ -1,4 +1,4 @@
 // RUN: not %clang -o /dev/null -v -fxray-instrument -c %s
-// XFAIL: -linux-, -freebsd, -darwin, -macos
+// XFAIL: -linux-, -freebsd, x86_64-apple-darwin, x86_64-apple-macos
 // REQUIRES: amd64 || x86_64 || x86_64h || arm || aarch64 || arm64
 typedef int a;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] d22985c - [Driver][XRay][test] Update the macOS support check

[Azharuddin Mohammed via cfe-commits]

Author: Azharuddin Mohammed
Date: 2020-08-27T14:05:43-07:00
New Revision: d22985c410873872a96ad3f53170df57c62aac9e

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

LOG: [Driver][XRay][test] Update the macOS support check

For macOS, the code says, the XRay flag is only supported on x86_64.
Updating the test and making that check explicit.

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

Added: 


Modified: 
clang/test/Driver/XRay/xray-instrument-os.c

Removed: 




diff  --git a/clang/test/Driver/XRay/xray-instrument-os.c 
b/clang/test/Driver/XRay/xray-instrument-os.c
index ba97328b54a6..3a0397208326 100644
--- a/clang/test/Driver/XRay/xray-instrument-os.c
+++ b/clang/test/Driver/XRay/xray-instrument-os.c
@@ -1,4 +1,4 @@
 // RUN: not %clang -o /dev/null -v -fxray-instrument -c %s
-// XFAIL: -linux-, -freebsd, -darwin, -macos
+// XFAIL: -linux-, -freebsd, x86_64-apple-darwin, x86_64-apple-macos
 // REQUIRES: amd64 || x86_64 || x86_64h || arm || aarch64 || arm64
 typedef int a;



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

[PATCH] D85773: [Driver][XRay][test] Update the macOS support check

[Azharuddin Mohammed via Phabricator via cfe-commits]

azharudd added a comment.

In D85773#2211929 , @dberris wrote:

> LGTM from me if XRay actually does work on non-x86_64 macOS.

The code says that it is supported only on x86_64 macOS. The way this test is 
written, it should fail on platforms where it is supported.

(Sorry about the delay here.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85773

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

[PATCH] D86135: [analyzer][MacroExpansion] Fix a crash where multiple parameters resolved to __VA_ARGS__

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:896-898
+RawLexer.reset(new Lexer(SM.getLocForStartOfFile(LocInfo.first), LangOpts,
+ MB->getBufferStart(), MacroNameTokenPos,
+ MB->getBufferEnd()));

steakhal wrote:
> I'm always puzzled if I see a naked `new`.
> Couldn't we use the assignment operator and `std::make_unique` here?
Wait, isn't it naked if its //not// surrounded by smart pointer stuff? In any 
case, explicit calls to operator `new` and `delete` are indeed discouraged by 
the 
[[http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#r11-avoid-calling-new-and-delete-explicitly
 | core guidelines]].



Comment at: clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:1150-1161
   // Acquire the macro's arguments at the expansion point.
   //
   // The rough idea here is to lex from the first left parentheses to the last
   // right parentheses, and map the macro's parameter to what they will be
   // expanded to. A macro argument may contain several token (like '3 + 4'), so
   // we'll lex until we find a tok::comma or tok::r_paren, at which point we
   // start lexing the next argument or finish.

steakhal wrote:
> By //lexing// one might think we use the actual lexer. Should we change this 
> comment?
I see what you mean, but this is why its phrased as a rough idea, not an 
in-depth step-by-step description of the process covering corner cases -- not 
in this comment, at least. Functionally, you could argue that we're lexing even 
if we get tokens from the injected range.


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

https://reviews.llvm.org/D86135

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

[clang] a1ac047 - [OpenMP] Fix a failing test after D85214

[Saiyedul Islam via cfe-commits]

Author: Saiyedul Islam
Date: 2020-08-27T20:57:17Z
New Revision: a1ac047b3453f205eacc36adc787ac31b952a502

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

LOG: [OpenMP] Fix a failing test after D85214

Removed version 45 testing from a failing test for now.

Added: 


Modified: 
clang/test/OpenMP/declare_target_ast_print.cpp

Removed: 




diff  --git a/clang/test/OpenMP/declare_target_ast_print.cpp 
b/clang/test/OpenMP/declare_target_ast_print.cpp
index 4831b3bde7a4..c086f8526147 100644
--- a/clang/test/OpenMP/declare_target_ast_print.cpp
+++ b/clang/test/OpenMP/declare_target_ast_print.cpp
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=45 -I %S/Inputs 
-ast-print %s | FileCheck %s
-// RUN: %clang_cc1 -fopenmp -fopenmp-version=45 -x c++ -std=c++11 -I %S/Inputs 
-emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp -fopenmp-version=45 -std=c++11 -include-pch %t 
-fsyntax-only -I %S/Inputs -verify %s -ast-print | FileCheck %s
+// RUN: %clang_cc1 -verify -fopenmp -I %S/Inputs -ast-print %s | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -I %S/Inputs -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -std=c++11 -include-pch %t -fsyntax-only -I 
%S/Inputs -verify %s -ast-print | FileCheck %s
 
 // RUN: %clang_cc1 -verify -fopenmp -I %S/Inputs -ast-print %s | FileCheck %s 
--check-prefix=CHECK --check-prefix=OMP50
 // RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -I %S/Inputs -emit-pch -o %t %s



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

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

[Alina Sbirlea via Phabricator via cfe-commits]

asbirlea added inline comments.



Comment at: llvm/include/llvm/Transforms/Scalar/LoopPassManager.h:273
   : nullptr;
+BlockFrequencyInfo *BFI = UseBlockFrequencyInfo
+  ? ((F))

Add `&& F.hasProfileData()` check here, in the NPM as well?


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

https://reviews.llvm.org/D86156

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

[PATCH] D85810: [clang] Pass-through remarks options to linker

[Wei Wang via Phabricator via cfe-commits]

weiwang updated this revision to Diff 288447.
weiwang added a comment.

remove redundant check


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85810

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/opt-record.c

Index: clang/test/Driver/opt-record.c
===
--- clang/test/Driver/opt-record.c
+++ clang/test/Driver/opt-record.c
@@ -18,7 +18,17 @@
 // RUN: %clang -### -S -o FOO -fsave-optimization-record -fsave-optimization-record=some-format %s 2>&1 | FileCheck %s -check-prefix=CHECK-EQ-FORMAT
 // RUN: %clang -### -S -o FOO -fsave-optimization-record=some-format %s 2>&1 | FileCheck %s -check-prefix=CHECK-EQ-FORMAT
 // RUN: %clang -### -S -o FOO -fsave-optimization-record=some-format -fno-save-optimization-record %s 2>&1 | FileCheck %s --check-prefix=CHECK-FOPT-DISABLE-FORMAT
-//
+
+// Test remarks options pass-through
+// No pass-through: lto is disabled
+// RUN: %clang -target x86_64-linux-gnu -### -o FOO -fdiagnostics-hotness-threshold=100 -fsave-optimization-record %s 2>&1 | not FileCheck %s -check-prefix=CHECK-PASS
+
+// Pass-through:
+// RUN: %clang -target x86_64-linux-gnu -### -o FOO -fuse-ld=lld -flto -fdiagnostics-hotness-threshold=100 -fsave-optimization-record -foptimization-record-passes=inline %s 2>&1 | FileCheck %s -check-prefix=CHECK-PASS
+// RUN: %clang -target x86_64-linux-gnu -### -o FOO -fuse-ld=gold -flto -fdiagnostics-hotness-threshold=100 -fsave-optimization-record -foptimization-record-passes=inline %s 2>&1 | FileCheck %s -check-prefix=CHECK-PASS
+// RUN: %clang -target x86_64-linux-gnu -### -o FOO -fuse-ld=lld -flto=thin -fdiagnostics-hotness-threshold=100 -fsave-optimization-record=some-format -foptimization-record-file=FOO.txt %s 2>&1 | FileCheck %s -check-prefix=CHECK-PASS-CUSTOM
+// RUN: %clang -target x86_64-linux-gnu -### -o FOO -fuse-ld=lld -flto=thin -fdiagnostics-hotness-threshold=100 -Rpass=inline -Rpass-missed=inline -Rpass-analysis=inline %s 2>&1 | FileCheck %s -check-prefix=CHECK-PASS-RPASS
+
 // CHECK: "-cc1"
 // CHECK: "-opt-record-file" "FOO.opt.yaml"
 
@@ -41,3 +51,17 @@
 // CHECK-EQ-FORMAT: "-opt-record-format" "some-format"
 
 // CHECK-FOPT-DISABLE-FORMAT-NOT: "-fno-save-optimization-record"
+
+// CHECK-PASS: "--plugin-opt=opt-remarks-filename=FOO.opt.ld.yaml"
+// CHECK-PASS: "--plugin-opt=opt-remarks-passes=inline"
+// CHECK-PASS: "--plugin-opt=opt-remarks-format=yaml"
+// CHECK-PASS: "--plugin-opt=opt-remarks-hotness-threshold=100"
+
+// CHECK-PASS-CUSTOM: "--plugin-opt=opt-remarks-filename=FOO.txt.opt.ld.some-format"
+// CHECK-PASS-CUSTOM: "--plugin-opt=opt-remarks-format=some-format"
+// CHECK-PASS-CUSTOM: "--plugin-opt=opt-remarks-hotness-threshold=100"
+
+// CHECK-PASS-RPASS: "--plugin-opt=-pass-remarks=inline"
+// CHECK-PASS-RPASS: "--plugin-opt=-pass-remarks-missed=inline"
+// CHECK-PASS-RPASS: "--plugin-opt=-pass-remarks-analysis=inline"
+// CHECK-PASS-RPASS: "--plugin-opt=opt-remarks-hotness-threshold=100"
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -60,6 +60,66 @@
 using namespace clang;
 using namespace llvm::opt;
 
+static void renderRpassOptions(const ArgList , ArgStringList ) {
+  if (const Arg *A = Args.getLastArg(options::OPT_Rpass_EQ))
+CmdArgs.push_back(Args.MakeArgString(Twine("--plugin-opt=-pass-remarks=") +
+ A->getValue()));
+
+  if (const Arg *A = Args.getLastArg(options::OPT_Rpass_missed_EQ))
+CmdArgs.push_back(Args.MakeArgString(
+Twine("--plugin-opt=-pass-remarks-missed=") + A->getValue()));
+
+  if (const Arg *A = Args.getLastArg(options::OPT_Rpass_analysis_EQ))
+CmdArgs.push_back(Args.MakeArgString(
+Twine("--plugin-opt=-pass-remarks-analysis=") + A->getValue()));
+}
+
+static void renderRemarksOptions(const ArgList , ArgStringList ,
+ const llvm::Triple ,
+ const InputInfo ,
+ const InputInfo ) {
+  StringRef Format = "yaml";
+  if (const Arg *A = Args.getLastArg(options::OPT_fsave_optimization_record_EQ))
+Format = A->getValue();
+
+  SmallString<128> F;
+  const Arg *A = Args.getLastArg(options::OPT_foptimization_record_file_EQ);
+  if (A) {
+F = A->getValue();
+  } else {
+if (Output.isFilename())
+  F = Output.getFilename();
+
+// Use the input filename.
+if (F.empty())
+  F = llvm::sys::path::stem(Input.getBaseInput());
+  }
+  // Append "opt.ld." to the end of the file name.
+  CmdArgs.push_back(
+  Args.MakeArgString(Twine("--plugin-opt=opt-remarks-filename=") + F +
+ Twine(".opt.ld.") + Format));
+
+  if (const Arg *A =
+  

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

[Di Mo via Phabricator via cfe-commits]

modimo added a comment.

In D86156#2242872 , @asbirlea wrote:

> As a general note, it may make sense to include BFI in the set of loop passes 
> always preserved (`getLoopPassPreservedAnalyses()`), if its nature is to 
> always be preserved (with some potential info loss) due to the callbacks 
> deleting blocks. But since we're only looking at LICM effect for now, this 
> can be a follow up when/if needed.

Certainly, that makes a lot of sense and makes it easier for any other loop 
optimization to take advantage of this data. I definitely want to make that 
happen, process-wise agreed that following up makes sense instead of tacking 
this on in addition.




Comment at: llvm/lib/Transforms/Scalar/LICM.cpp:220
   
().getDomTree(),
+  ().getBFI(),
   ().getTLI(

nikic wrote:
> I believe that to make this actually lazy the getBFI() call needs to be 
> guarded by some other check for presence of profiling data, otherwise it will 
> be computed unconditionally at this point. Typically something like 
> F.hasProfileData() or PSI.hasProfileSummary().
Appreciate the diligence here-your website is great! 

Your assessment is correct, the ".getBFI()" call for lazy will calculate BFI. 
I've guarded it under hasProfileData now.

I see in the "about" section that I can use your setup to test TP changes 
myself, here's my fork of llvm-project if that option is still available: 
https://github.com/modiking/llvm-project



Comment at: llvm/test/Other/opt-O2-pipeline.ll:281
 ; CHECK-NEXT: Loop Invariant Code Motion
 ; CHECK-NEXT:   Lazy Branch Probability Analysis
 ; CHECK-NEXT:   Lazy Block Frequency Analysis

asbirlea wrote:
> Mark LICM to preserve these passes so they get moved above LICM rather than 
> recomputed here (same as they are preserved in unswitch).
Marked, these are no longer calculated again.


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

https://reviews.llvm.org/D86156

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

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

[Di Mo via Phabricator via cfe-commits]

modimo updated this revision to Diff 288445.
modimo added a comment.

only use BFI when profile is enabled, have LICM mark BFI as preserved


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

https://reviews.llvm.org/D86156

Files:
  clang/test/CodeGen/thinlto-distributed-newpm.ll
  llvm/include/llvm/Analysis/BlockFrequencyInfo.h
  llvm/include/llvm/Analysis/BlockFrequencyInfoImpl.h
  llvm/include/llvm/Analysis/LoopAnalysisManager.h
  llvm/include/llvm/Transforms/Scalar/LoopPassManager.h
  llvm/lib/Analysis/BlockFrequencyInfo.cpp
  llvm/lib/Passes/PassBuilder.cpp
  llvm/lib/Transforms/Scalar/LICM.cpp
  llvm/lib/Transforms/Scalar/LoopDistribute.cpp
  llvm/lib/Transforms/Scalar/LoopLoadElimination.cpp
  llvm/lib/Transforms/Scalar/LoopUnswitch.cpp
  llvm/lib/Transforms/Utils/LoopVersioning.cpp
  llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
  llvm/test/Other/new-pm-defaults.ll
  llvm/test/Other/new-pm-thinlto-defaults.ll
  llvm/test/Other/opt-O2-pipeline.ll
  llvm/test/Other/opt-O3-pipeline-enable-matrix.ll
  llvm/test/Other/opt-O3-pipeline.ll
  llvm/test/Other/opt-Os-pipeline.ll
  llvm/unittests/Transforms/Scalar/LoopPassManagerTest.cpp

Index: llvm/unittests/Transforms/Scalar/LoopPassManagerTest.cpp
===
--- llvm/unittests/Transforms/Scalar/LoopPassManagerTest.cpp
+++ llvm/unittests/Transforms/Scalar/LoopPassManagerTest.cpp
@@ -9,7 +9,10 @@
 #include "llvm/Transforms/Scalar/LoopPassManager.h"
 #include "llvm/Analysis/AliasAnalysis.h"
 #include "llvm/Analysis/AssumptionCache.h"
+#include "llvm/Analysis/BlockFrequencyInfo.h"
+#include "llvm/Analysis/BranchProbabilityInfo.h"
 #include "llvm/Analysis/MemorySSA.h"
+#include "llvm/Analysis/PostDominators.h"
 #include "llvm/Analysis/ScalarEvolution.h"
 #include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/Analysis/TargetTransformInfo.h"
@@ -294,6 +297,9 @@
 // those.
 FAM.registerPass([&] { return AAManager(); });
 FAM.registerPass([&] { return AssumptionAnalysis(); });
+FAM.registerPass([&] { return BlockFrequencyAnalysis(); });
+FAM.registerPass([&] { return BranchProbabilityAnalysis(); });
+FAM.registerPass([&] { return PostDominatorTreeAnalysis(); });
 FAM.registerPass([&] { return MemorySSAAnalysis(); });
 FAM.registerPass([&] { return ScalarEvolutionAnalysis(); });
 FAM.registerPass([&] { return TargetLibraryAnalysis(); });
Index: llvm/test/Other/opt-Os-pipeline.ll
===
--- llvm/test/Other/opt-Os-pipeline.ll
+++ llvm/test/Other/opt-Os-pipeline.ll
@@ -97,6 +97,8 @@
 ; CHECK-NEXT: Loop Pass Manager
 ; CHECK-NEXT:   Rotate Loops
 ; CHECK-NEXT: Memory SSA
+; CHECK-NEXT: Lazy Branch Probability Analysis
+; CHECK-NEXT: Lazy Block Frequency Analysis
 ; CHECK-NEXT: Loop Pass Manager
 ; CHECK-NEXT:   Loop Invariant Code Motion
 ; CHECK-NEXT:   Unswitch loops
@@ -154,6 +156,8 @@
 ; CHECK-NEXT: LCSSA Verifier
 ; CHECK-NEXT: Loop-Closed SSA Form Pass
 ; CHECK-NEXT: Scalar Evolution Analysis
+; CHECK-NEXT: Lazy Branch Probability Analysis
+; CHECK-NEXT: Lazy Block Frequency Analysis
 ; CHECK-NEXT: Loop Pass Manager
 ; CHECK-NEXT:   Loop Invariant Code Motion
 ; CHECK-NEXT: Post-Dominator Tree Construction
@@ -256,10 +260,10 @@
 ; CHECK-NEXT:   LCSSA Verifier
 ; CHECK-NEXT:   Loop-Closed SSA Form Pass
 ; CHECK-NEXT:   Scalar Evolution Analysis
-; CHECK-NEXT:   Loop Pass Manager
-; CHECK-NEXT: Loop Invariant Code Motion
 ; CHECK-NEXT:   Lazy Branch Probability Analysis
 ; CHECK-NEXT:   Lazy Block Frequency Analysis
+; CHECK-NEXT:   Loop Pass Manager
+; CHECK-NEXT: Loop Invariant Code Motion
 ; CHECK-NEXT:   Optimization Remark Emitter
 ; CHECK-NEXT:   Warn about non-applied transformations
 ; CHECK-NEXT:   Alignment from assumptions
Index: llvm/test/Other/opt-O3-pipeline.ll
===
--- llvm/test/Other/opt-O3-pipeline.ll
+++ llvm/test/Other/opt-O3-pipeline.ll
@@ -116,6 +116,8 @@
 ; CHECK-NEXT: Loop Pass Manager
 ; CHECK-NEXT:   Rotate Loops
 ; CHECK-NEXT: Memory SSA
+; CHECK-NEXT: Lazy Branch Probability Analysis
+; CHECK-NEXT: Lazy Block Frequency Analysis
 ; CHECK-NEXT: Loop Pass Manager
 ; CHECK-NEXT:   Loop Invariant Code Motion
 ; CHECK-NEXT:   Unswitch loops
@@ -173,6 +175,8 @@
 ; CHECK-NEXT: LCSSA Verifier
 ; CHECK-NEXT: Loop-Closed SSA Form Pass
 ; CHECK-NEXT: Scalar Evolution Analysis
+; CHECK-NEXT: Lazy Branch Probability Analysis
+; CHECK-NEXT: Lazy Block Frequency Analysis
 ; CHECK-NEXT: Loop Pass Manager
 ; CHECK-NEXT:   Loop Invariant Code Motion
 ; CHECK-NEXT: Post-Dominator Tree Construction
@@ -275,10 

[PATCH] D86000: Add an unsigned shift base sanitizer

[Vedant Kumar via Phabricator via cfe-commits]

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

Lgtm, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86000

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

[PATCH] D86000: Add an unsigned shift base sanitizer

[JF Bastien via Phabricator via cfe-commits]

jfb added inline comments.



Comment at: compiler-rt/test/ubsan/TestCases/Integer/unsigned-shift.cpp:6
+  volatile unsigned _v = (val);\
+  volatile unsigned _a = (amount); \
+  unsigned res = _v << _a; \

vsk wrote:
> jfb wrote:
> > vsk wrote:
> > > Does the test not work without the volatiles?
> > It seems that LLVM sees too much without volatile, yes.
> The optimizer isn't running. Perhaps this is necessary because clang's 
> constant folder kicks in?
Probably, I haven't investigate. It's brittle is the main thing, and this 
forces it to not be.



Comment at: llvm/docs/ReleaseNotes.rst:151
+
+One can silence the check with masking, for example:
+

vsk wrote:
> This could use some more explanation, maybe s/with masking/by clearing bits 
> that would be shifted out/?
I just dropped it, I don't think release notes are the right place for this 
anyways.



Comment at: llvm/docs/ReleaseNotes.rst:153
+
+  `(lhs & ~(~1U << ((sizeof(lhs)*CHAR_BIT) - rhs))) << rhs`
+

lebedev.ri wrote:
> vsk wrote:
> > jfb wrote:
> > > lebedev.ri wrote:
> > > > Surely not `~1U`.
> > > Indeed, fixed.
> > I don't think this pattern works when rhs = 0 
> > (https://godbolt.org/z/rvEGqh).
> Surely not `1U` either :)
Right, I don't think it's something we want to explain here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86000

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

[PATCH] D86000: Add an unsigned shift base sanitizer

[JF Bastien via Phabricator via cfe-commits]

jfb updated this revision to Diff 288444.
jfb marked 4 inline comments as done.
jfb added a comment.

Remove the "suppress this" in release notes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86000

Files:
  clang/docs/UndefinedBehaviorSanitizer.rst
  clang/include/clang/Basic/Sanitizers.def
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/test/CodeGen/unsigned-shift-base.c
  clang/test/Driver/fsanitize.c
  compiler-rt/test/ubsan/TestCases/Integer/unsigned-shift.cpp
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -140,6 +140,14 @@
 Changes to LLDB
 -
 
+Changes to Sanitizers
+-
+
+The integer sanitizer `-fsanitize=integer` now has a new sanitizer:
+`-fsanitize=unsigned-shift-base`. It's not undefined behavior for an unsigned
+left shift to overflow (i.e. to shift bits out), but it has been the source of
+bugs and exploits in certain codebases in the past.
+
 External Open Source Projects Using LLVM 12
 ===
 
Index: compiler-rt/test/ubsan/TestCases/Integer/unsigned-shift.cpp
===
--- /dev/null
+++ compiler-rt/test/ubsan/TestCases/Integer/unsigned-shift.cpp
@@ -0,0 +1,54 @@
+// RUN: %clangxx -fsanitize=unsigned-shift-base %s -o %t1 && not %run %t1 2>&1 | FileCheck %s
+// RUN: %clangxx -fsanitize=unsigned-shift-base,shift-exponent %s -o %t1 && not %run %t1 2>&1 | FileCheck %s
+
+#define shift(val, amount) ({  \
+  volatile unsigned _v = (val);\
+  volatile unsigned _a = (amount); \
+  unsigned res = _v << _a; \
+  res; \
+})
+
+int main() {
+
+  shift(0b''', 31);
+  shift(0b'''0001, 31);
+  shift(0b'''0010, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 2 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b'''0100, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 4 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b'''1000, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 8 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b'''0001, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 16 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b'''0010, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 32 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b'''0100, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 64 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b'''1000, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 128 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b''0001', 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 256 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b''0010', 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 512 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b''0100', 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 1024 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b''1000', 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 2048 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b''0001', 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 4096 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b''0010', 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 8192 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b''0100', 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 16384 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b''1000', 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 32768 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b'0001'', 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 65536 by 31 places cannot be represented in type 

[PATCH] D85810: [clang] Pass-through remarks options to linker

[Wei Wang via Phabricator via cfe-commits]

weiwang added inline comments.



Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:596
+  // Handle remark diagnostics on screen options: '-Rpass-*'.
+  if (usesRpassOptions(Args))
+renderRpassOptions(Args, CmdArgs);

tejohnson wrote:
> Is this guard needed? It looks like renderRpassOptions will do the right 
> thing if none are specified (won't add anything). Right now you end up 
> checking each one a second time if any are enabled.
You are right! I will remove the redundant check.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85810

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

[PATCH] D86736: [analyzer][NFC] Don't bind values to ObjCForCollectionStmt, replace it with a GDM trait

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus created this revision.
Szelethus added reviewers: NoQ, vsavchenko, dcoughlin, baloghadamsoftware, 
martong, balazske, xazax.hun, steakhal.
Szelethus added a project: clang.
Herald added subscribers: cfe-commits, ASDenysPetrov, Charusso, gamesh411, 
dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, whisperity.
Szelethus requested review of this revision.

Based on the discussion in D82598#2171312 
. Thanks @NoQ!

Let's talk about how we got here. D82598  is 
titled "Get rid of statement liveness, because such a thing doesn't exist", and 
indeed, expressions //express// a value, non-expression statements don't.

  if (a && get() || []{ return true; }())
  ~~ has a value
  ~ has a value
  ~~ has a value
 has a value
  ~~~ doesn't have a value

That is simple enough, so it would only make sense if we only assigned symbolic 
values to expressions in the static analyzer. Yet the interface checkers can 
access presents, among other strange things, the following two methods:

ProgramState::BindExpr(const Stmt *S, const LocationContext *LCtx, SVal V, bool 
Invalidate=true) 

ProgramState::getSVal(const Stmt *S, const LocationContext *LCtx) 


So, what gives? Turns out, we make an exception for `ReturnStmt` (which we'll 
leave for another time) and `ObjCForCollectionStmt`. For any other loops, in 
order to know whether we should analyze another iteration, among other things, 
we evaluate it's condition. Which is a problem for `ObjCForCollectionStmt`, 
because it simply doesn't have one (`CXXForRangeStmt` has 

 an implicit one!). In its absence, we assigned the actual statement with a 
concrete 1 or 0 to indicate whether there are any more iterations left. 
However, this is wildly incorrect, its just simply not true that the `for` 
statement has a value of 1 or 0, we can't calculate its liveness because that 
doesn't make any sense either, so this patch turns it into a GDM trait.

This patch isn't quite done (its nowhere near up to my standards in terms of 
documentation), but I wanted to publish it to avoid sinking too much work into 
a doomed patch.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86736

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
  clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
  clang/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp
  clang/lib/StaticAnalyzer/Core/Environment.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
  clang/lib/StaticAnalyzer/Core/ProgramState.cpp
  clang/lib/StaticAnalyzer/Core/SymbolManager.cpp

Index: clang/lib/StaticAnalyzer/Core/SymbolManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/SymbolManager.cpp
+++ clang/lib/StaticAnalyzer/Core/SymbolManager.cpp
@@ -14,6 +14,7 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Expr.h"
+#include "clang/AST/StmtObjC.h"
 #include "clang/Analysis/Analyses/LiveVariables.h"
 #include "clang/Analysis/AnalysisDeclContext.h"
 #include "clang/Basic/LLVM.h"
@@ -494,7 +495,8 @@
 return true;
   }
 
-  // If no statement is provided, everything is this and parent contexts is live.
+  // If no statement is provided, everything in this and parent contexts are
+  // live.
   if (!Loc)
 return true;
 
Index: clang/lib/StaticAnalyzer/Core/ProgramState.cpp
===
--- clang/lib/StaticAnalyzer/Core/ProgramState.cpp
+++ clang/lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -11,6 +11,9 @@
 //===--===//
 
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
+#include "clang/AST/ASTDumperUtils.h"
+#include "clang/AST/StmtObjC.h"
+#include "clang/Analysis/AnalysisDeclContext.h"
 #include "clang/Analysis/CFG.h"
 #include "clang/Basic/JsonSupport.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
@@ -18,6 +21,7 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/DynamicType.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState_Fwd.h"
 #include "llvm/Support/raw_ostream.h"
 
 using namespace clang;
@@ -41,7 +45,8 @@
 Mgr.freeStates.push_back(s);

[PATCH] D85808: [Remarks][2/2] Expand remarks hotness threshold option support in more tools

[Wei Wang via Phabricator via cfe-commits]

weiwang added inline comments.



Comment at: llvm/lib/Analysis/OptimizationRemarkEmitter.cpp:103
 BFI = ().getBFI();
-  else
+// Get hotness threshold from PSI. This should only happen once.
+if (Context.isDiagnosticsHotnessThresholdSetFromPSI()) {

tejohnson wrote:
> Can you clarify what you mean by this only happening once?
The check of `Context.isDiagnosticsHotnessThresholdSetFromPSI()` guarantees 
that `ProfileSummaryInfoWrapperPass` and `setDiagnosticsHotnessThreshold` would 
only be called once. Once the threshold is set from PSI, the check always 
returns false. But strictly speaking, since PSI is immutable, it only happens 
once with/without the check.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85808

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

[PATCH] D86154: AMDGPU: Add llvm.amdgcn.{read,readfirst,write}lane2 intrinsics with type overloads

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added a comment.

>> ReplaceNodeResults expects the result type to be changed in semi-magical 
>> ways during vector type legalization, which is non-obvious since the method 
>> can be called from different places. I think it *could* be made to work with 
>> a lot of patience, but it's really a bad interface -- and besides, by doing 
>> it in IR we reduce code duplication between SelectionDAG and GlobalISel, 
>> which is an added benefit IMO.
>
> Well the globalisel handling should be much simpler. We have a lot of stuff 
> that's randomly handled in the IR to work around the DAG which long term 
> should be moved where it belongs in codegen

Is the GlobalISel handling simpler than the handling in IR? What would happen 
if we wanted to extend the handling to struct types?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86154

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

[PATCH] D85743: [CodeGen][AArch64] Support arm_sve_vector_bits attribute

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

Hi! The `attr-arm-sve-vector-bits-call.c` test seems to be failing on our clang 
builders:

  FAIL: Clang :: CodeGen/attr-arm-sve-vector-bits-call.c (3020 of 25924)
   TEST 'Clang :: CodeGen/attr-arm-sve-vector-bits-call.c' 
FAILED 
  Script:
  --
  : 'RUN: at line 3';   /b/s/w/ir/k/staging/llvm_build/bin/clang -cc1 
-internal-isystem /b/s/w/ir/k/staging/llvm_build/lib/clang/12.0.0/include 
-nostdsysteminc -triple aarch64-none-linux-gnu -target-feature +sve 
-msve-vector-bits=512 -fallow-half-arguments-and-returns -S -O1 -emit-llvm -o - 
/b/s/w/ir/k/llvm-project/clang/test/CodeGen/attr-arm-sve-vector-bits-call.c | 
/b/s/w/ir/k/staging/llvm_build/bin/FileCheck 
/b/s/w/ir/k/llvm-project/clang/test/CodeGen/attr-arm-sve-vector-bits-call.c
  --
  Exit Code: 1
  
  Command Output (stderr):
  --
  warning: Compiler has made implicit assumption that TypeSize is not scalable. 
This may or may not lead to broken code.
  warning: Compiler has made implicit assumption that TypeSize is not scalable. 
This may or may not lead to broken code.
  
/b/s/w/ir/k/llvm-project/clang/test/CodeGen/attr-arm-sve-vector-bits-call.c:67:16:
 error: CHECK-NEXT: is not on the line after the previous match
  // CHECK-NEXT: [[X_ADDR:%.*]] = alloca , align 16
 ^
  :52:2: note: 'next' match was here
   %retval.coerce.i = alloca , align 16
   ^
  :50:7: note: previous match ended here
  entry:
^
  :51:1: note: non-matching line after previous match is here
   %x.i = alloca <16 x i32>, align 16
  ^
  
  Input file: 
  Check file: 
/b/s/w/ir/k/llvm-project/clang/test/CodeGen/attr-arm-sve-vector-bits-call.c
  
  -dump-input=help explains the following input dump.
  
  Input was:
  <<
   .
   .
   .
  47: 
  48: ; Function Attrs: nounwind readnone
  49: define  @sizeless_caller( %x) 
local_unnamed_addr #1 {
  50: entry:
  51:  %x.i = alloca <16 x i32>, align 16
  52:  %retval.coerce.i = alloca , align 16
  next:67  !~ error: 
match on wrong line
  53:  %x.addr = alloca , align 16
  54:  %coerce.coerce = alloca , align 16
  55:  %coerce1 = alloca <16 x i32>, align 16
  56:  %saved-call-rvalue = alloca <16 x i32>, align 64
  57:  store  %x, * %x.addr, align 
16, !tbaa !5
   .
   .
   .
  >>
  
  --
  
  
  Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 
  
  Failed Tests (1):
Clang :: CodeGen/attr-arm-sve-vector-bits-call.c

Could you take a look? Thanks.

Builder: 
https://luci-milo.appspot.com/p/fuchsia/builders/ci/clang-linux-x64/b8870800848452818112?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85743

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

[clang] 17ceda9 - [CodeGen] Use an AttrBuilder to bulk remove 'target-cpu', 'target-features', and 'tune-cpu' before re-adding in CodeGenModule::setNonAliasAttributes.

[Craig Topper via cfe-commits]

Author: Craig Topper
Date: 2020-08-27T12:54:20-07:00
New Revision: 17ceda99d32035dc654b45ef7af62c571d8a8273

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

LOG: [CodeGen] Use an AttrBuilder to bulk remove 'target-cpu', 
'target-features', and 'tune-cpu' before re-adding in 
CodeGenModule::setNonAliasAttributes.

I think the removeAttributes interface should be faster than
calling removeAttribute 3 times.

Added: 


Modified: 
clang/lib/CodeGen/CodeGenModule.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index 65e5e27a5839..77a5079bd0f1 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -1829,9 +1829,11 @@ void CodeGenModule::setNonAliasAttributes(GlobalDecl GD,
 // We know that GetCPUAndFeaturesAttributes will always have the
 // newest set, since it has the newest possible FunctionDecl, so the
 // new ones should replace the old.
-F->removeFnAttr("target-cpu");
-F->removeFnAttr("target-features");
-F->removeFnAttr("tune-cpu");
+llvm::AttrBuilder RemoveAttrs;
+RemoveAttrs.addAttribute("target-cpu");
+RemoveAttrs.addAttribute("target-features");
+RemoveAttrs.addAttribute("tune-cpu");
+F->removeAttributes(llvm::AttributeList::FunctionIndex, RemoveAttrs);
 F->addAttributes(llvm::AttributeList::FunctionIndex, Attrs);
   }
 }



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

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

[Nikita Popov via Phabricator via cfe-commits]

nikic added a comment.

Current compile-time numbers show a 1% geomean regression: 
https://llvm-compile-time-tracker.com/compare.php?from=d7c119d89c5f6d0789cfd0a139c80e23912c0bb0=127615d90c7b4424ec83f5a8ab4256d08f7a8362=instructions

I've left a comment inline with a possible cause.




Comment at: llvm/lib/Transforms/Scalar/LICM.cpp:220
   
().getDomTree(),
+  ().getBFI(),
   ().getTLI(

I believe that to make this actually lazy the getBFI() call needs to be guarded 
by some other check for presence of profiling data, otherwise it will be 
computed unconditionally at this point. Typically something like 
F.hasProfileData() or PSI.hasProfileSummary().


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

https://reviews.llvm.org/D86156

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

[PATCH] D86707: [SystemZ][z/OS] Adding initial toolchain for z/OS

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Driver/ToolChains/ZOS.h:25
+
+  bool isPICDefault() const override { return false; }
+  bool isPIEDefault() const override { return false; }

According to the RFC re: LLVM on z/OS, the initial support in LLVM for z/OS is 
only for XPLink. My understanding is that all XPLink applications are 
DLL-enabled. Does being DLL-enabled not imply that the code is 
position-independent?

I understand that the value of the `__DLL__` predefined macro from the XL C 
compiler does not reflect the implicit DLL-enablement of XPLink code; however, 
I also note that the same compiler claims falsely that `Option NODLL is ignored 
because option XPLINK is specified` when `-qnodll` does actually suppress the 
effect of an earlier `-qdll` in causing `__DLL__` to be defined.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86707

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

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

[Alina Sbirlea via Phabricator via cfe-commits]

asbirlea added a comment.

Your understanding is correct. I only have one more comment on preserving 
passes in LICM too.

As a general note, it may make sense to include BFI in the set of loop passes 
always preserved (`getLoopPassPreservedAnalyses()`), if its nature is to always 
be preserved (with some potential info loss) due to the callbacks deleting 
blocks. But since we're only looking at LICM effect for now, this can be a 
follow up when/if needed.

Please allow some time for nikic@ to take a look too.




Comment at: llvm/test/Other/opt-O2-pipeline.ll:281
 ; CHECK-NEXT: Loop Invariant Code Motion
 ; CHECK-NEXT:   Lazy Branch Probability Analysis
 ; CHECK-NEXT:   Lazy Block Frequency Analysis

Mark LICM to preserve these passes so they get moved above LICM rather than 
recomputed here (same as they are preserved in unswitch).


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

https://reviews.llvm.org/D86156

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

[PATCH] D86089: [flang][driver]Add experimental flang driver and frontend with help screen

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski commandeered this revision.
awarzynski added a reviewer: CarolineConcatto.
awarzynski added a comment.

@sameeranjoshi Thanks for testing out-of-tree builds. We were missing some 
minor changes in CMake. I have now uploaded these. Out-of-tree will require 
`-DCLANG_DIR` when configuring build while the new driver depends on Clang.

As for your original issue - I suspect that you forgot to update your in-tree 
directory. This patch needs to be applied both in-tree (in `llvm-project`) and 
out-of-tree (`flang`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86089

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

[PATCH] D86089: [flang][driver]Add experimental flang driver and frontend with help screen

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski updated this revision to Diff 288424.
awarzynski added a comment.

Added support for out-of-tree builds. It requires 
-DCLANG_DIR=/lib/cmake/clang when configuring CMake. That's on 
top of LLVM_DIR and MLIR_DIR.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86089

Files:
  clang/include/clang/Driver/Driver.h
  clang/include/clang/Driver/Options.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Flang.cpp
  clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
  clang/lib/Tooling/Tooling.cpp
  clang/test/Driver/flang/flang.f90
  clang/test/Driver/flang/flang_ucase.F90
  clang/test/Driver/flang/multiple-inputs-mixed.f90
  clang/test/Driver/flang/multiple-inputs.f90
  clang/unittests/Driver/SanitizerArgsTest.cpp
  clang/unittests/Driver/ToolChainTest.cpp
  flang/CMakeLists.txt
  flang/include/flang/Frontend/CompilerInstance.h
  flang/include/flang/Frontend/CompilerInvocation.h
  flang/include/flang/Frontend/FrontendOptions.h
  flang/include/flang/FrontendTool/Utils.h
  flang/lib/CMakeLists.txt
  flang/lib/Frontend/CMakeLists.txt
  flang/lib/Frontend/CompilerInstance.cpp
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendOptions.cpp
  flang/lib/FrontendTool/CMakeLists.txt
  flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  flang/test/CMakeLists.txt
  flang/test/Flang-Driver/driver-error-cc1.c
  flang/test/Flang-Driver/driver-error-cc1.cpp
  flang/test/Flang-Driver/driver-help.f90
  flang/test/Flang-Driver/driver-version.f90
  flang/test/Flang-Driver/emit-obj.f90
  flang/test/lit.cfg.py
  flang/test/lit.site.cfg.py.in
  flang/tools/CMakeLists.txt
  flang/tools/flang-driver/CMakeLists.txt
  flang/tools/flang-driver/driver.cpp
  flang/tools/flang-driver/fc1_main.cpp
  flang/unittests/CMakeLists.txt
  flang/unittests/Frontend/CMakeLists.txt
  flang/unittests/Frontend/CompilerInstanceTest.cpp
  llvm/include/llvm/Option/OptTable.h
  llvm/lib/Option/OptTable.cpp

Index: llvm/lib/Option/OptTable.cpp
===
--- llvm/lib/Option/OptTable.cpp
+++ llvm/lib/Option/OptTable.cpp
@@ -610,9 +610,12 @@
   continue;
 
 unsigned Flags = getInfo(Id).Flags;
-if (FlagsToInclude && !(Flags & FlagsToInclude))
+
+unsigned ExplicitelyIncluded = Flags & FlagsToInclude;
+if (FlagsToInclude && !(ExplicitelyIncluded))
   continue;
-if (Flags & FlagsToExclude)
+
+if (!(ExplicitelyIncluded) && (Flags & FlagsToExclude))
   continue;
 
 // If an alias doesn't have a help text, show a help text for the aliased
Index: llvm/include/llvm/Option/OptTable.h
===
--- llvm/include/llvm/Option/OptTable.h
+++ llvm/include/llvm/Option/OptTable.h
@@ -225,7 +225,8 @@
   /// \param Usage - USAGE: Usage
   /// \param Title - OVERVIEW: Title
   /// \param FlagsToInclude - If non-zero, only include options with any
-  /// of these flags set.
+  /// of these flags set. Takes precedence over
+  /// FlagsToExclude.
   /// \param FlagsToExclude - Exclude options with any of these flags set.
   /// \param ShowAllAliases - If true, display all options including aliases
   /// that don't have help texts. By default, we display
Index: flang/unittests/Frontend/CompilerInstanceTest.cpp
===
--- /dev/null
+++ flang/unittests/Frontend/CompilerInstanceTest.cpp
@@ -0,0 +1,52 @@
+//===- unittests/Frontend/CompilerInstanceTest.cpp - CI tests -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "flang/Frontend/CompilerInstance.h"
+#include "gtest/gtest.h"
+#include "flang/Frontend/CompilerInvocation.h"
+#include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Driver/Options.h"
+#include "clang/Frontend/TextDiagnosticPrinter.h"
+#include "llvm/Support/raw_ostream.h"
+
+#include 
+using namespace llvm;
+using namespace Fortran::frontend;
+
+namespace {
+
+TEST(CompilerInstance, AllowDiagnosticLogWithUnownedDiagnosticConsumer) {
+  // 1. Set-up a basic DiagnosticConsumer
+  std::string diagnosticOutput;
+  llvm::raw_string_ostream diagnosticsOS(diagnosticOutput);
+  auto diagPrinter = std::make_unique(
+  diagnosticsOS, new clang::DiagnosticOptions());
+
+  // 2. Create a CompilerInstance (to manage a DiagnosticEngine)
+  CompilerInstance compInst;
+
+  // 3. Set-up DiagnosticOptions
+  auto diagOpts = new clang::DiagnosticOptions();
+  // Tell the diagnostics engine to emit the diagnostic log to STDERR. This
+  // 

[clang] 4448aff - [analyzer] pr47037: CastValueChecker: Support for the new variadic isa<>.

[Artem Dergachev via cfe-commits]

Author: Adam Balogh
Date: 2020-08-27T12:15:25-07:00
New Revision: 4448affede5100658530aea8793ae7a7bc05a110

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

LOG: [analyzer] pr47037: CastValueChecker: Support for the new variadic isa<>.

llvm::isa<>() and llvm::isa_and_not_null<>() template functions recently became
variadic. Unfortunately this causes crashes in case of isa_and_not_null<>()
and incorrect behavior in isa<>(). This patch fixes this issue.

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
clang/test/Analysis/Inputs/llvm.h
clang/test/Analysis/cast-value-logic.cpp
clang/test/Analysis/cast-value-notes.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
index ce8de99da99f..131c1345af99 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
@@ -135,6 +135,47 @@ static const NoteTag *getNoteTag(CheckerContext ,
   /*IsPrunable=*/true);
 }
 
+static const NoteTag *getNoteTag(CheckerContext ,
+ SmallVector CastToTyVec,
+ const Expr *Object,
+ bool IsKnownCast) {
+  Object = Object->IgnoreParenImpCasts();
+
+  return C.getNoteTag(
+  [=]() -> std::string {
+SmallString<128> Msg;
+llvm::raw_svector_ostream Out(Msg);
+
+if (!IsKnownCast)
+  Out << "Assuming ";
+
+if (const auto *DRE = dyn_cast(Object)) {
+  Out << '\'' << DRE->getDecl()->getNameAsString() << '\'';
+} else if (const auto *ME = dyn_cast(Object)) {
+  Out << (IsKnownCast ? "Field '" : "field '")
+  << ME->getMemberDecl()->getNameAsString() << '\'';
+} else {
+  Out << (IsKnownCast ? "The object" : "the object");
+}
+Out << " is";
+
+bool First = true;
+for (QualType CastToTy: CastToTyVec) {
+  std::string CastToName =
+CastToTy->getAsCXXRecordDecl() ?
+CastToTy->getAsCXXRecordDecl()->getNameAsString() :
+CastToTy->getPointeeCXXRecordDecl()->getNameAsString();
+  Out << ' ' << ((CastToTyVec.size() == 1) ? "not" :
+ (First ? "neither" : "nor")) << " a '" << CastToName
+  << '\'';
+  First = false;
+}
+
+return std::string(Out.str());
+  },
+  /*IsPrunable=*/true);
+}
+
 
//===--===//
 // Main logic to evaluate a cast.
 
//===--===//
@@ -220,40 +261,76 @@ static void addInstanceOfTransition(const CallEvent ,
 bool IsInstanceOf) {
   const FunctionDecl *FD = Call.getDecl()->getAsFunction();
   QualType CastFromTy = Call.parameters()[0]->getType();
-  QualType CastToTy = FD->getTemplateSpecializationArgs()->get(0).getAsType();
-  if (CastFromTy->isPointerType())
-CastToTy = C.getASTContext().getPointerType(CastToTy);
-  else if (CastFromTy->isReferenceType())
-CastToTy = alignReferenceTypes(CastToTy, CastFromTy, C.getASTContext());
-  else
-return;
+  SmallVector CastToTyVec;
+  for (unsigned idx = 0; idx < FD->getTemplateSpecializationArgs()->size() - 1;
+   ++idx) {
+TemplateArgument CastToTempArg =
+  FD->getTemplateSpecializationArgs()->get(idx);
+switch (CastToTempArg.getKind()) {
+default:
+  return;
+case TemplateArgument::Type:
+  CastToTyVec.push_back(CastToTempArg.getAsType());
+  break;
+case TemplateArgument::Pack:
+  for (TemplateArgument ArgInPack: CastToTempArg.pack_elements())
+CastToTyVec.push_back(ArgInPack.getAsType());
+  break;
+}
+  }
 
   const MemRegion *MR = DV.getAsRegion();
-  const DynamicCastInfo *CastInfo =
-  getDynamicCastInfo(State, MR, CastFromTy, CastToTy);
+  if (MR && CastFromTy->isReferenceType())
+MR = State->getSVal(DV.castAs()).getAsRegion();
+
+  bool Success = false;
+  bool IsAnyKnown = false;
+  for (QualType CastToTy: CastToTyVec) {
+if (CastFromTy->isPointerType())
+  CastToTy = C.getASTContext().getPointerType(CastToTy);
+else if (CastFromTy->isReferenceType())
+  CastToTy = alignReferenceTypes(CastToTy, CastFromTy, C.getASTContext());
+else
+  return;
 
-  bool CastSucceeds;
-  if (CastInfo)
-CastSucceeds = IsInstanceOf && CastInfo->succeeds();
-  else
-CastSucceeds = IsInstanceOf || CastFromTy == CastToTy;
+const DynamicCastInfo *CastInfo =
+  getDynamicCastInfo(State, MR, 

[PATCH] D85728: [Analyzer] Support for the new variadic isa<> and isa_and_not_null<> in CastValueChecker

[Artem Dergachev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4448affede51: [analyzer] pr47037: CastValueChecker: Support 
for the new variadic isa. (authored by baloghadamsoftware, committed by 
dergachev.a).

Changed prior to commit:
  https://reviews.llvm.org/D85728?vs=285047=288420#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85728

Files:
  clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
  clang/test/Analysis/Inputs/llvm.h
  clang/test/Analysis/cast-value-logic.cpp
  clang/test/Analysis/cast-value-notes.cpp

Index: clang/test/Analysis/cast-value-notes.cpp
===
--- clang/test/Analysis/cast-value-notes.cpp
+++ clang/test/Analysis/cast-value-notes.cpp
@@ -13,6 +13,8 @@
   const T *getAs() const;
 };
 class Triangle : public Shape {};
+class Rectangle : public Shape {};
+class Hexagon : public Shape {};
 class Circle : public Shape {};
 } // namespace clang
 
@@ -27,7 +29,6 @@
 }
 
 void evalNonNullParamNonNullReturnReference(const Shape ) {
-  // Unmodeled cast from reference to pointer.
   const auto *C = dyn_cast_or_null(S);
   // expected-note@-1 {{'C' initialized here}}
 
@@ -43,13 +44,37 @@
 return;
   }
 
+  if (dyn_cast_or_null(C)) {
+// expected-note@-1 {{Assuming 'C' is not a 'Rectangle'}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (dyn_cast_or_null(C)) {
+// expected-note@-1 {{Assuming 'C' is not a 'Hexagon'}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
   if (isa(C)) {
 // expected-note@-1 {{'C' is not a 'Triangle'}}
 // expected-note@-2 {{Taking false branch}}
 return;
   }
 
-  if (isa(C)) {
+  if (isa(C)) {
+// expected-note@-1 {{'C' is neither a 'Triangle' nor a 'Rectangle'}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (isa(C)) {
+// expected-note@-1 {{'C' is neither a 'Triangle' nor a 'Rectangle' nor a 'Hexagon'}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (isa(C)) {
 // expected-note@-1 {{'C' is a 'Circle'}}
 // expected-note@-2 {{Taking true branch}}
 
@@ -65,22 +90,57 @@
   // expected-note@-1 {{'S' is a 'Circle'}}
   // expected-note@-2 {{'C' initialized here}}
 
-  if (!isa(C)) {
-// expected-note@-1 {{Assuming 'C' is a 'Triangle'}}
+  if (!dyn_cast_or_null(C)) {
+// expected-note@-1 {{'C' is a 'Circle'}}
 // expected-note@-2 {{Taking false branch}}
 return;
   }
 
-  if (!isa(C)) {
-// expected-note@-1 {{'C' is a 'Triangle'}}
+  if (dyn_cast_or_null(C)) {
+// expected-note@-1 {{Assuming 'C' is not a 'Triangle'}}
 // expected-note@-2 {{Taking false branch}}
 return;
   }
 
-  (void)(1 / !C);
-  // expected-note@-1 {{'C' is non-null}}
-  // expected-note@-2 {{Division by zero}}
-  // expected-warning@-3 {{Division by zero}}
+  if (dyn_cast_or_null(C)) {
+// expected-note@-1 {{Assuming 'C' is not a 'Rectangle'}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (dyn_cast_or_null(C)) {
+// expected-note@-1 {{Assuming 'C' is not a 'Hexagon'}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (isa(C)) {
+// expected-note@-1 {{'C' is not a 'Triangle'}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (isa(C)) {
+// expected-note@-1 {{'C' is neither a 'Triangle' nor a 'Rectangle'}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (isa(C)) {
+// expected-note@-1 {{'C' is neither a 'Triangle' nor a 'Rectangle' nor a 'Hexagon'}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (isa(C)) {
+// expected-note@-1 {{'C' is a 'Circle'}}
+// expected-note@-2 {{Taking true branch}}
+
+(void)(1 / !C);
+// expected-note@-1 {{'C' is non-null}}
+// expected-note@-2 {{Division by zero}}
+// expected-warning@-3 {{Division by zero}}
+  }
 }
 
 void evalNonNullParamNullReturn(const Shape *S) {
Index: clang/test/Analysis/cast-value-logic.cpp
===
--- clang/test/Analysis/cast-value-logic.cpp
+++ clang/test/Analysis/cast-value-logic.cpp
@@ -19,6 +19,8 @@
   virtual double area();
 };
 class Triangle : public Shape {};
+class Rectangle : public Shape {};
+class Hexagon : public Shape {};
 class Circle : public Shape {
 public:
   ~Circle();
@@ -39,6 +41,23 @@
 clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
 }
 
+void test_regions_isa_variadic(const Shape *A, const Shape *B) {
+  if (isa(A) &&
+  !isa(B))
+clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+}
+
+void test_regions_isa_and_nonnull(const Shape *A, const Shape *B) {
+  if (isa_and_nonnull(A) && !isa_and_nonnull(B))
+clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+}
+
+void 

[PATCH] D86691: [analyzer] Fix wrong parameter name in printFormattedEntry

[Artem Dergachev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6e26e49edf0d: [analyzer] NFC: Fix wrong parameter name in 
printFormattedEntry. (authored by nullptr.cpp, committed by dergachev.a).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86691

Files:
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp


Index: clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
===
--- clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
+++ clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
@@ -40,7 +40,7 @@
   const size_t PadForDesc = InitialPad + EntryWidth;
 
   FOut.PadToColumn(InitialPad) << EntryDescPair.first;
-  // If the buffer's length is greater then PadForDesc, print a newline.
+  // If the buffer's length is greater than PadForDesc, print a newline.
   if (FOut.getColumn() > PadForDesc)
 FOut << '\n';
 
Index: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
===
--- clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
+++ clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
@@ -177,23 +177,23 @@
   /// description in a formatted manner. If \p MinLineWidth is set to 0, no 
line
   /// breaks are introduced for the description.
   ///
-  /// Format, depending whether the option name's length is less then
-  /// \p OptionWidth:
+  /// Format, depending whether the option name's length is less than
+  /// \p EntryWidth:
   ///
   ///   EntryNameDescription
   ///   <-padding->Description
   ///   <-padding->Description
   ///
-  ///   VeryVeryLongOptionName
+  ///   VeryVeryLongEntryName
   ///   <-padding->Description
   ///   <-padding->Description
-  ///   ^~~~ InitialPad
-  ///   ^~ EntryWidth
+  ///   ^InitialPad
+  ///^~EntryWidth
   ///   ^~MinLineWidth
-  static void printFormattedEntry(
-  llvm::raw_ostream ,
-  std::pair EntryDescPair,
-  size_t EntryWidth, size_t InitialPad, size_t MinLineWidth = 0);
+  static void printFormattedEntry(llvm::raw_ostream ,
+  std::pair 
EntryDescPair,
+  size_t InitialPad, size_t EntryWidth,
+  size_t MinLineWidth = 0);
 
   /// Pairs of checker/package name and enable/disable.
   std::vector> CheckersAndPackages;


Index: clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
===
--- clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
+++ clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
@@ -40,7 +40,7 @@
   const size_t PadForDesc = InitialPad + EntryWidth;
 
   FOut.PadToColumn(InitialPad) << EntryDescPair.first;
-  // If the buffer's length is greater then PadForDesc, print a newline.
+  // If the buffer's length is greater than PadForDesc, print a newline.
   if (FOut.getColumn() > PadForDesc)
 FOut << '\n';
 
Index: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
===
--- clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
+++ clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
@@ -177,23 +177,23 @@
   /// description in a formatted manner. If \p MinLineWidth is set to 0, no line
   /// breaks are introduced for the description.
   ///
-  /// Format, depending whether the option name's length is less then
-  /// \p OptionWidth:
+  /// Format, depending whether the option name's length is less than
+  /// \p EntryWidth:
   ///
   ///   EntryNameDescription
   ///   <-padding->Description
   ///   <-padding->Description
   ///
-  ///   VeryVeryLongOptionName
+  ///   VeryVeryLongEntryName
   ///   <-padding->Description
   ///   <-padding->Description
-  ///   ^~~~ InitialPad
-  ///   ^~ EntryWidth
+  ///   ^InitialPad
+  ///^~EntryWidth
   ///   ^~MinLineWidth
-  static void printFormattedEntry(
-  llvm::raw_ostream ,
-  std::pair EntryDescPair,
-  size_t EntryWidth, size_t InitialPad, size_t MinLineWidth = 0);
+  static void printFormattedEntry(llvm::raw_ostream ,
+  std::pair EntryDescPair,
+  size_t InitialPad, size_t EntryWidth,
+  size_t MinLineWidth = 0);
 
   /// Pairs of checker/package name and enable/disable.
   std::vector> CheckersAndPackages;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D86334: [analyzer] Remove redundant output errs

[Artem Dergachev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG37c21dbb3a32: [analyzer] Fix the debug print about debug 
egraph dumps requiring asserts. (authored by nullptr.cpp, committed by 
dergachev.a).

Changed prior to commit:
  https://reviews.llvm.org/D86334?vs=286960=288421#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86334

Files:
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp


Index: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -3154,8 +3154,9 @@
 #ifndef NDEBUG
   std::string Filename = DumpGraph(trim);
   llvm::DisplayGraph(Filename, false, llvm::GraphProgram::DOT);
-#endif
+#else
   llvm::errs() << "Warning: viewing graph requires assertions" << "\n";
+#endif
 }
 
 
@@ -3163,8 +3164,9 @@
 #ifndef NDEBUG
   std::string Filename = DumpGraph(Nodes);
   llvm::DisplayGraph(Filename, false, llvm::GraphProgram::DOT);
-#endif
+#else
   llvm::errs() << "Warning: viewing graph requires assertions" << "\n";
+#endif
 }
 
 std::string ExprEngine::DumpGraph(bool trim, StringRef Filename) {
@@ -3201,15 +3203,17 @@
 
   if (!TrimmedG.get()) {
 llvm::errs() << "warning: Trimmed ExplodedGraph is empty.\n";
+return "";
   } else {
 return llvm::WriteGraph(TrimmedG.get(), "TrimmedExprEngine",
 /*ShortNames=*/false,
 /*Title=*/"Trimmed Exploded Graph",
 /*Filename=*/std::string(Filename));
   }
-#endif
+#else
   llvm::errs() << "Warning: dumping graph requires assertions" << "\n";
   return "";
+#endif
 }
 
 void *ProgramStateTrait::GDMIndex() {


Index: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -3154,8 +3154,9 @@
 #ifndef NDEBUG
   std::string Filename = DumpGraph(trim);
   llvm::DisplayGraph(Filename, false, llvm::GraphProgram::DOT);
-#endif
+#else
   llvm::errs() << "Warning: viewing graph requires assertions" << "\n";
+#endif
 }
 
 
@@ -3163,8 +3164,9 @@
 #ifndef NDEBUG
   std::string Filename = DumpGraph(Nodes);
   llvm::DisplayGraph(Filename, false, llvm::GraphProgram::DOT);
-#endif
+#else
   llvm::errs() << "Warning: viewing graph requires assertions" << "\n";
+#endif
 }
 
 std::string ExprEngine::DumpGraph(bool trim, StringRef Filename) {
@@ -3201,15 +3203,17 @@
 
   if (!TrimmedG.get()) {
 llvm::errs() << "warning: Trimmed ExplodedGraph is empty.\n";
+return "";
   } else {
 return llvm::WriteGraph(TrimmedG.get(), "TrimmedExprEngine",
 /*ShortNames=*/false,
 /*Title=*/"Trimmed Exploded Graph",
 /*Filename=*/std::string(Filename));
   }
-#endif
+#else
   llvm::errs() << "Warning: dumping graph requires assertions" << "\n";
   return "";
+#endif
 }
 
 void *ProgramStateTrait::GDMIndex() {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 6e26e49 - [analyzer] NFC: Fix wrong parameter name in printFormattedEntry.

[Artem Dergachev via cfe-commits]

Author: Yang Fan
Date: 2020-08-27T12:15:26-07:00
New Revision: 6e26e49edf0d509fb3e76d984f2cbc8288fd6dc5

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

LOG: [analyzer] NFC: Fix wrong parameter name in printFormattedEntry.

Parameters were in a different order in the header and in the implementation.

Fix surrounding comments a bit.

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

Added: 


Modified: 
clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp

Removed: 




diff  --git a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h 
b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
index d2df24a6e21b..4907b0757a8a 100644
--- a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
+++ b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
@@ -177,23 +177,23 @@ class AnalyzerOptions : public 
RefCountedBase {
   /// description in a formatted manner. If \p MinLineWidth is set to 0, no 
line
   /// breaks are introduced for the description.
   ///
-  /// Format, depending whether the option name's length is less then
-  /// \p OptionWidth:
+  /// Format, depending whether the option name's length is less than
+  /// \p EntryWidth:
   ///
   ///   EntryNameDescription
   ///   <-padding->Description
   ///   <-padding->Description
   ///
-  ///   VeryVeryLongOptionName
+  ///   VeryVeryLongEntryName
   ///   <-padding->Description
   ///   <-padding->Description
-  ///   ^~~~ InitialPad
-  ///   ^~ EntryWidth
+  ///   ^InitialPad
+  ///^~EntryWidth
   ///   ^~MinLineWidth
-  static void printFormattedEntry(
-  llvm::raw_ostream ,
-  std::pair EntryDescPair,
-  size_t EntryWidth, size_t InitialPad, size_t MinLineWidth = 0);
+  static void printFormattedEntry(llvm::raw_ostream ,
+  std::pair 
EntryDescPair,
+  size_t InitialPad, size_t EntryWidth,
+  size_t MinLineWidth = 0);
 
   /// Pairs of checker/package name and enable/disable.
   std::vector> CheckersAndPackages;

diff  --git a/clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp 
b/clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
index 01ac2bc83bb6..8cd7f75e4e38 100644
--- a/clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
+++ b/clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
@@ -40,7 +40,7 @@ void AnalyzerOptions::printFormattedEntry(
   const size_t PadForDesc = InitialPad + EntryWidth;
 
   FOut.PadToColumn(InitialPad) << EntryDescPair.first;
-  // If the buffer's length is greater then PadForDesc, print a newline.
+  // If the buffer's length is greater than PadForDesc, print a newline.
   if (FOut.getColumn() > PadForDesc)
 FOut << '\n';
 



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

[clang] 5a9e778 - [analyzer] NFC: Store the pointee/referenced type for dynamic type tracking.

[Artem Dergachev via cfe-commits]

Author: Adam Balogh
Date: 2020-08-27T12:15:23-07:00
New Revision: 5a9e7789396e7618c1407aafc329e00584437a2f

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

LOG: [analyzer] NFC: Store the pointee/referenced type for dynamic type 
tracking.

The successfulness of a dynamic cast depends only on the C++ class, not the 
pointer or reference. Thus if *A is a *B, then  is a ,
const *A is a const *B etc. This patch changes DynamicCastInfo to store
and check the cast between the unqualified pointed/referenced types.
It also removes e.g. SubstTemplateTypeParmType from both the pointer
and the pointed type.

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
clang/lib/StaticAnalyzer/Core/DynamicType.cpp
clang/test/Analysis/cast-value-state-dump.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
index 56c6f8d02e0f..ce8de99da99f 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
@@ -106,7 +106,7 @@ static const NoteTag *getNoteTag(CheckerContext ,
  QualType CastToTy, const Expr *Object,
  bool CastSucceeds, bool IsKnownCast) {
   std::string CastToName =
-  CastInfo ? CastInfo->to()->getPointeeCXXRecordDecl()->getNameAsString()
+  CastInfo ? CastInfo->to()->getAsCXXRecordDecl()->getNameAsString()
: CastToTy->getPointeeCXXRecordDecl()->getNameAsString();
   Object = Object->IgnoreParenImpCasts();
 

diff  --git a/clang/lib/StaticAnalyzer/Core/DynamicType.cpp 
b/clang/lib/StaticAnalyzer/Core/DynamicType.cpp
index e9b64fd79614..9ed915aafcab 100644
--- a/clang/lib/StaticAnalyzer/Core/DynamicType.cpp
+++ b/clang/lib/StaticAnalyzer/Core/DynamicType.cpp
@@ -65,6 +65,13 @@ const DynamicTypeInfo *getRawDynamicTypeInfo(ProgramStateRef 
State,
   return State->get(MR);
 }
 
+static void unbox(QualType ) {
+  // FIXME: Why are we being fed references to pointers in the first place?
+  while (Ty->isReferenceType() || Ty->isPointerType())
+Ty = Ty->getPointeeType();
+  Ty = Ty.getCanonicalType().getUnqualifiedType();
+}
+
 const DynamicCastInfo *getDynamicCastInfo(ProgramStateRef State,
   const MemRegion *MR,
   QualType CastFromTy,
@@ -73,6 +80,9 @@ const DynamicCastInfo *getDynamicCastInfo(ProgramStateRef 
State,
   if (!Lookup)
 return nullptr;
 
+  unbox(CastFromTy);
+  unbox(CastToTy);
+
   for (const DynamicCastInfo  : *Lookup)
 if (Cast.equals(CastFromTy, CastToTy))
   return 
@@ -112,6 +122,9 @@ ProgramStateRef setDynamicTypeAndCastInfo(ProgramStateRef 
State,
 State = State->set(MR, CastToTy);
   }
 
+  unbox(CastFromTy);
+  unbox(CastToTy);
+
   DynamicCastInfo::CastResult ResultKind =
   CastSucceeds ? DynamicCastInfo::CastResult::Success
: DynamicCastInfo::CastResult::Failure;

diff  --git a/clang/test/Analysis/cast-value-state-dump.cpp 
b/clang/test/Analysis/cast-value-state-dump.cpp
index 3dffb78767cf..3e6a40cf1319 100644
--- a/clang/test/Analysis/cast-value-state-dump.cpp
+++ b/clang/test/Analysis/cast-value-state-dump.cpp
@@ -35,8 +35,8 @@ void evalNonNullParamNonNullReturn(const Shape *S) {
   // CHECK-NEXT: ],
   // CHECK-NEXT: "dynamic_casts": [
   // CHECK:{ "region": "SymRegion{reg_$0}", "casts": [
-  // CHECK-NEXT: { "from": "const struct clang::Shape *", "to": "const 
class clang::Circle *", "kind": "success" },
-  // CHECK-NEXT: { "from": "const struct clang::Shape *", "to": "const 
class clang::Square *", "kind": "fail" }
+  // CHECK-NEXT: { "from": "struct clang::Shape", "to": "class 
clang::Circle", "kind": "success" },
+  // CHECK-NEXT: { "from": "struct clang::Shape", "to": "class 
clang::Square", "kind": "fail" }
   // CHECK-NEXT:   ] }
 
   (void)(1 / !C);



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

[clang] 37c21db - [analyzer] Fix the debug print about debug egraph dumps requiring asserts.

[Artem Dergachev via cfe-commits]

Author: Yang Fan
Date: 2020-08-27T12:15:26-07:00
New Revision: 37c21dbb3a3209c31f22070c58f22c77357fa777

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

LOG: [analyzer] Fix the debug print about debug egraph dumps requiring asserts.

There's no need to remind people about that when clang *is* built with asserts.

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp 
b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 27b3e7ddb44e..a4b11b5e8a96 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -3154,8 +3154,9 @@ void ExprEngine::ViewGraph(bool trim) {
 #ifndef NDEBUG
   std::string Filename = DumpGraph(trim);
   llvm::DisplayGraph(Filename, false, llvm::GraphProgram::DOT);
-#endif
+#else
   llvm::errs() << "Warning: viewing graph requires assertions" << "\n";
+#endif
 }
 
 
@@ -3163,8 +3164,9 @@ void ExprEngine::ViewGraph(ArrayRef 
Nodes) {
 #ifndef NDEBUG
   std::string Filename = DumpGraph(Nodes);
   llvm::DisplayGraph(Filename, false, llvm::GraphProgram::DOT);
-#endif
+#else
   llvm::errs() << "Warning: viewing graph requires assertions" << "\n";
+#endif
 }
 
 std::string ExprEngine::DumpGraph(bool trim, StringRef Filename) {
@@ -3201,15 +3203,17 @@ std::string ExprEngine::DumpGraph(ArrayRef Nodes,
 
   if (!TrimmedG.get()) {
 llvm::errs() << "warning: Trimmed ExplodedGraph is empty.\n";
+return "";
   } else {
 return llvm::WriteGraph(TrimmedG.get(), "TrimmedExprEngine",
 /*ShortNames=*/false,
 /*Title=*/"Trimmed Exploded Graph",
 /*Filename=*/std::string(Filename));
   }
-#endif
+#else
   llvm::errs() << "Warning: dumping graph requires assertions" << "\n";
   return "";
+#endif
 }
 
 void *ProgramStateTrait::GDMIndex() {



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

[PATCH] D85752: [Analyzer] Store the pointed/referenced type for dynamic casts

[Artem Dergachev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5a9e7789396e: [analyzer] NFC: Store the pointee/referenced 
type for dynamic type tracking. (authored by baloghadamsoftware, committed by 
dergachev.a).

Changed prior to commit:
  https://reviews.llvm.org/D85752?vs=285036=288419#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85752

Files:
  clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
  clang/lib/StaticAnalyzer/Core/DynamicType.cpp
  clang/test/Analysis/cast-value-state-dump.cpp


Index: clang/test/Analysis/cast-value-state-dump.cpp
===
--- clang/test/Analysis/cast-value-state-dump.cpp
+++ clang/test/Analysis/cast-value-state-dump.cpp
@@ -35,8 +35,8 @@
   // CHECK-NEXT: ],
   // CHECK-NEXT: "dynamic_casts": [
   // CHECK:{ "region": "SymRegion{reg_$0}", "casts": [
-  // CHECK-NEXT: { "from": "const struct clang::Shape *", "to": "const 
class clang::Circle *", "kind": "success" },
-  // CHECK-NEXT: { "from": "const struct clang::Shape *", "to": "const 
class clang::Square *", "kind": "fail" }
+  // CHECK-NEXT: { "from": "struct clang::Shape", "to": "class 
clang::Circle", "kind": "success" },
+  // CHECK-NEXT: { "from": "struct clang::Shape", "to": "class 
clang::Square", "kind": "fail" }
   // CHECK-NEXT:   ] }
 
   (void)(1 / !C);
Index: clang/lib/StaticAnalyzer/Core/DynamicType.cpp
===
--- clang/lib/StaticAnalyzer/Core/DynamicType.cpp
+++ clang/lib/StaticAnalyzer/Core/DynamicType.cpp
@@ -65,6 +65,13 @@
   return State->get(MR);
 }
 
+static void unbox(QualType ) {
+  // FIXME: Why are we being fed references to pointers in the first place?
+  while (Ty->isReferenceType() || Ty->isPointerType())
+Ty = Ty->getPointeeType();
+  Ty = Ty.getCanonicalType().getUnqualifiedType();
+}
+
 const DynamicCastInfo *getDynamicCastInfo(ProgramStateRef State,
   const MemRegion *MR,
   QualType CastFromTy,
@@ -73,6 +80,9 @@
   if (!Lookup)
 return nullptr;
 
+  unbox(CastFromTy);
+  unbox(CastToTy);
+
   for (const DynamicCastInfo  : *Lookup)
 if (Cast.equals(CastFromTy, CastToTy))
   return 
@@ -112,6 +122,9 @@
 State = State->set(MR, CastToTy);
   }
 
+  unbox(CastFromTy);
+  unbox(CastToTy);
+
   DynamicCastInfo::CastResult ResultKind =
   CastSucceeds ? DynamicCastInfo::CastResult::Success
: DynamicCastInfo::CastResult::Failure;
Index: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
@@ -106,7 +106,7 @@
  QualType CastToTy, const Expr *Object,
  bool CastSucceeds, bool IsKnownCast) {
   std::string CastToName =
-  CastInfo ? CastInfo->to()->getPointeeCXXRecordDecl()->getNameAsString()
+  CastInfo ? CastInfo->to()->getAsCXXRecordDecl()->getNameAsString()
: CastToTy->getPointeeCXXRecordDecl()->getNameAsString();
   Object = Object->IgnoreParenImpCasts();
 


Index: clang/test/Analysis/cast-value-state-dump.cpp
===
--- clang/test/Analysis/cast-value-state-dump.cpp
+++ clang/test/Analysis/cast-value-state-dump.cpp
@@ -35,8 +35,8 @@
   // CHECK-NEXT: ],
   // CHECK-NEXT: "dynamic_casts": [
   // CHECK:{ "region": "SymRegion{reg_$0}", "casts": [
-  // CHECK-NEXT: { "from": "const struct clang::Shape *", "to": "const class clang::Circle *", "kind": "success" },
-  // CHECK-NEXT: { "from": "const struct clang::Shape *", "to": "const class clang::Square *", "kind": "fail" }
+  // CHECK-NEXT: { "from": "struct clang::Shape", "to": "class clang::Circle", "kind": "success" },
+  // CHECK-NEXT: { "from": "struct clang::Shape", "to": "class clang::Square", "kind": "fail" }
   // CHECK-NEXT:   ] }
 
   (void)(1 / !C);
Index: clang/lib/StaticAnalyzer/Core/DynamicType.cpp
===
--- clang/lib/StaticAnalyzer/Core/DynamicType.cpp
+++ clang/lib/StaticAnalyzer/Core/DynamicType.cpp
@@ -65,6 +65,13 @@
   return State->get(MR);
 }
 
+static void unbox(QualType ) {
+  // FIXME: Why are we being fed references to pointers in the first place?
+  while (Ty->isReferenceType() || Ty->isPointerType())
+Ty = Ty->getPointeeType();
+  Ty = Ty.getCanonicalType().getUnqualifiedType();
+}
+
 const DynamicCastInfo *getDynamicCastInfo(ProgramStateRef State,
   const MemRegion *MR,
   QualType CastFromTy,
@@ -73,6 +80,9 @@
   if (!Lookup)

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

[Scott Shawcroft via Phabricator via cfe-commits]

tannewt added a comment.

Is this superseded by: https://reviews.llvm.org/D79919 ?


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] D86719: [SyntaxTree][NFC] Refactor function templates into functions taking base class

[Dmitri Gribenko via Phabricator via cfe-commits]

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

> Perhaps we should inline getQualifiedNameStart and getInitializerRange and 
> make getDeclaratorRange a member function taking a Decl. What do you think?

These helpers seem to have well-defined semantics, so I'd rather keep them 
separate. If anything, I'd make helpers into free functions to emphasize that 
they don't use the class member variables and are standalone functions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86719

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

[PATCH] D85752: [Analyzer] Store the pointed/referenced type for dynamic casts

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/DynamicType.cpp:73
+Ty = STTPTy->getReplacementType();
+  if (Ty->isPointerType())
+Ty = Ty->getPointeeType();

NoQ wrote:
> baloghadamsoftware wrote:
> > xazax.hun wrote:
> > > Is this doing what you intended? What about a reference to a pointer? 
> > > Wouldn't you do too much unboxing? 
> > > 
> > > Also, I think a function returning a value would be more conventional. 
> > > 
> > > Other sugars like typedefs cannot interfere? I think this patch might 
> > > benefit from additional test coverage. I also see no tests for template 
> > > substitutions.
> > Reference to pointer cast using //LLVM//'s cast functions are syntactically 
> > invalid, they do not compile.
> > 
> > For `QualType` in-place modification is usual, since we use it by value.
> > 
> > I see no test coverage for this particular part of the analyzer 
> > specifically, it seems that its is only tested indirectly in the tests for 
> > `CastValueChecker`.
> The usual idiom is
> ```lang=c++
> if (Ty->isPointerType() || Ty->isReferenceType())
>   Ty = Ty->getPointeeType();
> ```
> It makes sure that you don't unwrap things twice. Also there's no need to 
> canonicalize before unwrapping the pointee type; all these methods 
> automatically operate over the canonical type.
> 
> You might want to add tests for typedefs.
Oof, this function is actually being fed references-to-pointers. I'll add a 
fixme.


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

https://reviews.llvm.org/D85752

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

[PATCH] D85091: [Sema, CodeGen] Implement [[likely]] and [[unlikely]] in IfStmt

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:1288
+def Likely : StmtAttr {
+  let Spellings = [CXX11<"", "likely", 201803>];
+  let Documentation = [LikelihoodDocs];

Mordante wrote:
> aaron.ballman wrote:
> > Mordante wrote:
> > > Mordante wrote:
> > > > aaron.ballman wrote:
> > > > > Mordante wrote:
> > > > > > Mordante wrote:
> > > > > > > aaron.ballman wrote:
> > > > > > > > Mordante wrote:
> > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > Hmm, I'm on the fence about specifying `201803` for these 
> > > > > > > > > > attributes. Given that this is only the start of supporting 
> > > > > > > > > > the attribute, do we want to claim it already matches the 
> > > > > > > > > > standard's behavior? Or do we just want to return `1` to 
> > > > > > > > > > signify that we understand this attribute but we don't yet 
> > > > > > > > > > fully support it in common cases (such as on labels in 
> > > > > > > > > > switch statements, etc)?
> > > > > > > > > > 
> > > > > > > > > > As another question, should we consider adding a C2x 
> > > > > > > > > > spelling `[[clang::likely]]` and `[[clang::unlikely]]` to 
> > > > > > > > > > add this functionality to C?
> > > > > > > > > I was also somewhat on the fence, for now I'll change it to 
> > > > > > > > > specify `1`.
> > > > > > > > > 
> > > > > > > > > I'll have a look at the C2x changes. Just curious do you know 
> > > > > > > > > whether there's a proposal to add this to C2x?
> > > > > > > > > I'll have a look at the C2x changes. Just curious do you know 
> > > > > > > > > whether there's a proposal to add this to C2x?
> > > > > > > > 
> > > > > > > > There isn't one currently because there is no implementation 
> > > > > > > > experience with the attributes in C. This is a way to get that 
> > > > > > > > implementation experience so it's easier to propose the feature 
> > > > > > > > to WG14.
> > > > > > > > I was also somewhat on the fence, for now I'll change it to 
> > > > > > > > specify `1`.
> > > > > > > 
> > > > > > > There seems to be an issue using the `1` so I used `2` instead. 
> > > > > > > (Didn't want to look closely at the issue.)
> > > > > > > 
> > > > > > > > I'll have a look at the C2x changes. Just curious do you know 
> > > > > > > > whether there's a proposal to add this to C2x?
> > > > > > > 
> > > > > > > There isn't one currently because there is no implementation 
> > > > > > > experience with the attributes in C. This is a way to get that 
> > > > > > > implementation experience so it's easier to propose the feature 
> > > > > > > to WG14.
> > > > > > 
> > > > > > Nice to know. It seems the C2x wasn't at straightforward as I 
> > > > > > hoped, so I didn't implement it yet. I intend to look at it later. 
> > > > > > I first want the initial part done to see whether this is the right 
> > > > > > approach.
> > > > > What issues are you running into? 1 is the default value when you 
> > > > > don't specify a value specifically.
> > > > It give the error "clang/include/clang/Basic/Attr.td:265:7: error: 
> > > > Standard attributes must have valid version information."
> > > > > > I'll have a look at the C2x changes. Just curious do you know 
> > > > > > whether there's a proposal to add this to C2x?
> > > > > 
> > > > > There isn't one currently because there is no implementation 
> > > > > experience with the attributes in C. This is a way to get that 
> > > > > implementation experience so it's easier to propose the feature to 
> > > > > WG14.
> > > > 
> > > > Nice to know. It seems the C2x wasn't at straightforward as I hoped, so 
> > > > I didn't implement it yet. I intend to look at it later. I first want 
> > > > the initial part done to see whether this is the right approach.
> > > 
> > > I had another look and I got it working in C.
> > If you leave the version number off entirely, it defaults to 1.
> Yes and that gives the same error. So the default value doesn't "compile". I 
> assume that's intentional to avoid setting a date of 1 for standard 
> attributes. So we could keep it at 2 or switch back to `201803`. Which value 
> do you prefer?
Ah, yeah, you're right (sorry for the noise). I think `2` is fine for now 
unless you find that the new patch actually hits enough of the important cases 
from the standard to justify `201803`. We can figure that out with the updated 
patch series.



Comment at: clang/include/clang/Basic/AttrDocs.td:1726
+  }
+  }];
+}

Mordante wrote:
> aaron.ballman wrote:
> > Mordante wrote:
> > > aaron.ballman wrote:
> > > > Something else we should document is what our behavior is when the 
> > > > attribute is not immediately inside of an `if` or `else` statement. 
> > > > e.g.,
> > > > ```
> > > > void func() { // Does not behave as though specified with [[gnu::hot]]
> > > >   [[likely]];
> > > > }
> > > > 
> > > > void func() {
> > > >   if (x) {
> > > > {
> > > >   [[likely]]; 

[PATCH] D86559: [Sema, CodeGen] Allow [[likely]] and [[unlikely]] on labels

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D86559#2242586 , @Mordante wrote:

>> That is exactly the behavior I am coming to believe we should follow. You 
>> can either write it on a compound statement that is guarded by a flow 
>> control decision (`if`/`else`/`while`) or you can write it on a label, 
>> otherwise the attribute is parsed and ignored (with a diagnostic). This 
>> feels like the right mixture of useful with understandable semantics so far, 
>> but perhaps we'll find other examples that change our minds.
>>
>> The fallthrough behavior question has one more case we may want to think 
>> about:
>>
>>   switch (x) {
>>   case 0:
>>   case 1:
>>   [[likely]] case 2: break;
>>   [[unlikely]] default:
>>   }
>>
>> Does this mark cases `0` and `1` as being likely or not? I could see users 
>> wanting to use shorthand rather than repeat themselves on all the cases. 
>> However, I'm also not certain whether there would be any performance impact 
>> if we marked only `case 2` as likely and left cases `0` and `1` with default 
>> likelihood. My gut feeling is that this should only mark `case 2`, but 
>> others may have different views.
>
> Not according to the standard: "A path of execution includes a label if and 
> only if it contains a jump to that label."

A switch statement contains a jump to all of its labels, though. So all of 
those labels are included in the path of execution, which is not likely what's 
intended by the standard.

> However for an if statement I use 2 weights: 2000 for likely, 1 for unlikely. 
> (These can be configured by a command-line option already used for 
> `__builtin_expect`.)
> For a switch I intent to use 3 weights: 2000 for likely, (2000 + 1)/2 for no 
> likelihood, 1 for unlikely. `__builtin_expect` doesn't have a 'neutral' 
> values since in a switch you can only mark one branch as likely. Instead of 
> adding a new option I picked the midpoint.
> So the weights in your example would be:
>
>   > switch (x) {
>   case 0:  // 1000
>   case 1:  // 1000
>   [[likely]] case 2: break; // 2000
>   [[unlikely]] default: // 1
>}

I think this is defensible.

> In this case the user could also write:
>
>   > switch (x) {
>   case 0:  // 1000
>   case 1:  // 1000
>   case 2: break; // 1000
>   [[unlikely]] default: // 1
>}
>
> where the values 0, 1, 2 still would be more likely than the default 
> statement. Obviously this will be documented when I implement the switch.
> How do you feel about this approach?

I think it's a reasonable approach; at least, I can't think of a case where 
this falls over.

>>> I fully agree the behaviour mandated by the standard is way too complex and 
>>> user unfriendly. It would have been nice if there were simpler rules, 
>>> making it easier to use and to teach. Still I think it would be best to use 
>>> the complex approach now, since that's what the standard specifies. During 
>>> that process we can see whether there are more pitfalls. Then we can 
>>> discuss it with other vendors and see whether we can change the wording of 
>>> the standard. Do you agree?
>>
>> The only requirement from the standard is that we parse `[[likely]]` or 
>> `[[unlikely]]` on a statement or label, that we don't allow conflicting 
>> attributes in the same attribute sequence, and that the attribute has no 
>> arguments to it. All the rest is recommended practice that's left up to the 
>> implementation as a matter of QoI. So we conform to the standard by 
>> following the approach on compound statements and labels + the constraint 
>> checking. We may not be following the recommended practice precisely, but we 
>> may not *want* to follow the recommended practice because it's bad for 
>> usability. So I'd like us to design the feature that we think gives the most 
>> value and is the easiest to use that matches the recommended practice as 
>> best we can, then start talking to other vendors. If other vendors don't 
>> want to follow our design, that's totally fine, but we should ensure our 
>> design *allows* users to write code that will behave similarly for other 
>> implementations without diagnostics. e.g., we don't want to design something 
>> where the user has to use macros to avoid diagnostics in cross-compiler 
>> code. After that, we may want to propose some changes to the recommended 
>> practice to WG21.
>>
>> From my current thinking, it seems that there may be agreement that allowing 
>> these on arbitrary statements may be really difficult for users to 
>> understand the behavior of and that compound statements and labels are what 
>> we think is an understandable and useful feature and is also a strict subset 
>> of what we COULD support. By that, I think we should limit the feature to 
>> just compound statements and labels; this leaves the door open to allow the 
>> attributes on arbitrary statements in the future without breaking code. If 
>> we allow on arbitrary statements from the start, we 

[PATCH] D86169: Initial support for letting plugins perform custom parsing of attribute arguments.

[Jonathan Protzenko via Phabricator via cfe-commits]

jonathan.protzenko added a comment.

In D86169#2241830 , @aaron.ballman 
wrote:

>> - Upon encountering `[[ attr(X) ]]` where `attr` is a plugin-handled 
>> attribute: clang parses `X` as token soup, only caring about finding a 
>> matching parenthesis
>
> Just to be clear, what you mean is to lex all of the tokens comprising `X` 
> and store them off for later?

Correct.

>> Does this stand any chance of working?
>
> This is along the same lines of what I was thinking, but there is one 
> potential snag. Clang layers our library components and Sema should never 
> refer back to the Parser (it doesn't link the library in). Because you have 
> an odd requirement where you need to do further parsing during the semantics 
> stage, you may run into awkwardness from this. Also, you may have to worry 
> about needing this functionality recursively. e.g., `int x [[your_attr(int y 
> [[your_attr(int z;)]];)]];` which could be... interesting.

I guess I'd have to i) call the action to create, say, a Decl, then later ii) 
call the plugin and extend the Decl with fresh attributes, assuming the 
attribute vector on the Decl can be grown *after* the Decl is created -- 
hopefully this can be driven from the parser, without a back-edge in the 
dependency graph? Or maybe I'm missing something?

Do you have any example somewhere of how to store tokens and re-fill the lex 
buffer with them later on? This is much more involved than my current patch so 
may take me a while to figure out.

>> Bonus question: seeing `diagAppertainsToDecl`, the name of this function led 
>> me to believe that for now, plugins could only handle attributes attached to 
>> declarations. Is this the case?
>
> Correct, the plugin system does not yet handle type or statement attributes 
> (it was focusing on declaration attributes first, which are the lion's share 
> of attributes in Clang).

Ok, I was asking because if there's no plans to handle type or statement 
attributes via a plugin, then we're somewhat over-engineering this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86169

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

[PATCH] D86000: Add an unsigned shift base sanitizer

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: llvm/docs/ReleaseNotes.rst:153
+
+  `(lhs & ~(~1U << ((sizeof(lhs)*CHAR_BIT) - rhs))) << rhs`
+

vsk wrote:
> jfb wrote:
> > lebedev.ri wrote:
> > > Surely not `~1U`.
> > Indeed, fixed.
> I don't think this pattern works when rhs = 0 (https://godbolt.org/z/rvEGqh).
Surely not `1U` either :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86000

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

[PATCH] D86146: [ARM][BFloat16] Change types of some Arm and AArch64 bf16 intrinsics

[Mikhail Maltsev 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 rGae1396c7d4d8: [ARM][BFloat16] Change types of some Arm and 
AArch64 bf16 intrinsics (authored by miyuki).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86146

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/aarch64-bf16-dotprod-intrinsics.c
  clang/test/CodeGen/arm-bf16-dotprod-intrinsics.c
  llvm/include/llvm/IR/IntrinsicsAArch64.td
  llvm/include/llvm/IR/IntrinsicsARM.td
  llvm/lib/IR/AutoUpgrade.cpp
  llvm/lib/Target/AArch64/AArch64InstrFormats.td
  llvm/lib/Target/AArch64/AArch64InstrInfo.td
  llvm/lib/Target/ARM/ARMInstrNEON.td
  llvm/test/Bitcode/aarch64-bf16-upgrade.ll
  llvm/test/Bitcode/aarch64-bf16-upgrade.ll.bc
  llvm/test/Bitcode/arm-bf16-upgrade.ll
  llvm/test/Bitcode/arm-bf16-upgrade.ll.bc
  llvm/test/CodeGen/AArch64/aarch64-bf16-dotprod-intrinsics.ll
  llvm/test/CodeGen/ARM/arm-bf16-dotprod-intrinsics.ll

Index: llvm/test/CodeGen/ARM/arm-bf16-dotprod-intrinsics.ll
===
--- llvm/test/CodeGen/ARM/arm-bf16-dotprod-intrinsics.ll
+++ llvm/test/CodeGen/ARM/arm-bf16-dotprod-intrinsics.ll
@@ -7,10 +7,8 @@
 ; CHECK-NEXT:vdot.bf16 d0, d1, d2
 ; CHECK-NEXT:bx lr
 entry:
-  %0 = bitcast <4 x bfloat> %a to <8 x i8>
-  %1 = bitcast <4 x bfloat> %b to <8 x i8>
-  %vbfdot1.i = call <2 x float> @llvm.arm.neon.bfdot.v2f32.v8i8(<2 x float> %r, <8 x i8> %0, <8 x i8> %1)
-  ret <2 x float> %vbfdot1.i
+  %vbfdot3.i = call <2 x float> @llvm.arm.neon.bfdot.v2f32.v4bf16(<2 x float> %r, <4 x bfloat> %a, <4 x bfloat> %b) #3
+  ret <2 x float> %vbfdot3.i
 }
 
 define <4 x float> @test_vbfdotq_f32(<4 x float> %r, <8 x bfloat> %a, <8 x bfloat> %b) {
@@ -19,10 +17,8 @@
 ; CHECK-NEXT:vdot.bf16 q0, q1, q2
 ; CHECK-NEXT:bx lr
 entry:
-  %0 = bitcast <8 x bfloat> %a to <16 x i8>
-  %1 = bitcast <8 x bfloat> %b to <16 x i8>
-  %vbfdot1.i = call <4 x float> @llvm.arm.neon.bfdot.v4f32.v16i8(<4 x float> %r, <16 x i8> %0, <16 x i8> %1)
-  ret <4 x float> %vbfdot1.i
+  %vbfdot3.i = call <4 x float> @llvm.arm.neon.bfdot.v4f32.v8bf16(<4 x float> %r, <8 x bfloat> %a, <8 x bfloat> %b) #3
+  ret <4 x float> %vbfdot3.i
 }
 
 define <2 x float> @test_vbfdot_lane_f32(<2 x float> %r, <4 x bfloat> %a, <4 x bfloat> %b) {
@@ -31,12 +27,11 @@
 ; CHECK-NEXT:vdot.bf16 d0, d1, d2[0]
 ; CHECK-NEXT:bx lr
 entry:
-  %0 = bitcast <4 x bfloat> %b to <2 x float>
-  %shuffle = shufflevector <2 x float> %0, <2 x float> undef, <2 x i32> zeroinitializer
-  %1 = bitcast <4 x bfloat> %a to <8 x i8>
-  %2 = bitcast <2 x float> %shuffle to <8 x i8>
-  %vbfdot1.i = call <2 x float> @llvm.arm.neon.bfdot.v2f32.v8i8(<2 x float> %r, <8 x i8> %1, <8 x i8> %2)
-  ret <2 x float> %vbfdot1.i
+  %.cast = bitcast <4 x bfloat> %b to <2 x float>
+  %lane = shufflevector <2 x float> %.cast, <2 x float> undef, <2 x i32> zeroinitializer
+  %.cast1 = bitcast <2 x float> %lane to <4 x bfloat>
+  %vbfdot3.i = call <2 x float> @llvm.arm.neon.bfdot.v2f32.v4bf16(<2 x float> %r, <4 x bfloat> %a, <4 x bfloat> %.cast1) #3
+  ret <2 x float> %vbfdot3.i
 }
 
 define <4 x float> @test_vbfdotq_laneq_f32(<4 x float> %r, <8 x bfloat> %a, <8 x bfloat> %b) {
@@ -46,12 +41,11 @@
 ; CHECK-NEXT:vdot.bf16 q0, q1, q8
 ; CHECK-NEXT:bx lr
 entry:
-  %0 = bitcast <8 x bfloat> %b to <4 x float>
-  %shuffle = shufflevector <4 x float> %0, <4 x float> undef, <4 x i32> 
-  %1 = bitcast <8 x bfloat> %a to <16 x i8>
-  %2 = bitcast <4 x float> %shuffle to <16 x i8>
-  %vbfdot1.i = call <4 x float> @llvm.arm.neon.bfdot.v4f32.v16i8(<4 x float> %r, <16 x i8> %1, <16 x i8> %2)
-  ret <4 x float> %vbfdot1.i
+  %.cast = bitcast <8 x bfloat> %b to <4 x float>
+  %lane = shufflevector <4 x float> %.cast, <4 x float> undef, <4 x i32> 
+  %.cast1 = bitcast <4 x float> %lane to <8 x bfloat>
+  %vbfdot3.i = call <4 x float> @llvm.arm.neon.bfdot.v4f32.v8bf16(<4 x float> %r, <8 x bfloat> %a, <8 x bfloat> %.cast1) #3
+  ret <4 x float> %vbfdot3.i
 }
 
 define <2 x float> @test_vbfdot_laneq_f32(<2 x float> %r, <4 x bfloat> %a, <8 x bfloat> %b) {
@@ -60,12 +54,11 @@
 ; CHECK-NEXT:vdot.bf16 d0, d1, d3[1]
 ; CHECK-NEXT:bx lr
 entry:
-  %0 = bitcast <8 x bfloat> %b to <4 x float>
-  %shuffle = shufflevector <4 x float> %0, <4 x float> undef, <2 x i32> 
-  %1 = bitcast <4 x bfloat> %a to <8 x i8>
-  %2 = bitcast <2 x float> %shuffle to <8 x i8>
-  %vbfdot1.i = call <2 x float> @llvm.arm.neon.bfdot.v2f32.v8i8(<2 x float> %r, <8 x i8> %1, <8 x i8> %2)
-  ret <2 x float> %vbfdot1.i
+  %.cast = bitcast <8 x bfloat> %b to <4 x float>
+  %lane = shufflevector <4 x float> %.cast, <4 x float> undef, <2 x i32> 
+  %.cast1 = bitcast <2 x float> %lane to <4 x bfloat>
+  %vbfdot3.i = call <2 x float> @llvm.arm.neon.bfdot.v2f32.v4bf16(<2 x float> %r, <4 x bfloat> %a, <4 x bfloat> %.cast1) #3
+  

[clang] ae1396c - [ARM][BFloat16] Change types of some Arm and AArch64 bf16 intrinsics

[Mikhail Maltsev via cfe-commits]

Author: Mikhail Maltsev
Date: 2020-08-27T18:43:16+01:00
New Revision: ae1396c7d4d83366695137f69f046719fd199408

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

LOG: [ARM][BFloat16] Change types of some Arm and AArch64 bf16 intrinsics

This patch adjusts the following ARM/AArch64 LLVM IR intrinsics:
- neon_bfmmla
- neon_bfmlalb
- neon_bfmlalt
so that they take and return bf16 and float types. Previously these
intrinsics used <8 x i8> and <4 x i8> vectors (a rudiment from
implementation lacking bf16 IR type).

The neon_vbfdot[q] intrinsics are adjusted similarly. This change
required some additional selection patterns for vbfdot itself and
also for vector shuffles (in a previous patch) because of SelectionDAG
transformations kicking in and mangling the original code.

This patch makes the generated IR cleaner (less useless bitcasts are
produced), but it does not affect the final assembly.

Reviewed By: dmgreen

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

Added: 
llvm/test/Bitcode/aarch64-bf16-upgrade.ll
llvm/test/Bitcode/aarch64-bf16-upgrade.ll.bc
llvm/test/Bitcode/arm-bf16-upgrade.ll
llvm/test/Bitcode/arm-bf16-upgrade.ll.bc

Modified: 
clang/lib/CodeGen/CGBuiltin.cpp
clang/test/CodeGen/aarch64-bf16-dotprod-intrinsics.c
clang/test/CodeGen/arm-bf16-dotprod-intrinsics.c
llvm/include/llvm/IR/IntrinsicsAArch64.td
llvm/include/llvm/IR/IntrinsicsARM.td
llvm/lib/IR/AutoUpgrade.cpp
llvm/lib/Target/AArch64/AArch64InstrFormats.td
llvm/lib/Target/AArch64/AArch64InstrInfo.td
llvm/lib/Target/ARM/ARMInstrNEON.td
llvm/test/CodeGen/AArch64/aarch64-bf16-dotprod-intrinsics.ll
llvm/test/CodeGen/ARM/arm-bf16-dotprod-intrinsics.ll

Removed: 




diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 24e33c164009..69899fd8e668 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -6241,28 +6241,10 @@ Value *CodeGenFunction::EmitCommonNeonBuiltinExpr(
   case NEON::BI__builtin_neon_vbfdot_v:
   case NEON::BI__builtin_neon_vbfdotq_v: {
 llvm::Type *InputTy =
-llvm::FixedVectorType::get(Int8Ty, Ty->getPrimitiveSizeInBits() / 8);
+llvm::FixedVectorType::get(BFloatTy, Ty->getPrimitiveSizeInBits() / 
16);
 llvm::Type *Tys[2] = { Ty, InputTy };
 return EmitNeonCall(CGM.getIntrinsic(Int, Tys), Ops, "vbfdot");
   }
-  case NEON::BI__builtin_neon_vbfmmlaq_v: {
-llvm::Type *InputTy =
-llvm::FixedVectorType::get(Int8Ty, Ty->getPrimitiveSizeInBits() / 8);
-llvm::Type *Tys[2] = { Ty, InputTy };
-return EmitNeonCall(CGM.getIntrinsic(Int, Tys), Ops, "vbfmmla");
-  }
-  case NEON::BI__builtin_neon_vbfmlalbq_v: {
-llvm::Type *InputTy =
-llvm::FixedVectorType::get(Int8Ty, Ty->getPrimitiveSizeInBits() / 8);
-llvm::Type *Tys[2] = { Ty, InputTy };
-return EmitNeonCall(CGM.getIntrinsic(Int, Tys), Ops, "vbfmlalb");
-  }
-  case NEON::BI__builtin_neon_vbfmlaltq_v: {
-llvm::Type *InputTy =
-llvm::FixedVectorType::get(Int8Ty, Ty->getPrimitiveSizeInBits() / 8);
-llvm::Type *Tys[2] = { Ty, InputTy };
-return EmitNeonCall(CGM.getIntrinsic(Int, Tys), Ops, "vbfmlalt");
-  }
   case NEON::BI__builtin_neon___a32_vcvt_bf16_v: {
 llvm::Type *Tys[1] = { Ty };
 Function *F = CGM.getIntrinsic(Int, Tys);

diff  --git a/clang/test/CodeGen/aarch64-bf16-dotprod-intrinsics.c 
b/clang/test/CodeGen/aarch64-bf16-dotprod-intrinsics.c
index 22e1396787ce..b9b5013bf6bd 100644
--- a/clang/test/CodeGen/aarch64-bf16-dotprod-intrinsics.c
+++ b/clang/test/CodeGen/aarch64-bf16-dotprod-intrinsics.c
@@ -1,146 +1,138 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
 // RUN: %clang_cc1 -triple aarch64-arm-none-eabi -target-feature +neon 
-target-feature +bf16 \
 // RUN: -disable-O0-optnone -emit-llvm %s -o - | opt -S -mem2reg -instcombine 
| FileCheck %s
 
 #include 
 
-// CHECK-LABEL: test_vbfdot_f32
-// CHECK-NEXT: entry:
-// CHECK-NEXT  %0 = bitcast <4 x bfloat> %a to <8 x i8>
-// CHECK-NEXT  %1 = bitcast <4 x bfloat> %b to <8 x i8>
-// CHECK-NEXT  %vbfdot1.i = tail call <2 x float> 
@llvm.aarch64.neon.bfdot.v2f32.v8i8(<2 x float> %r, <8 x i8> %0, <8 x i8> %1)
-// CHECK-NEXT  ret <2 x float> %vbfdot1.i
+// CHECK-LABEL: @test_vbfdot_f32(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[VBFDOT3_I:%.*]] = call <2 x float> 
@llvm.aarch64.neon.bfdot.v2f32.v4bf16(<2 x float> [[R:%.*]], <4 x bfloat> 
[[A:%.*]], <4 x bfloat> [[B:%.*]]) [[ATTR3:#.*]]
+// CHECK-NEXT:ret <2 x float> [[VBFDOT3_I]]
+//
 float32x2_t test_vbfdot_f32(float32x2_t r, bfloat16x4_t a, bfloat16x4_t b) {
   return vbfdot_f32(r, a, b);
 }
 
-// CHECK-LABEL: test_vbfdotq_f32
-// CHECK-NEXT: entry:
-// CHECK-NEXT  %0 = bitcast <8 x bfloat> %a to 

[PATCH] D86000: Add an unsigned shift base sanitizer

[Vedant Kumar via Phabricator via cfe-commits]

vsk added inline comments.



Comment at: compiler-rt/test/ubsan/TestCases/Integer/unsigned-shift.cpp:6
+  volatile unsigned _v = (val);\
+  volatile unsigned _a = (amount); \
+  unsigned res = _v << _a; \

jfb wrote:
> vsk wrote:
> > Does the test not work without the volatiles?
> It seems that LLVM sees too much without volatile, yes.
The optimizer isn't running. Perhaps this is necessary because clang's constant 
folder kicks in?



Comment at: llvm/docs/ReleaseNotes.rst:151
+
+One can silence the check with masking, for example:
+

This could use some more explanation, maybe s/with masking/by clearing bits 
that would be shifted out/?



Comment at: llvm/docs/ReleaseNotes.rst:153
+
+  `(lhs & ~(~1U << ((sizeof(lhs)*CHAR_BIT) - rhs))) << rhs`
+

jfb wrote:
> lebedev.ri wrote:
> > Surely not `~1U`.
> Indeed, fixed.
I don't think this pattern works when rhs = 0 (https://godbolt.org/z/rvEGqh).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86000

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

[PATCH] D86000: Add an unsigned shift base sanitizer

[JF Bastien via Phabricator via cfe-commits]

jfb added inline comments.



Comment at: llvm/docs/ReleaseNotes.rst:153
+
+  `(lhs & ~(~1U << ((sizeof(lhs)*CHAR_BIT) - rhs))) << rhs`
+

lebedev.ri wrote:
> Surely not `~1U`.
Indeed, fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86000

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

[PATCH] D86000: Add an unsigned shift base sanitizer

[JF Bastien via Phabricator via cfe-commits]

jfb updated this revision to Diff 288388.
jfb marked an inline comment as done.
jfb added a comment.

Fix notes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86000

Files:
  clang/docs/UndefinedBehaviorSanitizer.rst
  clang/include/clang/Basic/Sanitizers.def
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/test/CodeGen/unsigned-shift-base.c
  clang/test/Driver/fsanitize.c
  compiler-rt/test/ubsan/TestCases/Integer/unsigned-shift.cpp
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -140,6 +140,18 @@
 Changes to LLDB
 -
 
+Changes to Sanitizers
+-
+
+The integer sanitizer `-fsanitize=integer` now has a new sanitizer:
+`-fsanitize=unsigned-shift-base`. It's not undefined behavior for an unsigned
+left shift to overflow (i.e. to shift bits out), but it has been the source of
+bugs and exploits in certain codebases in the past.
+
+One can silence the check with masking, for example:
+
+  `(lhs & ~(1U << ((sizeof(lhs)*CHAR_BIT) - rhs))) << rhs`
+
 External Open Source Projects Using LLVM 12
 ===
 
Index: compiler-rt/test/ubsan/TestCases/Integer/unsigned-shift.cpp
===
--- /dev/null
+++ compiler-rt/test/ubsan/TestCases/Integer/unsigned-shift.cpp
@@ -0,0 +1,54 @@
+// RUN: %clangxx -fsanitize=unsigned-shift-base %s -o %t1 && not %run %t1 2>&1 | FileCheck %s
+// RUN: %clangxx -fsanitize=unsigned-shift-base,shift-exponent %s -o %t1 && not %run %t1 2>&1 | FileCheck %s
+
+#define shift(val, amount) ({  \
+  volatile unsigned _v = (val);\
+  volatile unsigned _a = (amount); \
+  unsigned res = _v << _a; \
+  res; \
+})
+
+int main() {
+
+  shift(0b''', 31);
+  shift(0b'''0001, 31);
+  shift(0b'''0010, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 2 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b'''0100, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 4 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b'''1000, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 8 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b'''0001, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 16 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b'''0010, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 32 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b'''0100, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 64 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b'''1000, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 128 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b''0001', 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 256 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b''0010', 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 512 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b''0100', 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 1024 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b''1000', 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 2048 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b''0001', 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 4096 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b''0010', 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 8192 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b''0100', 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 16384 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b''1000', 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 32768 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b'0001'', 31); // CHECK: 

[PATCH] D86065: [SVE] Make ElementCount members private

[Paul Walker via Phabricator via cfe-commits]

paulwalker-arm accepted this revision.
paulwalker-arm added a comment.
This revision is now accepted and ready to land.

There's probably a few .Min to .getKnownMinValue() conversions where the .Min 
could be dropped (calls to Builder.CreateVectorSplat for example) but they can 
be tidied up as part of a proper activity to reduce the places where 
getKnownMinValue is called.  So other than my suggested updated to 
EC::operator/ the patch looks good to my eye.  Please give other reviewers a 
little more time to provide other insights.




Comment at: llvm/include/llvm/Support/TypeSize.h:66
+
+  ElementCount /=(unsigned RHS) {
+Min /= RHS;

If you add an assert that the divide is lossless (i.e. MIN % RHS == 0) then 
asserts like:
```
assert(EltCnt.isKnownEven() && "Splitting vector, but not in half!");
```
are no longer required.  Plus those places which are not checking for lossless 
division will be automatically protected.  This feels like a sensible default 
to me.  If somebody wants a truncated result, they can do the maths using 
getKnownMinValue().


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

https://reviews.llvm.org/D86065

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

[PATCH] D86000: Add an unsigned shift base sanitizer

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: llvm/docs/ReleaseNotes.rst:153
+
+  `(lhs & ~(~1U << ((sizeof(lhs)*CHAR_BIT) - rhs))) << rhs`
+

Surely not `~1U`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86000

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

[PATCH] D86559: [Sema, CodeGen] Allow [[likely]] and [[unlikely]] on labels

[Mark de Wever via Phabricator via cfe-commits]

Mordante planned changes to this revision.
Mordante added a comment.

> That is exactly the behavior I am coming to believe we should follow. You can 
> either write it on a compound statement that is guarded by a flow control 
> decision (`if`/`else`/`while`) or you can write it on a label, otherwise the 
> attribute is parsed and ignored (with a diagnostic). This feels like the 
> right mixture of useful with understandable semantics so far, but perhaps 
> we'll find other examples that change our minds.
>
> The fallthrough behavior question has one more case we may want to think 
> about:
>
>   switch (x) {
>   case 0:
>   case 1:
>   [[likely]] case 2: break;
>   [[unlikely]] default:
>   }
>
> Does this mark cases `0` and `1` as being likely or not? I could see users 
> wanting to use shorthand rather than repeat themselves on all the cases. 
> However, I'm also not certain whether there would be any performance impact 
> if we marked only `case 2` as likely and left cases `0` and `1` with default 
> likelihood. My gut feeling is that this should only mark `case 2`, but others 
> may have different views.

Not according to the standard: "A path of execution includes a label if and 
only if it contains a jump to that label."
However for an if statement I use 2 weights: 2000 for likely, 1 for unlikely. 
(These can be configured by a command-line option already used for 
`__builtin_expect`.)
For a switch I intent to use 3 weights: 2000 for likely, (2000 + 1)/2 for no 
likelihood, 1 for unlikely. `__builtin_expect` doesn't have a 'neutral' values 
since in a switch you can only mark one branch as likely. Instead of adding a 
new option I picked the midpoint.
So the weights in your example would be:

  > switch (x) {
  case 0:  // 1000
  case 1:  // 1000
  [[likely]] case 2: break; // 2000
  [[unlikely]] default: // 1
   }

In this case the user could also write:

  > switch (x) {
  case 0:  // 1000
  case 1:  // 1000
  case 2: break; // 1000
  [[unlikely]] default: // 1
   }

where the values 0, 1, 2 still would be more likely than the default statement. 
Obviously this will be documented when I implement the switch.
How do you feel about this approach?

>> I fully agree the behaviour mandated by the standard is way too complex and 
>> user unfriendly. It would have been nice if there were simpler rules, making 
>> it easier to use and to teach. Still I think it would be best to use the 
>> complex approach now, since that's what the standard specifies. During that 
>> process we can see whether there are more pitfalls. Then we can discuss it 
>> with other vendors and see whether we can change the wording of the 
>> standard. Do you agree?
>
> The only requirement from the standard is that we parse `[[likely]]` or 
> `[[unlikely]]` on a statement or label, that we don't allow conflicting 
> attributes in the same attribute sequence, and that the attribute has no 
> arguments to it. All the rest is recommended practice that's left up to the 
> implementation as a matter of QoI. So we conform to the standard by following 
> the approach on compound statements and labels + the constraint checking. We 
> may not be following the recommended practice precisely, but we may not 
> *want* to follow the recommended practice because it's bad for usability. So 
> I'd like us to design the feature that we think gives the most value and is 
> the easiest to use that matches the recommended practice as best we can, then 
> start talking to other vendors. If other vendors don't want to follow our 
> design, that's totally fine, but we should ensure our design *allows* users 
> to write code that will behave similarly for other implementations without 
> diagnostics. e.g., we don't want to design something where the user has to 
> use macros to avoid diagnostics in cross-compiler code. After that, we may 
> want to propose some changes to the recommended practice to WG21.
>
> From my current thinking, it seems that there may be agreement that allowing 
> these on arbitrary statements may be really difficult for users to understand 
> the behavior of and that compound statements and labels are what we think is 
> an understandable and useful feature and is also a strict subset of what we 
> COULD support. By that, I think we should limit the feature to just compound 
> statements and labels; this leaves the door open to allow the attributes on 
> arbitrary statements in the future without breaking code. If we allow on 
> arbitrary statements from the start, we can't later restrict the feature 
> because we'd break code. This lets us start getting field experience with 
> that behavior to see how we like it, which may also help when talking to 
> other vendors because we'll have something concrete to talk about 
> (hopefully). WDYT?

I'm somewhat on the fence. In general I prefer to implement what the standard 
expects. However the more I try to do that, the more I'm convinced what the 
standard suggests is difficult to 

[PATCH] D86000: Add an unsigned shift base sanitizer

[JF Bastien via Phabricator via cfe-commits]

jfb added inline comments.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3872-3884
   llvm::Value *BitsShiftedOff = Builder.CreateLShr(
   Ops.LHS, Builder.CreateSub(PromotedWidthMinusOne, RHS, "shl.zeros",
  /*NUW*/ true, /*NSW*/ true),
   "shl.check");
-  if (CGF.getLangOpts().CPlusPlus) {
+  if (SanitizeUnsignedBase || CGF.getLangOpts().CPlusPlus) {
 // In C99, we are not permitted to shift a 1 bit into the sign bit.
 // Under C++11's rules, shifting a 1 bit into the sign bit is

lebedev.ri wrote:
> Why is this so complicated? Shouldn't this just be: 
> https://alive2.llvm.org/ce/z/scTqfX
> ```
> $ /repositories/alive2/build-Clang-release/alive-tv /tmp/test.ll 
> --smt-to=10 --disable-undef-input
> 
> 
> @2 = global 32 bytes, align 16
> 
> define i32 @src(i32 %arg, i32 %arg1) {
> %bb:
>   %i = icmp ugt i32 %arg1, 31
>   %i2 = sub nsw nuw i32 31, %arg1; NOPE
>   %i3 = lshr i32 %arg, %i2   ; NOPE
>   %i4 = icmp ult i32 %i3, 2  ; NOPE
>   %i5 = or i1 %i, %i4
>   br i1 %i5, label %bb9, label %bb6
> 
> %bb6:
>   %i7 = zext i32 %arg to i64
>   %i8 = zext i32 %arg1 to i64
>   %__constexpr_0 = bitcast * @2 to *
>   call void @__ubsan_handle_shift_out_of_bounds(* %__constexpr_0, i64 %i7, 
> i64 %i8)
>   br label %bb9
> 
> %bb9:
>   %i10 = shl i32 %arg, %arg1
>   ret i32 %i10
> }
> =>
> @2 = global 32 bytes, align 16
> 
> define i32 @tgt(i32 %arg, i32 %arg1) {
> %bb:
>   %i = icmp ugt i32 %arg1, 31
>   %iZZ0 = shl i32 %arg, %arg1; HI!
>   %iZZ1 = lshr i32 %iZZ0, %arg1  ; OVER HERE
>   %i4 = icmp eq i32 %arg, %iZZ1  ; LOOK!
>   %i5 = or i1 %i, %i4
>   br i1 %i5, label %bb9, label %bb6
> 
> %bb6:
>   %i7 = zext i32 %arg to i64
>   %i8 = zext i32 %arg1 to i64
>   %__constexpr_0 = bitcast * @2 to *
>   call void @__ubsan_handle_shift_out_of_bounds(* %__constexpr_0, i64 %i7, 
> i64 %i8)
>   br label %bb9
> 
> %bb9:
>   ret i32 %iZZ0
> }
> Transformation seems to be correct!
> 
> ```
> which will then be migrated to use `@llvm.ushl.with.overflow` once it's there.
Sure, but that's pre-existing and I'd rather not change it in this patch.



Comment at: compiler-rt/test/ubsan/TestCases/Integer/unsigned-shift.cpp:2
+// RUN: %clangxx -fsanitize=unsigned-shift-base %s -o %t1 && not %run %t1 2>&1 
| FileCheck %s
+// RUN: %clangxx -fsanitize=unsigned-shift-base,shift-exponent %s -o %t1 && 
not %run %t1 2>&1 | FileCheck %s
+

vsk wrote:
> jfb wrote:
> > I don't understand this test... when I run it with lit it fails (and it 
> > seems like the bots agree), but manually it works. Am I doing it wrong?
> Do you need to explicitly `return 1` or something to get a non-zero exit code?
That should be implicit, but I added it regardless to make sure. It's happy now 
路‍♂️



Comment at: compiler-rt/test/ubsan/TestCases/Integer/unsigned-shift.cpp:6
+  volatile unsigned _v = (val);\
+  volatile unsigned _a = (amount); \
+  unsigned res = _v << _a; \

vsk wrote:
> Does the test not work without the volatiles?
It seems that LLVM sees too much without volatile, yes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86000

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

[PATCH] D86000: Add an unsigned shift base sanitizer

[JF Bastien via Phabricator via cfe-commits]

jfb updated this revision to Diff 288383.
jfb marked 6 inline comments as done.
jfb added a comment.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86000

Files:
  clang/docs/UndefinedBehaviorSanitizer.rst
  clang/include/clang/Basic/Sanitizers.def
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/test/CodeGen/unsigned-shift-base.c
  clang/test/Driver/fsanitize.c
  compiler-rt/test/ubsan/TestCases/Integer/unsigned-shift.cpp
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -140,6 +140,18 @@
 Changes to LLDB
 -
 
+Changes to Sanitizers
+-
+
+The integer sanitizer `-fsanitize=integer` now has a new sanitizer:
+`-fsanitize=unsigned-shift-base`. It's not undefined behavior for an unsigned
+left shift to overflow (i.e. to shift bits out), but it has been the source of
+bugs and exploits in certain codebases in the past.
+
+One can silence the check with masking, for example:
+
+  `(lhs & ~(~1U << ((sizeof(lhs)*CHAR_BIT) - rhs))) << rhs`
+
 External Open Source Projects Using LLVM 12
 ===
 
Index: compiler-rt/test/ubsan/TestCases/Integer/unsigned-shift.cpp
===
--- /dev/null
+++ compiler-rt/test/ubsan/TestCases/Integer/unsigned-shift.cpp
@@ -0,0 +1,54 @@
+// RUN: %clangxx -fsanitize=unsigned-shift-base %s -o %t1 && not %run %t1 2>&1 | FileCheck %s
+// RUN: %clangxx -fsanitize=unsigned-shift-base,shift-exponent %s -o %t1 && not %run %t1 2>&1 | FileCheck %s
+
+#define shift(val, amount) ({  \
+  volatile unsigned _v = (val);\
+  volatile unsigned _a = (amount); \
+  unsigned res = _v << _a; \
+  res; \
+})
+
+int main() {
+
+  shift(0b''', 31);
+  shift(0b'''0001, 31);
+  shift(0b'''0010, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 2 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b'''0100, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 4 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b'''1000, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 8 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b'''0001, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 16 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b'''0010, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 32 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b'''0100, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 64 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b'''1000, 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 128 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b''0001', 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 256 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b''0010', 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 512 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b''0100', 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 1024 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b''1000', 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 2048 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b''0001', 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 4096 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b''0010', 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 8192 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b''0100', 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 16384 by 31 places cannot be represented in type 'unsigned int'
+  shift(0b''1000', 31); // CHECK: unsigned-shift.cpp:[[@LINE]]:3: runtime error: left shift of 32768 by 31 places cannot be represented in type 'unsigned int'
+  

[PATCH] D85474: Add -fbinutils-version= to gate ELF features on the specified binutils version

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 288382.
MaskRay marked an inline comment as done.
MaskRay added a comment.

Reword a comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85474

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  llvm/include/llvm/MC/MCAsmInfo.h
  llvm/include/llvm/Target/TargetMachine.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/CodeGen/LLVMTargetMachine.cpp
  llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
  llvm/lib/Target/TargetMachine.cpp
  llvm/test/CodeGen/X86/explicit-section-mergeable.ll
  llvm/tools/llc/llc.cpp

Index: llvm/tools/llc/llc.cpp
===
--- llvm/tools/llc/llc.cpp
+++ llvm/tools/llc/llc.cpp
@@ -81,6 +81,14 @@
  cl::value_desc("N"),
  cl::desc("Repeat compilation N times for timing"));
 
+static cl::opt BinutilsVersion(
+"binutils-version", cl::Hidden,
+cl::desc(
+"Used ELF features need to be supported by GNU ld "
+"from the specified binutils version. 'future' means that "
+"features not implemented by any known release can be used. If "
+"-no-integrated-as is specified, this also affects assembly output"));
+
 static cl::opt
 NoIntegratedAssembler("no-integrated-as", cl::Hidden,
   cl::desc("Disable integrated assembler"));
@@ -425,6 +433,8 @@
   }
 
   TargetOptions Options = codegen::InitTargetOptionsFromCodeGenFlags();
+  Options.BinutilsVersion =
+  TargetMachine::parseBinutilsVersion(BinutilsVersion);
   Options.DisableIntegratedAS = NoIntegratedAssembler;
   Options.MCOptions.ShowMCEncoding = ShowMCEncoding;
   Options.MCOptions.MCUseDwarfDirectory = EnableDwarfDirectory;
Index: llvm/test/CodeGen/X86/explicit-section-mergeable.ll
===
--- llvm/test/CodeGen/X86/explicit-section-mergeable.ll
+++ llvm/test/CodeGen/X86/explicit-section-mergeable.ll
@@ -282,15 +282,19 @@
 ;; --no-integrated-as avoids the use of ",unique," for compatibility with older binutils.
 
 ;; Error if an incompatible symbol is explicitly placed into a mergeable section.
-; RUN: not llc < %s -mtriple=x86_64 --no-integrated-as 2>&1 \
+; RUN: not llc < %s -mtriple=x86_64 --no-integrated-as -binutils-version=2.34 2>&1 \
 ; RUN: | FileCheck %s --check-prefix=NO-I-AS-ERR
 ; NO-I-AS-ERR: error: Symbol 'explicit_default_1' from module '' required a section with entry-size=0 but was placed in section '.rodata.cst16' with entry-size=16: Explicit assignment by pragma or attribute of an incompatible symbol to this section?
 ; NO-I-AS-ERR: error: Symbol 'explicit_default_4' from module '' required a section with entry-size=0 but was placed in section '.debug_str' with entry-size=1: Explicit assignment by pragma or attribute of an incompatible symbol to this section?
 ; NO-I-AS-ERR: error: Symbol 'explicit_implicit_2' from module '' required a section with entry-size=0 but was placed in section '.rodata.str1.1' with entry-size=1: Explicit assignment by pragma or attribute of an incompatible symbol to this section?
 ; NO-I-AS-ERR: error: Symbol 'explicit_implicit_4' from module '' required a section with entry-size=0 but was placed in section '.rodata.str1.1' with entry-size=1: Explicit assignment by pragma or attribute of an incompatible symbol to this section?
 
+;; For GNU as before 2.35,
 ;; Don't create mergeable sections for globals with an explicit section name.
 ; RUN: echo '@explicit = unnamed_addr constant [2 x i16] [i16 1, i16 1], section ".explicit"' > %t.no_i_as.ll
-; RUN: llc < %t.no_i_as.ll -mtriple=x86_64 --no-integrated-as 2>&1 \
-; RUN: | FileCheck %s --check-prefix=NO-I-AS
-; NO-I-AS: .section .explicit,"a",@progbits
+; RUN: llc < %t.no_i_as.ll -mtriple=x86_64 --no-integrated-as -binutils-version=2.34 2>&1 \
+; RUN: | FileCheck %s --check-prefix=NO-I-AS-OLD
+; NO-I-AS-OLD: .section .explicit,"a",@progbits
+; RUN: llc < %t.no_i_as.ll -mtriple=x86_64 --no-integrated-as -binutils-version=2.35 2>&1 \
+; RUN: | FileCheck %s --check-prefix=NO-I-AS-NEW
+; NO-I-AS-NEW: .section .explicit,"aM",@progbits,4,unique,1
Index: llvm/lib/Target/TargetMachine.cpp
===
--- llvm/lib/Target/TargetMachine.cpp
+++ llvm/lib/Target/TargetMachine.cpp
@@ -281,3 +281,12 @@
   return TargetIRAnalysis(
   [this](const Function ) { return this->getTargetTransformInfo(F); });
 }
+
+std::pair TargetMachine::parseBinutilsVersion(StringRef Version) {
+  if (Version == "future")
+return {(int)TargetOptions::FutureBinutilsVersion, 0};
+  std::pair Ret;
+  if (!Version.consumeInteger(10, Ret.first) && Version.consume_front("."))
+  

[PATCH] D86290: Move all fields of '-cc1' option related classes into def file databases

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese accepted this revision.
Bigcheese 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/D86290/new/

https://reviews.llvm.org/D86290

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

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

[Kostya Kortchinsky via Phabricator via cfe-commits]

cryptoad added a comment.

In D86694#2242150 , @russell.gallop 
wrote:

> In D86694#2242140 , @cryptoad wrote:
>
>> That's awesome! Is it meant to eventually be committed or only be used for 
>> comparison purposes?
>
> I'd like it to be committed, but can't claim I know the code from 
> https://reviews.llvm.org/D42519 well enough. The good news is that I can 
> build LLVM on Windows with this. Is there a good sanity check that it is 
> actually using Scudo rather than silently using the standard alloc?

Nothing except the tests. Compiling a sizeable application with Scudo as well.

> You marked D42519  as WIP, can you remember 
> what was still TBD?

Mostly because I didn't have the time and resources to make sure this would 
work over the long term.
I used my home windows box to do this as a proof of concept that it could be 
done.

> Also, it might make sense to separate out the "use" of sanitize=Scudo in 
> LLVM, from providing Windows support. I put them together here for evaluation 
> purposes.

Yeah this makes sense.

The other point that is worth mentioning is that we moved all dev efforts to 
the "standalone" version of Scudo (eg: the one not depending on 
sanitizer_common in the standalone/ subdirectory).
There is enough differences that there could be some significant 
performance/mem footprint changes between the 2.

Also depending on what you want to compare, disabling the Quarantine and other 
optional security features will make things faster and use less memory.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86694

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

[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given

[Zequan Wu via Phabricator via cfe-commits]

zequanwu updated this revision to Diff 288380.
zequanwu marked an inline comment as done.
zequanwu added a comment.

address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86369

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/ms_no_dynamic_cast.cpp
  clang/test/SemaCXX/no_dynamic_cast.cpp

Index: clang/test/SemaCXX/no_dynamic_cast.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/no_dynamic_cast.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 %s -fno-rtti-data -fsyntax-only -verify
+
+namespace std {
+struct type_info {};
+} // namespace std
+class B {
+public:
+  virtual ~B() = default;
+};
+
+class D1 : public B {
+public:
+  ~D1() = default;
+};
+
+void f() {
+  B *b = new D1();
+  auto d = dynamic_cast(b); // expected-warning{{dynamic_cast will not work since RTTI data is disabled by -fno-rtti-data}}
+  (void)typeid(int);  // expected-warning{{typeid will not work since RTTI data is disabled by -fno-rtti-data}}
+}
Index: clang/test/SemaCXX/ms_no_dynamic_cast.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/ms_no_dynamic_cast.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 %s -fdiagnostics-format msvc -fno-rtti-data -fsyntax-only -verify
+
+namespace std {
+struct type_info {};
+} // namespace std
+class B {
+public:
+  virtual ~B() = default;
+};
+
+class D1 : public B {
+public:
+  ~D1() = default;
+};
+
+void f() {
+  B *b = new D1();
+  auto d = dynamic_cast(b); // expected-warning{{dynamic_cast will not work since RTTI data is disabled by /GR-}}
+  (void)typeid(int);  // expected-warning{{typeid will not work since RTTI data is disabled by /GR-}}
+}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -646,6 +646,12 @@
 return ExprError(Diag(OpLoc, diag::err_no_typeid_with_fno_rtti));
   }
 
+  // Warns when typeid is used with RTTI disabled.
+  if (!getLangOpts().RTTIData)
+Diag(OpLoc, diag::warn_no_typeid_with_rtti_disabled)
+<< (getDiagnostics().getDiagnosticOptions().getFormat() ==
+DiagnosticOptions::MSVC);
+
   QualType TypeInfoType = Context.getTypeDeclType(CXXTypeInfoDecl);
 
   if (isType) {
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -890,6 +890,14 @@
 return;
   }
 
+  // Warns when dynamic_cast is used with RTTI disabled.
+  if (!Self.getLangOpts().RTTIData && !DestPointee->isVoidType()) {
+bool isClangCL = Self.getDiagnostics().getDiagnosticOptions().getFormat() ==
+ DiagnosticOptions::MSVC;
+Self.Diag(OpRange.getBegin(), diag::warn_no_dynamic_cast_with_rtti_disabled)
+<< isClangCL;
+  }
+
   // Done. Everything else is run-time checks.
   Kind = CK_Dynamic;
 }
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7433,6 +7433,12 @@
   "use of typeid requires -frtti">;
 def err_no_dynamic_cast_with_fno_rtti : Error<
   "use of dynamic_cast requires -frtti">;
+def warn_no_dynamic_cast_with_rtti_disabled: Warning<
+  "dynamic_cast will not work since RTTI data is disabled by " 
+  "%select{-fno-rtti-data|/GR-}0">, InGroup;
+def warn_no_typeid_with_rtti_disabled: Warning<
+  "typeid will not work since RTTI data is disabled by "
+  "%select{-fno-rtti-data|/GR-}0">, InGroup;
 
 def err_cannot_form_pointer_to_member_of_reference_type : Error<
   "cannot form a pointer-to-member to member %0 of reference type %1">;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -1229,3 +1229,5 @@
 }
 
 def WebAssemblyExceptionSpec : DiagGroup<"wasm-exception-spec">;
+
+def RTTI : DiagGroup<"rtti">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76496: [clang-tidy] add support to 3-arg find() in StringFindStartswith

[Niko Weh via Phabricator via cfe-commits]

niko updated this revision to Diff 288378.
niko added a comment.

Rebasing to HEAD


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76496

Files:
  clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/abseil-string-find-startswith.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/abseil-string-find-startswith.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/abseil-string-find-startswith.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/abseil-string-find-startswith.cpp
@@ -13,6 +13,7 @@
   ~basic_string();
   int find(basic_string s, int pos = 0);
   int find(const char *s, int pos = 0);
+  int find(const char *s, int pos, int n);
 };
 typedef basic_string string;
 typedef basic_string wstring;
@@ -41,6 +42,10 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use absl::StartsWith
   // CHECK-FIXES: {{^[[:space:]]*}}absl::StartsWith(s, s);{{$}}
 
+  s.find(s, 0) == 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use absl::StartsWith
+  // CHECK-FIXES: {{^[[:space:]]*}}absl::StartsWith(s, s);{{$}}
+
   s.find("aaa") != 0;
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StartsWith
   // CHECK-FIXES: {{^[[:space:]]*}}!absl::StartsWith(s, "aaa");{{$}}
@@ -61,9 +66,14 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use absl::StartsWith
   // CHECK-FIXES: {{^[[:space:]]*}}absl::StartsWith(s2, "a");{{$}}
 
+  s.find("a", 0, 1) == 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use absl::StartsWith
+  // CHECK-FIXES: {{^[[:space:]]*}}absl::StartsWith(s, absl::string_view("a", 
1));{{$}}
+
   // expressions that don't trigger the check are here.
   A_MACRO(s.find("a"), 0);
   s.find("a", 1) == 0;
   s.find("a", 1) == 1;
   s.find("a") == 1;
+  s.find("a", 1, 1) == 0;
 }
Index: clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp
===
--- clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp
+++ clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp
@@ -40,12 +40,13 @@
 
   auto StringFind = cxxMemberCallExpr(
   // .find()-call on a string...
-  callee(cxxMethodDecl(hasName("find"))),
-  on(hasType(StringType)),
+  callee(cxxMethodDecl(hasName("find"))), on(hasType(StringType)),
   // ... with some search expression ...
   hasArgument(0, expr().bind("needle")),
   // ... and either "0" as second argument or the default argument (also 
0).
-  anyOf(hasArgument(1, ZeroLiteral), hasArgument(1, cxxDefaultArgExpr(;
+  anyOf(hasArgument(1, ZeroLiteral), hasArgument(1, cxxDefaultArgExpr())),
+  // ... and optionally a third argument for 'search string length'.
+  optionally(hasArgument(2, expr().bind("needle_length";
 
   Finder->addMatcher(
   // Match [=!]= with a zero on one side and a string.find on the other.
@@ -69,6 +70,7 @@
   const Expr *Haystack = Result.Nodes.getNodeAs("findexpr")
  ->getImplicitObjectArgument();
   assert(Haystack != nullptr);
+  const auto *NeedleLength = Result.Nodes.getNodeAs("needle_length");
 
   if (ComparisonExpr->getBeginLoc().isMacroID())
 return;
@@ -82,6 +84,20 @@
   CharSourceRange::getTokenRange(Haystack->getSourceRange()), Source,
   Context.getLangOpts());
 
+  // If there is a third argument (search string length), create an
+  // absl::string_view(search string, search string length) and use that
+  // as the search string in StartsWith.
+  std::string NewNeedleStr;
+  if (NeedleLength != nullptr) {
+const StringRef NeedleLengthCode = Lexer::getSourceText(
+CharSourceRange::getTokenRange(NeedleLength->getSourceRange()), Source,
+Context.getLangOpts());
+NewNeedleStr = std::string("absl::string_view(") + NeedleExprCode.str() +
+   ", " + NeedleLengthCode.str() + ")";
+  } else {
+NewNeedleStr = NeedleExprCode.str();
+  }
+
   // Create the StartsWith string, negating if comparison was "!=".
   bool Neg = ComparisonExpr->getOpcodeStr() == "!=";
   StringRef StartswithStr;
@@ -100,7 +116,7 @@
 
   Diagnostic << FixItHint::CreateReplacement(
   ComparisonExpr->getSourceRange(),
-  (StartswithStr + "(" + HaystackExprCode + ", " + NeedleExprCode + ")")
+  (StartswithStr + "(" + HaystackExprCode + ", " + NewNeedleStr + ")")
   .str());
 
   // Create a preprocessor #include FixIt hint (CreateIncludeInsertion checks


Index: clang-tools-extra/test/clang-tidy/checkers/abseil-string-find-startswith.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/abseil-string-find-startswith.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/abseil-string-find-startswith.cpp
@@ -13,6 +13,7 @@
   ~basic_string();
   int find(basic_string s, int pos 

[PATCH] D86146: [ARM][BFloat16] Change types of some Arm and AArch64 bf16 intrinsics

[Mikhail Maltsev via Phabricator via cfe-commits]

miyuki updated this revision to Diff 288373.
miyuki added a comment.

Small refactoring of bitcode updater tests (use `some-name.ll.bc` names for 
bitcode files in order to be able to refer to them as `%s.bc` from 
`some-name.ll`)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86146

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/aarch64-bf16-dotprod-intrinsics.c
  clang/test/CodeGen/arm-bf16-dotprod-intrinsics.c
  llvm/include/llvm/IR/IntrinsicsAArch64.td
  llvm/include/llvm/IR/IntrinsicsARM.td
  llvm/lib/IR/AutoUpgrade.cpp
  llvm/lib/Target/AArch64/AArch64InstrFormats.td
  llvm/lib/Target/AArch64/AArch64InstrInfo.td
  llvm/lib/Target/ARM/ARMInstrNEON.td
  llvm/test/Bitcode/aarch64-bf16-upgrade.ll
  llvm/test/Bitcode/aarch64-bf16-upgrade.ll.bc
  llvm/test/Bitcode/arm-bf16-upgrade.ll
  llvm/test/Bitcode/arm-bf16-upgrade.ll.bc
  llvm/test/CodeGen/AArch64/aarch64-bf16-dotprod-intrinsics.ll
  llvm/test/CodeGen/ARM/arm-bf16-dotprod-intrinsics.ll

Index: llvm/test/CodeGen/ARM/arm-bf16-dotprod-intrinsics.ll
===
--- llvm/test/CodeGen/ARM/arm-bf16-dotprod-intrinsics.ll
+++ llvm/test/CodeGen/ARM/arm-bf16-dotprod-intrinsics.ll
@@ -7,10 +7,8 @@
 ; CHECK-NEXT:vdot.bf16 d0, d1, d2
 ; CHECK-NEXT:bx lr
 entry:
-  %0 = bitcast <4 x bfloat> %a to <8 x i8>
-  %1 = bitcast <4 x bfloat> %b to <8 x i8>
-  %vbfdot1.i = call <2 x float> @llvm.arm.neon.bfdot.v2f32.v8i8(<2 x float> %r, <8 x i8> %0, <8 x i8> %1)
-  ret <2 x float> %vbfdot1.i
+  %vbfdot3.i = call <2 x float> @llvm.arm.neon.bfdot.v2f32.v4bf16(<2 x float> %r, <4 x bfloat> %a, <4 x bfloat> %b) #3
+  ret <2 x float> %vbfdot3.i
 }
 
 define <4 x float> @test_vbfdotq_f32(<4 x float> %r, <8 x bfloat> %a, <8 x bfloat> %b) {
@@ -19,10 +17,8 @@
 ; CHECK-NEXT:vdot.bf16 q0, q1, q2
 ; CHECK-NEXT:bx lr
 entry:
-  %0 = bitcast <8 x bfloat> %a to <16 x i8>
-  %1 = bitcast <8 x bfloat> %b to <16 x i8>
-  %vbfdot1.i = call <4 x float> @llvm.arm.neon.bfdot.v4f32.v16i8(<4 x float> %r, <16 x i8> %0, <16 x i8> %1)
-  ret <4 x float> %vbfdot1.i
+  %vbfdot3.i = call <4 x float> @llvm.arm.neon.bfdot.v4f32.v8bf16(<4 x float> %r, <8 x bfloat> %a, <8 x bfloat> %b) #3
+  ret <4 x float> %vbfdot3.i
 }
 
 define <2 x float> @test_vbfdot_lane_f32(<2 x float> %r, <4 x bfloat> %a, <4 x bfloat> %b) {
@@ -31,12 +27,11 @@
 ; CHECK-NEXT:vdot.bf16 d0, d1, d2[0]
 ; CHECK-NEXT:bx lr
 entry:
-  %0 = bitcast <4 x bfloat> %b to <2 x float>
-  %shuffle = shufflevector <2 x float> %0, <2 x float> undef, <2 x i32> zeroinitializer
-  %1 = bitcast <4 x bfloat> %a to <8 x i8>
-  %2 = bitcast <2 x float> %shuffle to <8 x i8>
-  %vbfdot1.i = call <2 x float> @llvm.arm.neon.bfdot.v2f32.v8i8(<2 x float> %r, <8 x i8> %1, <8 x i8> %2)
-  ret <2 x float> %vbfdot1.i
+  %.cast = bitcast <4 x bfloat> %b to <2 x float>
+  %lane = shufflevector <2 x float> %.cast, <2 x float> undef, <2 x i32> zeroinitializer
+  %.cast1 = bitcast <2 x float> %lane to <4 x bfloat>
+  %vbfdot3.i = call <2 x float> @llvm.arm.neon.bfdot.v2f32.v4bf16(<2 x float> %r, <4 x bfloat> %a, <4 x bfloat> %.cast1) #3
+  ret <2 x float> %vbfdot3.i
 }
 
 define <4 x float> @test_vbfdotq_laneq_f32(<4 x float> %r, <8 x bfloat> %a, <8 x bfloat> %b) {
@@ -46,12 +41,11 @@
 ; CHECK-NEXT:vdot.bf16 q0, q1, q8
 ; CHECK-NEXT:bx lr
 entry:
-  %0 = bitcast <8 x bfloat> %b to <4 x float>
-  %shuffle = shufflevector <4 x float> %0, <4 x float> undef, <4 x i32> 
-  %1 = bitcast <8 x bfloat> %a to <16 x i8>
-  %2 = bitcast <4 x float> %shuffle to <16 x i8>
-  %vbfdot1.i = call <4 x float> @llvm.arm.neon.bfdot.v4f32.v16i8(<4 x float> %r, <16 x i8> %1, <16 x i8> %2)
-  ret <4 x float> %vbfdot1.i
+  %.cast = bitcast <8 x bfloat> %b to <4 x float>
+  %lane = shufflevector <4 x float> %.cast, <4 x float> undef, <4 x i32> 
+  %.cast1 = bitcast <4 x float> %lane to <8 x bfloat>
+  %vbfdot3.i = call <4 x float> @llvm.arm.neon.bfdot.v4f32.v8bf16(<4 x float> %r, <8 x bfloat> %a, <8 x bfloat> %.cast1) #3
+  ret <4 x float> %vbfdot3.i
 }
 
 define <2 x float> @test_vbfdot_laneq_f32(<2 x float> %r, <4 x bfloat> %a, <8 x bfloat> %b) {
@@ -60,12 +54,11 @@
 ; CHECK-NEXT:vdot.bf16 d0, d1, d3[1]
 ; CHECK-NEXT:bx lr
 entry:
-  %0 = bitcast <8 x bfloat> %b to <4 x float>
-  %shuffle = shufflevector <4 x float> %0, <4 x float> undef, <2 x i32> 
-  %1 = bitcast <4 x bfloat> %a to <8 x i8>
-  %2 = bitcast <2 x float> %shuffle to <8 x i8>
-  %vbfdot1.i = call <2 x float> @llvm.arm.neon.bfdot.v2f32.v8i8(<2 x float> %r, <8 x i8> %1, <8 x i8> %2)
-  ret <2 x float> %vbfdot1.i
+  %.cast = bitcast <8 x bfloat> %b to <4 x float>
+  %lane = shufflevector <4 x float> %.cast, <4 x float> undef, <2 x i32> 
+  %.cast1 = bitcast <2 x float> %lane to <4 x bfloat>
+  %vbfdot3.i = call <2 x float> @llvm.arm.neon.bfdot.v2f32.v4bf16(<2 x float> %r, <4 x bfloat> %a, <4 x bfloat> %.cast1) #3
+  ret <2 x float> 

[PATCH] D86719: [SyntaxTree][NFC] Refactor function templates into functions taking base class

[Eduardo Caldas via Phabricator via cfe-commits]

eduucaldas added a reviewer: gribozavr2.
eduucaldas added a comment.

As a result of this change `getDeclaratorRange` is used exclusively inside 
`processDeclaratorAndDeclaration` and the last two arguments are direct results 
of `getQualifiedNameStart(D)` and `getInitializerRange(D)`, which are used 
exclusively in this context.




Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:1462
 // Build node for the declarator, if any.
-auto ReturnDeclaratorRange =
-getDeclaratorRange(this->Builder.sourceManager(), ReturnedType,
-   /*Name=*/SourceLocation(),
-   /*Initializer=*/SourceLocation());
+auto ReturnDeclaratorRange = SourceRange(GetStartLoc().Visit(ReturnedType),
+ ReturnedType.getEndLoc());

1. TrailingReturn is not a Declarator
2. Inlining the arguments is actually less code and much than using the 
`getDeclaratorRange` here.



Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:393
 // Next sibling is not the same type, this one is responsible.
-if (NextT == nullptr) {
+if (D->getKind() != Next->getKind()) {
   return true;

I would argue that this is much clearer :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86719

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

[PATCH] D85091: [Sema, CodeGen] Implement [[likely]] and [[unlikely]] in IfStmt

[Mark de Wever via Phabricator via cfe-commits]

Mordante planned changes to this revision.
Mordante marked 15 inline comments as done.
Mordante added a comment.

I fixed the issues locally. Before I resubmit them I'd rather discuss in D86559 
 the direction we want to take, so we can 
avoid an additional review cycle.




Comment at: clang/include/clang/Basic/Attr.td:1288
+def Likely : StmtAttr {
+  let Spellings = [CXX11<"", "likely", 201803>];
+  let Documentation = [LikelihoodDocs];

aaron.ballman wrote:
> Mordante wrote:
> > Mordante wrote:
> > > aaron.ballman wrote:
> > > > Mordante wrote:
> > > > > Mordante wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > Mordante wrote:
> > > > > > > > aaron.ballman wrote:
> > > > > > > > > Hmm, I'm on the fence about specifying `201803` for these 
> > > > > > > > > attributes. Given that this is only the start of supporting 
> > > > > > > > > the attribute, do we want to claim it already matches the 
> > > > > > > > > standard's behavior? Or do we just want to return `1` to 
> > > > > > > > > signify that we understand this attribute but we don't yet 
> > > > > > > > > fully support it in common cases (such as on labels in switch 
> > > > > > > > > statements, etc)?
> > > > > > > > > 
> > > > > > > > > As another question, should we consider adding a C2x spelling 
> > > > > > > > > `[[clang::likely]]` and `[[clang::unlikely]]` to add this 
> > > > > > > > > functionality to C?
> > > > > > > > I was also somewhat on the fence, for now I'll change it to 
> > > > > > > > specify `1`.
> > > > > > > > 
> > > > > > > > I'll have a look at the C2x changes. Just curious do you know 
> > > > > > > > whether there's a proposal to add this to C2x?
> > > > > > > > I'll have a look at the C2x changes. Just curious do you know 
> > > > > > > > whether there's a proposal to add this to C2x?
> > > > > > > 
> > > > > > > There isn't one currently because there is no implementation 
> > > > > > > experience with the attributes in C. This is a way to get that 
> > > > > > > implementation experience so it's easier to propose the feature 
> > > > > > > to WG14.
> > > > > > > I was also somewhat on the fence, for now I'll change it to 
> > > > > > > specify `1`.
> > > > > > 
> > > > > > There seems to be an issue using the `1` so I used `2` instead. 
> > > > > > (Didn't want to look closely at the issue.)
> > > > > > 
> > > > > > > I'll have a look at the C2x changes. Just curious do you know 
> > > > > > > whether there's a proposal to add this to C2x?
> > > > > > 
> > > > > > There isn't one currently because there is no implementation 
> > > > > > experience with the attributes in C. This is a way to get that 
> > > > > > implementation experience so it's easier to propose the feature to 
> > > > > > WG14.
> > > > > 
> > > > > Nice to know. It seems the C2x wasn't at straightforward as I hoped, 
> > > > > so I didn't implement it yet. I intend to look at it later. I first 
> > > > > want the initial part done to see whether this is the right approach.
> > > > What issues are you running into? 1 is the default value when you don't 
> > > > specify a value specifically.
> > > It give the error "clang/include/clang/Basic/Attr.td:265:7: error: 
> > > Standard attributes must have valid version information."
> > > > > I'll have a look at the C2x changes. Just curious do you know whether 
> > > > > there's a proposal to add this to C2x?
> > > > 
> > > > There isn't one currently because there is no implementation experience 
> > > > with the attributes in C. This is a way to get that implementation 
> > > > experience so it's easier to propose the feature to WG14.
> > > 
> > > Nice to know. It seems the C2x wasn't at straightforward as I hoped, so I 
> > > didn't implement it yet. I intend to look at it later. I first want the 
> > > initial part done to see whether this is the right approach.
> > 
> > I had another look and I got it working in C.
> If you leave the version number off entirely, it defaults to 1.
Yes and that gives the same error. So the default value doesn't "compile". I 
assume that's intentional to avoid setting a date of 1 for standard attributes. 
So we could keep it at 2 or switch back to `201803`. Which value do you prefer?



Comment at: clang/include/clang/Basic/AttrDocs.td:1691
+The ``likely`` and ``unlikely`` attributes are used as compiler hints. When
+the next executed statement depends on a condition this attribute can
+annotate all possible statements with either ``likely`` or ``unlikely``.

aaron.ballman wrote:
> I think this paragraph is misleading because the attribute no longer impacts 
> the statement, it impacts the *entire branch the statement is contained 
> within*.
I'll update the documentation including the other remarks added.



Comment at: clang/include/clang/Basic/AttrDocs.td:1726
+  }
+  }];
+}

aaron.ballman wrote:
> Mordante wrote:
> > aaron.ballman 

[PATCH] D86065: [SVE] Make ElementCount members private

[David Sherwood via Phabricator via cfe-commits]

david-arm updated this revision to Diff 288370.
david-arm added a comment.

- Removed isPowerOf2() function since this is potentially misleading - it's 
only the known minimum value that we're checking.
- Renamed isEven to isKnownEven to try and make it clear that returning true 
indicates we know definitely that the total number of elements is even, whereas 
returning false could mean either the element count is odd or that we don't 
know.


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

https://reviews.llvm.org/D86065

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenTypes.cpp
  llvm/include/llvm/Analysis/TargetTransformInfo.h
  llvm/include/llvm/Analysis/VectorUtils.h
  llvm/include/llvm/CodeGen/ValueTypes.h
  llvm/include/llvm/IR/DataLayout.h
  llvm/include/llvm/IR/DerivedTypes.h
  llvm/include/llvm/IR/Instructions.h
  llvm/include/llvm/Support/MachineValueType.h
  llvm/include/llvm/Support/TypeSize.h
  llvm/lib/Analysis/InstructionSimplify.cpp
  llvm/lib/Analysis/VFABIDemangling.cpp
  llvm/lib/Analysis/ValueTracking.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/CodeGen/CodeGenPrepare.cpp
  llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/CodeGen/TargetLoweringBase.cpp
  llvm/lib/CodeGen/ValueTypes.cpp
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/IR/ConstantFold.cpp
  llvm/lib/IR/Constants.cpp
  llvm/lib/IR/DataLayout.cpp
  llvm/lib/IR/Function.cpp
  llvm/lib/IR/IRBuilder.cpp
  llvm/lib/IR/Instructions.cpp
  llvm/lib/IR/IntrinsicInst.cpp
  llvm/lib/IR/Type.cpp
  llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp
  llvm/lib/Transforms/Utils/FunctionComparator.cpp
  llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
  llvm/lib/Transforms/Vectorize/VPlan.cpp
  llvm/lib/Transforms/Vectorize/VPlan.h
  llvm/unittests/CodeGen/ScalableVectorMVTsTest.cpp
  llvm/unittests/IR/VectorTypesTest.cpp

Index: llvm/unittests/IR/VectorTypesTest.cpp
===
--- llvm/unittests/IR/VectorTypesTest.cpp
+++ llvm/unittests/IR/VectorTypesTest.cpp
@@ -119,8 +119,8 @@
   EXPECT_EQ(ConvTy->getElementType()->getScalarSizeInBits(), 64U);
 
   EltCnt = V8Int64Ty->getElementCount();
-  EXPECT_EQ(EltCnt.Min, 8U);
-  ASSERT_FALSE(EltCnt.Scalable);
+  EXPECT_EQ(EltCnt.getKnownMinValue(), 8U);
+  ASSERT_FALSE(EltCnt.isScalable());
 }
 
 TEST(VectorTypesTest, Scalable) {
@@ -215,8 +215,8 @@
   EXPECT_EQ(ConvTy->getElementType()->getScalarSizeInBits(), 64U);
 
   EltCnt = ScV8Int64Ty->getElementCount();
-  EXPECT_EQ(EltCnt.Min, 8U);
-  ASSERT_TRUE(EltCnt.Scalable);
+  EXPECT_EQ(EltCnt.getKnownMinValue(), 8U);
+  ASSERT_TRUE(EltCnt.isScalable());
 }
 
 TEST(VectorTypesTest, BaseVectorType) {
@@ -250,7 +250,7 @@
 // test I == J
 VectorType *VI = VTys[I];
 ElementCount ECI = VI->getElementCount();
-EXPECT_EQ(isa(VI), ECI.Scalable);
+EXPECT_EQ(isa(VI), ECI.isScalable());
 
 for (size_t J = I + 1, JEnd = VTys.size(); J < JEnd; ++J) {
   // test I < J
Index: llvm/unittests/CodeGen/ScalableVectorMVTsTest.cpp
===
--- llvm/unittests/CodeGen/ScalableVectorMVTsTest.cpp
+++ llvm/unittests/CodeGen/ScalableVectorMVTsTest.cpp
@@ -71,8 +71,8 @@
 
   // Check fields inside llvm::ElementCount
   EltCnt = Vnx4i32.getVectorElementCount();
-  EXPECT_EQ(EltCnt.Min, 4U);
-  ASSERT_TRUE(EltCnt.Scalable);
+  EXPECT_EQ(EltCnt.getKnownMinValue(), 4U);
+  ASSERT_TRUE(EltCnt.isScalable());
 
   // Check that fixed-length vector types aren't scalable.
   EVT V8i32 = EVT::getVectorVT(Ctx, MVT::i32, 8);
@@ -82,8 +82,8 @@
 
   // Check that llvm::ElementCount works for fixed-length types.
   EltCnt = V8i32.getVectorElementCount();
-  EXPECT_EQ(EltCnt.Min, 8U);
-  ASSERT_FALSE(EltCnt.Scalable);
+  EXPECT_EQ(EltCnt.getKnownMinValue(), 8U);
+  ASSERT_FALSE(EltCnt.isScalable());
 }
 
 TEST(ScalableVectorMVTsTest, IRToVTTranslation) {
Index: llvm/lib/Transforms/Vectorize/VPlan.h
===
--- llvm/lib/Transforms/Vectorize/VPlan.h
+++ llvm/lib/Transforms/Vectorize/VPlan.h
@@ -151,14 +151,15 @@
   /// \return True if the map has a scalar entry for \p Key and \p Instance.
   bool hasScalarValue(Value *Key, const VPIteration ) const {
 assert(Instance.Part < UF && "Queried Scalar Part is too large.");
-assert(Instance.Lane < VF.Min && "Queried Scalar Lane is too large.");
-assert(!VF.Scalable && "VF is assumed to be non scalable.");
+assert(Instance.Lane < VF.getKnownMinValue() &&
+   "Queried Scalar Lane is too large.");
+assert(!VF.isScalable() && "VF is assumed to be non scalable.");
 
 if (!hasAnyScalarValue(Key))
   return false;
 const ScalarParts  = ScalarMapStorage.find(Key)->second;
 assert(Entry.size() == UF && 

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

[Russell Gallop via Phabricator via cfe-commits]

russell.gallop added a comment.

In D86694#2242150 , @russell.gallop 
wrote:

> You marked D42519  as WIP, can you remember 
> what was still TBD?

Maybe more tests could be enabled...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86694

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

[PATCH] D86711: [clang-format] Parse __attribute((foo)) as a pointer qualifier

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:1895
tok::kw_restrict, tok::kw_volatile,
-   tok::kw_noexcept) ||
+   tok::kw___attribute, tok::kw_noexcept) ||
 (NextToken->is(tok::l_brace) && !NextToken->getNextNonComment()))

arichardson wrote:
> arichardson wrote:
> > aaron.ballman wrote:
> > > What about other attributes than GNU-style ones, like 
> > > `[[clang::address_space(0)]]`?
> > I guess those should also be handled. I can try to do that in a follow-up 
> > change.
> It appears that they are already handled for this case. I've handled them in 
> cast expressions in D86721. 
Ah, thank you for the update! Can you add a test case for that, as I don't see 
one in the current tests?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86711

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

[PATCH] D62574: Add support for target-configurable address spaces.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.
Herald added a subscriber: bjope.

This change looks good to me in general conceptually as it is completing 
missing logic in clang that let's targets to customize the address space 
behavior!

The change also looks good from the implementation side apart from some small 
details raised in the comments.

The only thing is that it would be good to test the new target setting logic 
somehow... do you have any ideas in mind? We could think of creating a dummy 
target for that or adding a dummy setting in existing targets - maybe SPIR 
could be a candidate. We have done something similar in the past if you look at 
`FakeAddrSpaceMap` in `LangOpts`.




Comment at: clang/include/clang/AST/ASTContext.h:2371
+  /// can be safely used as an object qualified with A.
+  bool compatiblyIncludes(Qualifiers A, Qualifiers B) const {
+return isAddressSpaceSupersetOf(A, B) &&

Perhaps not necessarily related to this change but I feel we should provide an 
explanation of how those functions behave wrt address spaces because terms more 
qualified or less qualified don't really apply there.



Comment at: clang/include/clang/AST/ASTContext.h:2588
 
+  /// Returns true if address space A is a superset of B.
+  /// The subspace/superspace relation governs the validity of implicit

I think we should finally start referring to section 5 of embedded C spec as it 
is not a secret that clang implements exactly this. It is rather a good thing 
to document the implemented behavior.



Comment at: clang/include/clang/AST/ASTContext.h:2612
+
+  /// Returns true if an explicit cast from address space A to B is legal.
+  /// Explicit conversion between address spaces is permitted if the address

Here we should say that this is an extension or enhancement of embedded C rules 
that clang implements.

Technically for OpenCL we could refactor to use this functionality as we don't 
support such explicit casts on disjoint address spaces. But then this would not 
be necessarily a target setting.



Comment at: clang/lib/AST/ASTContext.cpp:10959
+  // Otherwise, ask the target.
+  return Target->isAddressSpaceSupersetOf(A, B);
+}

I guess we should add a similar check here as below?

 
```
if (isTargetAddressSpace(From) || isTargetAddressSpace(To) ||
  From == LangAS::Default || To == LangAS::Default)
```



Comment at: clang/lib/AST/ASTContext.cpp:10963
+bool
+ASTContext::isExplicitAddrSpaceConversionLegal(LangAS From, LangAS To) const {
+  // If From and To overlap, the cast is legal.

Btw I assume that explicit cast can't reject what is not rejected by implicit 
cast?

I am not sure if we need to enforce or document this somehow considering that 
we provide full configurability now?



Comment at: clang/lib/Sema/SemaCast.cpp:2423
   auto DestPointeeType = DestPtrType->getPointeeType();
-  if (!DestPointeeType.isAddressSpaceOverlapping(SrcPointeeType)) {
+  if (!Self.Context.isExplicitAddrSpaceConversionLegal(
+SrcPointeeType.getQualifiers(), DestPointeeType.getQualifiers())) {

Btw just to point our that overlapping used to be a commutative operation so 
you could swap arguments and still get the same answer but for 
`isExplicitAddrSpaceConversionLegal` is not the same I assume?



Comment at: clang/lib/Sema/SemaOverload.cpp:3235
   //  - in non-top levels it is not a valid conversion.
+  // FIXME: This should probably be using isExplicitAddrSpaceConversionLegal,
+  // but we don't know if this is an implicit or explicit conversion.

Sorry if this has been discussed previously, do you refer to the first or the 
second case and is there any failing test case?



Comment at: clang/lib/Sema/SemaOverload.cpp:5289
 
+  // FIXME: hasAddressSpace is wrong; this check will be skipped if FromType is
+  // not qualified with an address space, but if there's no implicit conversion

Do you have a failing test case, if so feel free to create a bug?



Comment at: clang/test/CodeGenCXX/address-space-cast.cpp:41
+  // CHECK: %[[cast:.*]] = bitcast i32* %{{.*}} to i8*
+  // CHECK-NEXT: %[[cast2:.*]] = addrspacecast i8* %[[cast]] to i8 
addrspace(5)*
+  // CHECK-NEXT: store i8 addrspace(5)* %[[cast2]]

I am confused about why this is changed now? adrspacecast can cast a type and 
an address space too i.e. it is implicitly a bitcast too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62574

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

[PATCH] D86719: [SyntaxTree][NFC] Refactor function templates into functions taking base class

[Eduardo Caldas via Phabricator via cfe-commits]

eduucaldas updated this revision to Diff 288367.
eduucaldas added a comment.

Inline getDeclaratorRange inside buildTrailingReturn


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86719

Files:
  clang/lib/Tooling/Syntax/BuildTree.cpp


Index: clang/lib/Tooling/Syntax/BuildTree.cpp
===
--- clang/lib/Tooling/Syntax/BuildTree.cpp
+++ clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -208,7 +208,7 @@
   SourceLocation Name,
   SourceRange Initializer) {
   SourceLocation Start = GetStartLoc().Visit(T);
-  SourceLocation End = T.getSourceRange().getEnd();
+  SourceLocation End = T.getEndLoc();
   assert(End.isValid());
   if (Name.isValid()) {
 if (Start.isInvalid())
@@ -378,11 +378,9 @@
 
   /// Returns true if \p D is the last declarator in a chain and is thus
   /// reponsible for creating SimpleDeclaration for the whole chain.
-  template 
-  bool isResponsibleForCreatingDeclaration(const T *D) const {
-static_assert((std::is_base_of::value ||
-   std::is_base_of::value),
-  "only DeclaratorDecl and TypedefNameDecl are supported.");
+  bool isResponsibleForCreatingDeclaration(const Decl *D) const {
+assert((isa(D)) &&
+   "only DeclaratorDecl and TypedefNameDecl are supported.");
 
 const Decl *Next = D->getNextDeclInContext();
 
@@ -390,15 +388,14 @@
 if (Next == nullptr) {
   return true;
 }
-const auto *NextT = dyn_cast(Next);
 
 // Next sibling is not the same type, this one is responsible.
-if (NextT == nullptr) {
+if (D->getKind() != Next->getKind()) {
   return true;
 }
 // Next sibling doesn't begin at the same loc, it must be a different
 // declaration, so this declarator is responsible.
-if (NextT->getBeginLoc() != D->getBeginLoc()) {
+if (Next->getBeginLoc() != D->getBeginLoc()) {
   return true;
 }
 
@@ -1405,10 +1402,9 @@
   }
 
 private:
-  template  SourceLocation getQualifiedNameStart(T *D) {
-static_assert((std::is_base_of::value ||
-   std::is_base_of::value),
-  "only DeclaratorDecl and TypedefNameDecl are supported.");
+  SourceLocation getQualifiedNameStart(NamedDecl *D) {
+assert((isa(D)) &&
+   "only DeclaratorDecl and TypedefNameDecl are supported.");
 
 auto DN = D->getDeclName();
 bool IsAnonymous = DN.isIdentifier() && !DN.getAsIdentifierInfo();
@@ -1438,10 +1434,9 @@
   /// Folds SimpleDeclarator node (if present) and in case this is the last
   /// declarator in the chain it also folds SimpleDeclaration node.
   template  bool processDeclaratorAndDeclaration(T *D) {
-SourceRange Initializer = getInitializerRange(D);
-auto Range = getDeclaratorRange(Builder.sourceManager(),
-D->getTypeSourceInfo()->getTypeLoc(),
-getQualifiedNameStart(D), Initializer);
+auto Range = getDeclaratorRange(
+Builder.sourceManager(), D->getTypeSourceInfo()->getTypeLoc(),
+getQualifiedNameStart(D), getInitializerRange(D));
 
 // There doesn't have to be a declarator (e.g. `void foo(int)` only has
 // declaration, but no declarator).
@@ -1464,10 +1459,8 @@
 
 auto ReturnedType = L.getReturnLoc();
 // Build node for the declarator, if any.
-auto ReturnDeclaratorRange =
-getDeclaratorRange(this->Builder.sourceManager(), ReturnedType,
-   /*Name=*/SourceLocation(),
-   /*Initializer=*/SourceLocation());
+auto ReturnDeclaratorRange = SourceRange(GetStartLoc().Visit(ReturnedType),
+ ReturnedType.getEndLoc());
 syntax::SimpleDeclarator *ReturnDeclarator = nullptr;
 if (ReturnDeclaratorRange.isValid()) {
   ReturnDeclarator = new (allocator()) syntax::SimpleDeclarator;


Index: clang/lib/Tooling/Syntax/BuildTree.cpp
===
--- clang/lib/Tooling/Syntax/BuildTree.cpp
+++ clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -208,7 +208,7 @@
   SourceLocation Name,
   SourceRange Initializer) {
   SourceLocation Start = GetStartLoc().Visit(T);
-  SourceLocation End = T.getSourceRange().getEnd();
+  SourceLocation End = T.getEndLoc();
   assert(End.isValid());
   if (Name.isValid()) {
 if (Start.isInvalid())
@@ -378,11 +378,9 @@
 
   /// Returns true if \p D is the last declarator in a chain and is thus
   /// reponsible for creating SimpleDeclaration for the whole chain.
-  template 
-  bool isResponsibleForCreatingDeclaration(const T *D) const {
-static_assert((std::is_base_of::value ||
-   std::is_base_of::value),
-  "only DeclaratorDecl and 

[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given

[Zequan Wu via Phabricator via cfe-commits]

zequanwu added inline comments.



Comment at: clang/lib/Sema/SemaCast.cpp:895
+  if (!Self.getLangOpts().RTTIData) {
+bool isMSVC = Self.getDiagnostics().getDiagnosticOptions().getFormat() ==
+  DiagnosticOptions::MSVC;

hans wrote:
> I'm not sure isMSVC is the best name (it's not clear what aspect is MSVC 
> exactly -- in this case it's the diagnostics format).
> 
> It's possible to target MSVC both with clang-cl and with regular clang.
> 
> For example, one could use
> 
>   clang-cl /c /tmp/a.cpp
> 
> or
> 
>   clang -c /tmp/a.cpp -target i686-pc-windows-msvc19.11.0 -fms-extensions
> 
> 
> My understanding is that the purpose of "isMSVC" here is to try and detect if 
> we're using clang-cl or clang so that the diagnostic can say "/GR-" or 
> "-fno-rtti-data". So maybe it's better to call it "isClangCL" or something 
> like that.
> 
> Also, I don't think we should check "isMSVC" in the if-statement below. We 
> want the warning to fire both when using clang and clang-cl: as long as 
> -fno-rtti-data or /GR- is used, the warning makes sense.
> 
> So I think the code could be more like:
> 
> ```
> if (!Self.getLangOpts().RTTIData && !DestPointee->isVoidType()) {
>   bool isClangCL = ...;
>   Self.Diag(...) << isClangCL;
> }
> ```
MSVC will warn even if the DestPointee is void type. What I thought is if 
invoked by clang-cl warn regardless of DeskPointee type. If invoked by clang, 
warn if it's not void type. 
https://godbolt.org/z/475q5v. I noticed MSVC won't warn at typeid if /GR- is 
given. Probably I should remove the warning in typeid.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86369

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

[PATCH] D85176: [Coverage] Enable emitting gap area between macros

[Zequan Wu via Phabricator via cfe-commits]

zequanwu added a comment.

In D85176#2241257 , @phosek wrote:

> We started seeing assertion failure after rolling a toolchain that contains 
> this change and `git bisect` identified this change. I have filed a bug with 
> a reproducer as PR47324.

It has been reverted for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85176

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

[PATCH] D85031: [builtins] Unify the softfloat division implementation

[Anatoly Trosinenko via Phabricator via cfe-commits]

atrosinenko added inline comments.



Comment at: compiler-rt/lib/builtins/fp_div_impl.inc:142
+  // An error due to truncated C + an error due to truncated x_UQ0_hw:
+  // e_0 <= 3/4 - 1/sqrt(2) + 2^-W0 + 2^-HW
+

sepavloff wrote:
> Should the right part contain `1/b`?
What line are you referring to? For line 142, `e_0` is defined as `x_n - 
1/b_hw` in infinite precision (please note it intentionally refers `b_hw` that 
is a truncated version of `b`, see lines 113-114).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85031

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

[PATCH] D86465: [analyzer][solver] Redesign constraint ranges data structure

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

Previously, I liked this. Now I love it!
The benchmarks look promising as well.

I think I can not convince you about not modifying iterators using `++`,`--`, 
etc. outside of //loop-expressions//. So be it :D




Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:110
+public:
+  using iterator = ImplType::const_iterator;
+

Shouldn't we call this `const_iterator`?



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:209
+  private:
+/// Return a persistent version of the given container.
+RangeSet makePersistent(ContainerType &);





Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:211
+RangeSet makePersistent(ContainerType &);
+/// Construct a new persistent version of the given container.
+ContainerType *construct(ContainerType &);





Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:278
+private:
+  /* implicit */ RangeSet(ContainerType *RawContainer) : Impl(RawContainer) {}
+  /* implicit */ RangeSet(UnderlyingType Ptr) : Impl(Ptr) {}

Btw in C++20 we will have a way expressing this: `explicit(false)`



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:281-290
+  /// Pin given points to the type represented by the current range set.
+  ///
+  /// This makes parameter points to be in-out parameters.
+  /// In order to maintain consistent types across all of the ranges in the set
+  /// and to keep all the operations to compare ONLY points of the same type, 
we
+  /// need to pin every point before any operation.
+  ///

Aww, excellent. Thanks.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:51
   }
-};
+  void print(raw_ostream ) const;
 

steakhal wrote:
> We have `dump` methods all over the place. Except this.
> I propose to rename this to `dump`.
How about marking this `LLVM_DUMP_METHOD`?
Also, defining the `operator<<` would come handly later.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:53-55
+  // In order to keep non-overlapping ranges sorted, we can compare only From
+  // points.
+  inline bool operator<(const Range ) const { return From() < RHS.From(); }

vsavchenko wrote:
> steakhal wrote:
> > It's a good practice to define comparison operators as //friend// functions 
> > inline.
> > Even if we don't rely on implicit conversions.
> It doesn't seem like there is a single opinion on this in the codebase.
Okay.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:135
+/// @{
+RangeSet getSet(Range Origin);
+RangeSet getSet(const llvm::APSInt , const llvm::APSInt ) {

vsavchenko wrote:
> steakhal wrote:
> > NoQ wrote:
> > > "But what about `setGet()`???" - some user, probably :)
> > Why don't we call this `createSetOf`?
> > And `createEmptySet`.
> > I know that we don't create the empty set, but then what does a //factory// 
> > if not create stuff?
> The naming is this way to be consistent with `ImmutableSet::Factory`
Makes sense.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:272-273
+private:
+  RangeSet(ContainerType *RawContainer) : Impl(RawContainer) {}
+  RangeSet(UnderlyingType Ptr) : Impl(Ptr) {}
 

vsavchenko wrote:
> steakhal wrote:
> > vsavchenko wrote:
> > > steakhal wrote:
> > > > Missing `explicit`.
> > > More like missing `/* implicit */` because it is intentional
> > It doesn't have a too long identifier.
> > The users can always refer to it `auto R = RangeSet(...)`, so we still 
> > don't repeat ourselves.
> > Do you have any particular counterexample?
> > Probably the tests will become slightly more bloated but eh. whatever.
> These constructors are `private` and used in `Factory` methods.  IMO they 
> make those methods less cluttered.
Okay.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:326
+
+  // If we ran out of ranges in one set, but not the other,
+  // it means that those elements are definitely not in the





Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:462-470
   // Negate all other ranges.
-  for (iterator e = end(); i != e; ++i) {
+  for (; It != End; ++It) {
 // Negate int values.
-const llvm::APSInt  = BV.getValue(-i->To());
-const llvm::APSInt  = BV.getValue(-i->From());
-// Add a negated range.
-newRanges = F.add(newRanges, Range(newFrom, newTo));
-  }
-
-  if (newRanges.isSingleton())
-return 

[PATCH] D86146: [ARM][BFloat16] Change types of some Arm and AArch64 bf16 intrinsics

[Mikhail Maltsev via Phabricator via cfe-commits]

miyuki added inline comments.



Comment at: llvm/include/llvm/IR/IntrinsicsAArch64.td:473
   def int_aarch64_neon_bfdot : AdvSIMD_Dot_Intrinsic;
-  def int_aarch64_neon_bfmmla : AdvSIMD_MatMul_Intrinsic;
-  def int_aarch64_neon_bfmlalb : AdvSIMD_FML_Intrinsic;
-  def int_aarch64_neon_bfmlalt : AdvSIMD_FML_Intrinsic;
+  def int_aarch64_neon_bfmmla
+: Intrinsic<[llvm_v4f32_ty],

dmgreen wrote:
> This can be a AdvSIMD_BF16FML_Intrinsic?
I want to make a distinction because FML is "Fused multiply-add" and bfmmla is 
a matrix multiplication intrinsic (even though its prototype matches 
AdvSIMD_BF16FML_Intrinsic).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86146

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

[PATCH] D86719: [SyntaxTree][NFC] Refactor function templates into functions taking base class

[Eduardo Caldas via Phabricator via cfe-commits]

eduucaldas updated this revision to Diff 288362.
eduucaldas added a comment.

Fix macro


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86719

Files:
  clang/lib/Tooling/Syntax/BuildTree.cpp


Index: clang/lib/Tooling/Syntax/BuildTree.cpp
===
--- clang/lib/Tooling/Syntax/BuildTree.cpp
+++ clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -378,11 +378,9 @@
 
   /// Returns true if \p D is the last declarator in a chain and is thus
   /// reponsible for creating SimpleDeclaration for the whole chain.
-  template 
-  bool isResponsibleForCreatingDeclaration(const T *D) const {
-static_assert((std::is_base_of::value ||
-   std::is_base_of::value),
-  "only DeclaratorDecl and TypedefNameDecl are supported.");
+  bool isResponsibleForCreatingDeclaration(const Decl *D) const {
+assert((isa(D)) &&
+   "only DeclaratorDecl and TypedefNameDecl are supported.");
 
 const Decl *Next = D->getNextDeclInContext();
 
@@ -390,15 +388,14 @@
 if (Next == nullptr) {
   return true;
 }
-const auto *NextT = dyn_cast(Next);
 
 // Next sibling is not the same type, this one is responsible.
-if (NextT == nullptr) {
+if (D->getKind() != Next->getKind()) {
   return true;
 }
 // Next sibling doesn't begin at the same loc, it must be a different
 // declaration, so this declarator is responsible.
-if (NextT->getBeginLoc() != D->getBeginLoc()) {
+if (Next->getBeginLoc() != D->getBeginLoc()) {
   return true;
 }
 
@@ -1405,10 +1402,9 @@
   }
 
 private:
-  template  SourceLocation getQualifiedNameStart(T *D) {
-static_assert((std::is_base_of::value ||
-   std::is_base_of::value),
-  "only DeclaratorDecl and TypedefNameDecl are supported.");
+  SourceLocation getQualifiedNameStart(NamedDecl *D) {
+assert((isa(D)) &&
+   "only DeclaratorDecl and TypedefNameDecl are supported.");
 
 auto DN = D->getDeclName();
 bool IsAnonymous = DN.isIdentifier() && !DN.getAsIdentifierInfo();
@@ -1438,10 +1434,9 @@
   /// Folds SimpleDeclarator node (if present) and in case this is the last
   /// declarator in the chain it also folds SimpleDeclaration node.
   template  bool processDeclaratorAndDeclaration(T *D) {
-SourceRange Initializer = getInitializerRange(D);
-auto Range = getDeclaratorRange(Builder.sourceManager(),
-D->getTypeSourceInfo()->getTypeLoc(),
-getQualifiedNameStart(D), Initializer);
+auto Range = getDeclaratorRange(
+Builder.sourceManager(), D->getTypeSourceInfo()->getTypeLoc(),
+getQualifiedNameStart(D), getInitializerRange(D));
 
 // There doesn't have to be a declarator (e.g. `void foo(int)` only has
 // declaration, but no declarator).


Index: clang/lib/Tooling/Syntax/BuildTree.cpp
===
--- clang/lib/Tooling/Syntax/BuildTree.cpp
+++ clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -378,11 +378,9 @@
 
   /// Returns true if \p D is the last declarator in a chain and is thus
   /// reponsible for creating SimpleDeclaration for the whole chain.
-  template 
-  bool isResponsibleForCreatingDeclaration(const T *D) const {
-static_assert((std::is_base_of::value ||
-   std::is_base_of::value),
-  "only DeclaratorDecl and TypedefNameDecl are supported.");
+  bool isResponsibleForCreatingDeclaration(const Decl *D) const {
+assert((isa(D)) &&
+   "only DeclaratorDecl and TypedefNameDecl are supported.");
 
 const Decl *Next = D->getNextDeclInContext();
 
@@ -390,15 +388,14 @@
 if (Next == nullptr) {
   return true;
 }
-const auto *NextT = dyn_cast(Next);
 
 // Next sibling is not the same type, this one is responsible.
-if (NextT == nullptr) {
+if (D->getKind() != Next->getKind()) {
   return true;
 }
 // Next sibling doesn't begin at the same loc, it must be a different
 // declaration, so this declarator is responsible.
-if (NextT->getBeginLoc() != D->getBeginLoc()) {
+if (Next->getBeginLoc() != D->getBeginLoc()) {
   return true;
 }
 
@@ -1405,10 +1402,9 @@
   }
 
 private:
-  template  SourceLocation getQualifiedNameStart(T *D) {
-static_assert((std::is_base_of::value ||
-   std::is_base_of::value),
-  "only DeclaratorDecl and TypedefNameDecl are supported.");
+  SourceLocation getQualifiedNameStart(NamedDecl *D) {
+assert((isa(D)) &&
+   "only DeclaratorDecl and TypedefNameDecl are supported.");
 
 auto DN = D->getDeclName();
 bool IsAnonymous = DN.isIdentifier() && !DN.getAsIdentifierInfo();
@@ -1438,10 +1434,9 @@
   /// Folds SimpleDeclarator node (if present) and in case this is the last
   /// 

[PATCH] D86713: [clang-format] Parse nullability attributes as a pointer qualifier

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a comment.

In D86713#2242354 , @JakeMerdichAMD 
wrote:

> In D86713#2242300 , @arichardson 
> wrote:
>
>> In D86713#2242253 , @JakeMerdichAMD 
>> wrote:
>>
>>> LGTM, again assuming tests pass locally (patch did not resolve).
>>>
>>> Out of curiosity, is _Atomic on your radar? I found some code in clang 
>>> proper that handled restrict and _Atomic together. C/C++ have way too many 
>>> qualifiers...
>>
>> I have not looked at _Atomic yet and it's probably low priority for me 
>> unless it's a trivial change.
>> My main motivation with these changes is to format a `__capability` pointer 
>> qualifier correctly (an extension that we add for our out-of-tree CHERI 
>> C/C++ dialect).
>
> I don't know of anyone who uses it yet, so just adding it for posterity, 
> definitely not a blocker. Were you planning on handling `__capability` 
> directly or in a user-configurable option? I can imagine other ad-hoc pointer 
> qualifiers specific to static analysis tools or as properly-ifdef'd aliases 
> of `__attribute__(...)`, so an option might be useful.

I'm not sure yet what the best solution is. I was thinking of either

  a) option to treat double-underscore prefixed strings after `*/&` as 
qualifiers (probably on by default, but not sure about that).
  b) adding a config option with a list of strings that should be treated as 
qualifiers
  c) Adding a new __capability keyword to clang-format since it already 
includes things such as Qt-specific keywords.
  d) Option b) but with __capability included in the default list of qualifiers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86713

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

[PATCH] D85948: [HeapProf] Clang and LLVM support for heap profiling instrumentation

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

In D85948#2242370 , @tejohnson wrote:

> In D85948#2242352 , @lebedev.ri 
> wrote:
>
>> This really needs some docs changes.
>
> Good point, I'll send a patch for that later today.

Actually, that might be better after the runtime is committed. Since the 
generated code with this option won't actually work without that part.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85948

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