[PATCH] D128845: [HLSL]Add -O and -Od option for dxc mode.

2022-08-05 Thread Xiang Li via Phabricator via cfe-commits
python3kgae updated this revision to Diff 450243.
python3kgae added a comment.

Rebase and fix test fail.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128845

Files:
  clang/include/clang/Basic/TargetOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGHLSLRuntime.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/HLSL.cpp
  clang/test/CodeGenHLSL/disable_opt.hlsl
  clang/test/Driver/dxc_O.hlsl
  clang/test/Driver/dxc_fcgl.hlsl

Index: clang/test/Driver/dxc_fcgl.hlsl
===
--- clang/test/Driver/dxc_fcgl.hlsl
+++ clang/test/Driver/dxc_fcgl.hlsl
@@ -1,5 +1,6 @@
 // RUN: %clang_dxc -fcgl -T lib_6_7 foo.hlsl -### %s 2>&1 | FileCheck %s
 
 // Make sure fcgl option flag which translated into "-S" "-emit-llvm" "-disable-llvm-passes".
-// CHECK:"-S" "-emit-llvm" "-disable-llvm-passes"
+// CHECK:"-S"
+// CHECK-SAME:"-emit-llvm" "-disable-llvm-passes"
 
Index: clang/test/Driver/dxc_O.hlsl
===
--- /dev/null
+++ clang/test/Driver/dxc_O.hlsl
@@ -0,0 +1,19 @@
+// RUN: %clang_dxc -T lib_6_7  foo.hlsl -### %s 2>&1 | FileCheck %s
+// RUN: %clang_dxc -T lib_6_7 -Od foo.hlsl -### %s 2>&1 | FileCheck %s --check-prefix=Od
+// RUN: %clang_dxc -T lib_6_7 -O0 foo.hlsl -### %s 2>&1 | FileCheck %s --check-prefix=O0
+// RUN: %clang_dxc -T lib_6_7 -O1 foo.hlsl -### %s 2>&1 | FileCheck %s --check-prefix=O1
+// RUN: %clang_dxc -T lib_6_7 -O2 foo.hlsl -### %s 2>&1 | FileCheck %s --check-prefix=O2
+// RUN: %clang_dxc -T lib_6_7 -O3 foo.hlsl -### %s 2>&1 | FileCheck %s --check-prefix=O3
+
+// Make sure default is O3.
+// CHECK: "-O3"
+
+// Make sure Od option flag which translated into "-O0" "-dxc-opt-disable"
+// Od: "-O0"
+// Od-SAME: "-dxc-opt-disable"
+
+// Make sure O0/O1/O2/O3 is send to cc1.
+// O0: "-O0"
+// O1: "-O1"
+// O2: "-O2"
+// O3: "-O3"
Index: clang/test/CodeGenHLSL/disable_opt.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/disable_opt.hlsl
@@ -0,0 +1,12 @@
+// RUN: %clang -cc1 -S -triple dxil-pc-shadermodel6.3-library -dxc-opt-disable -emit-llvm -xhlsl  -o - %s | FileCheck %s
+// RUN: %clang -cc1 -S -triple dxil-pc-shadermodel6.3-library -emit-llvm -xhlsl  -o - %s | FileCheck %s --check-prefix=OPT
+
+// CHECK:!"dx.disable_optimizations", i32 1}
+
+// OPT-NOT:"dx.disable_optimizations"
+
+float bar(float a, float b);
+
+float foo(float a, float b) {
+  return bar(a, b);
+}
Index: clang/lib/Driver/ToolChains/HLSL.cpp
===
--- clang/lib/Driver/ToolChains/HLSL.cpp
+++ clang/lib/Driver/ToolChains/HLSL.cpp
@@ -164,6 +164,19 @@
   A->claim();
   continue;
 }
+if (A->getOption().getID() == options::OPT__SLASH_O) {
+  StringRef OStr = A->getValue();
+  if (OStr == "d") {
+DAL->AddFlagArg(nullptr, Opts.getOption(options::OPT_dxc_opt_disable));
+DAL->AddFlagArg(nullptr, Opts.getOption(options::OPT_O0));
+A->claim();
+continue;
+  } else {
+DAL->AddJoinedArg(nullptr, Opts.getOption(options::OPT_O), OStr);
+A->claim();
+continue;
+  }
+}
 if (A->getOption().getID() == options::OPT_emit_pristine_llvm) {
   // Translate fcgl into -S -emit-llvm and -disable-llvm-passes.
   DAL->AddFlagArg(nullptr, Opts.getOption(options::OPT_S));
@@ -183,6 +196,9 @@
 Opts.getOption(options::OPT_dxil_validator_version),
 DefaultValidatorVer);
   }
+  if (!DAL->hasArg(options::OPT_O_Group)) {
+DAL->AddJoinedArg(nullptr, Opts.getOption(options::OPT_O), "3");
+  }
   // FIXME: add validation for enable_16bit_types should be after HLSL 2018 and
   // shader model 6.2.
   return DAL;
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3512,6 +3512,8 @@
  options::OPT_D,
  options::OPT_I,
  options::OPT_S,
+ options::OPT_O,
+ options::OPT_dxc_opt_disable,
  options::OPT_emit_llvm,
  options::OPT_disable_llvm_passes,
  options::OPT_fnative_half_type,
Index: clang/lib/CodeGen/CGHLSLRuntime.cpp
===
--- clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -43,6 +43,11 @@
   StringRef DxilValKey = "dx.valver";
   M.addModuleFlag(llvm::Module::ModFlagBehavior::AppendUnique, DxilValKey,

[PATCH] D130754: [X86] Support ``-mindirect-branch-cs-prefix`` for call and jmp to indirect thunk

2022-08-05 Thread Phoebe Wang via Phabricator via cfe-commits
pengfei added a comment.

In D130754#3701837 , @nikic wrote:

> This change caused a significant compile-time regression for `O0` builds 
> (about 1%): 
> http://llvm-compile-time-tracker.com/compare.php?from=45bae1be90472c696f6ba3bb4f8fabee76040fa9&to=6f867f9102838ebe314c1f3661fdf95700386e5a&stat=instructions
>
> At a guess, fetching a module flag for every single instruction is slow?

Thanks for reporting it. I guess so. I'll try to move it outside :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130754

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


[PATCH] D128379: [clangd] Change the url for clang-tidy check documentation

2022-08-05 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4c106c93eb68: [clangd] Change the url for clang-tidy check 
documentation (authored by njames93).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128379

Files:
  clang-tools-extra/clangd/Diagnostics.cpp
  clang-tools-extra/clangd/test/diagnostics-tidy.test


Index: clang-tools-extra/clangd/test/diagnostics-tidy.test
===
--- clang-tools-extra/clangd/test/diagnostics-tidy.test
+++ clang-tools-extra/clangd/test/diagnostics-tidy.test
@@ -9,7 +9,7 @@
 # CHECK-NEXT:  {
 # CHECK-NEXT:"code": "bugprone-sizeof-expression",
 # CHECK-NEXT:"codeDescription": {
-# CHECK-NEXT:  "href": 
"https://clang.llvm.org/extra/clang-tidy/checks/bugprone-sizeof-expression.html";
+# CHECK-NEXT:  "href": 
"https://clang.llvm.org/extra/clang-tidy/checks/bugprone/sizeof-expression.html";
 # CHECK-NEXT:},
 # CHECK-NEXT:"message": "Suspicious usage of 'sizeof(K)'; did you mean 
'K'?",
 # CHECK-NEXT:"range": {
Index: clang-tools-extra/clangd/Diagnostics.cpp
===
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -918,9 +918,19 @@
 // information to be worth linking.
 // https://clang.llvm.org/docs/DiagnosticsReference.html
 break;
-  case Diag::ClangTidy:
-return {("https://clang.llvm.org/extra/clang-tidy/checks/"; + Name + 
".html")
-.str()};
+  case Diag::ClangTidy: {
+StringRef Module, Check;
+// This won't correctly get the module for clang-analyzer checks, but as we
+// don't link in the analyzer that shouldn't be an issue.
+// This would also need updating if anyone decides to create a module with 
a
+// '-' in the name.
+std::tie(Module, Check) = Name.split('-');
+if (Module.empty() || Check.empty())
+  return llvm::None;
+return ("https://clang.llvm.org/extra/clang-tidy/checks/"; + Module + "/" +
+Check + ".html")
+.str();
+  }
   case Diag::Clangd:
 if (Name == "unused-includes")
   return {"https://clangd.llvm.org/guides/include-cleaner"};


Index: clang-tools-extra/clangd/test/diagnostics-tidy.test
===
--- clang-tools-extra/clangd/test/diagnostics-tidy.test
+++ clang-tools-extra/clangd/test/diagnostics-tidy.test
@@ -9,7 +9,7 @@
 # CHECK-NEXT:  {
 # CHECK-NEXT:"code": "bugprone-sizeof-expression",
 # CHECK-NEXT:"codeDescription": {
-# CHECK-NEXT:  "href": "https://clang.llvm.org/extra/clang-tidy/checks/bugprone-sizeof-expression.html";
+# CHECK-NEXT:  "href": "https://clang.llvm.org/extra/clang-tidy/checks/bugprone/sizeof-expression.html";
 # CHECK-NEXT:},
 # CHECK-NEXT:"message": "Suspicious usage of 'sizeof(K)'; did you mean 'K'?",
 # CHECK-NEXT:"range": {
Index: clang-tools-extra/clangd/Diagnostics.cpp
===
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -918,9 +918,19 @@
 // information to be worth linking.
 // https://clang.llvm.org/docs/DiagnosticsReference.html
 break;
-  case Diag::ClangTidy:
-return {("https://clang.llvm.org/extra/clang-tidy/checks/"; + Name + ".html")
-.str()};
+  case Diag::ClangTidy: {
+StringRef Module, Check;
+// This won't correctly get the module for clang-analyzer checks, but as we
+// don't link in the analyzer that shouldn't be an issue.
+// This would also need updating if anyone decides to create a module with a
+// '-' in the name.
+std::tie(Module, Check) = Name.split('-');
+if (Module.empty() || Check.empty())
+  return llvm::None;
+return ("https://clang.llvm.org/extra/clang-tidy/checks/"; + Module + "/" +
+Check + ".html")
+.str();
+  }
   case Diag::Clangd:
 if (Name == "unused-includes")
   return {"https://clangd.llvm.org/guides/include-cleaner"};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] 4c106c9 - [clangd] Change the url for clang-tidy check documentation

2022-08-05 Thread Nathan James via cfe-commits

Author: Nathan James
Date: 2022-08-05T08:42:52+01:00
New Revision: 4c106c93eb68f8f9f201202677cd31e326c16823

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

LOG: [clangd] Change the url for clang-tidy check documentation

In 
https://github.com/llvm/llvm-project/commit/6e566bc5523f743bc34a7e26f050f1f2b4d699a8,
 The directory structure of the documentation for clang-tidy checks was 
changed, however clangd wasn't updated.
Now all the links generated will point to old dead pages.
This updated clangd to use the new page structure.

Reviewed By: sammccall, kadircet

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

Added: 


Modified: 
clang-tools-extra/clangd/Diagnostics.cpp
clang-tools-extra/clangd/test/diagnostics-tidy.test

Removed: 




diff  --git a/clang-tools-extra/clangd/Diagnostics.cpp 
b/clang-tools-extra/clangd/Diagnostics.cpp
index e70d45769fe46..031192f763e6e 100644
--- a/clang-tools-extra/clangd/Diagnostics.cpp
+++ b/clang-tools-extra/clangd/Diagnostics.cpp
@@ -918,9 +918,19 @@ llvm::Optional 
getDiagnosticDocURI(Diag::DiagSource Source,
 // information to be worth linking.
 // https://clang.llvm.org/docs/DiagnosticsReference.html
 break;
-  case Diag::ClangTidy:
-return {("https://clang.llvm.org/extra/clang-tidy/checks/"; + Name + 
".html")
-.str()};
+  case Diag::ClangTidy: {
+StringRef Module, Check;
+// This won't correctly get the module for clang-analyzer checks, but as we
+// don't link in the analyzer that shouldn't be an issue.
+// This would also need updating if anyone decides to create a module with 
a
+// '-' in the name.
+std::tie(Module, Check) = Name.split('-');
+if (Module.empty() || Check.empty())
+  return llvm::None;
+return ("https://clang.llvm.org/extra/clang-tidy/checks/"; + Module + "/" +
+Check + ".html")
+.str();
+  }
   case Diag::Clangd:
 if (Name == "unused-includes")
   return {"https://clangd.llvm.org/guides/include-cleaner"};

diff  --git a/clang-tools-extra/clangd/test/diagnostics-tidy.test 
b/clang-tools-extra/clangd/test/diagnostics-tidy.test
index c7e79b0c6d5f5..a100c9f4359d1 100644
--- a/clang-tools-extra/clangd/test/diagnostics-tidy.test
+++ b/clang-tools-extra/clangd/test/diagnostics-tidy.test
@@ -9,7 +9,7 @@
 # CHECK-NEXT:  {
 # CHECK-NEXT:"code": "bugprone-sizeof-expression",
 # CHECK-NEXT:"codeDescription": {
-# CHECK-NEXT:  "href": 
"https://clang.llvm.org/extra/clang-tidy/checks/bugprone-sizeof-expression.html";
+# CHECK-NEXT:  "href": 
"https://clang.llvm.org/extra/clang-tidy/checks/bugprone/sizeof-expression.html";
 # CHECK-NEXT:},
 # CHECK-NEXT:"message": "Suspicious usage of 'sizeof(K)'; did you mean 
'K'?",
 # CHECK-NEXT:"range": {



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


[PATCH] D131241: [Clang][Lex] Extend HeaderSearch::LookupFile to control OpenFile behavior.

2022-08-05 Thread Vassil Vassilev via Phabricator via cfe-commits
v.g.vassilev added a comment.

Maybe we can add a little more context. In the case of static compilation the 
file system is pretty much read-only and taking a snapshot of it usually is 
sufficient. In the interactive C++ case the compilation is longer and people 
can create and include files, etc. In that case we often do not want to open 
files or cache failures unless is absolutely necessary. We might be able to 
write a test for this in `clang/unittests/Lex/HeaderSearchTest.cpp`. However, 
this patch improves consistency in the interfaces by forwarding the already 
provided flags to the callees and if we cannot write a reasonable test we 
should be able to accept it without.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131241

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


[PATCH] D130754: [X86] Support ``-mindirect-branch-cs-prefix`` for call and jmp to indirect thunk

2022-08-05 Thread Phoebe Wang via Phabricator via cfe-commits
pengfei added a comment.

In D130754#3701858 , @pengfei wrote:

> In D130754#3701837 , @nikic wrote:
>
>> This change caused a significant compile-time regression for `O0` builds 
>> (about 1%): 
>> http://llvm-compile-time-tracker.com/compare.php?from=45bae1be90472c696f6ba3bb4f8fabee76040fa9&to=6f867f9102838ebe314c1f3661fdf95700386e5a&stat=instructions
>>
>> At a guess, fetching a module flag for every single instruction is slow?
>
> Thanks for reporting it. I guess so. I'll try to move it outside :)

D131245  should help with it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130754

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


[clang] 501faaa - [clang][analyzer] Add more wide-character functions to CStringChecker

2022-08-05 Thread Balázs Kéri via cfe-commits

Author: Balázs Kéri
Date: 2022-08-05T10:32:53+02:00
New Revision: 501faaa0d65e9814566ef65e58d834c13b8a

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

LOG: [clang][analyzer] Add more wide-character functions to CStringChecker

Support for functions wmempcpy, wmemmove, wmemcmp is added to the checker.
The same tests are copied that exist for the non-wide versions, with
non-wide functions and character types changed to the wide version.

Reviewed By: martong

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

Added: 


Modified: 
clang/docs/analyzer/checkers.rst
clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
clang/test/Analysis/wstring.c

Removed: 




diff  --git a/clang/docs/analyzer/checkers.rst 
b/clang/docs/analyzer/checkers.rst
index 81312a8fa229b..623a520574e2b 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -2699,7 +2699,7 @@ Check stream handling functions: ``fopen, tmpfile, 
fclose, fread, fwrite, fseek,
 
 alpha.unix.cstring.BufferOverlap (C)
 
-Checks for overlap in two buffer arguments. Applies to:  ``memcpy, mempcpy, 
wmemcpy``.
+Checks for overlap in two buffer arguments. Applies to:  ``memcpy, mempcpy, 
wmemcpy, wmempcpy``.
 
 .. code-block:: c
 

diff  --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 9a6c013bcf66a..09549534fcfd2 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -75,6 +75,16 @@ static ErrorMessage createOutOfBoundErrorMsg(StringRef 
FunctionDescription,
 }
 
 enum class ConcatFnKind { none = 0, strcat = 1, strlcat = 2 };
+
+enum class CharKind { Regular = 0, Wide };
+constexpr CharKind CK_Regular = CharKind::Regular;
+constexpr CharKind CK_Wide = CharKind::Wide;
+
+static QualType getCharPtrType(ASTContext &Ctx, CharKind CK) {
+  return Ctx.getPointerType(CK == CharKind::Regular ? Ctx.CharTy
+: Ctx.WideCharTy);
+}
+
 class CStringChecker : public Checker< eval::Call,
  check::PreStmt,
  check::LiveSymbols,
@@ -125,12 +135,21 @@ class CStringChecker : public Checker< eval::Call,
 
   CallDescriptionMap Callbacks = {
   {{CDF_MaybeBuiltin, "memcpy", 3},
-   std::bind(&CStringChecker::evalMemcpy, _1, _2, _3, false)},
+   std::bind(&CStringChecker::evalMemcpy, _1, _2, _3, CK_Regular)},
   {{CDF_MaybeBuiltin, "wmemcpy", 3},
-   std::bind(&CStringChecker::evalMemcpy, _1, _2, _3, true)},
-  {{CDF_MaybeBuiltin, "mempcpy", 3}, &CStringChecker::evalMempcpy},
-  {{CDF_MaybeBuiltin, "memcmp", 3}, &CStringChecker::evalMemcmp},
-  {{CDF_MaybeBuiltin, "memmove", 3}, &CStringChecker::evalMemmove},
+   std::bind(&CStringChecker::evalMemcpy, _1, _2, _3, CK_Wide)},
+  {{CDF_MaybeBuiltin, "mempcpy", 3},
+   std::bind(&CStringChecker::evalMempcpy, _1, _2, _3, CK_Regular)},
+  {{CDF_None, "wmempcpy", 3},
+   std::bind(&CStringChecker::evalMempcpy, _1, _2, _3, CK_Wide)},
+  {{CDF_MaybeBuiltin, "memcmp", 3},
+   std::bind(&CStringChecker::evalMemcmp, _1, _2, _3, CK_Regular)},
+  {{CDF_MaybeBuiltin, "wmemcmp", 3},
+   std::bind(&CStringChecker::evalMemcmp, _1, _2, _3, CK_Wide)},
+  {{CDF_MaybeBuiltin, "memmove", 3},
+   std::bind(&CStringChecker::evalMemmove, _1, _2, _3, CK_Regular)},
+  {{CDF_MaybeBuiltin, "wmemmove", 3},
+   std::bind(&CStringChecker::evalMemmove, _1, _2, _3, CK_Wide)},
   {{CDF_MaybeBuiltin, "memset", 3}, &CStringChecker::evalMemset},
   {{CDF_MaybeBuiltin, "explicit_memset", 3}, &CStringChecker::evalMemset},
   {{CDF_MaybeBuiltin, "strcpy", 2}, &CStringChecker::evalStrcpy},
@@ -150,7 +169,8 @@ class CStringChecker : public Checker< eval::Call,
   {{CDF_MaybeBuiltin, "strncasecmp", 3}, &CStringChecker::evalStrncasecmp},
   {{CDF_MaybeBuiltin, "strsep", 2}, &CStringChecker::evalStrsep},
   {{CDF_MaybeBuiltin, "bcopy", 3}, &CStringChecker::evalBcopy},
-  {{CDF_MaybeBuiltin, "bcmp", 3}, &CStringChecker::evalMemcmp},
+  {{CDF_MaybeBuiltin, "bcmp", 3},
+   std::bind(&CStringChecker::evalMemcmp, _1, _2, _3, CK_Regular)},
   {{CDF_MaybeBuiltin, "bzero", 2}, &CStringChecker::evalBzero},
   {{CDF_MaybeBuiltin, "explicit_bzero", 2}, &CStringChecker::evalBzero},
   };
@@ -160,16 +180,16 @@ class CStringChecker : public Checker< eval::Call,
   StdCopyBackward{{"std", "copy_backward"}, 3};
 
   FnCheck identifyCall(const CallEvent &Call, CheckerContext &C) const;
-  void evalMemcpy(CheckerContext &C, const CallExpr *CE, bool IsWide) const;
-  void evalMempc

[PATCH] D130470: [clang][analyzer] Add more wide-character functions to CStringChecker

2022-08-05 Thread Balázs Kéri 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 rG501faaa0d65e: [clang][analyzer] Add more wide-character 
functions to CStringChecker (authored by balazske).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130470

Files:
  clang/docs/analyzer/checkers.rst
  clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  clang/test/Analysis/wstring.c

Index: clang/test/Analysis/wstring.c
===
--- clang/test/Analysis/wstring.c
+++ clang/test/Analysis/wstring.c
@@ -33,7 +33,7 @@
 void clang_analyzer_eval(int);
 
 //===--===
-// wwmemcpy()
+// wmemcpy()
 //===--===
 
 #define wmemcpy BUILTIN(wmemcpy)
@@ -139,6 +139,255 @@
   clang_analyzer_eval(result == a); // no-warning (above is fatal)
 }
 
+//===--===
+// wmempcpy()
+//===--===
+
+wchar_t *wmempcpy(wchar_t *restrict s1, const wchar_t *restrict s2, size_t n);
+
+void wmempcpy0 (void) {
+  wchar_t src[] = {1, 2, 3, 4};
+  wchar_t dst[5] = {0};
+
+  wmempcpy(dst, src, 4); // no-warning
+
+  clang_analyzer_eval(wmempcpy(dst, src, 4) == &dst[4]); // expected-warning{{TRUE}}
+
+  // If we actually model the copy, we can make this known.
+  // The important thing for now is that the old value has been invalidated.
+  clang_analyzer_eval(dst[0] != 0); // expected-warning{{UNKNOWN}}
+}
+
+void wmempcpy1 (void) {
+  wchar_t src[] = {1, 2, 3, 4};
+  wchar_t dst[10];
+
+  wmempcpy(dst, src, 5); // expected-warning{{Memory copy function accesses out-of-bound array element}}
+}
+
+void wmempcpy2 (void) {
+  wchar_t src[] = {1, 2, 3, 4};
+  wchar_t dst[1];
+
+  wmempcpy(dst, src, 4); // expected-warning{{Memory copy function overflows the destination buffer}}
+}
+
+void wmempcpy3 (void) {
+  wchar_t src[] = {1, 2, 3, 4};
+  wchar_t dst[3];
+
+  wmempcpy(dst+1, src+2, 2); // no-warning
+}
+
+void wmempcpy4 (void) {
+  wchar_t src[] = {1, 2, 3, 4};
+  wchar_t dst[10];
+
+  wmempcpy(dst+2, src+2, 3); // expected-warning{{Memory copy function accesses out-of-bound array element}}
+}
+
+void wmempcpy5(void) {
+  wchar_t src[] = {1, 2, 3, 4};
+  wchar_t dst[3];
+
+  wmempcpy(dst + 2, src + 2, 2); // expected-warning{{Memory copy function overflows the destination buffer}}
+}
+
+void wmempcpy6(void) {
+  wchar_t a[4] = {0};
+  wmempcpy(a, a, 2); // expected-warning{{overlapping}}
+}
+
+void wmempcpy7(void) {
+  wchar_t a[4] = {0};
+  wmempcpy(a+2, a+1, 2); // expected-warning{{overlapping}}
+}
+
+void wmempcpy8(void) {
+  wchar_t a[4] = {0};
+  wmempcpy(a+1, a+2, 2); // expected-warning{{overlapping}}
+}
+
+void wmempcpy9(void) {
+  wchar_t a[4] = {0};
+  wmempcpy(a+2, a+1, 1); // no-warning
+  wmempcpy(a+1, a+2, 1); // no-warning
+}
+
+void wmempcpy10(void) {
+  wchar_t a[4] = {0};
+  wmempcpy(0, a, 1); // expected-warning{{Null pointer passed as 1st argument to memory copy function}}
+}
+
+void wmempcpy11(void) {
+  wchar_t a[4] = {0};
+  wmempcpy(a, 0, 1); // expected-warning{{Null pointer passed as 2nd argument to memory copy function}}
+}
+
+void wmempcpy12(void) {
+  wchar_t a[4] = {0};
+  wmempcpy(0, a, 0); // no-warning
+}
+
+void wmempcpy13(void) {
+  wchar_t a[4] = {0};
+  wmempcpy(a, 0, 0); // no-warning
+}
+
+void wmempcpy14(void) {
+  wchar_t src[] = {1, 2, 3, 4};
+  wchar_t dst[5] = {0};
+  wchar_t *p;
+
+  p = wmempcpy(dst, src, 4);
+
+  clang_analyzer_eval(p == &dst[4]); // expected-warning{{TRUE}}
+}
+
+struct st {
+  wchar_t i;
+  wchar_t j;
+};
+
+void wmempcpy15(void) {
+  struct st s1 = {0};
+  struct st s2;
+  struct st *p1;
+  struct st *p2;
+
+  p1 = (&s2) + 1;
+  p2 = (struct st *)wmempcpy((wchar_t *)&s2, (wchar_t *)&s1, 2);
+
+  clang_analyzer_eval(p1 == p2); // expected-warning{{TRUE}}
+}
+
+void wmempcpy16(void) {
+  struct st s1[10] = {{0}};
+  struct st s2[10];
+  struct st *p1;
+  struct st *p2;
+
+  p1 = (&s2[0]) + 5;
+  p2 = (struct st *)wmempcpy((wchar_t *)&s2[0], (wchar_t *)&s1[0], 5 * 2);
+
+  clang_analyzer_eval(p1 == p2); // expected-warning{{TRUE}}
+}
+
+void wmempcpy_unknown_size_warn (size_t n) {
+  wchar_t a[4];
+  void *result = wmempcpy(a, 0, n); // expected-warning{{Null pointer passed as 2nd argument to memory copy function}}
+  clang_analyzer_eval(result == a); // no-warning (above is fatal)
+}
+
+void wmempcpy_unknownable_size (wchar_t *src, float n) {
+  wchar_t a[4];
+  // This used to crash because we don't model floats.
+  wmempcpy(a, src, (size_t)n);
+}
+
+//===--===
+// wmemmove()
+//===--=

[PATCH] D131134: [X86] Report error if the amx enabled on the non-64-bits target

2022-08-05 Thread LiuChen via Phabricator via cfe-commits
LiuChen3 added a comment.

In D131134#3699742 , @craig.topper 
wrote:

> How does this interact with `-march=native -m32`. Won't that pick up the amx 
> flag from CPUID?



In D131134#3699344 , @aaron.ballman 
wrote:

> I think the precommit CI failures might be relevant here -- can you 
> double-check those?

Yes. This test is only supported on  amdgpu-registered-target so I didn't catch 
this fail on my local machine. Thanks for point out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131134

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


[PATCH] D131134: [X86] Report error if the amx enabled on the non-64-bits target

2022-08-05 Thread LiuChen via Phabricator via cfe-commits
LiuChen3 added a comment.

In D131134#3699742 , @craig.topper 
wrote:

> How does this interact with `-march=native -m32`. Won't that pick up the amx 
> flag from CPUID?

Good point. I will continue the work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131134

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


[clang] 809b416 - [NFC] Requires x86-registered-target for test/pr56919.cpp

2022-08-05 Thread Chuanqi Xu via cfe-commits

Author: Chuanqi Xu
Date: 2022-08-05T16:46:38+08:00
New Revision: 809b4166413f157ce8e45ba0f80438701bf2e133

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

LOG: [NFC] Requires x86-registered-target for test/pr56919.cpp

Added: 


Modified: 
clang/test/CodeGenCoroutines/pr56919.cpp

Removed: 




diff  --git a/clang/test/CodeGenCoroutines/pr56919.cpp 
b/clang/test/CodeGenCoroutines/pr56919.cpp
index ecd9515acb81..dd6df99a5623 100644
--- a/clang/test/CodeGenCoroutines/pr56919.cpp
+++ b/clang/test/CodeGenCoroutines/pr56919.cpp
@@ -1,4 +1,7 @@
 // Test for PR56919. Tests the destroy function contains the call to delete 
function only.
+//
+// REQUIRES: x86-registered-target
+//
 // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c++20 %s -O3 -S -o - 
| FileCheck %s
 
 #include "Inputs/coroutine.h"



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


[PATCH] D130268: [NFC] Add SmallVector constructor to allow creation of SmallVector from ArrayRef of items convertible to type T

2022-08-05 Thread Dawid Jurczak via Phabricator via cfe-commits
yurai007 updated this revision to Diff 450259.
yurai007 added a comment.

Fix std::initializer_list lifetime issue in test case revealed on CI


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130268

Files:
  llvm/include/llvm/ADT/SmallVector.h
  llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
  llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
  llvm/lib/Target/ARM/ARMISelLowering.cpp
  llvm/lib/Target/Hexagon/HexagonISelDAGToDAGHVX.cpp
  llvm/lib/Target/PowerPC/PPCISelLowering.cpp
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86MCInstLower.cpp
  llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
  llvm/unittests/ADT/SmallVectorTest.cpp

Index: llvm/unittests/ADT/SmallVectorTest.cpp
===
--- llvm/unittests/ADT/SmallVectorTest.cpp
+++ llvm/unittests/ADT/SmallVectorTest.cpp
@@ -246,6 +246,18 @@
   assertValuesInOrder(V, 3u, 1, 2, 3);
 }
 
+// Constructor test.
+TYPED_TEST(SmallVectorTest, ConstructorFromArrayRefSimpleTest) {
+  SCOPED_TRACE("ConstructorFromArrayRefSimpleTest");
+  std::array StdArray = {Constructable(1), Constructable(2),
+   Constructable(3)};
+  ArrayRef Array = StdArray;
+  auto &V = this->theVector;
+  V = SmallVector(Array);
+  assertValuesInOrder(V, 3u, 1, 2, 3);
+  ASSERT_EQ(NumBuiltinElts(TypeParam{}), NumBuiltinElts(V));
+}
+
 // New vector test.
 TYPED_TEST(SmallVectorTest, EmptyVectorTest) {
   SCOPED_TRACE("EmptyVectorTest");
@@ -1130,6 +1142,44 @@
   EXPECT_TRUE(makeArrayRef(V2).equals({4, 5, 3, 2}));
 }
 
+struct To {
+  int Content;
+  friend bool operator==(const To &LHS, const To &RHS) {
+return LHS.Content == RHS.Content;
+  }
+};
+
+class From {
+public:
+  From() = default;
+  From(To M) { T = M; }
+  operator To() const { return T; }
+
+private:
+  To T;
+};
+
+TEST(SmallVectorTest, ConstructFromArrayRefOfConvertibleType) {
+  To to1{1}, to2{2}, to3{3};
+  std::vector StdVector = {From(to1), From(to2), From(to3)};
+  ArrayRef Array = StdVector;
+  {
+llvm::SmallVector Vector(Array);
+
+ASSERT_EQ(Array.size(), Vector.size());
+for (size_t I = 0; I < Array.size(); ++I)
+  EXPECT_EQ(Array[I], Vector[I]);
+  }
+  {
+llvm::SmallVector Vector(Array);
+
+ASSERT_EQ(Array.size(), Vector.size());
+ASSERT_EQ(4u, NumBuiltinElts(Vector));
+for (size_t I = 0; I < Array.size(); ++I)
+  EXPECT_EQ(Array[I], Vector[I]);
+  }
+}
+
 template 
 class SmallVectorReferenceInvalidationTest : public SmallVectorTestBase {
 protected:
Index: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
===
--- llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -7662,7 +7662,7 @@
 
   /// Functions adds masks, merging them into  single one.
   void addMask(ArrayRef SubMask) {
-SmallVector NewMask(SubMask.begin(), SubMask.end());
+SmallVector NewMask(SubMask);
 addMask(NewMask);
   }
 
@@ -8758,7 +8758,7 @@
   return PoisonValue::get(FixedVectorType::get(
   cast(V1->getType())->getElementType(), Mask.size()));
 Value *Op = V1;
-SmallVector CombinedMask(Mask.begin(), Mask.end());
+SmallVector CombinedMask(Mask);
 PeekThroughShuffles(Op, CombinedMask);
 if (!isa(Op->getType()) ||
 !IsIdentityMask(CombinedMask, cast(Op->getType( {
Index: llvm/lib/Target/X86/X86MCInstLower.cpp
===
--- llvm/lib/Target/X86/X86MCInstLower.cpp
+++ llvm/lib/Target/X86/X86MCInstLower.cpp
@@ -1856,7 +1856,7 @@
   SrcOp2.isReg() ? GetRegisterName(SrcOp2.getReg()) : "mem";
 
   // One source operand, fix the mask to print all elements in one span.
-  SmallVector ShuffleMask(Mask.begin(), Mask.end());
+  SmallVector ShuffleMask(Mask);
   if (Src1Name == Src2Name)
 for (int i = 0, e = ShuffleMask.size(); i != e; ++i)
   if (ShuffleMask[i] >= e)
Index: llvm/lib/Target/X86/X86ISelLowering.cpp
===
--- llvm/lib/Target/X86/X86ISelLowering.cpp
+++ llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -6158,7 +6158,7 @@
 SmallVectorImpl &WidenedMask) {
   // Create an alternative mask with info about zeroable elements.
   // Here we do not set undef elements as zeroable.
-  SmallVector ZeroableMask(Mask.begin(), Mask.end());
+  SmallVector ZeroableMask(Mask);
   if (V2IsZero) {
 assert(!Zeroable.isZero() && "V2's non-undef elements are used?!");
 for (int i = 0, Size = Mask.size(); i != Size; ++i)
@@ -11927,7 +11927,7 @@
   MVT VT = MVT::getVectorVT(EltVT, Mask.size());
 
   // We can't assume a canonical shuffle mas

[PATCH] D130015: [clangd] Add "usedAsMutablePointer" highlighting modifier

2022-08-05 Thread Christian Kandeler via Phabricator via cfe-commits
ckandeler added a comment.

IMO the relevant point is the (non-)const-ness of the corresponding parameter, 
i.e. can the passed object get mutated or not. The fact that there's an 
ampersand at the call site (which is not even the case if the variable is 
itself a pointer) does not tell me that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130015

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


[PATCH] D131067: [analyzer] Treat values passed as parameter as escaped

2022-08-05 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added a comment.

Please, consider stating the motivation behind this change.
For me, by looking at the modified test, it feels like we would lose legitimate 
findings (aka. true-positive reports) by applying this change.
From my perspective, the store to `i` is //dead//, since it will be immediately 
overwritten in the `referenceParameters()` function.




Comment at: clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp:536-537
+  void findReferenceParameter(const FunctionDecl *FD, const Expr *const *Args) 
{
+unsigned numParams = FD->getNumParams();
+for (unsigned i = 0; i < numParams; ++i) {
+  if (FD->getParamDecl(i)->getType()->isReferenceType()) {

You don't seem to use the index, only for iterating over the parameters.
I believe, `FD->parameters()` would simplify this code.



Comment at: clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp:538-545
+  if (FD->getParamDecl(i)->getType()->isReferenceType()) {
+if (auto *DRE = dyn_cast(Args[i])) {
+  if (auto *VD = dyn_cast(DRE->getDecl())) {
+Escaped.insert(VD);
+  }
+}
+  }

By looking at e.g. `findLambdaReferenceCaptures()`, it seems like we should 
prefer `continue` statements for filtering; probably to avoid such deeply 
nested intendations.
Along with that, I think the llvm style guide prefers not wrapping single 
statement branches by courly braces.



Comment at: clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp:549
+  void findCallReferenceParameters(const CallExpr *CE) {
+if (auto *FD = dyn_cast(CE->getCalleeDecl())) {
+  findReferenceParameter(FD, CE->getArgs());

I'm not sure `getCalleeDecl()` always returns a valid pointer.
I believe the `dyn_cast_or_null` would be more appropriate in this case.
This theory is underpinned by the other uses of the `getCalleeDecl()` API.



Comment at: clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp:555
+  void findConstructorReferenceParameters(const CXXConstructExpr *CE) {
+findReferenceParameter(CE->getConstructor(), CE->getArgs());
+  }

Why don't you use the `CE->arguments()` method instead? That would result in a 
range (begin + end).
I believe that is superior in most cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131067

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


[PATCH] D130394: [clang][CodeGen] Factor out Swift ABI hooks (NFCI)

2022-08-05 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

This seems like a reasonable set of changes, but I'd prefer someone more 
well-versed in codegen make the final call.




Comment at: clang/lib/CodeGen/TargetInfo.h:63
+  /// Returns Swift ABI info helper for the target.
+  const SwiftABIInfo &getSwiftABIInfo() const { return *SwiftInfo; }
+

`SwiftInfo` isn't always initialized to a nonnull pointer; should this have an 
assert to help point out when someone's forgotten to initialize that properly?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130394

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


[PATCH] D130268: [NFC] Add SmallVector constructor to allow creation of SmallVector from ArrayRef of items convertible to type T

2022-08-05 Thread Dawid Jurczak 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 rG1bd31a689844: [NFC] Add SmallVector constructor to allow 
creation of SmallVector from… (authored by yurai007).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130268

Files:
  llvm/include/llvm/ADT/SmallVector.h
  llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
  llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
  llvm/lib/Target/ARM/ARMISelLowering.cpp
  llvm/lib/Target/Hexagon/HexagonISelDAGToDAGHVX.cpp
  llvm/lib/Target/PowerPC/PPCISelLowering.cpp
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86MCInstLower.cpp
  llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
  llvm/unittests/ADT/SmallVectorTest.cpp

Index: llvm/unittests/ADT/SmallVectorTest.cpp
===
--- llvm/unittests/ADT/SmallVectorTest.cpp
+++ llvm/unittests/ADT/SmallVectorTest.cpp
@@ -246,6 +246,18 @@
   assertValuesInOrder(V, 3u, 1, 2, 3);
 }
 
+// Constructor test.
+TYPED_TEST(SmallVectorTest, ConstructorFromArrayRefSimpleTest) {
+  SCOPED_TRACE("ConstructorFromArrayRefSimpleTest");
+  std::array StdArray = {Constructable(1), Constructable(2),
+   Constructable(3)};
+  ArrayRef Array = StdArray;
+  auto &V = this->theVector;
+  V = SmallVector(Array);
+  assertValuesInOrder(V, 3u, 1, 2, 3);
+  ASSERT_EQ(NumBuiltinElts(TypeParam{}), NumBuiltinElts(V));
+}
+
 // New vector test.
 TYPED_TEST(SmallVectorTest, EmptyVectorTest) {
   SCOPED_TRACE("EmptyVectorTest");
@@ -1130,6 +1142,44 @@
   EXPECT_TRUE(makeArrayRef(V2).equals({4, 5, 3, 2}));
 }
 
+struct To {
+  int Content;
+  friend bool operator==(const To &LHS, const To &RHS) {
+return LHS.Content == RHS.Content;
+  }
+};
+
+class From {
+public:
+  From() = default;
+  From(To M) { T = M; }
+  operator To() const { return T; }
+
+private:
+  To T;
+};
+
+TEST(SmallVectorTest, ConstructFromArrayRefOfConvertibleType) {
+  To to1{1}, to2{2}, to3{3};
+  std::vector StdVector = {From(to1), From(to2), From(to3)};
+  ArrayRef Array = StdVector;
+  {
+llvm::SmallVector Vector(Array);
+
+ASSERT_EQ(Array.size(), Vector.size());
+for (size_t I = 0; I < Array.size(); ++I)
+  EXPECT_EQ(Array[I], Vector[I]);
+  }
+  {
+llvm::SmallVector Vector(Array);
+
+ASSERT_EQ(Array.size(), Vector.size());
+ASSERT_EQ(4u, NumBuiltinElts(Vector));
+for (size_t I = 0; I < Array.size(); ++I)
+  EXPECT_EQ(Array[I], Vector[I]);
+  }
+}
+
 template 
 class SmallVectorReferenceInvalidationTest : public SmallVectorTestBase {
 protected:
Index: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
===
--- llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -7659,7 +7659,7 @@
 
   /// Functions adds masks, merging them into  single one.
   void addMask(ArrayRef SubMask) {
-SmallVector NewMask(SubMask.begin(), SubMask.end());
+SmallVector NewMask(SubMask);
 addMask(NewMask);
   }
 
@@ -8755,7 +8755,7 @@
   return PoisonValue::get(FixedVectorType::get(
   cast(V1->getType())->getElementType(), Mask.size()));
 Value *Op = V1;
-SmallVector CombinedMask(Mask.begin(), Mask.end());
+SmallVector CombinedMask(Mask);
 PeekThroughShuffles(Op, CombinedMask);
 if (!isa(Op->getType()) ||
 !IsIdentityMask(CombinedMask, cast(Op->getType( {
Index: llvm/lib/Target/X86/X86MCInstLower.cpp
===
--- llvm/lib/Target/X86/X86MCInstLower.cpp
+++ llvm/lib/Target/X86/X86MCInstLower.cpp
@@ -1865,7 +1865,7 @@
   SrcOp2.isReg() ? GetRegisterName(SrcOp2.getReg()) : "mem";
 
   // One source operand, fix the mask to print all elements in one span.
-  SmallVector ShuffleMask(Mask.begin(), Mask.end());
+  SmallVector ShuffleMask(Mask);
   if (Src1Name == Src2Name)
 for (int i = 0, e = ShuffleMask.size(); i != e; ++i)
   if (ShuffleMask[i] >= e)
Index: llvm/lib/Target/X86/X86ISelLowering.cpp
===
--- llvm/lib/Target/X86/X86ISelLowering.cpp
+++ llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -6158,7 +6158,7 @@
 SmallVectorImpl &WidenedMask) {
   // Create an alternative mask with info about zeroable elements.
   // Here we do not set undef elements as zeroable.
-  SmallVector ZeroableMask(Mask.begin(), Mask.end());
+  SmallVector ZeroableMask(Mask);
   if (V2IsZero) {
 assert(!Zeroable.isZero() && "V2's non-undef elements are used?!");
 for (int i = 0, Size = Mask.size(); i != Size; ++i)
@

[PATCH] D131067: [analyzer] Treat values passed as parameter as escaped

2022-08-05 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added inline comments.



Comment at: clang/test/Analysis/dead-stores.cpp:225
+void functionReferenceParameter(int &i) {
+  i = 5; // no warning
+}

By checking it on godbolt, this line never raised a warning.
https://godbolt.org/z/n7fP5od5q

You must be referring to L230 and L241 instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131067

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


[PATCH] D131065: [clang][dataflow] Store DeclContext of block being analysed in Environment if available.

2022-08-05 Thread Stanislav Gatev via Phabricator via cfe-commits
sgatev accepted this revision.
sgatev added inline comments.



Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:537-539
+  auto *Caller = Env.getDeclCtx();
+  Env = Environment(ExitState->Env);
+  Env.setDeclCtx(Caller);

I believe you'll need to rebase because this has changed: 
https://github.com/llvm/llvm-project/blob/main/clang/lib/Analysis/FlowSensitive/Transfer.cpp#L567


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131065

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


[PATCH] D131213: [clang][Headers] Fix unintentional error in D130800

2022-08-05 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

> Undefined macros evaluate to zero, so when checking for a smaller value, we 
> need to include the case when the macro is undefined.

The code being changed already checks `defined(__cplusplus)` so there no 
undefined macro value being tested. What's more, if `__cplusplus` is not 
defined, we wouldn't even get into this block because we'd have hit line 19 
instead. I think the current form is easier to read given the subsequent 
comment talking about being in C++98 mode (which would be weird to consider for 
when `__cplusplus` is not defined), even if the test for `defined(__cplusplus)` 
isn't strictly needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131213

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


[PATCH] D130394: [clang][CodeGen] Factor out Swift ABI hooks (NFCI)

2022-08-05 Thread Sergei Barannikov via Phabricator via cfe-commits
barannikov88 added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.h:63
+  /// Returns Swift ABI info helper for the target.
+  const SwiftABIInfo &getSwiftABIInfo() const { return *SwiftInfo; }
+

aaron.ballman wrote:
> `SwiftInfo` isn't always initialized to a nonnull pointer; should this have 
> an assert to help point out when someone's forgotten to initialize that 
> properly?
Good point, thank you


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130394

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


[PATCH] D131194: [C++20] Fix crash-on-valid with consteval temporary construction through list initialization

2022-08-05 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman updated this revision to Diff 450271.
aaron.ballman added a comment.

Improved from review feedback.


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

https://reviews.llvm.org/D131194

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/CodeGen/CGExprConstant.cpp
  clang/test/CodeGenCXX/cxx20-consteval-crash.cpp


Index: clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
===
--- clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
+++ clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
@@ -56,3 +56,18 @@
 // CHECK: define{{.*}} signext i8 @_ZN10Issue545782f2IcEEcT_(
 // CHECK: ret i8 4
 }
+
+namespace Issue55871 {
+struct Item {
+  consteval Item(char c) :_char{c}{}
+  char _char;
+};
+
+int function(const Item& item1, const Item& item2) {
+  return 0;
+}
+
+int foo() {
+  return function(Item{'a'}, Item{'a'});
+}
+} // namespace Issue58871
Index: clang/lib/CodeGen/CGExprConstant.cpp
===
--- clang/lib/CodeGen/CGExprConstant.cpp
+++ clang/lib/CodeGen/CGExprConstant.cpp
@@ -1395,15 +1395,13 @@
 llvm::Constant *ConstantEmitter::tryEmitConstantExpr(const ConstantExpr *CE) {
   if (!CE->hasAPValueResult())
 return nullptr;
-  const Expr *Inner = CE->getSubExpr()->IgnoreImplicit();
-  QualType RetType;
-  if (auto *Call = dyn_cast(Inner))
-RetType = Call->getCallReturnType(CGM.getContext());
-  else if (auto *Ctor = dyn_cast(Inner))
-RetType = Ctor->getType();
-  llvm::Constant *Res =
-  emitAbstract(CE->getBeginLoc(), CE->getAPValueResult(), RetType);
-  return Res;
+
+  QualType RetType = CE->getType();
+  if (CE->isGLValue())
+RetType = CGM.getContext().getLValueReferenceType(RetType);
+  assert(!RetType.isNull() && "Not certain of the constant expression's type");
+
+  return emitAbstract(CE->getBeginLoc(), CE->getAPValueResult(), RetType);
 }
 
 llvm::Constant *
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -65,7 +65,9 @@
 - Fix a crash when evaluating a multi-dimensional array's array filler
   expression is element-dependent. This fixes
   `Issue 50601 `_.
-
+- Fixed a crash-on-valid with consteval evaluation of a list-initialized
+  constructor for a temporary object. This fixes
+  `Issue 55871 `_.
 
 Improvements to Clang's diagnostics
 ^^^


Index: clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
===
--- clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
+++ clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
@@ -56,3 +56,18 @@
 // CHECK: define{{.*}} signext i8 @_ZN10Issue545782f2IcEEcT_(
 // CHECK: ret i8 4
 }
+
+namespace Issue55871 {
+struct Item {
+  consteval Item(char c) :_char{c}{}
+  char _char;
+};
+
+int function(const Item& item1, const Item& item2) {
+  return 0;
+}
+
+int foo() {
+  return function(Item{'a'}, Item{'a'});
+}
+} // namespace Issue58871
Index: clang/lib/CodeGen/CGExprConstant.cpp
===
--- clang/lib/CodeGen/CGExprConstant.cpp
+++ clang/lib/CodeGen/CGExprConstant.cpp
@@ -1395,15 +1395,13 @@
 llvm::Constant *ConstantEmitter::tryEmitConstantExpr(const ConstantExpr *CE) {
   if (!CE->hasAPValueResult())
 return nullptr;
-  const Expr *Inner = CE->getSubExpr()->IgnoreImplicit();
-  QualType RetType;
-  if (auto *Call = dyn_cast(Inner))
-RetType = Call->getCallReturnType(CGM.getContext());
-  else if (auto *Ctor = dyn_cast(Inner))
-RetType = Ctor->getType();
-  llvm::Constant *Res =
-  emitAbstract(CE->getBeginLoc(), CE->getAPValueResult(), RetType);
-  return Res;
+
+  QualType RetType = CE->getType();
+  if (CE->isGLValue())
+RetType = CGM.getContext().getLValueReferenceType(RetType);
+  assert(!RetType.isNull() && "Not certain of the constant expression's type");
+
+  return emitAbstract(CE->getBeginLoc(), CE->getAPValueResult(), RetType);
 }
 
 llvm::Constant *
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -65,7 +65,9 @@
 - Fix a crash when evaluating a multi-dimensional array's array filler
   expression is element-dependent. This fixes
   `Issue 50601 `_.
-
+- Fixed a crash-on-valid with consteval evaluation of a list-initialized
+  constructor for a temporary object. This fixes
+  `Issue 55871 `_.
 
 Improvements to Clang's diagnostics
 ^^^
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/

[PATCH] D131194: [C++20] Fix crash-on-valid with consteval temporary construction through list initialization

2022-08-05 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D131194#3700688 , @efriedma wrote:

> Instead of trying to dig through subexpressions of the ConstantExpr to try to 
> infer the type we need, can we just compute it directly?
>
>   QualType RetType = CE->getType();
>   if (CE->isGLValue())
> RetType = CGM.getContext().getLValueReferenceType(RetType);

Oh wow, that's a *much* better approach, thank you for the suggestion! It 
passed my local testing, so hopefully precommit CI doesn't spot any concerns 
from it.


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

https://reviews.llvm.org/D131194

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


[PATCH] D130058: [Clang] Diagnose ill-formed constant expression when setting a non fixed enum to a value outside the range of the enumeration values

2022-08-05 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment.

+1 on downgrading to a warning, so that users can have time to do cleanup on 
their codebase.

We also see internal breakages caused by this change. It breaks some code using 
the magic_enum  library, a simple repro 
example:

  #include
  
  #include "magic_enum.hpp"

  
  enum Color { Red, Black };
  
  int main(int argc, char* argv[]) {
  
 Color color = Black;   
   
 switch (color) {   
   
   case Red:
   
 break; 
   
   default: 
   
 std::cout << magic_enum::enum_name(color); 
   
 break; 
   
 }  

   
 return 0;  
   
   }

Errors:

  ./magic_enum/magic_enum.hpp:436:10: note: in instantiation of function 
template specialization 'magic_enum::detail::values' requested here
return values>(std::make_index_sequence{});
   ^
  ./magic_enum/magic_enum.hpp:440:34: note: in instantiation of function 
template specialization 'magic_enum::detail::values' requested here
  inline constexpr auto values_v = values();
   ^
  ./magic_enum/magic_enum.hpp:446:33: note: in instantiation of variable 
template specialization 'magic_enum::detail::values_v' requested here
  inline constexpr auto count_v = values_v.size();
  ^
  ./magic_enum/magic_enum.hpp:567:17: note: in instantiation of variable 
template specialization 'magic_enum::detail::count_v' requested here
static_assert(count_v > 0, "magic_enum requires enum 
implementation and valid max and min.");
  ^
  ./magic_enum/magic_enum.hpp:579:1: note: in instantiation of template class 
'magic_enum::detail::enable_if_enum' 
requested here
  using enable_if_enum_t = typename 
enable_if_enum>, false, T, R>::type;
  ^
  ./magic_enum/magic_enum.hpp:690:69: note: in instantiation of template type 
alias 'enable_if_enum_t' requested here
  [[nodiscard]] constexpr auto enum_name(E value) noexcept -> 
detail::enable_if_enum_t {
  ^
  main.cc:17:20: note: while substituting deduced template arguments into 
function template 'enum_name' [with E = Color]
std::cout << magic_enum::enum_name(color);


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

https://reviews.llvm.org/D130058

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


[PATCH] D131255: Fix Wbitfield-constant-conversion on 1-bit signed bitfield

2022-08-05 Thread Shawn Zhong via Phabricator via cfe-commits
ShawnZhong created this revision.
ShawnZhong added a reviewer: rsmith.
ShawnZhong added a project: clang.
Herald added a project: All.
ShawnZhong requested review of this revision.
Herald added a subscriber: cfe-commits.

Fix https://github.com/llvm/llvm-project/issues/53253


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131255

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/constant-conversion.c


Index: clang/test/Sema/constant-conversion.c
===
--- clang/test/Sema/constant-conversion.c
+++ clang/test/Sema/constant-conversion.c
@@ -131,3 +131,15 @@
   s.a = -9;  // expected-warning{{implicit truncation from 'int' to bit-field 
changes value from -9 to 7}}
   s.a = 16;  // expected-warning{{implicit truncation from 'int' to bit-field 
changes value from 16 to 0}}
 }
+
+void test11(void) {
+  struct S {
+signed char c : 1;  // the only valid values are 0 and -1
+  } s;
+
+  s.c = -2; // expected-warning {{implicit conversion from 'int' to 'char' 
changes value from -2 to 0}}
+  s.c = -1; // no-warning
+  s.c = 0;  // no-warning
+  s.c = 1;  // expected-warning {{implicit conversion from 'int' to 'char' 
changes value from 1 to -1}}
+  s.c = 2;  // expected-warning {{implicit conversion from 'int' to 'char' 
changes value from 2 to 0}}
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -13067,11 +13067,6 @@
   if (llvm::APSInt::isSameValue(Value, TruncatedValue))
 return false;
 
-  // Special-case bitfields of width 1: booleans are naturally 0/1, and
-  // therefore don't strictly fit into a signed bitfield of width 1.
-  if (FieldWidth == 1 && Value == 1)
-return false;
-
   std::string PrettyValue = toString(Value, 10);
   std::string PrettyTrunc = toString(TruncatedValue, 10);
 


Index: clang/test/Sema/constant-conversion.c
===
--- clang/test/Sema/constant-conversion.c
+++ clang/test/Sema/constant-conversion.c
@@ -131,3 +131,15 @@
   s.a = -9;  // expected-warning{{implicit truncation from 'int' to bit-field changes value from -9 to 7}}
   s.a = 16;  // expected-warning{{implicit truncation from 'int' to bit-field changes value from 16 to 0}}
 }
+
+void test11(void) {
+  struct S {
+signed char c : 1;  // the only valid values are 0 and -1
+  } s;
+
+  s.c = -2; // expected-warning {{implicit conversion from 'int' to 'char' changes value from -2 to 0}}
+  s.c = -1; // no-warning
+  s.c = 0;  // no-warning
+  s.c = 1;  // expected-warning {{implicit conversion from 'int' to 'char' changes value from 1 to -1}}
+  s.c = 2;  // expected-warning {{implicit conversion from 'int' to 'char' changes value from 2 to 0}}
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -13067,11 +13067,6 @@
   if (llvm::APSInt::isSameValue(Value, TruncatedValue))
 return false;
 
-  // Special-case bitfields of width 1: booleans are naturally 0/1, and
-  // therefore don't strictly fit into a signed bitfield of width 1.
-  if (FieldWidth == 1 && Value == 1)
-return false;
-
   std::string PrettyValue = toString(Value, 10);
   std::string PrettyTrunc = toString(TruncatedValue, 10);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D130894: [clang] Print more information about failed static assertions

2022-08-05 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added reviewers: erichkeane, cjdb, clang-language-wg, jyknight.
aaron.ballman added a comment.

In D130894#3701749 , @tbaeder wrote:

> For the record, the output now looks something like this:
>
>   test.cpp:24:1: error: static assertion failed due to requirement 'c != c'
>   static_assert(c != c);
>   ^ ~~
>   test.cpp:24:17: note: expression evaluates to '('0' != '0')'
>   static_assert(c != c);
> ~~^~~~

This looks hard to read to me...

> test.cpp:25:1: error: static assertion failed due to requirement 'c > 'a''
> static_assert(c > 'a');
> ^ ~~~
> test.cpp:25:17: note: expression evaluates to '('0' > 'a')'

Same confusion here -- it took me a while to realize what's happening is that 
we're quoting the part outside of the parens and that's throwing me for a loop. 
We typically quote things that are syntax but in this case, the parens are not 
part of the syntax and so the surrounding quotes are catching me. Given that 
parens are being used as a visual delimiter and a single quote serves the same 
purpose, I would expect something more like:

  test.cpp:25:17: note: expression evaluates to ''0' > 'a''
  test.cpp:26:17: note: expression evaluates to '0 > 'a''
  test.cpp:27:17: note: expression evaluates to '14 > 5'
  test.cpp:28:17: note: expression evaluates to 'true == false'
  test.cpp:29:17: note: expression evaluates to 'nullptr != nullptr'

Adding a few more folks as reviewers to see if there is a consensus position on 
how to proceed.


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

https://reviews.llvm.org/D130894

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


[PATCH] D131255: Fix Wbitfield-constant-conversion on 1-bit signed bitfield

2022-08-05 Thread Shawn Zhong via Phabricator via cfe-commits
ShawnZhong updated this revision to Diff 450274.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131255

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/constant-conversion.c


Index: clang/test/Sema/constant-conversion.c
===
--- clang/test/Sema/constant-conversion.c
+++ clang/test/Sema/constant-conversion.c
@@ -15,8 +15,16 @@
 }
 
 void test(void) {
-  struct { int bit : 1; } a;
-  a.bit = 1; // shouldn't warn
+  struct S {
+signed char c : 1;  // the only valid values are 0 and -1
+  } s;
+
+  s.c = -3; // expected-warning {{implicit truncation from 'int' to bit-field 
changes value from -3 to -1}}
+  s.c = -2; // expected-warning {{implicit truncation from 'int' to bit-field 
changes value from -2 to 0}}
+  s.c = -1; // no-warning
+  s.c = 0;  // no-warning
+  s.c = 1;  // expected-warning {{implicit truncation from 'int' to bit-field 
changes value from 1 to -1}}
+  s.c = 2;  // expected-warning {{implicit truncation from 'int' to bit-field 
changes value from 2 to 0}}
 }
 
 enum Test2 { K_zero, K_one };
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -13067,11 +13067,6 @@
   if (llvm::APSInt::isSameValue(Value, TruncatedValue))
 return false;
 
-  // Special-case bitfields of width 1: booleans are naturally 0/1, and
-  // therefore don't strictly fit into a signed bitfield of width 1.
-  if (FieldWidth == 1 && Value == 1)
-return false;
-
   std::string PrettyValue = toString(Value, 10);
   std::string PrettyTrunc = toString(TruncatedValue, 10);
 


Index: clang/test/Sema/constant-conversion.c
===
--- clang/test/Sema/constant-conversion.c
+++ clang/test/Sema/constant-conversion.c
@@ -15,8 +15,16 @@
 }
 
 void test(void) {
-  struct { int bit : 1; } a;
-  a.bit = 1; // shouldn't warn
+  struct S {
+signed char c : 1;  // the only valid values are 0 and -1
+  } s;
+
+  s.c = -3; // expected-warning {{implicit truncation from 'int' to bit-field changes value from -3 to -1}}
+  s.c = -2; // expected-warning {{implicit truncation from 'int' to bit-field changes value from -2 to 0}}
+  s.c = -1; // no-warning
+  s.c = 0;  // no-warning
+  s.c = 1;  // expected-warning {{implicit truncation from 'int' to bit-field changes value from 1 to -1}}
+  s.c = 2;  // expected-warning {{implicit truncation from 'int' to bit-field changes value from 2 to 0}}
 }
 
 enum Test2 { K_zero, K_one };
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -13067,11 +13067,6 @@
   if (llvm::APSInt::isSameValue(Value, TruncatedValue))
 return false;
 
-  // Special-case bitfields of width 1: booleans are naturally 0/1, and
-  // therefore don't strictly fit into a signed bitfield of width 1.
-  if (FieldWidth == 1 && Value == 1)
-return false;
-
   std::string PrettyValue = toString(Value, 10);
   std::string PrettyTrunc = toString(TruncatedValue, 10);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D130894: [clang] Print more information about failed static assertions

2022-08-05 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment.

or

  test.cpp:25:17: note: evaluated expression: '0' > 'a'


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

https://reviews.llvm.org/D130894

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


[PATCH] D131255: Fix Wbitfield-constant-conversion on 1-bit signed bitfield

2022-08-05 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added reviewers: aaron.ballman, clang-language-wg.
aaron.ballman added a comment.

Thank you for working on this! I think this is going in the right direction, 
but I think there's some more work needed here because the rules depend on the 
type of the bit-field (at least in C, I've not checked C++ yet). See C2x 
6.7.2.1p12 (and it's footnote): "A bit-field is interpreted as having a signed 
or unsigned integer type consisting of the specified number of bits. ..." (and 
the footnote goes on to say "As specified in 6.7.2 above, if the actual type 
specifier used is `int` or a typedef-name defined as `int`, then it is 
implementation-defined whether the bit-field is signed or unsigned. ...").

Assuming that we actually do treat plain `int` bit-fields as being signed 
(which I'm pretty sure we do), then I agree that these changes are fine. But 
you should add some CodeGen test coverage (or find existing coverage) that 
proves how we behave in that case and an `int` test in `Sema` showing we match 
the codegen behavior.

Also, can you add a release note for the fix?




Comment at: clang/test/Sema/constant-conversion.c:137
+  struct S {
+signed char c : 1;  // the only valid values are 0 and -1
+  } s;




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131255

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


[clang] 4bc9e60 - Removing redundant code; NFC

2022-08-05 Thread Aaron Ballman via cfe-commits

Author: Aaron Ballman
Date: 2022-08-05T09:17:20-04:00
New Revision: 4bc9e603065b59976f3a16b7ecc6b5fb109f5f2d

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

LOG: Removing redundant code; NFC

The same predicate is checked on line 12962 just above the removed code.

Added: 


Modified: 
clang/lib/Sema/SemaChecking.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index d71114e33f31f..293239c7b0d78 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -12976,9 +12976,6 @@ static bool AnalyzeBitFieldAssignment(Sema &S, 
FieldDecl *Bitfield, Expr *Init,
 }
   }
 
-  if (Bitfield->getType()->isBooleanType())
-return false;
-
   // Ignore value- or type-dependent expressions.
   if (Bitfield->getBitWidth()->isValueDependent() ||
   Bitfield->getBitWidth()->isTypeDependent() ||



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


[PATCH] D131065: [clang][dataflow] Store DeclContext of block being analysed in Environment if available.

2022-08-05 Thread weiyi via Phabricator via cfe-commits
wyt updated this revision to Diff 450276.
wyt marked an inline comment as done.
wyt added a comment.

Rebase to head.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131065

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp

Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -154,7 +154,7 @@
 : DACtx(&DACtx), FlowConditionToken(&DACtx.makeFlowConditionToken()) {}
 
 Environment::Environment(const Environment &Other)
-: DACtx(Other.DACtx), ReturnLoc(Other.ReturnLoc),
+: DACtx(Other.DACtx), DeclCtx(Other.DeclCtx), ReturnLoc(Other.ReturnLoc),
   ThisPointeeLoc(Other.ThisPointeeLoc), DeclToLoc(Other.DeclToLoc),
   ExprToLoc(Other.ExprToLoc), LocToVal(Other.LocToVal),
   MemberLocToStruct(Other.MemberLocToStruct),
@@ -168,9 +168,11 @@
 }
 
 Environment::Environment(DataflowAnalysisContext &DACtx,
- const DeclContext &DeclCtx)
+ const DeclContext &DeclCtxArg)
 : Environment(DACtx) {
-  if (const auto *FuncDecl = dyn_cast(&DeclCtx)) {
+  setDeclCtx(&DeclCtxArg);
+
+  if (const auto *FuncDecl = dyn_cast(DeclCtx)) {
 assert(FuncDecl->getBody() != nullptr);
 initGlobalVars(*FuncDecl->getBody(), *this);
 for (const auto *ParamDecl : FuncDecl->parameters()) {
@@ -185,7 +187,7 @@
 ReturnLoc = &createStorageLocation(ReturnType);
   }
 
-  if (const auto *MethodDecl = dyn_cast(&DeclCtx)) {
+  if (const auto *MethodDecl = dyn_cast(DeclCtx)) {
 auto *Parent = MethodDecl->getParent();
 assert(Parent != nullptr);
 if (Parent->isLambda())
@@ -210,6 +212,9 @@
 
   const auto *FuncDecl = Call->getDirectCallee();
   assert(FuncDecl != nullptr);
+
+  Env.setDeclCtx(FuncDecl);
+
   // FIXME: In order to allow the callee to reference globals, we probably need
   // to call `initGlobalVars` here in some way.
 
@@ -252,12 +257,12 @@
 
 void Environment::popCall(const Environment &CalleeEnv) {
   // We ignore `DACtx` because it's already the same in both. We don't want the
-  // callee's `ReturnLoc` or `ThisPointeeLoc`. We don't bring back `DeclToLoc`
-  // and `ExprToLoc` because we want to be able to later analyze the same callee
-  // in a different context, and `setStorageLocation` requires there to not
-  // already be a storage location assigned. Conceptually, these maps capture
-  // information from the local scope, so when popping that scope, we do not
-  // propagate the maps.
+  // callee's `DeclCtx`, `ReturnLoc` or `ThisPointeeLoc`. We don't bring back
+  // `DeclToLoc` and `ExprToLoc` because we want to be able to later analyze the
+  // same callee in a different context, and `setStorageLocation` requires there
+  // to not already be a storage location assigned. Conceptually, these maps
+  // capture information from the local scope, so when popping that scope, we do
+  // not propagate the maps.
   this->LocToVal = std::move(CalleeEnv.LocToVal);
   this->MemberLocToStruct = std::move(CalleeEnv.MemberLocToStruct);
   this->FlowConditionToken = std::move(CalleeEnv.FlowConditionToken);
@@ -304,11 +309,13 @@
   assert(DACtx == Other.DACtx);
   assert(ReturnLoc == Other.ReturnLoc);
   assert(ThisPointeeLoc == Other.ThisPointeeLoc);
+  assert(DeclCtx == Other.DeclCtx);
 
   auto Effect = LatticeJoinEffect::Unchanged;
 
   Environment JoinedEnv(*DACtx);
 
+  JoinedEnv.setDeclCtx(DeclCtx);
   JoinedEnv.ReturnLoc = ReturnLoc;
   JoinedEnv.ThisPointeeLoc = ThisPointeeLoc;
 
Index: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
===
--- clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -172,6 +172,13 @@
   LatticeJoinEffect join(const Environment &Other,
  Environment::ValueModel &Model);
 
+  /// Returns the `DeclContext` of the block being analysed if provided,
+  /// otherwise returns nullptr.
+  const DeclContext *getDeclCtx() { return DeclCtx; }
+
+  /// Sets the `DeclContext` of the block being analysed.
+  void setDeclCtx(const DeclContext *Ctx) { DeclCtx = Ctx; }
+
   // FIXME: Rename `createOrGetStorageLocation` to `getOrCreateStorageLocation`,
   // `getStableStorageLocation`, or something more appropriate.
 
@@ -377,6 +384,9 @@
   // `DACtx` is not null and not owned by this object.
   DataflowAnalysisContext *DACtx;
 
+  // `DeclContext` of the block being analysed if provided.
+  const DeclContext *DeclCtx;
+
   // In a properly initialized `Environment`, `ReturnLoc` should only be null if
   // its `DeclContext` could not be cast to a `FunctionDecl`.
   Stor

[PATCH] D128440: [WebAssembly] Initial support for reference type funcref in clang

2022-08-05 Thread Paulo Matos via Phabricator via cfe-commits
pmatos added a comment.

It would be great to get this landed. Can someone pls take a look at it? It's 
quite independent from the remainder of the work and self-contained.

maybe @tlively ping?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128440

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


[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

2022-08-05 Thread Paulo Matos via Phabricator via cfe-commits
pmatos added a comment.

It would be great to get this landed. Can someone pls take a look at it? It's 
quite independent from the remainder of the work and self-contained.

maybe @tlively ping?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122215

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


[PATCH] D130951: [HLSL] CodeGen hlsl resource binding.

2022-08-05 Thread Chris Bieneman via Phabricator via cfe-commits
beanz added inline comments.



Comment at: clang/lib/CodeGen/CGHLSLRuntime.h:64
   CodeGenModule &CGM;
   uint32_t ResourceCounters[static_cast(
   hlsl::ResourceClass::NumClasses)] = {0};

Anastasia wrote:
> This is not part of this change but any reason why it is a protected member 
> and not private?
I wrote that code, and had a vague idea of a use case in my head where we might 
provide subclasses of the HLSLRuntime, but in retrospect that is unlikely 
anytime soon, so we could safely make that `private.`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130951

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


[PATCH] D130894: [clang] Print more information about failed static assertions

2022-08-05 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added a comment.

I don't know much about fixit hints, would they be appropriate to display it 
below the line itself, e.g.

  test.cpp:24:17: note: expression evaluates to
  static_assert(c != c);
  '0' != '0'
~~^~~~

Or would it be even better to just inline the evaluated expression into the 
`static_assert` ala

  test.cpp:24:17: note: expression evaluates to
  static_assert('0' != '0');
  ~~^~~~


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

https://reviews.llvm.org/D130894

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


[PATCH] D131258: [Sema] Merge variable template specializations

2022-08-05 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov created this revision.
ilya-biryukov added a reviewer: ChuanqiXu.
Herald added a project: All.
ilya-biryukov requested review of this revision.
Herald added a project: clang.

Clang used to produce redefinition errors, see tests for examples.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131258

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Modules/merge-var-template-spec-cxx-modules.cpp
  clang/test/Modules/merge-var-template-spec.cpp

Index: clang/test/Modules/merge-var-template-spec.cpp
===
--- /dev/null
+++ clang/test/Modules/merge-var-template-spec.cpp
@@ -0,0 +1,67 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -xc++ -std=c++17 -fmodules -fmodule-name=library \
+// RUN: -emit-module %t/modules.map \
+// RUN: -o %t/module.pcm
+//
+//
+// RUN: %clang_cc1 -xc++ -std=c++17 -fmodules -fmodule-file=%t/module.pcm  \
+// RUN: -fmodule-map-file=%t/modules.map \
+// RUN: -fsyntax-only -verify %t/use.cpp
+//
+//--- use.cpp
+
+#include "var1.h"
+#include "var2.h"
+
+auto foo = zero;
+auto bar = zero;
+auto baz = zero;
+
+template  constexpr T zero = 0; // expected-error {{redefinition}} expected-note@* {{previous}}
+template <> constexpr Int zero = {0}; // expected-error {{redefinition}} expected-note@* {{previous}}
+template  constexpr T* zero = nullptr; // expected-error {{redefinition}} expected-note@* {{previous}}
+
+template <> constexpr int** zero = nullptr; // ok, new specialization.
+template  constexpr T** zero = nullptr; // ok, new partial specilization.
+
+//--- modules.map
+module "library" {
+	export *
+	module "var1" {
+		export *
+		header "var1.h"
+	}
+	module "var2" {
+		export *
+		header "var2.h"
+	}
+}
+
+//--- var1.h
+#ifndef VAR1_H
+#define VAR1_H
+
+template  constexpr T zero = 0;
+struct Int {
+int value;
+};
+template <> constexpr int zero = {0};
+template  constexpr T* zero = nullptr;
+
+#endif // VAR1_H
+
+//--- var2.h
+#ifndef VAR2_H
+#define VAR2_H
+
+template  constexpr T zero = 0;
+struct Int {
+int value;
+};
+template <> constexpr int zero = {0};
+template  constexpr T* zero = nullptr;
+
+#endif // VAR2_H
\ No newline at end of file
Index: clang/test/Modules/merge-var-template-spec-cxx-modules.cpp
===
--- /dev/null
+++ clang/test/Modules/merge-var-template-spec-cxx-modules.cpp
@@ -0,0 +1,44 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/var_def.cppm -o %t/var_def.pcm
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -fprebuilt-module-path=%t %t/reexport1.cppm -o %t/reexport1.pcm
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -fprebuilt-module-path=%t %t/reexport2.cppm -o %t/reexport2.pcm
+// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t %t/use.cppm -fsyntax-only -verify
+
+//--- use.cppm
+import reexport1;
+import reexport2;
+
+auto foo = zero;
+auto bar = zero;
+auto baz = zero;
+
+template  constexpr T zero = 0; // expected-error-re {{declaration{{.*}}in the global module follows declaration in module var_def}}
+ // expected-note@* {{previous}}
+template <> constexpr Int zero = {0}; // expected-error-re {{declaration{{.*}}in the global module follows declaration in module var_def}}
+   // expected-note@* {{previous}}
+template  constexpr T* zero = nullptr; // expected-error-re {{declaration{{.*}}in the global module follows declaration in module var_def}}
+// expected-note@* {{previous}}
+
+template <> constexpr int** zero = nullptr; // ok, new specialization.
+template  constexpr T** zero = nullptr; // ok, new partial specilization.
+
+//--- var_def.cppm
+export module var_def;
+
+export template  constexpr T zero = 0;
+export struct Int {
+int value;
+};
+export template <> constexpr Int zero = {0};
+export template  constexpr T* zero = nullptr;
+
+//--- reexport1.cppm
+export module reexport1;
+export import var_def;
+
+//--- reexport2.cppm
+export module reexport2;
+export import var_def;
\ No newline at end of file
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -4665,6 +4665,7 @@
   if (!hasVisibleDefinition(Old) &&
   (New->getFormalLinkage() == InternalLinkage ||
New->isInline() ||
+   isa(New) ||
New->getDescribedVarTemplate() ||
New->getNumTemplateParameterLists() ||
New->getDeclContext()->isDependentContext())) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D131255: Fix Wbitfield-constant-conversion on 1-bit signed bitfield

2022-08-05 Thread Shawn Zhong via Phabricator via cfe-commits
ShawnZhong added a comment.

Thanks for the quick reply and the reference on the C standard!

On the C++ side, Section C.1.8 specified that `int` bit-fields are signed:

> Change: Bit-fields of type plain int are signed.
> Rationale: Leaving the choice of signedness to implementations could lead to 
> inconsistent definitions of template specializations. For consistency, the 
> implementation freedom was eliminated for non-dependent types, too.
> Effect on original feature: The choise is implementation-defined in C, but 
> not so in C++.

Implementation-wise, I'll see what I can find in CodeGen on whether `int` 
bit-fields are signed for C


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131255

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


[PATCH] D131255: Fix Wbitfield-constant-conversion on 1-bit signed bitfield

2022-08-05 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D131255#3702286 , @ShawnZhong 
wrote:

> Thanks for the quick reply and the reference on the C standard!
>
> On the C++ side, Section C.1.8 specified that `int` bit-fields are signed:
>
>> Change: Bit-fields of type plain int are signed.
>> Rationale: Leaving the choice of signedness to implementations could lead to 
>> inconsistent definitions of template specializations. For consistency, the 
>> implementation freedom was eliminated for non-dependent types, too.
>> Effect on original feature: The choise is implementation-defined in C, but 
>> not so in C++.

Agreed; I think that's falling out from 
https://eel.is/c++draft/class.bit#4.sentence-1 and 
https://eel.is/c++draft/basic.fundamental#1.sentence-1.

> Implementation-wise, I'll see what I can find in CodeGen on whether `int` 
> bit-fields are signed for C

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131255

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


[PATCH] D130894: [clang] Print more information about failed static assertions

2022-08-05 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D130894#3702233 , @tbaeder wrote:

> I don't know much about fixit hints, would they be appropriate to display it 
> below the line itself, e.g.
>
>   test.cpp:24:17: note: expression evaluates to
>   static_assert(c != c);
>   '0' != '0'
> ~~^~~~

Nope, that would be a bad thing -- Clang has the ability to automatically apply 
fix-it hints to the source, so this would modify the static assertion to 
continue to be false which isn't much of a fix.

> Or would it be even better to just inline the evaluated expression into the 
> `static_assert` ala
>
>   test.cpp:24:17: note: expression evaluates to
>   static_assert('0' != '0');
>   ~~^~~~

Making sure I'm following along, you mean that we'd display the error for the 
static assertion, and then the note would show `static_assert()`?

I think this might work for very simple static asserts, but what about more 
complicated ones like `static_assert(foo() && (bar() == 12 || baz()) && 'a' != 
quux(4000));`?


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

https://reviews.llvm.org/D130894

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


[PATCH] D130894: [clang] Print more information about failed static assertions

2022-08-05 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment.

Any tests with macros?


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

https://reviews.llvm.org/D130894

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


[PATCH] D129231: [Builtins] Do not claim all libfuncs are readnone with trapping math.

2022-08-05 Thread Florian Hahn via Phabricator via cfe-commits
fhahn added a comment.

ping :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129231

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


[PATCH] D130394: [clang][CodeGen] Factor out Swift ABI hooks (NFCI)

2022-08-05 Thread Sergei Barannikov via Phabricator via cfe-commits
barannikov88 updated this revision to Diff 450298.
barannikov88 added a comment.

Assert that Swift ABI info is initialized before accessing it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130394

Files:
  clang/lib/CodeGen/ABIInfo.h
  clang/lib/CodeGen/SwiftCallingConv.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/CodeGen/TargetInfo.h

Index: clang/lib/CodeGen/TargetInfo.h
===
--- clang/lib/CodeGen/TargetInfo.h
+++ clang/lib/CodeGen/TargetInfo.h
@@ -38,6 +38,7 @@
 class CallArgList;
 class CodeGenFunction;
 class CGBlockInfo;
+class SwiftABIInfo;
 
 /// TargetCodeGenInfo - This class organizes various target-specific
 /// codegeneration issues, like target-specific attributes, builtins and so
@@ -45,6 +46,12 @@
 class TargetCodeGenInfo {
   std::unique_ptr Info;
 
+protected:
+  // Target hooks supporting Swift calling conventions. The target must
+  // initialize this field if it claims to support these calling conventions
+  // by returning true from TargetInfo::checkCallingConvention for them.
+  std::unique_ptr SwiftInfo;
+
 public:
   TargetCodeGenInfo(std::unique_ptr Info);
   virtual ~TargetCodeGenInfo();
@@ -52,6 +59,12 @@
   /// getABIInfo() - Returns ABI info helper for the target.
   const ABIInfo &getABIInfo() const { return *Info; }
 
+  /// Returns Swift ABI info helper for the target.
+  const SwiftABIInfo &getSwiftABIInfo() const {
+assert(SwiftInfo && "Swift ABI info has not been initialized");
+return *SwiftInfo;
+  }
+
   /// setTargetAttributes - Provides a convenient hook to handle extra
   /// target-specific attributes for the given global.
   virtual void setTargetAttributes(const Decl *D, llvm::GlobalValue *GV,
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -23,7 +23,6 @@
 #include "clang/Basic/CodeGenOptions.h"
 #include "clang/Basic/DiagnosticFrontend.h"
 #include "clang/CodeGen/CGFunctionInfo.h"
-#include "clang/CodeGen/SwiftCallingConv.h"
 #include "llvm/ADT/SmallBitVector.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSwitch.h"
@@ -117,7 +116,9 @@
   return false;
 }
 
-ABIInfo::~ABIInfo() {}
+ABIInfo::~ABIInfo() = default;
+
+SwiftABIInfo::~SwiftABIInfo() = default;
 
 /// Does the given lowering require more than the given number of
 /// registers when expanded?
@@ -151,12 +152,16 @@
   return (intCount + fpCount > maxAllRegisters);
 }
 
-bool SwiftABIInfo::isLegalVectorTypeForSwift(CharUnits vectorSize,
- llvm::Type *eltTy,
- unsigned numElts) const {
+bool SwiftABIInfo::shouldPassIndirectly(ArrayRef ComponentTys,
+bool AsReturnValue) const {
+  return occupiesMoreThan(CGT, ComponentTys, /*total=*/4);
+}
+
+bool SwiftABIInfo::isLegalVectorType(CharUnits VectorSize, llvm::Type *EltTy,
+ unsigned NumElts) const {
   // The default implementation of this assumes that the target guarantees
   // 128-bit SIMD support but nothing more.
-  return (vectorSize.getQuantity() > 8 && vectorSize.getQuantity() <= 16);
+  return (VectorSize.getQuantity() > 8 && VectorSize.getQuantity() <= 16);
 }
 
 static CGCXXABI::RecordArgABI getRecordArgABI(const RecordType *RT,
@@ -814,7 +819,7 @@
 // This is a very simple ABI that relies a lot on DefaultABIInfo.
 //===--===//
 
-class WebAssemblyABIInfo final : public SwiftABIInfo {
+class WebAssemblyABIInfo final : public ABIInfo {
 public:
   enum ABIKind {
 MVP = 0,
@@ -827,7 +832,7 @@
 
 public:
   explicit WebAssemblyABIInfo(CodeGen::CodeGenTypes &CGT, ABIKind Kind)
-  : SwiftABIInfo(CGT), defaultInfo(CGT), Kind(Kind) {}
+  : ABIInfo(CGT), defaultInfo(CGT), Kind(Kind) {}
 
 private:
   ABIArgInfo classifyReturnType(QualType RetTy) const;
@@ -845,22 +850,16 @@
 
   Address EmitVAArg(CodeGenFunction &CGF, Address VAListAddr,
 QualType Ty) const override;
-
-  bool shouldPassIndirectlyForSwift(ArrayRef scalars,
-bool asReturnValue) const override {
-return occupiesMoreThan(CGT, scalars, /*total*/ 4);
-  }
-
-  bool isSwiftErrorInRegister() const override {
-return false;
-  }
 };
 
 class WebAssemblyTargetCodeGenInfo final : public TargetCodeGenInfo {
 public:
   explicit WebAssemblyTargetCodeGenInfo(CodeGen::CodeGenTypes &CGT,
 WebAssemblyABIInfo::ABIKind K)
-  : TargetCodeGenInfo(std::make_unique(CGT, K)) {}
+  : TargetCodeGenInfo(std::make_unique(CGT, K)) {
+SwiftInfo =
+std::make_unique(CGT, /*SwiftErrorInRegister=*/false);
+  }
 
   void setTargetAttributes(const 

[PATCH] D130394: [clang][CodeGen] Factor out Swift ABI hooks (NFCI)

2022-08-05 Thread Sergei Barannikov via Phabricator via cfe-commits
barannikov88 updated this revision to Diff 450294.
barannikov88 added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130394

Files:
  clang/lib/CodeGen/ABIInfo.h
  clang/lib/CodeGen/SwiftCallingConv.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/CodeGen/TargetInfo.h

Index: clang/lib/CodeGen/TargetInfo.h
===
--- clang/lib/CodeGen/TargetInfo.h
+++ clang/lib/CodeGen/TargetInfo.h
@@ -38,6 +38,7 @@
 class CallArgList;
 class CodeGenFunction;
 class CGBlockInfo;
+class SwiftABIInfo;
 
 /// TargetCodeGenInfo - This class organizes various target-specific
 /// codegeneration issues, like target-specific attributes, builtins and so
@@ -45,6 +46,12 @@
 class TargetCodeGenInfo {
   std::unique_ptr Info;
 
+protected:
+  // Target hooks supporting Swift calling conventions. The target must
+  // initialize this field if it claims to support these calling conventions
+  // by returning true from TargetInfo::checkCallingConvention for them.
+  std::unique_ptr SwiftInfo;
+
 public:
   TargetCodeGenInfo(std::unique_ptr Info);
   virtual ~TargetCodeGenInfo();
@@ -52,6 +59,9 @@
   /// getABIInfo() - Returns ABI info helper for the target.
   const ABIInfo &getABIInfo() const { return *Info; }
 
+  /// Returns Swift ABI info helper for the target.
+  const SwiftABIInfo &getSwiftABIInfo() const { return *SwiftInfo; }
+
   /// setTargetAttributes - Provides a convenient hook to handle extra
   /// target-specific attributes for the given global.
   virtual void setTargetAttributes(const Decl *D, llvm::GlobalValue *GV,
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -23,7 +23,6 @@
 #include "clang/Basic/CodeGenOptions.h"
 #include "clang/Basic/DiagnosticFrontend.h"
 #include "clang/CodeGen/CGFunctionInfo.h"
-#include "clang/CodeGen/SwiftCallingConv.h"
 #include "llvm/ADT/SmallBitVector.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSwitch.h"
@@ -117,7 +116,9 @@
   return false;
 }
 
-ABIInfo::~ABIInfo() {}
+ABIInfo::~ABIInfo() = default;
+
+SwiftABIInfo::~SwiftABIInfo() = default;
 
 /// Does the given lowering require more than the given number of
 /// registers when expanded?
@@ -151,12 +152,16 @@
   return (intCount + fpCount > maxAllRegisters);
 }
 
-bool SwiftABIInfo::isLegalVectorTypeForSwift(CharUnits vectorSize,
- llvm::Type *eltTy,
- unsigned numElts) const {
+bool SwiftABIInfo::shouldPassIndirectly(ArrayRef ComponentTys,
+bool AsReturnValue) const {
+  return occupiesMoreThan(CGT, ComponentTys, /*total=*/4);
+}
+
+bool SwiftABIInfo::isLegalVectorType(CharUnits VectorSize, llvm::Type *EltTy,
+ unsigned NumElts) const {
   // The default implementation of this assumes that the target guarantees
   // 128-bit SIMD support but nothing more.
-  return (vectorSize.getQuantity() > 8 && vectorSize.getQuantity() <= 16);
+  return (VectorSize.getQuantity() > 8 && VectorSize.getQuantity() <= 16);
 }
 
 static CGCXXABI::RecordArgABI getRecordArgABI(const RecordType *RT,
@@ -814,7 +819,7 @@
 // This is a very simple ABI that relies a lot on DefaultABIInfo.
 //===--===//
 
-class WebAssemblyABIInfo final : public SwiftABIInfo {
+class WebAssemblyABIInfo final : public ABIInfo {
 public:
   enum ABIKind {
 MVP = 0,
@@ -827,7 +832,7 @@
 
 public:
   explicit WebAssemblyABIInfo(CodeGen::CodeGenTypes &CGT, ABIKind Kind)
-  : SwiftABIInfo(CGT), defaultInfo(CGT), Kind(Kind) {}
+  : ABIInfo(CGT), defaultInfo(CGT), Kind(Kind) {}
 
 private:
   ABIArgInfo classifyReturnType(QualType RetTy) const;
@@ -845,22 +850,16 @@
 
   Address EmitVAArg(CodeGenFunction &CGF, Address VAListAddr,
 QualType Ty) const override;
-
-  bool shouldPassIndirectlyForSwift(ArrayRef scalars,
-bool asReturnValue) const override {
-return occupiesMoreThan(CGT, scalars, /*total*/ 4);
-  }
-
-  bool isSwiftErrorInRegister() const override {
-return false;
-  }
 };
 
 class WebAssemblyTargetCodeGenInfo final : public TargetCodeGenInfo {
 public:
   explicit WebAssemblyTargetCodeGenInfo(CodeGen::CodeGenTypes &CGT,
 WebAssemblyABIInfo::ABIKind K)
-  : TargetCodeGenInfo(std::make_unique(CGT, K)) {}
+  : TargetCodeGenInfo(std::make_unique(CGT, K)) {
+SwiftInfo =
+std::make_unique(CGT, /*SwiftErrorInRegister=*/false);
+  }
 
   void setTargetAttributes(const Decl *D, llvm::GlobalValue *GV,
CodeGen::CodeGenModule &CGM) const override {
@@ -1136,7 +1135,7 @@
 };
 

[PATCH] D131265: Fixed sm version for .and bmma operator.

2022-08-05 Thread Jack Kirk via Phabricator via cfe-commits
JackAKirk created this revision.
JackAKirk added a reviewer: tra.
Herald added subscribers: mattd, gchakrabarti, asavonic.
Herald added a project: All.
JackAKirk requested review of this revision.
Herald added subscribers: cfe-commits, jholewinski.
Herald added a project: clang.

As stated here 
(https://docs.nvidia.com/cuda/parallel-thread-execution/index.html#warp-level-matrix-instructions-wmma-mma):

".and operation in single-bit wmma requires sm_80 or higher."


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131265

Files:
  clang/include/clang/Basic/BuiltinsNVPTX.def


Index: clang/include/clang/Basic/BuiltinsNVPTX.def
===
--- clang/include/clang/Basic/BuiltinsNVPTX.def
+++ clang/include/clang/Basic/BuiltinsNVPTX.def
@@ -853,7 +853,7 @@
 TARGET_BUILTIN(__bmma_m8n8k128_ld_a_b1, "vi*iC*UiIi", "", AND(SM_75,PTX63))
 TARGET_BUILTIN(__bmma_m8n8k128_ld_b_b1, "vi*iC*UiIi", "", AND(SM_75,PTX63))
 TARGET_BUILTIN(__bmma_m8n8k128_ld_c, "vi*iC*UiIi", "", AND(SM_75,PTX63))
-TARGET_BUILTIN(__bmma_m8n8k128_mma_and_popc_b1, "vi*iC*iC*iC*Ii", "", 
AND(SM_75,PTX71))
+TARGET_BUILTIN(__bmma_m8n8k128_mma_and_popc_b1, "vi*iC*iC*iC*Ii", "", 
AND(SM_80,PTX71))
 TARGET_BUILTIN(__bmma_m8n8k128_mma_xor_popc_b1, "vi*iC*iC*iC*Ii", "", 
AND(SM_75,PTX63))
 TARGET_BUILTIN(__bmma_m8n8k128_st_c_i32, "vi*iC*UiIi", "", AND(SM_75,PTX63))
 TARGET_BUILTIN(__imma_m16n16k16_ld_a_s8, "vi*iC*UiIi", "", AND(SM_72,PTX63))


Index: clang/include/clang/Basic/BuiltinsNVPTX.def
===
--- clang/include/clang/Basic/BuiltinsNVPTX.def
+++ clang/include/clang/Basic/BuiltinsNVPTX.def
@@ -853,7 +853,7 @@
 TARGET_BUILTIN(__bmma_m8n8k128_ld_a_b1, "vi*iC*UiIi", "", AND(SM_75,PTX63))
 TARGET_BUILTIN(__bmma_m8n8k128_ld_b_b1, "vi*iC*UiIi", "", AND(SM_75,PTX63))
 TARGET_BUILTIN(__bmma_m8n8k128_ld_c, "vi*iC*UiIi", "", AND(SM_75,PTX63))
-TARGET_BUILTIN(__bmma_m8n8k128_mma_and_popc_b1, "vi*iC*iC*iC*Ii", "", AND(SM_75,PTX71))
+TARGET_BUILTIN(__bmma_m8n8k128_mma_and_popc_b1, "vi*iC*iC*iC*Ii", "", AND(SM_80,PTX71))
 TARGET_BUILTIN(__bmma_m8n8k128_mma_xor_popc_b1, "vi*iC*iC*iC*Ii", "", AND(SM_75,PTX63))
 TARGET_BUILTIN(__bmma_m8n8k128_st_c_i32, "vi*iC*UiIi", "", AND(SM_75,PTX63))
 TARGET_BUILTIN(__imma_m16n16k16_ld_a_s8, "vi*iC*UiIi", "", AND(SM_72,PTX63))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D130894: [clang] Print more information about failed static assertions

2022-08-05 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment.

Thank you so much for working on this! It's been on my todo list for a while 
and just haven't gotten around to it.

In D130894#3702166 , @aaron.ballman 
wrote:

> In D130894#3701749 , @tbaeder wrote:
>
>> For the record, the output now looks something like this:
>>
>>   test.cpp:24:1: error: static assertion failed due to requirement 'c != c'
>>   static_assert(c != c);
>>   ^ ~~
>>   test.cpp:24:17: note: expression evaluates to '('0' != '0')'
>>   static_assert(c != c);
>> ~~^~~~
>
> This looks hard to read to me...
>
>> test.cpp:25:1: error: static assertion failed due to requirement 'c > 'a''
>> static_assert(c > 'a');
>> ^ ~~~
>> test.cpp:25:17: note: expression evaluates to '('0' > 'a')'
>
> Same confusion here -- it took me a while to realize what's happening is that 
> we're quoting the part outside of the parens and that's throwing me for a 
> loop. We typically quote things that are syntax but in this case, the parens 
> are not part of the syntax and so the surrounding quotes are catching me. 
> Given that parens are being used as a visual delimiter and a single quote 
> serves the same purpose, I would expect something more like:
>
>   test.cpp:25:17: note: expression evaluates to ''0' > 'a''
>   test.cpp:26:17: note: expression evaluates to '0 > 'a''
>   test.cpp:27:17: note: expression evaluates to '14 > 5'
>   test.cpp:28:17: note: expression evaluates to 'true == false'
>   test.cpp:29:17: note: expression evaluates to 'nullptr != nullptr'
>
> Adding a few more folks as reviewers to see if there is a consensus position 
> on how to proceed.

Agreed on confusion, but `''0' > 'a''` is painful to read. Sadly, short of 
changing quotes to backticks (something that'd be good for non-terminal-based 
consumers of SARIF), I'm not sure I can suggest anything better in one line. If 
we can span multiple lines, which I think would improve readability:

  test.cpp:25:17: note: expression evaluates with:
LHS: '0'
RHS: 'a'
  test.cpp:26:17: note: expression evaluates with:
LHS: 0
RHS: 'a'
  test.cpp:27:17: note: expression evaluates with:
LHS: 14
RHS: 5
  test.cpp:28:17: note: expression evaluates with:
LHS: true
RHS: false
  test.cpp:29:17: note: expression evaluates with:
LHS: nullptr
RHS: nullptr

My assertion library follows what Catch2 and simple-test do, and uses `Actual` 
and `Expected`, but I'm not sure hardcoding those terms into Clang is a good 
idea.


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

https://reviews.llvm.org/D130894

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


[PATCH] D131143: [Clang] Interaction of FP pragmas and function template instantiation

2022-08-05 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

I completely agree with Eli.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131143

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


[PATCH] D131143: [Clang] Interaction of FP pragmas and function template instantiation

2022-08-05 Thread Serge Pavlov via Phabricator via cfe-commits
sepavloff added a comment.

In D131143#3700373 , @efriedma wrote:

>> In the case is explicit instantiation there is apparent connection
>> between a point in source code and instantiated function. It can
>> support interaction of pragmas that act in that point. For example, in
>> the code:
>>
>>   #pragma STDC FENV_ROUND FE_DOWNWARD
>>   template float func_05(float, float);
>>
>> the instantiated function is created with the specified constant
>> rounding mode. The effect is same as if the template pattern were
>> textually inserted into the code with needed replacements.
>
> These pragmas aren't currently part of the C++ standard, but I strongly doubt 
> the C++ standards committee would choose to do this.

This is an attempt to implement the interaction as proposed in the review of 
D129464 . It is, of course, an extension, 
which may be convenient but nothing serious would happen if it is not 
implemented.

> You would end up with undefined behavior due to mismatched definitions, if 
> the template is implicitly instantiated elsewhere.

Indeed, it makes the solution fragile.

> The "obvious" rule here is just to always use the pragma in effect at the 
> point the template body is written.

Sure, it is already works in this way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131143

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


[PATCH] D130907: [Clang] Use pragma FENV_ROUND to reset rounding mode settings.

2022-08-05 Thread Serge Pavlov via Phabricator via cfe-commits
sepavloff added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:398
+// If dynamic rounding mode is specified in absence of FENV_ACCESS, treat 
it
+// as setting default rounding mode.
+FpPragmaStack.CurrentValue.clearConstRoundingModeOverride();

efriedma wrote:
> I'm not really happy with the way this is written... like I mentioned in the 
> other patch, keep the state transitions simple, and put the logic into the 
> code that computes the derived properties.
This is not about any floating-point property, it is only how to distinguish 
between the cases of user-defined properties (which must be preserved) and 
default ones (which may be overridden). So far the only case, where this 
difference is important, is the interaction of FP pragmas and template 
instantiations, implemented in D131143. If that change is not accepted, this 
one loses its value.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130907

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


[PATCH] D131268: [HLSL] Generate buffer subscript operators

2022-08-05 Thread Chris Bieneman via Phabricator via cfe-commits
beanz created this revision.
beanz added reviewers: bogner, aaron.ballman, python3kgae, pow2clk.
Herald added a subscriber: Anastasia.
Herald added a project: All.
beanz requested review of this revision.
Herald added a project: clang.

In HLSL buffer types support array subscripting syntax for loads and
stores. This change fleshes out the subscript operators to become array
accesses on the underlying handle pointer. This will allow LLVM
optimization passes to optimize resource accesses the same way any other
memory access would be optimized.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131268

Files:
  clang/lib/Sema/HLSLExternalSemaSource.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/AST/HLSL/RWBuffer-AST.hlsl
  clang/test/CodeGenHLSL/buffer-array-operator.hlsl

Index: clang/test/CodeGenHLSL/buffer-array-operator.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/buffer-array-operator.hlsl
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -emit-llvm -disable-llvm-passes -o - %s | FileCheck %s
+
+const RWBuffer In;
+RWBuffer Out;
+
+void fn(int Idx) {
+  Out[Idx] = In[Idx];
+}
+
+// This test is intended to verify reasonable code generation of the subscript
+// operator. In this test case we should be generating both the const and
+// non-const operators so we verify both cases.
+
+// Non-const comes first.
+// CHECK: float @"??A?$RWBuffer@M@hlsl@@QBAMI@Z"
+// CHECK: %this1 = load ptr, ptr %this.addr, align 4
+// CHECK-NEXT: %h = getelementptr inbounds %"class.hlsl::RWBuffer", ptr %this1, i32 0, i32 0
+// CHECK-NEXT: %0 = load ptr, ptr %h, align 4
+// CHECK-NEXT: %1 = load i32, ptr %Idx.addr, align 4
+// CHECK-NEXT: %arrayidx = getelementptr inbounds float, ptr %0, i32 %1
+// CHECK-NEXT: %2 = load float, ptr %arrayidx, align 4
+// CHECK-NEXT: ret float %2
+
+// Const comes next, and returns the pointer instead of the value.
+// CHECK: ptr @"??A?$RWBuffer@M@hlsl@@QMI@Z"
+// CHECK: %this1 = load ptr, ptr %this.addr, align 4
+// CHECK-NEXT: %h = getelementptr inbounds %"class.hlsl::RWBuffer", ptr %this1, i32 0, i32 0
+// CHECK-NEXT: %0 = load ptr, ptr %h, align 4
+// CHECK-NEXT: %1 = load i32, ptr %Idx.addr, align 4
+// CHECK-NEXT: %arrayidx = getelementptr inbounds float, ptr %0, i32 %1
+// CHECK-NEXT: ret ptr %arrayidx
\ No newline at end of file
Index: clang/test/AST/HLSL/RWBuffer-AST.hlsl
===
--- clang/test/AST/HLSL/RWBuffer-AST.hlsl
+++ clang/test/AST/HLSL/RWBuffer-AST.hlsl
@@ -39,11 +39,30 @@
 
 // CHECK: FinalAttr 0x{{[0-9A-Fa-f]+}} <> Implicit final
 // CHECK-NEXT: HLSLResourceAttr 0x{{[0-9A-Fa-f]+}} <> Implicit UAV
-// CHECK-NEXT: FieldDecl 0x{{[0-9A-Fa-f]+}} <>  implicit h 'void *'
+// CHECK-NEXT: FieldDecl 0x{{[0-9A-Fa-f]+}} <>  implicit h 'element_type *'
+
+// CHECK: CXXMethodDecl 0x{{[0-9A-Fa-f]+}} <>  operator[] 'element_type (unsigned int) const'
+// CHECK-NEXT: ParmVarDecl 0x{{[0-9A-Fa-f]+}} <>  Idx 'unsigned int'
+// CHECK-NEXT: CompoundStmt 0x{{[0-9A-Fa-f]+}} <>
+// CHECK-NEXT: ReturnStmt 0x{{[0-9A-Fa-f]+}} <>
+// CHECK-NEXT: ArraySubscriptExpr 0x{{[0-9A-Fa-f]+}} <> 'element_type' lvalue
+// CHECK-NEXT: MemberExpr 0x{{[0-9A-Fa-f]+}} <> 'element_type *' lvalue ->h 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT: CXXThisExpr 0x{{[0-9A-Fa-f]+}} <> 'const RWBuffer *' implicit this
+// CHECK-NEXT: DeclRefExpr 0x{{[0-9A-Fa-f]+}} <> 'unsigned int' ParmVar 0x{{[0-9A-Fa-f]+}} 'Idx' 'unsigned int'
+
+// CHECK-NEXT: CXXMethodDecl 0x{{[0-9A-Fa-f]+}} <>  operator[] 'element_type &(unsigned int)'
+// CHECK-NEXT: ParmVarDecl 0x{{[0-9A-Fa-f]+}} <>  Idx 'unsigned int'
+// CHECK-NEXT: CompoundStmt 0x{{[0-9A-Fa-f]+}} <>
+// CHECK-NEXT: ReturnStmt 0x{{[0-9A-Fa-f]+}} <>
+// CHECK-NEXT: ArraySubscriptExpr 0x{{[0-9A-Fa-f]+}} <> 'element_type' lvalue
+// CHECK-NEXT: MemberExpr 0x{{[0-9A-Fa-f]+}} <> 'element_type *' lvalue ->h 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT: CXXThisExpr 0x{{[0-9A-Fa-f]+}} <> 'RWBuffer *' implicit this
+// CHECK-NEXT: DeclRefExpr 0x{{[0-9A-Fa-f]+}} <> 'unsigned int' ParmVar 0x{{[0-9A-Fa-f]+}} 'Idx' 'unsigned int'
+
 // CHECK: ClassTemplateSpecializationDecl 0x{{[0-9A-Fa-f]+}} <>  class RWBuffer definition
 
 // CHECK: TemplateArgument type 'float'
 // CHECK-NEXT: BuiltinType 0x{{[0-9A-Fa-f]+}} 'float'
 // CHECK-NEXT: FinalAttr 0x{{[0-9A-Fa-f]+}} <> Implicit final
 // CHECK-NEXT: HLSLResourceAttr 0x{{[0-9A-Fa-f]+}} <> Implicit UAV
-// CHECK-NEXT: FieldDecl 0x{{[0-9A-Fa-f]+}} <>   implicit referenced h 'void *'
+// CHECK-NEXT: FieldDecl 0x{{[0-9A-Fa-f]+}} <>   implicit referenced h 'float *'
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2158,7 +2158,7 @@
 return QualType();
   }
 
-  if (getLangOpts().HLSL) {
+  if (getLangOpts().HLSL && Loc.isValid()) {
 Diag(Loc, diag::err_hlsl_pointers_unsuppo

[PATCH] D129231: [Builtins] Do not claim all libfuncs are readnone with trapping math.

2022-08-05 Thread John Brawn via Phabricator via cfe-commits
john.brawn accepted this revision.
john.brawn 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/D129231/new/

https://reviews.llvm.org/D129231

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


[PATCH] D131265: Fixed sm version for .and bmma operator.

2022-08-05 Thread Artem Belevich via Phabricator via cfe-commits
tra accepted this revision.
tra added a comment.
This revision is now accepted and ready to land.

Good catch. LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131265

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


[PATCH] D131076: [clang][modules] Don't depend on sharing FileManager during module build

2022-08-05 Thread Ben Langmuir via Phabricator via cfe-commits
benlangmuir updated this revision to Diff 450313.
benlangmuir added a comment.
Herald added a subscriber: ormris.

Attempt to fix test failure seen on Windows. It revealed two bugs

- Avoid reusing the FileManager at the top-level in clang-scan-deps to make the 
test behave as expected
- Make the FileManager return the cached redirecting entry, to match how it 
behaves when it's a new file, and test this case explicitly.

I think it was mostly luck of exactly which file lookups happen that we didn't 
hit this on other platforms.


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

https://reviews.llvm.org/D131076

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Basic/FileManager.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/test/ClangScanDeps/modulemap-via-vfs.m
  clang/test/Modules/submodule-in-private-mmap-vfs.m
  clang/test/VFS/module-import.m
  clang/unittests/Basic/FileManagerTest.cpp

Index: clang/unittests/Basic/FileManagerTest.cpp
===
--- clang/unittests/Basic/FileManagerTest.cpp
+++ clang/unittests/Basic/FileManagerTest.cpp
@@ -340,6 +340,7 @@
   auto F1Again = manager.getFileRef("dir/f1.cpp");
   auto F1Also = manager.getFileRef("dir/f1-also.cpp");
   auto F1Redirect = manager.getFileRef("dir/f1-redirect.cpp");
+  auto F1RedirectAgain = manager.getFileRef("dir/f1-redirect.cpp");
   auto F2 = manager.getFileRef("dir/f2.cpp");
 
   // Check Expected for error.
@@ -347,6 +348,7 @@
   ASSERT_FALSE(!F1Also);
   ASSERT_FALSE(!F1Again);
   ASSERT_FALSE(!F1Redirect);
+  ASSERT_FALSE(!F1RedirectAgain);
   ASSERT_FALSE(!F2);
 
   // Check names.
@@ -354,6 +356,7 @@
   EXPECT_EQ("dir/f1.cpp", F1Again->getName());
   EXPECT_EQ("dir/f1-also.cpp", F1Also->getName());
   EXPECT_EQ("dir/f1.cpp", F1Redirect->getName());
+  EXPECT_EQ("dir/f1.cpp", F1RedirectAgain->getName());
   EXPECT_EQ("dir/f2.cpp", F2->getName());
 
   EXPECT_EQ("dir/f1.cpp", F1->getNameAsRequested());
@@ -363,6 +366,7 @@
   EXPECT_EQ(&F1->getFileEntry(), *F1);
   EXPECT_EQ(*F1, &F1->getFileEntry());
   EXPECT_EQ(&F1->getFileEntry(), &F1Redirect->getFileEntry());
+  EXPECT_EQ(&F1->getFileEntry(), &F1RedirectAgain->getFileEntry());
   EXPECT_NE(&F2->getFileEntry(), *F1);
   EXPECT_NE(*F1, &F2->getFileEntry());
 
@@ -371,6 +375,7 @@
   EXPECT_EQ(*F1, *F1Again);
   EXPECT_EQ(*F1, *F1Redirect);
   EXPECT_EQ(*F1Also, *F1Redirect);
+  EXPECT_EQ(*F1, *F1RedirectAgain);
   EXPECT_NE(*F2, *F1);
   EXPECT_NE(*F2, *F1Also);
   EXPECT_NE(*F2, *F1Again);
@@ -381,6 +386,7 @@
   EXPECT_FALSE(F1->isSameRef(*F1Redirect));
   EXPECT_FALSE(F1->isSameRef(*F1Also));
   EXPECT_FALSE(F1->isSameRef(*F2));
+  EXPECT_TRUE(F1Redirect->isSameRef(*F1RedirectAgain));
 }
 
 // getFile() Should return the same entry as getVirtualFile if the file actually
Index: clang/test/VFS/module-import.m
===
--- clang/test/VFS/module-import.m
+++ clang/test/VFS/module-import.m
@@ -1,6 +1,7 @@
-// RUN: rm -rf %t
+// RUN: rm -rf %t %t-unshared
 // RUN: sed -e "s@INPUT_DIR@%{/S:regex_replacement}/Inputs@g" -e "s@OUT_DIR@%{/t:regex_replacement}@g" %S/Inputs/vfsoverlay.yaml > %t.yaml
 // RUN: %clang_cc1 -Werror -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -ivfsoverlay %t.yaml -I %t -fsyntax-only %s
+// RUN: %clang_cc1 -Werror -fmodules -fno-modules-share-filemanager -fimplicit-module-maps -fmodules-cache-path=%t-unshared -ivfsoverlay %t.yaml -I %t -fsyntax-only %s
 
 @import not_real;
 
@@ -18,9 +19,12 @@
 // Override the module map (vfsoverlay2 on top)
 // RUN: sed -e "s@INPUT_DIR@%{/S:regex_replacement}/Inputs@g" -e "s@OUT_DIR@%{/t:regex_replacement}@g" %S/Inputs/vfsoverlay2.yaml > %t2.yaml
 // RUN: %clang_cc1 -Werror -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -ivfsoverlay %t.yaml -ivfsoverlay %t2.yaml -I %t -fsyntax-only %s
+// RUN: %clang_cc1 -Werror -fmodules -fno-modules-share-filemanager -fimplicit-module-maps -fmodules-cache-path=%t-unshared -ivfsoverlay %t.yaml -ivfsoverlay %t2.yaml -I %t -fsyntax-only %s
 
 // vfsoverlay2 not present
 // RUN: not %clang_cc1 -Werror -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -ivfsoverlay %t.yaml -I %t -fsyntax-only %s -DIMPORT2 2>&1 | FileCheck -check-prefix=CHECK-VFS2 %s
+// RUN: not %clang_cc1 -Werror -fmodules -fno-modules-share-filemanager -fimplicit-module-maps -fmodules-cache-path=%t-unshared -ivfsoverlay %t.yaml -I %t -fsyntax-only %s -DIMPORT2 2>&1 | FileCheck -check-prefix=CHECK-VFS2 %s
 
 // vfsoverlay2 on the bottom
 // RUN: not %clang_cc1 -Werror -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -ivfsoverlay %t2.yaml -ivfsoverlay %t.yaml -I %t -fsyntax-only %s -DIMPORT2 2>&1 | FileCheck -check-prefix=CHECK-VFS2 %s
+// RUN: not %clang_cc1 -Werror -fmodules -fno-modules-share-filemanager -fimplicit-module-maps -fmodules-cache-path=%t-unshared -ivfsoverlay %t2.yaml -ivfsoverlay %t.yaml -

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-08-05 Thread Amy Kwan via Phabricator via cfe-commits
amyk added a comment.

Hi, 
I have come across a failure involving `flang/test/Driver/linker-flags.f90` 
that occurs when the default linker is lld. I have opened an external issue for 
it here: https://github.com/llvm/llvm-project/issues/56955


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126291

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


[PATCH] D131272: [Clang][OpenMP] Fix the issue that `llvm.lifetime.end` is emitted too early for variables captured in linear clause

2022-08-05 Thread Shilei Tian via Phabricator via cfe-commits
tianshilei1992 created this revision.
tianshilei1992 added reviewers: jdoerfert, ABataev.
Herald added subscribers: pengfei, guansong, yaxunl.
Herald added a project: All.
tianshilei1992 requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

Currently if an OpenMP program uses `linear` clause, and is compiled with
optimization, `llvm.lifetime.end` for variables listed in `linear` clause are
emitted too early such that there could still be uses after that. Let's take the
following code as example:

  c
  // loop.c
  int j;
  int *u;
  
  void loop(int n) {
int i;
for (i = 0; i < n; ++i) {
  ++j;
  u = &j;
}
  }

We compile using the command:

  shell
  clang -cc1 -fopenmp-simd -O3 -x c -triple x86_64-apple-darwin10 -emit-llvm 
loop.c -o loop.ll

The following IR (simplified) will be generated:

  llvm
  @j = local_unnamed_addr global i32 0, align 4
  @u = local_unnamed_addr global ptr null, align 8
  
  define void @loop(i32 noundef %n) local_unnamed_addr {
  entry:
%j = alloca i32, align 4
%cmp = icmp sgt i32 %n, 0
br i1 %cmp, label %simd.if.then, label %simd.if.end
  
  simd.if.then: ; preds = %entry
call void @llvm.lifetime.start.p0(i64 4, ptr nonnull %j)
store ptr %j, ptr @u, align 8
call void @llvm.lifetime.end.p0(i64 4, ptr nonnull %j)
%0 = load i32, ptr %j, align 4
store i32 %0, ptr @j, align 4
br label %simd.if.end
  
  simd.if.end:  ; preds = %simd.if.then, 
%entry
ret void
  }

The most important part is:

  llvm
call void @llvm.lifetime.end.p0(i64 4, ptr nonnull %j)
%0 = load i32, ptr %j, align 4
store i32 %0, ptr @j, align 4

`%j` is still loaded after `@llvm.lifetime.end.p0(i64 4, ptr nonnull %j)`. This
could cause the backend incorrectly optimizes the code and further generates
incorrect code. The root cause is, when we emit a construct that could have
`linear` clause, it usually has the following pattern:

  cpp
  EmitOMPLinearClauseInit(S)
  {
OMPPrivateScope LoopScope(*this);
...
EmitOMPLinearClause(S, LoopScope);
...
(void)LoopScope.Privatize();
...
  }
  EmitOMPLinearClauseFinal(S, [](CodeGenFunction &) { return nullptr; });

Variables that need to be privatized are added into `LoopScope`, which also
serves as a RAII object. When `LoopScope` is destructed and if optimization is
enabled, a `@llvm.lifetime.end` is also emitted for each privatized variable.
However, the writing back to original variables in `linear` clause happens after
the scope in `EmitOMPLinearClauseFinal`, causing the issue we see above.

A quick "fix" seems to be, moving `EmitOMPLinearClauseFinal` inside the scope.
However, it doesn't work. That's because the local variable map has been updated
by `LoopScope` such that a variable declaration is mapped to the privatized
variable, instead of the actual one. In that way, the following code will be
generated:

  llvm
%0 = load i32, ptr %j, align 4
store i32 %0, ptr %j, align 4
call void @llvm.lifetime.end.p0(i64 4, ptr nonnull %j)

Well, now the life time is correct, but apparently the writing back is broken.

In this patch, a new function `OMPPrivateScope::restoreMap` is added and called
before calling `EmitOMPLinearClauseFinal`. This can make sure that
`EmitOMPLinearClauseFinal` can find the orignal varaibls to write back.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131272

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/OpenMP/bug56913.c
  clang/test/OpenMP/for_linear_codegen.cpp
  clang/test/OpenMP/parallel_master_taskloop_simd_lastprivate_codegen.cpp

Index: clang/test/OpenMP/parallel_master_taskloop_simd_lastprivate_codegen.cpp
===
--- clang/test/OpenMP/parallel_master_taskloop_simd_lastprivate_codegen.cpp
+++ clang/test/OpenMP/parallel_master_taskloop_simd_lastprivate_codegen.cpp
@@ -1847,8 +1847,12 @@
 // CHECK6-NEXT:[[TMP38:%.*]] = icmp ne i32 [[TMP37]], 0
 // CHECK6-NEXT:br i1 [[TMP38]], label [[DOTOMP_LINEAR_PU_I:%.*]], label [[DOTOMP_OUTLINED__1_EXIT:%.*]]
 // CHECK6:   .omp.linear.pu.i:
-// CHECK6-NEXT:[[TMP39:%.*]] = load i32, i32* [[J_I]], align 4, !noalias !14
-// CHECK6-NEXT:store i32 [[TMP39]], i32* [[J_I]], align 4, !noalias !14
+// CHECK6-NEXT:[[TMP39:%.*]] = getelementptr inbounds [[STRUCT_ANON]], %struct.anon* [[TMP20]], i32 0, i32 0
+// CHECK6-NEXT:[[TMP40:%.*]] = load i32*, i32** [[TMP39]], align 8
+// CHECK6-NEXT:[[TMP41:%.*]] = getelementptr inbounds [[STRUCT_ANON]], %struct.anon* [[TMP20]], i32 0, i32 1
+// CHECK6-NEXT:[[TMP42:%.*]] = load i32*, i32** [[TMP41]], align 8
+// CHECK6-NEXT:[[TMP43:%.*]] = load i32, i32* [[J_I]], align 4, !noalias !14
+// CHECK6-NEXT:store i32 [[TMP43]], i32* [[TMP42]], align 4
 // CHECK6-NEXT:br label [

[PATCH] D131213: [clang][Headers] Fix unintentional error in D130800

2022-08-05 Thread Dominic Chen via Phabricator via cfe-commits
ddcc added a comment.

I missed line 19, yeah that makes sense. @iana is that ok with you?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131213

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


[PATCH] D131213: [clang][Headers] Fix unintentional error in D130800

2022-08-05 Thread Ian Anderson via Phabricator via cfe-commits
iana added a comment.

In D131213#3702699 , @ddcc wrote:

> I missed line 19, yeah that makes sense. @iana is that ok with you?

Ah, I didn't see that either. Can we just lose the `defined` on line 26 then? 
It's redundant and little confusing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131213

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


[PATCH] D131213: [clang][Headers] Fix unintentional error in D130800

2022-08-05 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D131213#3702707 , @iana wrote:

> In D131213#3702699 , @ddcc wrote:
>
>> I missed line 19, yeah that makes sense. @iana is that ok with you?
>
> Ah, I didn't see that either. Can we just lose the `defined` on line 26 then? 
> It's redundant and little confusing.

I'm fine dropping it (ever so slightly less work for the preprocessor to do in 
this file), but I don't see what the confusion is with checking whether 
`__cplusplus` is defined before testing its value, so I'm also fine with 
leaving it as-is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131213

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


[PATCH] D131272: [Clang][OpenMP] Fix the issue that `llvm.lifetime.end` is emitted too early for variables captured in linear clause

2022-08-05 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments.



Comment at: clang/lib/CodeGen/CodeGenFunction.h:1070
 void operator=(const OMPPrivateScope &) = delete;
+bool MapRestored = false;
 

Better to make it a property of MappedVars. Also, are there any issues if just 
restore it several times without checking?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131272

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


[PATCH] D131273: [clang] Fix redirection behaviour for cached FileEntryRef

2022-08-05 Thread Ben Langmuir via Phabricator via cfe-commits
benlangmuir created this revision.
benlangmuir added a reviewer: bnbarham.
Herald added a project: All.
benlangmuir requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

In 6a79e2ff1989b 
 we 
changed Filemanager::getEntryRef() to return the
redirecting FileEntryRef instead of looking through the redirection.
This commit fixes the case when looking up a cached file path to also
return the redirecting FileEntryRef. This mainly affects the behaviour
of calling getNameAsRequested() on the resulting entry ref.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131273

Files:
  clang/lib/Basic/FileManager.cpp
  clang/unittests/Basic/FileManagerTest.cpp


Index: clang/unittests/Basic/FileManagerTest.cpp
===
--- clang/unittests/Basic/FileManagerTest.cpp
+++ clang/unittests/Basic/FileManagerTest.cpp
@@ -340,6 +340,7 @@
   auto F1Again = manager.getFileRef("dir/f1.cpp");
   auto F1Also = manager.getFileRef("dir/f1-also.cpp");
   auto F1Redirect = manager.getFileRef("dir/f1-redirect.cpp");
+  auto F1RedirectAgain = manager.getFileRef("dir/f1-redirect.cpp");
   auto F2 = manager.getFileRef("dir/f2.cpp");
 
   // Check Expected for error.
@@ -347,6 +348,7 @@
   ASSERT_FALSE(!F1Also);
   ASSERT_FALSE(!F1Again);
   ASSERT_FALSE(!F1Redirect);
+  ASSERT_FALSE(!F1RedirectAgain);
   ASSERT_FALSE(!F2);
 
   // Check names.
@@ -354,6 +356,7 @@
   EXPECT_EQ("dir/f1.cpp", F1Again->getName());
   EXPECT_EQ("dir/f1-also.cpp", F1Also->getName());
   EXPECT_EQ("dir/f1.cpp", F1Redirect->getName());
+  EXPECT_EQ("dir/f1.cpp", F1RedirectAgain->getName());
   EXPECT_EQ("dir/f2.cpp", F2->getName());
 
   EXPECT_EQ("dir/f1.cpp", F1->getNameAsRequested());
@@ -363,6 +366,7 @@
   EXPECT_EQ(&F1->getFileEntry(), *F1);
   EXPECT_EQ(*F1, &F1->getFileEntry());
   EXPECT_EQ(&F1->getFileEntry(), &F1Redirect->getFileEntry());
+  EXPECT_EQ(&F1->getFileEntry(), &F1RedirectAgain->getFileEntry());
   EXPECT_NE(&F2->getFileEntry(), *F1);
   EXPECT_NE(*F1, &F2->getFileEntry());
 
@@ -371,6 +375,7 @@
   EXPECT_EQ(*F1, *F1Again);
   EXPECT_EQ(*F1, *F1Redirect);
   EXPECT_EQ(*F1Also, *F1Redirect);
+  EXPECT_EQ(*F1, *F1RedirectAgain);
   EXPECT_NE(*F2, *F1);
   EXPECT_NE(*F2, *F1Also);
   EXPECT_NE(*F2, *F1Again);
@@ -381,6 +386,7 @@
   EXPECT_FALSE(F1->isSameRef(*F1Redirect));
   EXPECT_FALSE(F1->isSameRef(*F1Also));
   EXPECT_FALSE(F1->isSameRef(*F2));
+  EXPECT_TRUE(F1Redirect->isSameRef(*F1RedirectAgain));
 }
 
 // getFile() Should return the same entry as getVirtualFile if the file 
actually
Index: clang/lib/Basic/FileManager.cpp
===
--- clang/lib/Basic/FileManager.cpp
+++ clang/lib/Basic/FileManager.cpp
@@ -212,13 +212,7 @@
 if (!SeenFileInsertResult.first->second)
   return llvm::errorCodeToError(
   SeenFileInsertResult.first->second.getError());
-// Construct and return and FileEntryRef, unless it's a redirect to another
-// filename.
-FileEntryRef::MapValue Value = *SeenFileInsertResult.first->second;
-if (LLVM_LIKELY(Value.V.is()))
-  return FileEntryRef(*SeenFileInsertResult.first);
-return FileEntryRef(*reinterpret_cast(
-Value.V.get()));
+return FileEntryRef(*SeenFileInsertResult.first);
   }
 
   // We've not seen this before. Fill it in.


Index: clang/unittests/Basic/FileManagerTest.cpp
===
--- clang/unittests/Basic/FileManagerTest.cpp
+++ clang/unittests/Basic/FileManagerTest.cpp
@@ -340,6 +340,7 @@
   auto F1Again = manager.getFileRef("dir/f1.cpp");
   auto F1Also = manager.getFileRef("dir/f1-also.cpp");
   auto F1Redirect = manager.getFileRef("dir/f1-redirect.cpp");
+  auto F1RedirectAgain = manager.getFileRef("dir/f1-redirect.cpp");
   auto F2 = manager.getFileRef("dir/f2.cpp");
 
   // Check Expected for error.
@@ -347,6 +348,7 @@
   ASSERT_FALSE(!F1Also);
   ASSERT_FALSE(!F1Again);
   ASSERT_FALSE(!F1Redirect);
+  ASSERT_FALSE(!F1RedirectAgain);
   ASSERT_FALSE(!F2);
 
   // Check names.
@@ -354,6 +356,7 @@
   EXPECT_EQ("dir/f1.cpp", F1Again->getName());
   EXPECT_EQ("dir/f1-also.cpp", F1Also->getName());
   EXPECT_EQ("dir/f1.cpp", F1Redirect->getName());
+  EXPECT_EQ("dir/f1.cpp", F1RedirectAgain->getName());
   EXPECT_EQ("dir/f2.cpp", F2->getName());
 
   EXPECT_EQ("dir/f1.cpp", F1->getNameAsRequested());
@@ -363,6 +366,7 @@
   EXPECT_EQ(&F1->getFileEntry(), *F1);
   EXPECT_EQ(*F1, &F1->getFileEntry());
   EXPECT_EQ(&F1->getFileEntry(), &F1Redirect->getFileEntry());
+  EXPECT_EQ(&F1->getFileEntry(), &F1RedirectAgain->getFileEntry());
   EXPECT_NE(&F2->getFileEntry(), *F1);
   EXPECT_NE(*F1, &F2->getFileEntry());
 
@@ -371,6 +375,7 @@
   EXPECT_EQ(*F1, *F1Again);
   EXPECT_EQ(*F1, *F1Redirect);
   EXPECT_EQ(*F1Also, *F1Redirect);
+  EXPECT_EQ(*F1, *F1Redirect

[PATCH] D131265: Fixed sm version for .and bmma operator.

2022-08-05 Thread Jack Kirk via Phabricator via cfe-commits
JackAKirk added a comment.

In D131265#3702594 , @tra wrote:

> Good catch. LGTM.

Thanks. If you could land it for me that would be much appreciated. I don't 
have the rights.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131265

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


[PATCH] D131274: [clang][Darwin] Re-apply "Always set the default C++ Standard Library to libc++"

2022-08-05 Thread Louis Dionne via Phabricator via cfe-commits
ldionne created this revision.
ldionne added a reviewer: leonardchan.
Herald added a project: All.
ldionne requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

Newer SDKs don't even provide libstdc++ headers, so it's effectively
never valid to build for libstdc++ unless the user explicitly asks
for it (in which case they will need to provide include paths and more).

This is a re-application of c5ccb78ade81 
 which had 
been reverted in
33171df9cc7f 
 because 
it broke the Fuchsia CI bots. The issue was that
the test was XPASSing because it didn't fail anymore when the
CLANG_DEFAULT_CXX_LIB was set to libc++, which seems to be done for
Fuchsia. Instead, the test only fails if CLANG_DEFAULT_CXX_LIB is
set to libstdc++.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131274

Files:
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/darwin-iphone-defaults.m
  clang/test/Driver/darwin-stdlib.cpp
  clang/test/lit.cfg.py


Index: clang/test/lit.cfg.py
===
--- clang/test/lit.cfg.py
+++ clang/test/lit.cfg.py
@@ -134,7 +134,7 @@
 # Set available features we allow tests to conditionalize on.
 #
 if config.clang_default_cxx_stdlib != '':
-config.available_features.add('default-cxx-stdlib-set')
+
config.available_features.add('default-cxx-stdlib={}'.format(config.clang_default_cxx_stdlib))
 
 # As of 2011.08, crash-recovery tests still do not pass on FreeBSD.
 if platform.system() not in ['FreeBSD']:
Index: clang/test/Driver/darwin-stdlib.cpp
===
--- clang/test/Driver/darwin-stdlib.cpp
+++ clang/test/Driver/darwin-stdlib.cpp
@@ -1,14 +1,10 @@
-// This test will fail if CLANG_DEFAULT_CXX_STDLIB is set to anything different
-// than the platform default. (see https://llvm.org/bugs/show_bug.cgi?id=30548)
-// XFAIL: default-cxx-stdlib-set
+// This test will fail if CLANG_DEFAULT_CXX_STDLIB is set to libstdc++.
+// XFAIL: default-cxx-stdlib=libstdc++
 
-// RUN: %clang -target x86_64-apple-darwin -ccc-install-dir 
%S/Inputs/darwin_toolchain_tree/bin/ -arch arm64 -miphoneos-version-min=7.0 %s 
-### 2>&1 | FileCheck %s --check-prefix=CHECK-LIBCXX
-// RUN: %clang -target x86_64-apple-darwin -ccc-install-dir 
%S/Inputs/darwin_toolchain_tree/bin/ -mmacosx-version-min=10.8 
-Wno-stdlibcxx-not-found %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-LIBSTDCXX
-// RUN: %clang -target x86_64-apple-darwin -ccc-install-dir 
%S/Inputs/darwin_toolchain_tree/bin/ -mmacosx-version-min=10.9 %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-LIBCXX
-// RUN: %clang -target x86_64-apple-darwin -ccc-install-dir 
%S/Inputs/darwin_toolchain_tree/bin/ -arch armv7s -miphoneos-version-min=6.1 
-Wno-stdlibcxx-not-found %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-LIBSTDCXX
-// RUN: %clang -target x86_64-apple-darwin -ccc-install-dir 
%S/Inputs/darwin_toolchain_tree/bin/ -arch armv7s -miphoneos-version-min=7.0 %s 
-### 2>&1 | FileCheck %s --check-prefix=CHECK-LIBCXX
-// RUN: %clang -target x86_64-apple-darwin -ccc-install-dir 
%S/Inputs/darwin_toolchain_tree/bin/ -arch armv7k %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-LIBCXX
+// RUN: %clang -target x86_64-apple-darwin -ccc-install-dir 
%S/Inputs/darwin_toolchain_tree/bin/ -arch arm64 -miphoneos-version-min=7.0 %s 
-### 2>&1 | FileCheck %s
+// RUN: %clang -target x86_64-apple-darwin -ccc-install-dir 
%S/Inputs/darwin_toolchain_tree/bin/ -mmacosx-version-min=10.9 %s -### 2>&1 | 
FileCheck %s
+// RUN: %clang -target x86_64-apple-darwin -ccc-install-dir 
%S/Inputs/darwin_toolchain_tree/bin/ -arch armv7s -miphoneos-version-min=7.0 %s 
-### 2>&1 | FileCheck %s
+// RUN: %clang -target x86_64-apple-darwin -ccc-install-dir 
%S/Inputs/darwin_toolchain_tree/bin/ -arch armv7k %s -### 2>&1 | FileCheck %s
 
-// CHECK-LIBCXX: "-stdlib=libc++"
-// CHECK-LIBSTDCXX-NOT: -stdlib=libc++
-// CHECK-LIBSTDCXX-NOT: -stdlib=libstdc++
+// CHECK: "-stdlib=libc++"
+// CHECK-NOT: "-stdlib=libstdc++"
Index: clang/test/Driver/darwin-iphone-defaults.m
===
--- clang/test/Driver/darwin-iphone-defaults.m
+++ clang/test/Driver/darwin-iphone-defaults.m
@@ -1,4 +1,4 @@
-// RUN: %clang -target i386-apple-darwin9 -miphoneos-version-min=3.0 -arch 
armv7 -stdlib=platform -flto -S -o - %s | FileCheck %s
+// RUN: %clang -target i386-apple-darwin -miphoneos-version-min=5.0 -arch 
armv7 -stdlib=platform -flto -S -o - %s | FileCheck %s
 
 // CHECK: @f0() [[F0:#[0-9]+]]
 // CHECK: @__f0_block_invoke
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -896,12 +896,7 @@

[PATCH] D131274: [clang][Darwin] Re-apply "Always set the default C++ Standard Library to libc++"

2022-08-05 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment.

@leonardchan Does this fix the Fuchsia issues you were seeing? I think I 
understand what went wrong and I think this version of the patch should address 
the issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131274

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


[PATCH] D130516: [llvm] compression classes

2022-08-05 Thread Cole Kissane via Phabricator via cfe-commits
ckissane added inline comments.



Comment at: llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp:54-60
+  OptionalCompressionScheme = compression::noneIfUnsupported(
+  (Compress && DoInstrProfNameCompression) ? OptionalCompressionScheme
+   : llvm::NoneType());
+
+  bool doCompression = bool(OptionalCompressionScheme);
+
+  if (doCompression) {

dblaikie wrote:
> This still seems like a lot of hoops to jump through - why 
> "noneIfUnsupported" rather than either having the compression scheme (I think 
> it could be the CompressionAlgorithm itself, rather than having the separate 
> OptionalCompressionKind abstraction) either be null itself, or expose an 
> "isAvailable" operation directly on the CompressionAlgorithm?
> 
> Even if the CompressionKind/OptionalCompressionKind/CompressionAlgorithm 
> abstractions are kept, I'm not sure why the above code is preferred over, say:
> 
> ```
> if (Compress && DoInstrProfNameCompression && OptionalCompressionScheme /* 
> .isAvailable(), if we want to be more explicit */) {
>   ...
> }
> ```
> 
> What's the benefit that `noneIfUnsupported` is providing? (& generally I'd 
> expect the `Compress && DoInstrProfNameCompression` to be tested/exit early 
> before even naming/constructing/querying/doing anything with the compression 
> scheme/algorithm/etc - so there'd be no need to combine the tests for 
> availability and the tests for whether compression was requested)
> 
> Perhaps this API is motivated by a desire to implement something much closer 
> to the original code than is necessary/suitable? Or some other use 
> case/benefit I'm not quite understanding yet?
I shall remove `noneIfUnsupported`. You express good points, we can simply 
check `if(OptionalCompressionScheme && *OptionalCompressionScheme)` where 
necessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130516

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


[PATCH] D130516: [llvm] compression classes

2022-08-05 Thread Cole Kissane via Phabricator via cfe-commits
ckissane added inline comments.



Comment at: llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp:54-60
+  OptionalCompressionScheme = compression::noneIfUnsupported(
+  (Compress && DoInstrProfNameCompression) ? OptionalCompressionScheme
+   : llvm::NoneType());
+
+  bool doCompression = bool(OptionalCompressionScheme);
+
+  if (doCompression) {

ckissane wrote:
> dblaikie wrote:
> > This still seems like a lot of hoops to jump through - why 
> > "noneIfUnsupported" rather than either having the compression scheme (I 
> > think it could be the CompressionAlgorithm itself, rather than having the 
> > separate OptionalCompressionKind abstraction) either be null itself, or 
> > expose an "isAvailable" operation directly on the CompressionAlgorithm?
> > 
> > Even if the CompressionKind/OptionalCompressionKind/CompressionAlgorithm 
> > abstractions are kept, I'm not sure why the above code is preferred over, 
> > say:
> > 
> > ```
> > if (Compress && DoInstrProfNameCompression && OptionalCompressionScheme /* 
> > .isAvailable(), if we want to be more explicit */) {
> >   ...
> > }
> > ```
> > 
> > What's the benefit that `noneIfUnsupported` is providing? (& generally I'd 
> > expect the `Compress && DoInstrProfNameCompression` to be tested/exit early 
> > before even naming/constructing/querying/doing anything with the 
> > compression scheme/algorithm/etc - so there'd be no need to combine the 
> > tests for availability and the tests for whether compression was requested)
> > 
> > Perhaps this API is motivated by a desire to implement something much 
> > closer to the original code than is necessary/suitable? Or some other use 
> > case/benefit I'm not quite understanding yet?
> I shall remove `noneIfUnsupported`. You express good points, we can simply 
> check `if(OptionalCompressionScheme && *OptionalCompressionScheme)` where 
> necessary.
though it will make a lot of existing code patterns less clear, and more verbose


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130516

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


[PATCH] D130516: [llvm] compression classes

2022-08-05 Thread Cole Kissane via Phabricator via cfe-commits
ckissane added inline comments.



Comment at: llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp:54-60
+  OptionalCompressionScheme = compression::noneIfUnsupported(
+  (Compress && DoInstrProfNameCompression) ? OptionalCompressionScheme
+   : llvm::NoneType());
+
+  bool doCompression = bool(OptionalCompressionScheme);
+
+  if (doCompression) {

ckissane wrote:
> ckissane wrote:
> > dblaikie wrote:
> > > This still seems like a lot of hoops to jump through - why 
> > > "noneIfUnsupported" rather than either having the compression scheme (I 
> > > think it could be the CompressionAlgorithm itself, rather than having the 
> > > separate OptionalCompressionKind abstraction) either be null itself, or 
> > > expose an "isAvailable" operation directly on the CompressionAlgorithm?
> > > 
> > > Even if the CompressionKind/OptionalCompressionKind/CompressionAlgorithm 
> > > abstractions are kept, I'm not sure why the above code is preferred over, 
> > > say:
> > > 
> > > ```
> > > if (Compress && DoInstrProfNameCompression && OptionalCompressionScheme 
> > > /* .isAvailable(), if we want to be more explicit */) {
> > >   ...
> > > }
> > > ```
> > > 
> > > What's the benefit that `noneIfUnsupported` is providing? (& generally 
> > > I'd expect the `Compress && DoInstrProfNameCompression` to be tested/exit 
> > > early before even naming/constructing/querying/doing anything with the 
> > > compression scheme/algorithm/etc - so there'd be no need to combine the 
> > > tests for availability and the tests for whether compression was 
> > > requested)
> > > 
> > > Perhaps this API is motivated by a desire to implement something much 
> > > closer to the original code than is necessary/suitable? Or some other use 
> > > case/benefit I'm not quite understanding yet?
> > I shall remove `noneIfUnsupported`. You express good points, we can simply 
> > check `if(OptionalCompressionScheme && *OptionalCompressionScheme)` where 
> > necessary.
> though it will make a lot of existing code patterns less clear, and more 
> verbose
and sometimes you really do need to re code the exact thing `noneIfUnsupported` 
encapsulates...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130516

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


[PATCH] D130516: [llvm] compression classes

2022-08-05 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added inline comments.



Comment at: llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp:54-60
+  OptionalCompressionScheme = compression::noneIfUnsupported(
+  (Compress && DoInstrProfNameCompression) ? OptionalCompressionScheme
+   : llvm::NoneType());
+
+  bool doCompression = bool(OptionalCompressionScheme);
+
+  if (doCompression) {

ckissane wrote:
> ckissane wrote:
> > ckissane wrote:
> > > dblaikie wrote:
> > > > This still seems like a lot of hoops to jump through - why 
> > > > "noneIfUnsupported" rather than either having the compression scheme (I 
> > > > think it could be the CompressionAlgorithm itself, rather than having 
> > > > the separate OptionalCompressionKind abstraction) either be null 
> > > > itself, or expose an "isAvailable" operation directly on the 
> > > > CompressionAlgorithm?
> > > > 
> > > > Even if the 
> > > > CompressionKind/OptionalCompressionKind/CompressionAlgorithm 
> > > > abstractions are kept, I'm not sure why the above code is preferred 
> > > > over, say:
> > > > 
> > > > ```
> > > > if (Compress && DoInstrProfNameCompression && OptionalCompressionScheme 
> > > > /* .isAvailable(), if we want to be more explicit */) {
> > > >   ...
> > > > }
> > > > ```
> > > > 
> > > > What's the benefit that `noneIfUnsupported` is providing? (& generally 
> > > > I'd expect the `Compress && DoInstrProfNameCompression` to be 
> > > > tested/exit early before even naming/constructing/querying/doing 
> > > > anything with the compression scheme/algorithm/etc - so there'd be no 
> > > > need to combine the tests for availability and the tests for whether 
> > > > compression was requested)
> > > > 
> > > > Perhaps this API is motivated by a desire to implement something much 
> > > > closer to the original code than is necessary/suitable? Or some other 
> > > > use case/benefit I'm not quite understanding yet?
> > > I shall remove `noneIfUnsupported`. You express good points, we can 
> > > simply check `if(OptionalCompressionScheme && 
> > > *OptionalCompressionScheme)` where necessary.
> > though it will make a lot of existing code patterns less clear, and more 
> > verbose
> and sometimes you really do need to re code the exact thing 
> `noneIfUnsupported` encapsulates...
Are there examples within LLVM that you can show compare/contrast 
`noneIfUnsupported` helps?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130516

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


[PATCH] D128441: [CUDA] Do not embed a fatbinary when using the new driver

2022-08-05 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

This change  breaks `clang++ --cuda-device-only` compilation. Clang does not 
create any output in this case. Reverting the change fixes the problem.

Reproducible with:

  echo '__global__ void k(){}' | bin/clang++  --offload-arch=sm_70 -x cuda -  
--cuda-device-only -v  -c -o foo123.o

Compilation succeeds, but there's no `foo123.o` to be found.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128441

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


[PATCH] D131272: [Clang][OpenMP] Fix the issue that `llvm.lifetime.end` is emitted too early for variables captured in linear clause

2022-08-05 Thread Shilei Tian via Phabricator via cfe-commits
tianshilei1992 added inline comments.



Comment at: clang/lib/CodeGen/CodeGenFunction.h:1070
 void operator=(const OMPPrivateScope &) = delete;
+bool MapRestored = false;
 

ABataev wrote:
> Better to make it a property of MappedVars. Also, are there any issues if 
> just restore it several times without checking?
> Better to make it a property of MappedVars.
Will do.
> Also, are there any issues if just restore it several times without checking?
TBH, I don't know clearly, but I feel it should not be a problem. We will not 
have two scopes at the same time, will we? But I will add an assertion that if 
the mapped variables have been restored, `addPrivate` should not be called 
anymore.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131272

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


[PATCH] D128441: [CUDA] Do not embed a fatbinary when using the new driver

2022-08-05 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 added a comment.

In D128441#3702800 , @tra wrote:

> This change  breaks `clang++ --cuda-device-only` compilation. Clang does not 
> create any output in this case. Reverting the change fixes the problem.
>
> Reproducible with:
>
>   echo '__global__ void k(){}' | bin/clang++  --offload-arch=sm_70 -x cuda -  
> --cuda-device-only -v  -c -o foo123.o
>
> Compilation succeeds, but there's no `foo123.o` to be found.

Is it spitting it out as `foo123.cubin` instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128441

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


[PATCH] D131268: [HLSL] Generate buffer subscript operators

2022-08-05 Thread Xiang Li via Phabricator via cfe-commits
python3kgae added inline comments.



Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:220
+
+// Const subscript operators return copies of elements, non-const return a
+// reference so that they are assignable.

If we reuse this function for StructuredBuffer, then const subscript return a 
const reference could be better?



Comment at: clang/test/CodeGenHLSL/buffer-array-operator.hlsl:4
+const RWBuffer In;
+RWBuffer Out;
+

Maybe change In to RWBuffer
And Out[Idx] = In[Idx].z;
So we also test vector case?



Comment at: clang/test/CodeGenHLSL/buffer-array-operator.hlsl:16
+// CHECK: float @"??A?$RWBuffer@M@hlsl@@QBAMI@Z"
+// CHECK: %this1 = load ptr, ptr %this.addr, align 4
+// CHECK-NEXT: %h = getelementptr inbounds %"class.hlsl::RWBuffer", ptr 
%this1, i32 0, i32 0

Where is the this.addr coming from?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131268

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


[PATCH] D103562: [NFC][compiler-rt][hwasan] Refactor hwasan functions

2022-08-05 Thread Vitaly Buka via Phabricator via cfe-commits
vitalybuka added a subscriber: smd.
vitalybuka added inline comments.
Herald added a subscriber: Enna1.
Herald added a project: All.



Comment at: compiler-rt/lib/hwasan/hwasan.cpp:205
+  if (registers_frame && stack->trace && stack->size > 0) {
+stack->trace++;
+stack->size--;

maybe we should pop everything up to "pc" to avoid issues with nested calls?

For most users hwasan frames are not very useful. However if you work on 
sanitizer, some frames can be a useful info. So I don't mind we just relax test 
cases to accommodate this nesting.

cc @smd 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103562

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


[PATCH] D130516: [llvm] compression classes

2022-08-05 Thread Cole Kissane via Phabricator via cfe-commits
ckissane updated this revision to Diff 450338.
ckissane added a comment.

- cleanup some compression nits


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130516

Files:
  clang-tools-extra/clangd/index/Serialization.cpp
  clang-tools-extra/clangd/unittests/SerializationTests.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  lld/ELF/Driver.cpp
  lld/ELF/InputSection.cpp
  llvm/include/llvm/Object/Decompressor.h
  llvm/include/llvm/ProfileData/InstrProf.h
  llvm/include/llvm/Support/Compression.h
  llvm/lib/MC/ELFObjectWriter.cpp
  llvm/lib/ObjCopy/ELF/ELFObject.cpp
  llvm/lib/Object/Decompressor.cpp
  llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
  llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp
  llvm/lib/ProfileData/InstrProf.cpp
  llvm/lib/ProfileData/InstrProfCorrelator.cpp
  llvm/lib/ProfileData/SampleProfReader.cpp
  llvm/lib/ProfileData/SampleProfWriter.cpp
  llvm/lib/Support/Compression.cpp
  llvm/tools/llvm-mc/llvm-mc.cpp
  llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
  llvm/unittests/ProfileData/InstrProfTest.cpp
  llvm/unittests/Support/CompressionTest.cpp

Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -22,31 +22,41 @@
 
 namespace {
 
-#if LLVM_ENABLE_ZLIB
-static void testZlibCompression(StringRef Input, int Level) {
+static void testCompressionAlgorithm(
+StringRef Input, int Level, CompressionKind CompressionScheme,
+std::string ExpectedDestinationBufferTooSmallErrorMessage) {
   SmallVector Compressed;
   SmallVector Uncompressed;
-  zlib::compress(arrayRefFromStringRef(Input), Compressed, Level);
+  CompressionScheme->compress(arrayRefFromStringRef(Input), Compressed, Level);
 
   // Check that uncompressed buffer is the same as original.
-  Error E = zlib::uncompress(Compressed, Uncompressed, Input.size());
+  Error E =
+  CompressionScheme->decompress(Compressed, Uncompressed, Input.size());
   consumeError(std::move(E));
 
   EXPECT_EQ(Input, toStringRef(Uncompressed));
   if (Input.size() > 0) {
 // Uncompression fails if expected length is too short.
-E = zlib::uncompress(Compressed, Uncompressed, Input.size() - 1);
-EXPECT_EQ("zlib error: Z_BUF_ERROR", llvm::toString(std::move(E)));
+E = CompressionScheme->decompress(Compressed, Uncompressed,
+  Input.size() - 1);
+EXPECT_EQ(ExpectedDestinationBufferTooSmallErrorMessage,
+  llvm::toString(std::move(E)));
   }
 }
 
+#if LLVM_ENABLE_ZLIB
+static void testZlibCompression(StringRef Input, int Level) {
+  testCompressionAlgorithm(Input, Level, CompressionKind::Zlib,
+   "zlib error: Z_BUF_ERROR");
+}
+
 TEST(CompressionTest, Zlib) {
-  testZlibCompression("", zlib::DefaultCompression);
+  CompressionKind CompressionScheme = CompressionKind::Zlib;
+  testZlibCompression("", CompressionScheme->DefaultLevel);
 
-  testZlibCompression("hello, world!", zlib::NoCompression);
-  testZlibCompression("hello, world!", zlib::BestSizeCompression);
-  testZlibCompression("hello, world!", zlib::BestSpeedCompression);
-  testZlibCompression("hello, world!", zlib::DefaultCompression);
+  testZlibCompression("hello, world!", CompressionScheme->BestSizeLevel);
+  testZlibCompression("hello, world!", CompressionScheme->BestSpeedLevel);
+  testZlibCompression("hello, world!", CompressionScheme->DefaultLevel);
 
   const size_t kSize = 1024;
   char BinaryData[kSize];
@@ -54,38 +64,26 @@
 BinaryData[i] = i & 255;
   StringRef BinaryDataStr(BinaryData, kSize);
 
-  testZlibCompression(BinaryDataStr, zlib::NoCompression);
-  testZlibCompression(BinaryDataStr, zlib::BestSizeCompression);
-  testZlibCompression(BinaryDataStr, zlib::BestSpeedCompression);
-  testZlibCompression(BinaryDataStr, zlib::DefaultCompression);
+  testZlibCompression(BinaryDataStr, CompressionScheme->BestSizeLevel);
+  testZlibCompression(BinaryDataStr, CompressionScheme->BestSpeedLevel);
+  testZlibCompression(BinaryDataStr, CompressionScheme->DefaultLevel);
 }
 #endif
 
 #if LLVM_ENABLE_ZSTD
-static void testZstdCompression(StringRef Input, int Level) {
-  SmallVector Compressed;
-  SmallVector Uncompressed;
-  zstd::compress(arrayRefFromStringRef(Input), Compressed, Level);
 
-  // Check that uncompressed buffer is the same as original.
-  Error E = zstd::uncompress(Compressed, Uncompressed, Input.size());
-  consumeError(std::move(E));
-
-  EXPECT_EQ(Input, toStringRef(Uncompressed));
-  if (Input.size() > 0) {
-// Uncompression fails if expected length is too short.
-E = zstd::uncompress(Compressed, Uncompressed, Input.size() - 1);
-EXPECT_EQ("Destination buffer is too small", llvm::toString(std::move(E)));
-  }
+static void te

[PATCH] D130516: [llvm] compression classes

2022-08-05 Thread Cole Kissane via Phabricator via cfe-commits
ckissane added inline comments.



Comment at: llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp:54-60
+  OptionalCompressionScheme = compression::noneIfUnsupported(
+  (Compress && DoInstrProfNameCompression) ? OptionalCompressionScheme
+   : llvm::NoneType());
+
+  bool doCompression = bool(OptionalCompressionScheme);
+
+  if (doCompression) {

dblaikie wrote:
> ckissane wrote:
> > ckissane wrote:
> > > ckissane wrote:
> > > > dblaikie wrote:
> > > > > This still seems like a lot of hoops to jump through - why 
> > > > > "noneIfUnsupported" rather than either having the compression scheme 
> > > > > (I think it could be the CompressionAlgorithm itself, rather than 
> > > > > having the separate OptionalCompressionKind abstraction) either be 
> > > > > null itself, or expose an "isAvailable" operation directly on the 
> > > > > CompressionAlgorithm?
> > > > > 
> > > > > Even if the 
> > > > > CompressionKind/OptionalCompressionKind/CompressionAlgorithm 
> > > > > abstractions are kept, I'm not sure why the above code is preferred 
> > > > > over, say:
> > > > > 
> > > > > ```
> > > > > if (Compress && DoInstrProfNameCompression && 
> > > > > OptionalCompressionScheme /* .isAvailable(), if we want to be more 
> > > > > explicit */) {
> > > > >   ...
> > > > > }
> > > > > ```
> > > > > 
> > > > > What's the benefit that `noneIfUnsupported` is providing? (& 
> > > > > generally I'd expect the `Compress && DoInstrProfNameCompression` to 
> > > > > be tested/exit early before even naming/constructing/querying/doing 
> > > > > anything with the compression scheme/algorithm/etc - so there'd be no 
> > > > > need to combine the tests for availability and the tests for whether 
> > > > > compression was requested)
> > > > > 
> > > > > Perhaps this API is motivated by a desire to implement something much 
> > > > > closer to the original code than is necessary/suitable? Or some other 
> > > > > use case/benefit I'm not quite understanding yet?
> > > > I shall remove `noneIfUnsupported`. You express good points, we can 
> > > > simply check `if(OptionalCompressionScheme && 
> > > > *OptionalCompressionScheme)` where necessary.
> > > though it will make a lot of existing code patterns less clear, and more 
> > > verbose
> > and sometimes you really do need to re code the exact thing 
> > `noneIfUnsupported` encapsulates...
> Are there examples within LLVM that you can show compare/contrast 
> `noneIfUnsupported` helps?
yes, I'll paste a couple here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130516

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


[PATCH] D131277: clang: fix typo availbility

2022-08-05 Thread Aarush Bhat via Phabricator via cfe-commits
sloorush created this revision.
Herald added a reviewer: dang.
Herald added a reviewer: ributzka.
Herald added a project: All.
sloorush requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fixes #56787. Fixing the spelling of availability. I am unsure about if this 
change will have any side effects. If someone can help on how to check if it 
has any side effects, I can test those out as well.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131277

Files:
  clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp


Index: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
===
--- clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
+++ clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
@@ -146,19 +146,19 @@
   if (Avail.isDefault())
 return None;
 
-  Object Availbility;
-  serializeObject(Availbility, "introducedVersion",
+  Object Availability;
+  serializeObject(Availability, "introducedVersion",
   serializeSemanticVersion(Avail.Introduced));
-  serializeObject(Availbility, "deprecatedVersion",
+  serializeObject(Availability, "deprecatedVersion",
   serializeSemanticVersion(Avail.Deprecated));
-  serializeObject(Availbility, "obsoletedVersion",
+  serializeObject(Availability, "obsoletedVersion",
   serializeSemanticVersion(Avail.Obsoleted));
   if (Avail.isUnavailable())
-Availbility["isUnconditionallyUnavailable"] = true;
+Availability["isUnconditionallyUnavailable"] = true;
   if (Avail.isUnconditionallyDeprecated())
-Availbility["isUnconditionallyDeprecated"] = true;
+Availability["isUnconditionallyDeprecated"] = true;
 
-  return Availbility;
+  return Availability;
 }
 
 /// Get the language name string for interface language references.
@@ -494,7 +494,7 @@
   serializeObject(
   Obj, "location",
   serializeSourceLocation(Record.Location, /*IncludeFileURI=*/true));
-  serializeObject(Obj, "availbility",
+  serializeObject(Obj, "availability",
   serializeAvailability(Record.Availability));
   serializeObject(Obj, "docComment", serializeDocComment(Record.Comment));
   serializeArray(Obj, "declarationFragments",


Index: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
===
--- clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
+++ clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
@@ -146,19 +146,19 @@
   if (Avail.isDefault())
 return None;
 
-  Object Availbility;
-  serializeObject(Availbility, "introducedVersion",
+  Object Availability;
+  serializeObject(Availability, "introducedVersion",
   serializeSemanticVersion(Avail.Introduced));
-  serializeObject(Availbility, "deprecatedVersion",
+  serializeObject(Availability, "deprecatedVersion",
   serializeSemanticVersion(Avail.Deprecated));
-  serializeObject(Availbility, "obsoletedVersion",
+  serializeObject(Availability, "obsoletedVersion",
   serializeSemanticVersion(Avail.Obsoleted));
   if (Avail.isUnavailable())
-Availbility["isUnconditionallyUnavailable"] = true;
+Availability["isUnconditionallyUnavailable"] = true;
   if (Avail.isUnconditionallyDeprecated())
-Availbility["isUnconditionallyDeprecated"] = true;
+Availability["isUnconditionallyDeprecated"] = true;
 
-  return Availbility;
+  return Availability;
 }
 
 /// Get the language name string for interface language references.
@@ -494,7 +494,7 @@
   serializeObject(
   Obj, "location",
   serializeSourceLocation(Record.Location, /*IncludeFileURI=*/true));
-  serializeObject(Obj, "availbility",
+  serializeObject(Obj, "availability",
   serializeAvailability(Record.Availability));
   serializeObject(Obj, "docComment", serializeDocComment(Record.Comment));
   serializeArray(Obj, "declarationFragments",
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122874: [clang] Add GNU spelling for no_unqiue_address attribute

2022-08-05 Thread Nikolas Klauser via Phabricator via cfe-commits
philnik planned changes to this revision.
philnik added a comment.

This is useless until we have some `no_unique_address` in clang-cl.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122874

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


[PATCH] D131084: Add support for specifying the severity of a SARIF Result.

2022-08-05 Thread Abraham Corea Diaz via Phabricator via cfe-commits
abrahamcd added a comment.

In D131084#3697211 , @vaibhav.y wrote:

> Submitting for review:
>
> Some notes:
>
> There are a couple of ways I think we can acheive this, per the spec:
>
> 1. The reportingDescriptor (rule) objects can be given a default 
> configuration property 
> ,
>  which can set the default warning level and other data such as rule 
> parameters etc.
> 2. The reportingDescriptor objects can omit the default configuration (which 
> then allows operating with warning as default), and the level is then set 
> when the result is reported.
>
> The first approach would be "more correct", what are your thoughts on this? 
> Would we benefit from having per-diagnostic configuration?
>
> There is also the question about the "kind" of results in clang. From my 
> reading of the spec 
> ,
>  it seems that "fail" is the only case that applies to us because:
>
> - "pass": Implies no issue was found.
> - "open": This value is used by proof-based tools. It could also mean 
> additional assertions required
> - "informational": The specified rule was evaluated and produced a purely 
> informational result that does not indicate the presence of a problem
> - "notApplicable": The rule specified by ruleId was not evaluated, because it 
> does not apply to the analysis target.
>
> Of these "open" and "notApplicable" seem to be ones that *could* come to use 
> but I'm not sure where / what kind of diagnostics would use these. Probably 
> clang-tidy's `bugprone-*` suite?
>
> Let me know what you think is a good way to approach this wrt clang's 
> diagnostics system.

@cjdb, @denik, and I discussed the two approaches yesterday, and we thought the 
first would be the better option (add setDefaultConfig to SarifRule instead of 
setDiagnosticLevel in SarifResult). We could make use of the 
defaultConfiguration's "level", "enabled", and "rank" properties to define the 
different diagnostic types that are currently present in Clang, and it would 
consolidate that information in a single rule rather than copying it across 
every result related to that rule. Then we could just have a small set of 
defaultConfigurations that correspond to the current Clang diagnostic 
categories that we can choose from when making new rules.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131084

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


[PATCH] D131278: [CUDA] Fix output name being replaced in device-only mode

2022-08-05 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 created this revision.
jhuber6 added a reviewer: tra.
Herald added subscribers: mattd, yaxunl.
Herald added a project: All.
jhuber6 requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

When performing device only compilation, there was an issue where
`cubin` outputs were being renamed to `cubin` despite the user's name.
This is required in a normal compilation flow as the Nvidia tools only
understand specific filenames instead of checking magic bytes for some
unknown reason. We do not want to perform this transformation when the
user is performing device only compilation.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131278

Files:
  clang/lib/Driver/ToolChains/Cuda.cpp
  clang/test/Driver/cuda-bindings.cu


Index: clang/test/Driver/cuda-bindings.cu
===
--- clang/test/Driver/cuda-bindings.cu
+++ clang/test/Driver/cuda-bindings.cu
@@ -135,3 +135,14 @@
 // RUN: | FileCheck -check-prefix=DASM2 %s
 // DASM2: # "nvptx64-nvidia-cuda" - "clang",{{.*}} output: 
"cuda-bindings-cuda-nvptx64-nvidia-cuda-sm_30.s"
 // DASM2: # "nvptx64-nvidia-cuda" - "clang",{{.*}} output: 
"cuda-bindings-cuda-nvptx64-nvidia-cuda-sm_35.s"
+
+//
+// Ensure we output the user's specified name in device-only mode.
+//
+// RUN: %clang -target powerpc64le-ibm-linux-gnu -### \
+// RUN:--cuda-gpu-arch=sm_52 --cuda-device-only -c -o foo.o %s 2>&1 \
+// RUN: | FileCheck -check-prefix=D_ONLY %s
+// RUN: %clang -target powerpc64le-ibm-linux-gnu -### --offload-new-driver \
+// RUN:--cuda-gpu-arch=sm_52 --cuda-device-only -c -o foo.o %s 2>&1 \
+// RUN: | FileCheck -check-prefix=D_ONLY %s
+// D_ONLY: "foo.o"
Index: clang/lib/Driver/ToolChains/Cuda.cpp
===
--- clang/lib/Driver/ToolChains/Cuda.cpp
+++ clang/lib/Driver/ToolChains/Cuda.cpp
@@ -693,8 +693,8 @@
 
 std::string CudaToolChain::getInputFilename(const InputInfo &Input) const {
   // Only object files are changed, for example assembly files keep their .s
-  // extensions. 
-  if (Input.getType() != types::TY_Object)
+  // extensions. If the user requested device-only compilation don't change it.
+  if (Input.getType() != types::TY_Object || getDriver().offloadDeviceOnly())
 return ToolChain::getInputFilename(Input);
 
   // Replace extension for object files with cubin because nvlink relies on


Index: clang/test/Driver/cuda-bindings.cu
===
--- clang/test/Driver/cuda-bindings.cu
+++ clang/test/Driver/cuda-bindings.cu
@@ -135,3 +135,14 @@
 // RUN: | FileCheck -check-prefix=DASM2 %s
 // DASM2: # "nvptx64-nvidia-cuda" - "clang",{{.*}} output: "cuda-bindings-cuda-nvptx64-nvidia-cuda-sm_30.s"
 // DASM2: # "nvptx64-nvidia-cuda" - "clang",{{.*}} output: "cuda-bindings-cuda-nvptx64-nvidia-cuda-sm_35.s"
+
+//
+// Ensure we output the user's specified name in device-only mode.
+//
+// RUN: %clang -target powerpc64le-ibm-linux-gnu -### \
+// RUN:--cuda-gpu-arch=sm_52 --cuda-device-only -c -o foo.o %s 2>&1 \
+// RUN: | FileCheck -check-prefix=D_ONLY %s
+// RUN: %clang -target powerpc64le-ibm-linux-gnu -### --offload-new-driver \
+// RUN:--cuda-gpu-arch=sm_52 --cuda-device-only -c -o foo.o %s 2>&1 \
+// RUN: | FileCheck -check-prefix=D_ONLY %s
+// D_ONLY: "foo.o"
Index: clang/lib/Driver/ToolChains/Cuda.cpp
===
--- clang/lib/Driver/ToolChains/Cuda.cpp
+++ clang/lib/Driver/ToolChains/Cuda.cpp
@@ -693,8 +693,8 @@
 
 std::string CudaToolChain::getInputFilename(const InputInfo &Input) const {
   // Only object files are changed, for example assembly files keep their .s
-  // extensions. 
-  if (Input.getType() != types::TY_Object)
+  // extensions. If the user requested device-only compilation don't change it.
+  if (Input.getType() != types::TY_Object || getDriver().offloadDeviceOnly())
 return ToolChain::getInputFilename(Input);
 
   // Replace extension for object files with cubin because nvlink relies on
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D128441: [CUDA] Do not embed a fatbinary when using the new driver

2022-08-05 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

In D128441#3702816 , @jhuber6 wrote:

> Is it spitting it out as `foo123.cubin` instead?

That's the output name it passes to `ptxas`, but it's treated as a temporary 
file and is removed at the end, so the user gets nothing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128441

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


[PATCH] D130372: [analyzer] Add a new factory function RangeSet::Factory::invert

2022-08-05 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov added a comment.

In D130372#3699036 , @martong wrote:

> LGTM! Nice work!

Thank you. I prefer to load this along with the motivation part D131006 
.


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

https://reviews.llvm.org/D130372

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


[PATCH] D130516: [llvm] compression classes

2022-08-05 Thread Cole Kissane via Phabricator via cfe-commits
ckissane added inline comments.



Comment at: llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp:54-60
+  OptionalCompressionScheme = compression::noneIfUnsupported(
+  (Compress && DoInstrProfNameCompression) ? OptionalCompressionScheme
+   : llvm::NoneType());
+
+  bool doCompression = bool(OptionalCompressionScheme);
+
+  if (doCompression) {

ckissane wrote:
> dblaikie wrote:
> > ckissane wrote:
> > > ckissane wrote:
> > > > ckissane wrote:
> > > > > dblaikie wrote:
> > > > > > This still seems like a lot of hoops to jump through - why 
> > > > > > "noneIfUnsupported" rather than either having the compression 
> > > > > > scheme (I think it could be the CompressionAlgorithm itself, rather 
> > > > > > than having the separate OptionalCompressionKind abstraction) 
> > > > > > either be null itself, or expose an "isAvailable" operation 
> > > > > > directly on the CompressionAlgorithm?
> > > > > > 
> > > > > > Even if the 
> > > > > > CompressionKind/OptionalCompressionKind/CompressionAlgorithm 
> > > > > > abstractions are kept, I'm not sure why the above code is preferred 
> > > > > > over, say:
> > > > > > 
> > > > > > ```
> > > > > > if (Compress && DoInstrProfNameCompression && 
> > > > > > OptionalCompressionScheme /* .isAvailable(), if we want to be more 
> > > > > > explicit */) {
> > > > > >   ...
> > > > > > }
> > > > > > ```
> > > > > > 
> > > > > > What's the benefit that `noneIfUnsupported` is providing? (& 
> > > > > > generally I'd expect the `Compress && DoInstrProfNameCompression` 
> > > > > > to be tested/exit early before even 
> > > > > > naming/constructing/querying/doing anything with the compression 
> > > > > > scheme/algorithm/etc - so there'd be no need to combine the tests 
> > > > > > for availability and the tests for whether compression was 
> > > > > > requested)
> > > > > > 
> > > > > > Perhaps this API is motivated by a desire to implement something 
> > > > > > much closer to the original code than is necessary/suitable? Or 
> > > > > > some other use case/benefit I'm not quite understanding yet?
> > > > > I shall remove `noneIfUnsupported`. You express good points, we can 
> > > > > simply check `if(OptionalCompressionScheme && 
> > > > > *OptionalCompressionScheme)` where necessary.
> > > > though it will make a lot of existing code patterns less clear, and 
> > > > more verbose
> > > and sometimes you really do need to re code the exact thing 
> > > `noneIfUnsupported` encapsulates...
> > Are there examples within LLVM that you can show compare/contrast 
> > `noneIfUnsupported` helps?
> yes, I'll paste a couple here
Ok, So I believe I was mistaken.
In older versions of this patch there was a none compression implementation 
that just did a memcpy,  this made a natural fall through state, which made 
this type of pattern advantageous.
However, this is no longer the case.
Hence I will remove this without further adue.
Thank you for your astute observation!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130516

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


[PATCH] D130516: [llvm] compression classes

2022-08-05 Thread Cole Kissane via Phabricator via cfe-commits
ckissane updated this revision to Diff 450343.
ckissane added a comment.

- remove OptionalCompressionKind noneIfUnsupported


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130516

Files:
  clang-tools-extra/clangd/index/Serialization.cpp
  clang-tools-extra/clangd/unittests/SerializationTests.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  lld/ELF/Driver.cpp
  lld/ELF/InputSection.cpp
  llvm/include/llvm/Object/Decompressor.h
  llvm/include/llvm/ProfileData/InstrProf.h
  llvm/include/llvm/Support/Compression.h
  llvm/lib/MC/ELFObjectWriter.cpp
  llvm/lib/ObjCopy/ELF/ELFObject.cpp
  llvm/lib/Object/Decompressor.cpp
  llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
  llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp
  llvm/lib/ProfileData/InstrProf.cpp
  llvm/lib/ProfileData/InstrProfCorrelator.cpp
  llvm/lib/ProfileData/SampleProfReader.cpp
  llvm/lib/ProfileData/SampleProfWriter.cpp
  llvm/lib/Support/Compression.cpp
  llvm/tools/llvm-mc/llvm-mc.cpp
  llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
  llvm/unittests/ProfileData/InstrProfTest.cpp
  llvm/unittests/Support/CompressionTest.cpp

Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -22,31 +22,41 @@
 
 namespace {
 
-#if LLVM_ENABLE_ZLIB
-static void testZlibCompression(StringRef Input, int Level) {
+static void testCompressionAlgorithm(
+StringRef Input, int Level, CompressionKind CompressionScheme,
+std::string ExpectedDestinationBufferTooSmallErrorMessage) {
   SmallVector Compressed;
   SmallVector Uncompressed;
-  zlib::compress(arrayRefFromStringRef(Input), Compressed, Level);
+  CompressionScheme->compress(arrayRefFromStringRef(Input), Compressed, Level);
 
   // Check that uncompressed buffer is the same as original.
-  Error E = zlib::uncompress(Compressed, Uncompressed, Input.size());
+  Error E =
+  CompressionScheme->decompress(Compressed, Uncompressed, Input.size());
   consumeError(std::move(E));
 
   EXPECT_EQ(Input, toStringRef(Uncompressed));
   if (Input.size() > 0) {
 // Uncompression fails if expected length is too short.
-E = zlib::uncompress(Compressed, Uncompressed, Input.size() - 1);
-EXPECT_EQ("zlib error: Z_BUF_ERROR", llvm::toString(std::move(E)));
+E = CompressionScheme->decompress(Compressed, Uncompressed,
+  Input.size() - 1);
+EXPECT_EQ(ExpectedDestinationBufferTooSmallErrorMessage,
+  llvm::toString(std::move(E)));
   }
 }
 
+#if LLVM_ENABLE_ZLIB
+static void testZlibCompression(StringRef Input, int Level) {
+  testCompressionAlgorithm(Input, Level, CompressionKind::Zlib,
+   "zlib error: Z_BUF_ERROR");
+}
+
 TEST(CompressionTest, Zlib) {
-  testZlibCompression("", zlib::DefaultCompression);
+  CompressionKind CompressionScheme = CompressionKind::Zlib;
+  testZlibCompression("", CompressionScheme->DefaultLevel);
 
-  testZlibCompression("hello, world!", zlib::NoCompression);
-  testZlibCompression("hello, world!", zlib::BestSizeCompression);
-  testZlibCompression("hello, world!", zlib::BestSpeedCompression);
-  testZlibCompression("hello, world!", zlib::DefaultCompression);
+  testZlibCompression("hello, world!", CompressionScheme->BestSizeLevel);
+  testZlibCompression("hello, world!", CompressionScheme->BestSpeedLevel);
+  testZlibCompression("hello, world!", CompressionScheme->DefaultLevel);
 
   const size_t kSize = 1024;
   char BinaryData[kSize];
@@ -54,38 +64,26 @@
 BinaryData[i] = i & 255;
   StringRef BinaryDataStr(BinaryData, kSize);
 
-  testZlibCompression(BinaryDataStr, zlib::NoCompression);
-  testZlibCompression(BinaryDataStr, zlib::BestSizeCompression);
-  testZlibCompression(BinaryDataStr, zlib::BestSpeedCompression);
-  testZlibCompression(BinaryDataStr, zlib::DefaultCompression);
+  testZlibCompression(BinaryDataStr, CompressionScheme->BestSizeLevel);
+  testZlibCompression(BinaryDataStr, CompressionScheme->BestSpeedLevel);
+  testZlibCompression(BinaryDataStr, CompressionScheme->DefaultLevel);
 }
 #endif
 
 #if LLVM_ENABLE_ZSTD
-static void testZstdCompression(StringRef Input, int Level) {
-  SmallVector Compressed;
-  SmallVector Uncompressed;
-  zstd::compress(arrayRefFromStringRef(Input), Compressed, Level);
 
-  // Check that uncompressed buffer is the same as original.
-  Error E = zstd::uncompress(Compressed, Uncompressed, Input.size());
-  consumeError(std::move(E));
-
-  EXPECT_EQ(Input, toStringRef(Uncompressed));
-  if (Input.size() > 0) {
-// Uncompression fails if expected length is too short.
-E = zstd::uncompress(Compressed, Uncompressed, Input.size() - 1);
-EXPECT_EQ("Destination buffer is too small", llvm::toString(std::move(E)));
-

[PATCH] D130516: [llvm] compression classes

2022-08-05 Thread Cole Kissane via Phabricator via cfe-commits
ckissane added inline comments.



Comment at: clang/lib/Serialization/ASTReader.cpp:1474-1475
+  if (!CompressionScheme) {
+Error("compression class " +
+  (CompressionScheme->Name + " is not available").str());
 return nullptr;

leonardchan wrote:
> I think this `Error` takes a StringRef, so I think you could have:
> 
> ```
> return Error("compression class " + CompressionScheme->Name + " is not 
> available");
> ```
unfortunately that doesn't work (I tried)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130516

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


[PATCH] D131268: [HLSL] Generate buffer subscript operators

2022-08-05 Thread Chris Bieneman via Phabricator via cfe-commits
beanz added inline comments.



Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:220
+
+// Const subscript operators return copies of elements, non-const return a
+// reference so that they are assignable.

python3kgae wrote:
> If we reuse this function for StructuredBuffer, then const subscript return a 
> const reference could be better?
Why? Since this should get inlined return value optimization should eliminate 
the redundant copies.



Comment at: clang/test/CodeGenHLSL/buffer-array-operator.hlsl:4
+const RWBuffer In;
+RWBuffer Out;
+

python3kgae wrote:
> Maybe change In to RWBuffer
> And Out[Idx] = In[Idx].z;
> So we also test vector case?
None of the added code here depends on the template parameter type. Putting a 
vector in increases the complexity of the output, but I'm unsure it extends the 
test coverage meaningfully.



Comment at: clang/test/CodeGenHLSL/buffer-array-operator.hlsl:16
+// CHECK: float @"??A?$RWBuffer@M@hlsl@@QBAMI@Z"
+// CHECK: %this1 = load ptr, ptr %this.addr, align 4
+// CHECK-NEXT: %h = getelementptr inbounds %"class.hlsl::RWBuffer", ptr 
%this1, i32 0, i32 0

python3kgae wrote:
> Where is the this.addr coming from?
That is how clang generates stored locations for input parameters.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131268

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


[PATCH] D130516: [llvm] compression classes

2022-08-05 Thread Cole Kissane via Phabricator via cfe-commits
ckissane updated this revision to Diff 450345.
ckissane added a comment.

- format error string


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130516

Files:
  clang-tools-extra/clangd/index/Serialization.cpp
  clang-tools-extra/clangd/unittests/SerializationTests.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  lld/ELF/Driver.cpp
  lld/ELF/InputSection.cpp
  llvm/include/llvm/Object/Decompressor.h
  llvm/include/llvm/ProfileData/InstrProf.h
  llvm/include/llvm/Support/Compression.h
  llvm/lib/MC/ELFObjectWriter.cpp
  llvm/lib/ObjCopy/ELF/ELFObject.cpp
  llvm/lib/Object/Decompressor.cpp
  llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
  llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp
  llvm/lib/ProfileData/InstrProf.cpp
  llvm/lib/ProfileData/InstrProfCorrelator.cpp
  llvm/lib/ProfileData/SampleProfReader.cpp
  llvm/lib/ProfileData/SampleProfWriter.cpp
  llvm/lib/Support/Compression.cpp
  llvm/tools/llvm-mc/llvm-mc.cpp
  llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
  llvm/unittests/ProfileData/InstrProfTest.cpp
  llvm/unittests/Support/CompressionTest.cpp

Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -22,31 +22,41 @@
 
 namespace {
 
-#if LLVM_ENABLE_ZLIB
-static void testZlibCompression(StringRef Input, int Level) {
+static void testCompressionAlgorithm(
+StringRef Input, int Level, CompressionKind CompressionScheme,
+std::string ExpectedDestinationBufferTooSmallErrorMessage) {
   SmallVector Compressed;
   SmallVector Uncompressed;
-  zlib::compress(arrayRefFromStringRef(Input), Compressed, Level);
+  CompressionScheme->compress(arrayRefFromStringRef(Input), Compressed, Level);
 
   // Check that uncompressed buffer is the same as original.
-  Error E = zlib::uncompress(Compressed, Uncompressed, Input.size());
+  Error E =
+  CompressionScheme->decompress(Compressed, Uncompressed, Input.size());
   consumeError(std::move(E));
 
   EXPECT_EQ(Input, toStringRef(Uncompressed));
   if (Input.size() > 0) {
 // Uncompression fails if expected length is too short.
-E = zlib::uncompress(Compressed, Uncompressed, Input.size() - 1);
-EXPECT_EQ("zlib error: Z_BUF_ERROR", llvm::toString(std::move(E)));
+E = CompressionScheme->decompress(Compressed, Uncompressed,
+  Input.size() - 1);
+EXPECT_EQ(ExpectedDestinationBufferTooSmallErrorMessage,
+  llvm::toString(std::move(E)));
   }
 }
 
+#if LLVM_ENABLE_ZLIB
+static void testZlibCompression(StringRef Input, int Level) {
+  testCompressionAlgorithm(Input, Level, CompressionKind::Zlib,
+   "zlib error: Z_BUF_ERROR");
+}
+
 TEST(CompressionTest, Zlib) {
-  testZlibCompression("", zlib::DefaultCompression);
+  CompressionKind CompressionScheme = CompressionKind::Zlib;
+  testZlibCompression("", CompressionScheme->DefaultLevel);
 
-  testZlibCompression("hello, world!", zlib::NoCompression);
-  testZlibCompression("hello, world!", zlib::BestSizeCompression);
-  testZlibCompression("hello, world!", zlib::BestSpeedCompression);
-  testZlibCompression("hello, world!", zlib::DefaultCompression);
+  testZlibCompression("hello, world!", CompressionScheme->BestSizeLevel);
+  testZlibCompression("hello, world!", CompressionScheme->BestSpeedLevel);
+  testZlibCompression("hello, world!", CompressionScheme->DefaultLevel);
 
   const size_t kSize = 1024;
   char BinaryData[kSize];
@@ -54,38 +64,26 @@
 BinaryData[i] = i & 255;
   StringRef BinaryDataStr(BinaryData, kSize);
 
-  testZlibCompression(BinaryDataStr, zlib::NoCompression);
-  testZlibCompression(BinaryDataStr, zlib::BestSizeCompression);
-  testZlibCompression(BinaryDataStr, zlib::BestSpeedCompression);
-  testZlibCompression(BinaryDataStr, zlib::DefaultCompression);
+  testZlibCompression(BinaryDataStr, CompressionScheme->BestSizeLevel);
+  testZlibCompression(BinaryDataStr, CompressionScheme->BestSpeedLevel);
+  testZlibCompression(BinaryDataStr, CompressionScheme->DefaultLevel);
 }
 #endif
 
 #if LLVM_ENABLE_ZSTD
-static void testZstdCompression(StringRef Input, int Level) {
-  SmallVector Compressed;
-  SmallVector Uncompressed;
-  zstd::compress(arrayRefFromStringRef(Input), Compressed, Level);
 
-  // Check that uncompressed buffer is the same as original.
-  Error E = zstd::uncompress(Compressed, Uncompressed, Input.size());
-  consumeError(std::move(E));
-
-  EXPECT_EQ(Input, toStringRef(Uncompressed));
-  if (Input.size() > 0) {
-// Uncompression fails if expected length is too short.
-E = zstd::uncompress(Compressed, Uncompressed, Input.size() - 1);
-EXPECT_EQ("Destination buffer is too small", llvm::toString(std::move(E)));
-  }
+static void testZStdComp

[PATCH] D131280: [clang][dataflow] Parameterize analysis by explicit map of analyzable functions.

2022-08-05 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel created this revision.
ymandel added reviewers: samestep, xazax.hun, sgatev.
Herald added subscribers: martong, rnkovacs.
Herald added a reviewer: NoQ.
Herald added a project: All.
ymandel requested review of this revision.
Herald added a project: clang.

This patch evolves the cache of `ControlFlowContext`s into a parameter that the
user can supply. Specifically, the system now limits inline analysis of function
calls specifically to those functions contained in a user-supplied map from
"fully qualified name" to `ControlFlowContext`.

To avoid the cost of generating the fully qualified function name for every
`CallExpr`, we cache the results of the map lookup in an intermediary map over
`FunctionDecl*`s.

Part of issue #56879


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131280

Files:
  clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
  clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -11,12 +11,15 @@
 #include "clang/AST/Decl.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/FlowSensitive/ControlFlowContext.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
 #include "clang/Analysis/FlowSensitive/NoopAnalysis.h"
 #include "clang/Analysis/FlowSensitive/StorageLocation.h"
 #include "clang/Analysis/FlowSensitive/Value.h"
 #include "clang/Basic/LangStandard.h"
+#include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Testing/Support/Error.h"
@@ -40,11 +43,39 @@
 template 
 void runDataflow(llvm::StringRef Code, Matcher Match,
  DataflowAnalysisOptions Options,
+ llvm::StringRef ModeledFunctionsCode = {},
  LangStandard::Kind Std = LangStandard::lang_cxx17,
  llvm::StringRef TargetFun = "target") {
+  const std::vector Args = {
+  "-fsyntax-only", "-fno-delayed-template-parsing",
+  "-std=" +
+  std::string(LangStandard::getLangStandardForKind(Std).getName())};
+
+  auto MainASTUnit = tooling::buildASTFromCodeWithArgs(Code, Args);
+  ASSERT_NE(MainASTUnit, nullptr);
+
+  // The unique_ptr has to live outside the if-body since the lifetime of the
+  // function map is linked to the lifetime of the ASTUnit. The raw pointer
+  // allows us to select between the main TU and the (optional) model TU.
+  std::unique_ptr ModelASTUnit = nullptr;
+  ASTUnit *UnitPtr = nullptr;
+  if (ModeledFunctionsCode.empty()) {
+UnitPtr = MainASTUnit.get();
+  } else {
+ModelASTUnit =
+tooling::buildASTFromCodeWithArgs(ModeledFunctionsCode, Args);
+ASSERT_NE(ModelASTUnit, nullptr);
+UnitPtr = ModelASTUnit.get();
+  }
+
+  llvm::Expected> Result =
+  buildFunctionMapFromAST(*UnitPtr);
+  ASSERT_TRUE((bool)Result);
+  llvm::StringMap AnalyzableFunctions = std::move(*Result);
+
   ASSERT_THAT_ERROR(
   test::checkDataflow(
-  Code, TargetFun,
+  Code, std::move(MainASTUnit), ast_matchers::hasName(TargetFun),
   [Options](ASTContext &C, Environment &) {
 return NoopAnalysis(C, Options);
   },
@@ -53,18 +84,18 @@
   std::pair>>
   Results,
   ASTContext &ASTCtx) { Match(Results, ASTCtx); },
-  {"-fsyntax-only", "-fno-delayed-template-parsing",
-   "-std=" + std::string(
- LangStandard::getLangStandardForKind(Std).getName())}),
+  std::move(AnalyzableFunctions)),
   llvm::Succeeded());
 }
 
+// FIXME: inline this overload and remove it.
 template 
 void runDataflow(llvm::StringRef Code, Matcher Match,
  LangStandard::Kind Std = LangStandard::lang_cxx17,
  bool ApplyBuiltinTransfer = true,
  llvm::StringRef TargetFun = "target") {
-  runDataflow(Code, Match, {ApplyBuiltinTransfer, {}}, Std, TargetFun);
+  runDataflow(Code, Match, /*Options=*/{ApplyBuiltinTransfer, {}},
+  /*AnalyzableFunctions=*/{}, Std, TargetFun);
 }
 
 TEST(TransferTest, IntVarDeclNotTrackedWhenTransferDisabled) {
@@ -1320,9 +1351,19 @@
   }
 };
   )";
+
+  auto ASTUnit = tooling::buildASTFromCodeWithArgs(
+  Code,
+  /*Ar

[clang] 3e0e556 - [CUDA] Fixed sm version constrain for __bmma_m8n8k128_mma_and_popc_b1.

2022-08-05 Thread Artem Belevich via cfe-commits

Author: Jack Kirk
Date: 2022-08-05T12:14:06-07:00
New Revision: 3e0e5568a6a8c744d26f79a1e55360fe2655867c

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

LOG: [CUDA] Fixed sm version constrain for __bmma_m8n8k128_mma_and_popc_b1.

As stated in
https://docs.nvidia.com/cuda/parallel-thread-execution/index.html#warp-level-matrix-instructions-wmma-mma:
".and operation in single-bit wmma requires sm_80 or higher."

tra@: Fixed a bug in builtins-nvptx-mma.py test generator and regenerated the 
tests.

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

Added: 


Modified: 
clang/include/clang/Basic/BuiltinsNVPTX.def
clang/test/CodeGen/builtins-nvptx-mma.cu
clang/test/CodeGen/builtins-nvptx-mma.py

Removed: 




diff  --git a/clang/include/clang/Basic/BuiltinsNVPTX.def 
b/clang/include/clang/Basic/BuiltinsNVPTX.def
index a5ec77a6112c0..ea0efcef2ca5b 100644
--- a/clang/include/clang/Basic/BuiltinsNVPTX.def
+++ b/clang/include/clang/Basic/BuiltinsNVPTX.def
@@ -853,7 +853,7 @@ TARGET_BUILTIN(__hmma_m8n32k16_mma_f16f32, 
"vi*iC*iC*fC*IiIi", "", AND(SM_70,PTX
 TARGET_BUILTIN(__bmma_m8n8k128_ld_a_b1, "vi*iC*UiIi", "", AND(SM_75,PTX63))
 TARGET_BUILTIN(__bmma_m8n8k128_ld_b_b1, "vi*iC*UiIi", "", AND(SM_75,PTX63))
 TARGET_BUILTIN(__bmma_m8n8k128_ld_c, "vi*iC*UiIi", "", AND(SM_75,PTX63))
-TARGET_BUILTIN(__bmma_m8n8k128_mma_and_popc_b1, "vi*iC*iC*iC*Ii", "", 
AND(SM_75,PTX71))
+TARGET_BUILTIN(__bmma_m8n8k128_mma_and_popc_b1, "vi*iC*iC*iC*Ii", "", 
AND(SM_80,PTX71))
 TARGET_BUILTIN(__bmma_m8n8k128_mma_xor_popc_b1, "vi*iC*iC*iC*Ii", "", 
AND(SM_75,PTX63))
 TARGET_BUILTIN(__bmma_m8n8k128_st_c_i32, "vi*iC*UiIi", "", AND(SM_75,PTX63))
 TARGET_BUILTIN(__imma_m16n16k16_ld_a_s8, "vi*iC*UiIi", "", AND(SM_72,PTX63))

diff  --git a/clang/test/CodeGen/builtins-nvptx-mma.cu 
b/clang/test/CodeGen/builtins-nvptx-mma.cu
index aaa44bcaa7e22..5375d88032b7d 100644
--- a/clang/test/CodeGen/builtins-nvptx-mma.cu
+++ b/clang/test/CodeGen/builtins-nvptx-mma.cu
@@ -10,7 +10,7 @@
 // RUN:-fcuda-is-device -target-feature +ptx71 \
 // RUN:-DPTX=71 -DSM=80 \
 // RUN:-S -emit-llvm -o - -x cuda %s \
-// RUN:   | FileCheck 
-check-prefixes=CHECK_PTX70_SM80,CHECK_PTX60_SM70,CHECK_PTX63_SM72,CHECK_PTX61_SM70,CHECK_PTX63_SM75,CHECK_PTX71_SM75
 %s
+// RUN:   | FileCheck 
-check-prefixes=CHECK_PTX70_SM80,CHECK_PTX60_SM70,CHECK_PTX63_SM72,CHECK_PTX61_SM70,CHECK_PTX63_SM75,CHECK_PTX71_SM80
 %s
 // Verify that all builtins have correct constraints.
 // RUN: %clang_cc1 -triple nvptx-unknown-unknown \
 // RUN:   -target-cpu sm_60 -target-feature +ptx42 \
@@ -167,7 +167,7 @@ __device__ void test_wmma_buitins(int *src, int *dst,
   // CHECK_PTX60_SM70: call {{.*}} 
@llvm.nvvm.wmma.m16n16k16.mma.row.row.f32.f32.satfinite
   // expected-error-re@+1 {{'__hmma_m16n16k16_mma_f32f32' needs target feature 
(sm_70{{.*}},(ptx60{{.*
   __hmma_m16n16k16_mma_f32f32(fdst, src, src, fsrc, 0, 1);
-#endif // (PTX >= 60) && (SM >= 70) 
+#endif // (PTX >= 60) && (SM >= 70)
 
 #if (PTX >= 61) && (SM >= 70)
 
@@ -435,7 +435,7 @@ __device__ void test_wmma_buitins(int *src, int *dst,
   // CHECK_PTX61_SM70: call {{.*}} 
@llvm.nvvm.wmma.m8n32k16.mma.row.row.f32.f32.satfinite
   // expected-error-re@+1 {{'__hmma_m8n32k16_mma_f32f32' needs target feature 
(sm_70{{.*}},(ptx61{{.*
   __hmma_m8n32k16_mma_f32f32(fdst, src, src, fsrc, 0, 1);
-#endif // (PTX >= 61) && (SM >= 70) 
+#endif // (PTX >= 61) && (SM >= 70)
 
 #if (PTX >= 63) && (SM >= 72)
 
@@ -691,7 +691,7 @@ __device__ void test_wmma_buitins(int *src, int *dst,
   // CHECK_PTX63_SM72: call {{.*}} 
@llvm.nvvm.wmma.m8n32k16.mma.row.row.u8.satfinite
   // expected-error-re@+1 {{'__imma_m8n32k16_mma_u8' needs target feature 
(sm_72{{.*}},(ptx63{{.*
   __imma_m8n32k16_mma_u8(dst, src, src, src, 0, 1);
-#endif // (PTX >= 63) && (SM >= 72) 
+#endif // (PTX >= 63) && (SM >= 72)
 
 #if (PTX >= 63) && (SM >= 75)
 
@@ -752,7 +752,7 @@ __device__ void test_wmma_buitins(int *src, int *dst,
   // CHECK_PTX63_SM75: call {{.*}} 
@llvm.nvvm.wmma.m8n8k32.mma.row.col.u4.satfinite
   // expected-error-re@+1 {{'__imma_m8n8k32_mma_u4' needs target feature 
(sm_75{{.*}},(ptx63{{.*
   __imma_m8n8k32_mma_u4(dst, src, src, src, 1, 1);
-#endif // (PTX >= 63) && (SM >= 75) 
+#endif // (PTX >= 63) && (SM >= 75)
 
 #if (PTX >= 70) && (SM >= 80)
 
@@ -900,12 +900,12 @@ __device__ void test_wmma_buitins(int *src, int *dst,
   // CHECK_PTX70_SM80: call {{.*}} @llvm.nvvm.wmma.m8n8k4.mma.row.row.f64
   // expected-error-re@+1 {{'__dmma_m8n8k4_mma_f64' needs target feature 
(sm_80{{.*}},(ptx70{{.*
   __dmma_m8n8k4_mma_f64(ddst, dsrc, dsrc, dsrc, 0, 0);
-#endif // (PTX >= 70) && (SM >= 80) 
+#endif // (PTX >= 70) && (SM >= 80)
 
-#if (PTX >= 71) && (SM >= 75)
+#if (PTX >= 7

[PATCH] D131265: Fixed sm version for .and bmma operator.

2022-08-05 Thread Artem Belevich 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 rG3e0e5568a6a8: [CUDA] Fixed sm version constrain for 
__bmma_m8n8k128_mma_and_popc_b1. (authored by JackAKirk, committed by tra).

Changed prior to commit:
  https://reviews.llvm.org/D131265?vs=450302&id=450351#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131265

Files:
  clang/include/clang/Basic/BuiltinsNVPTX.def
  clang/test/CodeGen/builtins-nvptx-mma.cu
  clang/test/CodeGen/builtins-nvptx-mma.py

Index: clang/test/CodeGen/builtins-nvptx-mma.py
===
--- clang/test/CodeGen/builtins-nvptx-mma.py
+++ clang/test/CodeGen/builtins-nvptx-mma.py
@@ -202,7 +202,7 @@
   if frag.ptx_type in ["f64", "bf16", "tf32"]:
 return 80
   if frag.ptx_type in ["u4", "s4", "b1"]:
-if b1op == "_and_popc":
+if b1op == ".and.popc":
   return 80
 return 75
   if frag.ptx_type in ["s8", "u8"]:
@@ -409,7 +409,7 @@
 print()
 print("#if (PTX >= %d) && (SM >= %d)" % (ptx, sm))
 print(tests)
-print("#endif // (PTX >= %d) && (SM >= %d) "% (ptx, sm))
+print("#endif // (PTX >= %d) && (SM >= %d)"% (ptx, sm))
 
   print("}")
 
Index: clang/test/CodeGen/builtins-nvptx-mma.cu
===
--- clang/test/CodeGen/builtins-nvptx-mma.cu
+++ clang/test/CodeGen/builtins-nvptx-mma.cu
@@ -10,7 +10,7 @@
 // RUN:-fcuda-is-device -target-feature +ptx71 \
 // RUN:-DPTX=71 -DSM=80 \
 // RUN:-S -emit-llvm -o - -x cuda %s \
-// RUN:   | FileCheck -check-prefixes=CHECK_PTX70_SM80,CHECK_PTX60_SM70,CHECK_PTX63_SM72,CHECK_PTX61_SM70,CHECK_PTX63_SM75,CHECK_PTX71_SM75 %s
+// RUN:   | FileCheck -check-prefixes=CHECK_PTX70_SM80,CHECK_PTX60_SM70,CHECK_PTX63_SM72,CHECK_PTX61_SM70,CHECK_PTX63_SM75,CHECK_PTX71_SM80 %s
 // Verify that all builtins have correct constraints.
 // RUN: %clang_cc1 -triple nvptx-unknown-unknown \
 // RUN:   -target-cpu sm_60 -target-feature +ptx42 \
@@ -167,7 +167,7 @@
   // CHECK_PTX60_SM70: call {{.*}} @llvm.nvvm.wmma.m16n16k16.mma.row.row.f32.f32.satfinite
   // expected-error-re@+1 {{'__hmma_m16n16k16_mma_f32f32' needs target feature (sm_70{{.*}},(ptx60{{.*
   __hmma_m16n16k16_mma_f32f32(fdst, src, src, fsrc, 0, 1);
-#endif // (PTX >= 60) && (SM >= 70) 
+#endif // (PTX >= 60) && (SM >= 70)
 
 #if (PTX >= 61) && (SM >= 70)
 
@@ -435,7 +435,7 @@
   // CHECK_PTX61_SM70: call {{.*}} @llvm.nvvm.wmma.m8n32k16.mma.row.row.f32.f32.satfinite
   // expected-error-re@+1 {{'__hmma_m8n32k16_mma_f32f32' needs target feature (sm_70{{.*}},(ptx61{{.*
   __hmma_m8n32k16_mma_f32f32(fdst, src, src, fsrc, 0, 1);
-#endif // (PTX >= 61) && (SM >= 70) 
+#endif // (PTX >= 61) && (SM >= 70)
 
 #if (PTX >= 63) && (SM >= 72)
 
@@ -691,7 +691,7 @@
   // CHECK_PTX63_SM72: call {{.*}} @llvm.nvvm.wmma.m8n32k16.mma.row.row.u8.satfinite
   // expected-error-re@+1 {{'__imma_m8n32k16_mma_u8' needs target feature (sm_72{{.*}},(ptx63{{.*
   __imma_m8n32k16_mma_u8(dst, src, src, src, 0, 1);
-#endif // (PTX >= 63) && (SM >= 72) 
+#endif // (PTX >= 63) && (SM >= 72)
 
 #if (PTX >= 63) && (SM >= 75)
 
@@ -752,7 +752,7 @@
   // CHECK_PTX63_SM75: call {{.*}} @llvm.nvvm.wmma.m8n8k32.mma.row.col.u4.satfinite
   // expected-error-re@+1 {{'__imma_m8n8k32_mma_u4' needs target feature (sm_75{{.*}},(ptx63{{.*
   __imma_m8n8k32_mma_u4(dst, src, src, src, 1, 1);
-#endif // (PTX >= 63) && (SM >= 75) 
+#endif // (PTX >= 63) && (SM >= 75)
 
 #if (PTX >= 70) && (SM >= 80)
 
@@ -900,12 +900,12 @@
   // CHECK_PTX70_SM80: call {{.*}} @llvm.nvvm.wmma.m8n8k4.mma.row.row.f64
   // expected-error-re@+1 {{'__dmma_m8n8k4_mma_f64' needs target feature (sm_80{{.*}},(ptx70{{.*
   __dmma_m8n8k4_mma_f64(ddst, dsrc, dsrc, dsrc, 0, 0);
-#endif // (PTX >= 70) && (SM >= 80) 
+#endif // (PTX >= 70) && (SM >= 80)
 
-#if (PTX >= 71) && (SM >= 75)
+#if (PTX >= 71) && (SM >= 80)
 
-  // CHECK_PTX71_SM75: call {{.*}} @llvm.nvvm.wmma.m8n8k128.mma.and.popc.row.col.b1
-  // expected-error-re@+1 {{'__bmma_m8n8k128_mma_and_popc_b1' needs target feature (sm_75{{.*}},(ptx71{{.*
+  // CHECK_PTX71_SM80: call {{.*}} @llvm.nvvm.wmma.m8n8k128.mma.and.popc.row.col.b1
+  // expected-error-re@+1 {{'__bmma_m8n8k128_mma_and_popc_b1' needs target feature (sm_80{{.*}},(ptx71{{.*
   __bmma_m8n8k128_mma_and_popc_b1(dst, src, src, src, 1);
-#endif // (PTX >= 71) && (SM >= 75) 
+#endif // (PTX >= 71) && (SM >= 80)
 }
Index: clang/include/clang/Basic/BuiltinsNVPTX.def
===
--- clang/include/clang/Basic/BuiltinsNVPTX.def
+++ clang/include/clang/Basic/BuiltinsNVPTX.def
@@ -853,7 +853,7 @@
 TARGET_BUILTIN(__bmma_m8n8k128_ld_a_b1, "vi*iC*UiIi", "", AND(SM_75,PTX63))
 TARGET_BUILTIN(__bmma_m8n8k128_ld_b_b1, "vi*iC*UiIi", "", AND(SM

[PATCH] D131265: Fixed sm version for .and bmma operator.

2022-08-05 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

Looks like the tests needed to be updated (and I've found one bug which 
explains how we've missed this).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131265

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


[PATCH] D103562: [NFC][compiler-rt][hwasan] Refactor hwasan functions

2022-08-05 Thread Alexey Baturo via Phabricator via cfe-commits
smd added a comment.

Hi folks,

I've been working on support hwasan for risc-v and I believe I've found an 
issue with the existing lit tests this commit causes.
Tests stack-{oob,uar,uas}.c check for correct backtrace being printed. From the 
code and comments the idea is to not to print any hwasan related frames(see the 
code and comments below).

  void HandleTagMismatch(AccessInfo ai, uptr pc, uptr frame, void *uc,  
  
 uptr *registers_frame) {   
  
InternalMmapVector stack_buffer(1); 
  
BufferedStackTrace *stack = stack_buffer.data();
  
stack->Reset(); 
  
stack->Unwind(pc, frame, uc, common_flags()->fast_unwind_on_fatal); 
  

  
// The second stack frame contains the failure __hwasan_check function, as  
  
// we have a stack frame for the registers saved in __hwasan_tag_mismatch 
that
// we wish to ignore. This (currently) only occurs on AArch64, as x64   
   
// implementations use SIGTRAP to implement the failure, and thus do not go 
  
// through the stack saver. 
  
if (registers_frame && stack->trace && stack->size > 0) {   
  
  stack->trace++;   
   
  stack->size--;
   
} 

Before this commit the return address and frame pointer to were taken directly 
from **hwasan_tag_mismatch4**, while after the commit those addresses are 
calculated after another function being called from hwasan_tag_mismatch4 (the 
**HwasanTagMismatch** one).
So, if I understand it correctly, now it looks like 2 stack frames must be 
ignored(for **hwasan_tag_mismatch4** and **HwasanTagMismatch**) to get a proper 
backtrace.
What do you think?

Thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103562

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


[PATCH] D103562: [NFC][compiler-rt][hwasan] Refactor hwasan functions

2022-08-05 Thread Florian Mayer via Phabricator via cfe-commits
fmayer added a comment.

In D103562#3702962 , @smd wrote:

> Hi folks,
>
> I've been working on support hwasan for risc-v and I believe I've found an 
> issue with the existing lit tests this commit causes.
> Tests stack-{oob,uar,uas}.c check for correct backtrace being printed. From 
> the code and comments the idea is to not to print any hwasan related 
> frames(see the code and comments below).
>
>   void HandleTagMismatch(AccessInfo ai, uptr pc, uptr frame, void *uc,
> 
>  uptr *registers_frame) { 
> 
> InternalMmapVector stack_buffer(1);   
> 
> BufferedStackTrace *stack = stack_buffer.data();  
> 
> stack->Reset();   
> 
> stack->Unwind(pc, frame, uc, common_flags()->fast_unwind_on_fatal);   
> 
>   
> 
> // The second stack frame contains the failure __hwasan_check function, 
> as
> // we have a stack frame for the registers saved in __hwasan_tag_mismatch 
> that
> // we wish to ignore. This (currently) only occurs on AArch64, as x64 
>  
> // implementations use SIGTRAP to implement the failure, and thus do not 
> go   
> // through the stack saver.   
> 
> if (registers_frame && stack->trace && stack->size > 0) { 
> 
>   stack->trace++; 
>  
>   stack->size--;  
>  
> } 
>
> Before this commit the return address and frame pointer to were taken 
> directly from **hwasan_tag_mismatch4**, while after the commit those 
> addresses are calculated after another function being called from 
> hwasan_tag_mismatch4 (the **HwasanTagMismatch** one).
> So, if I understand it correctly, now it looks like 2 stack frames must be 
> ignored(for **hwasan_tag_mismatch4** and **HwasanTagMismatch**) to get a 
> proper backtrace.
> What do you think?
>
> Thanks

Yes, but I am not sure we can *rely* on that being the case as is. LTO could 
conceivably inline this – in which case it would be one, right?




Comment at: compiler-rt/lib/hwasan/hwasan.cpp:205
+  if (registers_frame && stack->trace && stack->size > 0) {
+stack->trace++;
+stack->size--;

vitalybuka wrote:
> maybe we should pop everything up to "pc" to avoid issues with nested calls?
> 
> For most users hwasan frames are not very useful. However if you work on 
> sanitizer, some frames can be a useful info. So I don't mind we just relax 
> test cases to accommodate this nesting.
> 
> cc @smd 
This is probably for another patch though, right? This is already like this on 
the LHS.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103562

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


[PATCH] D103562: [NFC][compiler-rt][hwasan] Refactor hwasan functions

2022-08-05 Thread Florian Mayer via Phabricator via cfe-commits
fmayer added inline comments.



Comment at: compiler-rt/lib/hwasan/hwasan.cpp:205
+  if (registers_frame && stack->trace && stack->size > 0) {
+stack->trace++;
+stack->size--;

fmayer wrote:
> vitalybuka wrote:
> > maybe we should pop everything up to "pc" to avoid issues with nested calls?
> > 
> > For most users hwasan frames are not very useful. However if you work on 
> > sanitizer, some frames can be a useful info. So I don't mind we just relax 
> > test cases to accommodate this nesting.
> > 
> > cc @smd 
> This is probably for another patch though, right? This is already like this 
> on the LHS.
nevermind. i accidentally had this left, sent https://reviews.llvm.org/D131279 
for that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103562

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


[PATCH] D131280: [clang][dataflow] Parameterize analysis by explicit map of analyzable functions.

2022-08-05 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 450353.
ymandel added a comment.

add comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131280

Files:
  clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
  clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -11,12 +11,15 @@
 #include "clang/AST/Decl.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/FlowSensitive/ControlFlowContext.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
 #include "clang/Analysis/FlowSensitive/NoopAnalysis.h"
 #include "clang/Analysis/FlowSensitive/StorageLocation.h"
 #include "clang/Analysis/FlowSensitive/Value.h"
 #include "clang/Basic/LangStandard.h"
+#include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Testing/Support/Error.h"
@@ -40,11 +43,39 @@
 template 
 void runDataflow(llvm::StringRef Code, Matcher Match,
  DataflowAnalysisOptions Options,
+ llvm::StringRef ModeledFunctionsCode = {},
  LangStandard::Kind Std = LangStandard::lang_cxx17,
  llvm::StringRef TargetFun = "target") {
+  const std::vector Args = {
+  "-fsyntax-only", "-fno-delayed-template-parsing",
+  "-std=" +
+  std::string(LangStandard::getLangStandardForKind(Std).getName())};
+
+  auto MainASTUnit = tooling::buildASTFromCodeWithArgs(Code, Args);
+  ASSERT_NE(MainASTUnit, nullptr);
+
+  // The unique_ptr has to live outside the if-body since the lifetime of the
+  // function map is linked to the lifetime of the ASTUnit. The raw pointer
+  // allows us to select between the main TU and the (optional) model TU.
+  std::unique_ptr ModelASTUnit = nullptr;
+  ASTUnit *UnitPtr = nullptr;
+  if (ModeledFunctionsCode.empty()) {
+UnitPtr = MainASTUnit.get();
+  } else {
+ModelASTUnit =
+tooling::buildASTFromCodeWithArgs(ModeledFunctionsCode, Args);
+ASSERT_NE(ModelASTUnit, nullptr);
+UnitPtr = ModelASTUnit.get();
+  }
+
+  llvm::Expected> Result =
+  buildFunctionMapFromAST(*UnitPtr);
+  ASSERT_TRUE((bool)Result);
+  llvm::StringMap AnalyzableFunctions = std::move(*Result);
+
   ASSERT_THAT_ERROR(
   test::checkDataflow(
-  Code, TargetFun,
+  Code, std::move(MainASTUnit), ast_matchers::hasName(TargetFun),
   [Options](ASTContext &C, Environment &) {
 return NoopAnalysis(C, Options);
   },
@@ -53,18 +84,18 @@
   std::pair>>
   Results,
   ASTContext &ASTCtx) { Match(Results, ASTCtx); },
-  {"-fsyntax-only", "-fno-delayed-template-parsing",
-   "-std=" + std::string(
- LangStandard::getLangStandardForKind(Std).getName())}),
+  std::move(AnalyzableFunctions)),
   llvm::Succeeded());
 }
 
+// FIXME: inline this overload and remove it.
 template 
 void runDataflow(llvm::StringRef Code, Matcher Match,
  LangStandard::Kind Std = LangStandard::lang_cxx17,
  bool ApplyBuiltinTransfer = true,
  llvm::StringRef TargetFun = "target") {
-  runDataflow(Code, Match, {ApplyBuiltinTransfer, {}}, Std, TargetFun);
+  runDataflow(Code, Match, /*Options=*/{ApplyBuiltinTransfer, {}},
+  /*AnalyzableFunctions=*/{}, Std, TargetFun);
 }
 
 TEST(TransferTest, IntVarDeclNotTrackedWhenTransferDisabled) {
@@ -1320,9 +1351,19 @@
   }
 };
   )";
+
+  auto ASTUnit = tooling::buildASTFromCodeWithArgs(
+  Code,
+  /*Args=*/
+  {"-fsyntax-only", "-fno-delayed-template-parsing",
+   "-std=" + std::string(LangStandard::getLangStandardForKind(
+ LangStandard::lang_cxx17)
+ .getName())},
+  "input.cc", "clang-dataflow-test");
+
   ASSERT_THAT_ERROR(
   test::checkDataflow(
-  Code, cxxConstructorDecl(ofClass(hasName("B"))),
+  Code, std::move(ASTUnit), cxxConstructorDecl(ofClass(hasName("B"))),
   [](ASTContext &C, Environment &) {
 return NoopAnalysis(C, /*ApplyBuiltinTransfer=*/true);
 

[clang] d038bb1 - [clang] Fix redirection behaviour for cached FileEntryRef

2022-08-05 Thread Ben Langmuir via cfe-commits

Author: Ben Langmuir
Date: 2022-08-05T12:23:38-07:00
New Revision: d038bb196c51dcf80cbe771f4229b4e227c6c5b6

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

LOG: [clang] Fix redirection behaviour for cached FileEntryRef

In 6a79e2ff1989b we changed Filemanager::getEntryRef() to return the
redirecting FileEntryRef instead of looking through the redirection.
This commit fixes the case when looking up a cached file path to also
return the redirecting FileEntryRef. This mainly affects the behaviour
of calling getNameAsRequested() on the resulting entry ref.

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

Added: 


Modified: 
clang/lib/Basic/FileManager.cpp
clang/unittests/Basic/FileManagerTest.cpp

Removed: 




diff  --git a/clang/lib/Basic/FileManager.cpp b/clang/lib/Basic/FileManager.cpp
index cb719762ec7c8..4ef25358ec82f 100644
--- a/clang/lib/Basic/FileManager.cpp
+++ b/clang/lib/Basic/FileManager.cpp
@@ -212,13 +212,7 @@ FileManager::getFileRef(StringRef Filename, bool openFile, 
bool CacheFailure) {
 if (!SeenFileInsertResult.first->second)
   return llvm::errorCodeToError(
   SeenFileInsertResult.first->second.getError());
-// Construct and return and FileEntryRef, unless it's a redirect to another
-// filename.
-FileEntryRef::MapValue Value = *SeenFileInsertResult.first->second;
-if (LLVM_LIKELY(Value.V.is()))
-  return FileEntryRef(*SeenFileInsertResult.first);
-return FileEntryRef(*reinterpret_cast(
-Value.V.get()));
+return FileEntryRef(*SeenFileInsertResult.first);
   }
 
   // We've not seen this before. Fill it in.

diff  --git a/clang/unittests/Basic/FileManagerTest.cpp 
b/clang/unittests/Basic/FileManagerTest.cpp
index 419c497e95d68..6fe4a3d65d172 100644
--- a/clang/unittests/Basic/FileManagerTest.cpp
+++ b/clang/unittests/Basic/FileManagerTest.cpp
@@ -340,6 +340,7 @@ TEST_F(FileManagerTest, getFileRefEquality) {
   auto F1Again = manager.getFileRef("dir/f1.cpp");
   auto F1Also = manager.getFileRef("dir/f1-also.cpp");
   auto F1Redirect = manager.getFileRef("dir/f1-redirect.cpp");
+  auto F1RedirectAgain = manager.getFileRef("dir/f1-redirect.cpp");
   auto F2 = manager.getFileRef("dir/f2.cpp");
 
   // Check Expected for error.
@@ -347,6 +348,7 @@ TEST_F(FileManagerTest, getFileRefEquality) {
   ASSERT_FALSE(!F1Also);
   ASSERT_FALSE(!F1Again);
   ASSERT_FALSE(!F1Redirect);
+  ASSERT_FALSE(!F1RedirectAgain);
   ASSERT_FALSE(!F2);
 
   // Check names.
@@ -354,6 +356,7 @@ TEST_F(FileManagerTest, getFileRefEquality) {
   EXPECT_EQ("dir/f1.cpp", F1Again->getName());
   EXPECT_EQ("dir/f1-also.cpp", F1Also->getName());
   EXPECT_EQ("dir/f1.cpp", F1Redirect->getName());
+  EXPECT_EQ("dir/f1.cpp", F1RedirectAgain->getName());
   EXPECT_EQ("dir/f2.cpp", F2->getName());
 
   EXPECT_EQ("dir/f1.cpp", F1->getNameAsRequested());
@@ -363,6 +366,7 @@ TEST_F(FileManagerTest, getFileRefEquality) {
   EXPECT_EQ(&F1->getFileEntry(), *F1);
   EXPECT_EQ(*F1, &F1->getFileEntry());
   EXPECT_EQ(&F1->getFileEntry(), &F1Redirect->getFileEntry());
+  EXPECT_EQ(&F1->getFileEntry(), &F1RedirectAgain->getFileEntry());
   EXPECT_NE(&F2->getFileEntry(), *F1);
   EXPECT_NE(*F1, &F2->getFileEntry());
 
@@ -371,6 +375,7 @@ TEST_F(FileManagerTest, getFileRefEquality) {
   EXPECT_EQ(*F1, *F1Again);
   EXPECT_EQ(*F1, *F1Redirect);
   EXPECT_EQ(*F1Also, *F1Redirect);
+  EXPECT_EQ(*F1, *F1RedirectAgain);
   EXPECT_NE(*F2, *F1);
   EXPECT_NE(*F2, *F1Also);
   EXPECT_NE(*F2, *F1Again);
@@ -381,6 +386,7 @@ TEST_F(FileManagerTest, getFileRefEquality) {
   EXPECT_FALSE(F1->isSameRef(*F1Redirect));
   EXPECT_FALSE(F1->isSameRef(*F1Also));
   EXPECT_FALSE(F1->isSameRef(*F2));
+  EXPECT_TRUE(F1Redirect->isSameRef(*F1RedirectAgain));
 }
 
 // getFile() Should return the same entry as getVirtualFile if the file 
actually



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


[PATCH] D131273: [clang] Fix redirection behaviour for cached FileEntryRef

2022-08-05 Thread Ben Langmuir via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd038bb196c51: [clang] Fix redirection behaviour for cached 
FileEntryRef (authored by benlangmuir).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131273

Files:
  clang/lib/Basic/FileManager.cpp
  clang/unittests/Basic/FileManagerTest.cpp


Index: clang/unittests/Basic/FileManagerTest.cpp
===
--- clang/unittests/Basic/FileManagerTest.cpp
+++ clang/unittests/Basic/FileManagerTest.cpp
@@ -340,6 +340,7 @@
   auto F1Again = manager.getFileRef("dir/f1.cpp");
   auto F1Also = manager.getFileRef("dir/f1-also.cpp");
   auto F1Redirect = manager.getFileRef("dir/f1-redirect.cpp");
+  auto F1RedirectAgain = manager.getFileRef("dir/f1-redirect.cpp");
   auto F2 = manager.getFileRef("dir/f2.cpp");
 
   // Check Expected for error.
@@ -347,6 +348,7 @@
   ASSERT_FALSE(!F1Also);
   ASSERT_FALSE(!F1Again);
   ASSERT_FALSE(!F1Redirect);
+  ASSERT_FALSE(!F1RedirectAgain);
   ASSERT_FALSE(!F2);
 
   // Check names.
@@ -354,6 +356,7 @@
   EXPECT_EQ("dir/f1.cpp", F1Again->getName());
   EXPECT_EQ("dir/f1-also.cpp", F1Also->getName());
   EXPECT_EQ("dir/f1.cpp", F1Redirect->getName());
+  EXPECT_EQ("dir/f1.cpp", F1RedirectAgain->getName());
   EXPECT_EQ("dir/f2.cpp", F2->getName());
 
   EXPECT_EQ("dir/f1.cpp", F1->getNameAsRequested());
@@ -363,6 +366,7 @@
   EXPECT_EQ(&F1->getFileEntry(), *F1);
   EXPECT_EQ(*F1, &F1->getFileEntry());
   EXPECT_EQ(&F1->getFileEntry(), &F1Redirect->getFileEntry());
+  EXPECT_EQ(&F1->getFileEntry(), &F1RedirectAgain->getFileEntry());
   EXPECT_NE(&F2->getFileEntry(), *F1);
   EXPECT_NE(*F1, &F2->getFileEntry());
 
@@ -371,6 +375,7 @@
   EXPECT_EQ(*F1, *F1Again);
   EXPECT_EQ(*F1, *F1Redirect);
   EXPECT_EQ(*F1Also, *F1Redirect);
+  EXPECT_EQ(*F1, *F1RedirectAgain);
   EXPECT_NE(*F2, *F1);
   EXPECT_NE(*F2, *F1Also);
   EXPECT_NE(*F2, *F1Again);
@@ -381,6 +386,7 @@
   EXPECT_FALSE(F1->isSameRef(*F1Redirect));
   EXPECT_FALSE(F1->isSameRef(*F1Also));
   EXPECT_FALSE(F1->isSameRef(*F2));
+  EXPECT_TRUE(F1Redirect->isSameRef(*F1RedirectAgain));
 }
 
 // getFile() Should return the same entry as getVirtualFile if the file 
actually
Index: clang/lib/Basic/FileManager.cpp
===
--- clang/lib/Basic/FileManager.cpp
+++ clang/lib/Basic/FileManager.cpp
@@ -212,13 +212,7 @@
 if (!SeenFileInsertResult.first->second)
   return llvm::errorCodeToError(
   SeenFileInsertResult.first->second.getError());
-// Construct and return and FileEntryRef, unless it's a redirect to another
-// filename.
-FileEntryRef::MapValue Value = *SeenFileInsertResult.first->second;
-if (LLVM_LIKELY(Value.V.is()))
-  return FileEntryRef(*SeenFileInsertResult.first);
-return FileEntryRef(*reinterpret_cast(
-Value.V.get()));
+return FileEntryRef(*SeenFileInsertResult.first);
   }
 
   // We've not seen this before. Fill it in.


Index: clang/unittests/Basic/FileManagerTest.cpp
===
--- clang/unittests/Basic/FileManagerTest.cpp
+++ clang/unittests/Basic/FileManagerTest.cpp
@@ -340,6 +340,7 @@
   auto F1Again = manager.getFileRef("dir/f1.cpp");
   auto F1Also = manager.getFileRef("dir/f1-also.cpp");
   auto F1Redirect = manager.getFileRef("dir/f1-redirect.cpp");
+  auto F1RedirectAgain = manager.getFileRef("dir/f1-redirect.cpp");
   auto F2 = manager.getFileRef("dir/f2.cpp");
 
   // Check Expected for error.
@@ -347,6 +348,7 @@
   ASSERT_FALSE(!F1Also);
   ASSERT_FALSE(!F1Again);
   ASSERT_FALSE(!F1Redirect);
+  ASSERT_FALSE(!F1RedirectAgain);
   ASSERT_FALSE(!F2);
 
   // Check names.
@@ -354,6 +356,7 @@
   EXPECT_EQ("dir/f1.cpp", F1Again->getName());
   EXPECT_EQ("dir/f1-also.cpp", F1Also->getName());
   EXPECT_EQ("dir/f1.cpp", F1Redirect->getName());
+  EXPECT_EQ("dir/f1.cpp", F1RedirectAgain->getName());
   EXPECT_EQ("dir/f2.cpp", F2->getName());
 
   EXPECT_EQ("dir/f1.cpp", F1->getNameAsRequested());
@@ -363,6 +366,7 @@
   EXPECT_EQ(&F1->getFileEntry(), *F1);
   EXPECT_EQ(*F1, &F1->getFileEntry());
   EXPECT_EQ(&F1->getFileEntry(), &F1Redirect->getFileEntry());
+  EXPECT_EQ(&F1->getFileEntry(), &F1RedirectAgain->getFileEntry());
   EXPECT_NE(&F2->getFileEntry(), *F1);
   EXPECT_NE(*F1, &F2->getFileEntry());
 
@@ -371,6 +375,7 @@
   EXPECT_EQ(*F1, *F1Again);
   EXPECT_EQ(*F1, *F1Redirect);
   EXPECT_EQ(*F1Also, *F1Redirect);
+  EXPECT_EQ(*F1, *F1RedirectAgain);
   EXPECT_NE(*F2, *F1);
   EXPECT_NE(*F2, *F1Also);
   EXPECT_NE(*F2, *F1Again);
@@ -381,6 +386,7 @@
   EXPECT_FALSE(F1->isSameRef(*F1Redirect));
   EXPECT_FALSE(F1->isSameRef(*F1Also));
   EXPECT_FALSE(F1->isSameRef(*F2));
+  EXPECT_TRUE(F1Redirect->isSameRef(*F1RedirectAgain));
 }
 
 // getFile() Should return the same entry as getVirtualFile if the file actually

[PATCH] D131076: [clang][modules] Don't depend on sharing FileManager during module build

2022-08-05 Thread Ben Langmuir via Phabricator via cfe-commits
benlangmuir updated this revision to Diff 450356.
benlangmuir added a comment.

Split out the FileManager change into https://reviews.llvm.org/D131273


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

https://reviews.llvm.org/D131076

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Frontend/CompilerInstance.cpp
  clang/test/ClangScanDeps/modulemap-via-vfs.m
  clang/test/Modules/submodule-in-private-mmap-vfs.m
  clang/test/VFS/module-import.m

Index: clang/test/VFS/module-import.m
===
--- clang/test/VFS/module-import.m
+++ clang/test/VFS/module-import.m
@@ -1,6 +1,7 @@
-// RUN: rm -rf %t
+// RUN: rm -rf %t %t-unshared
 // RUN: sed -e "s@INPUT_DIR@%{/S:regex_replacement}/Inputs@g" -e "s@OUT_DIR@%{/t:regex_replacement}@g" %S/Inputs/vfsoverlay.yaml > %t.yaml
 // RUN: %clang_cc1 -Werror -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -ivfsoverlay %t.yaml -I %t -fsyntax-only %s
+// RUN: %clang_cc1 -Werror -fmodules -fno-modules-share-filemanager -fimplicit-module-maps -fmodules-cache-path=%t-unshared -ivfsoverlay %t.yaml -I %t -fsyntax-only %s
 
 @import not_real;
 
@@ -18,9 +19,12 @@
 // Override the module map (vfsoverlay2 on top)
 // RUN: sed -e "s@INPUT_DIR@%{/S:regex_replacement}/Inputs@g" -e "s@OUT_DIR@%{/t:regex_replacement}@g" %S/Inputs/vfsoverlay2.yaml > %t2.yaml
 // RUN: %clang_cc1 -Werror -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -ivfsoverlay %t.yaml -ivfsoverlay %t2.yaml -I %t -fsyntax-only %s
+// RUN: %clang_cc1 -Werror -fmodules -fno-modules-share-filemanager -fimplicit-module-maps -fmodules-cache-path=%t-unshared -ivfsoverlay %t.yaml -ivfsoverlay %t2.yaml -I %t -fsyntax-only %s
 
 // vfsoverlay2 not present
 // RUN: not %clang_cc1 -Werror -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -ivfsoverlay %t.yaml -I %t -fsyntax-only %s -DIMPORT2 2>&1 | FileCheck -check-prefix=CHECK-VFS2 %s
+// RUN: not %clang_cc1 -Werror -fmodules -fno-modules-share-filemanager -fimplicit-module-maps -fmodules-cache-path=%t-unshared -ivfsoverlay %t.yaml -I %t -fsyntax-only %s -DIMPORT2 2>&1 | FileCheck -check-prefix=CHECK-VFS2 %s
 
 // vfsoverlay2 on the bottom
 // RUN: not %clang_cc1 -Werror -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -ivfsoverlay %t2.yaml -ivfsoverlay %t.yaml -I %t -fsyntax-only %s -DIMPORT2 2>&1 | FileCheck -check-prefix=CHECK-VFS2 %s
+// RUN: not %clang_cc1 -Werror -fmodules -fno-modules-share-filemanager -fimplicit-module-maps -fmodules-cache-path=%t-unshared -ivfsoverlay %t2.yaml -ivfsoverlay %t.yaml -I %t -fsyntax-only %s -DIMPORT2 2>&1 | FileCheck -check-prefix=CHECK-VFS2 %s
Index: clang/test/Modules/submodule-in-private-mmap-vfs.m
===
--- /dev/null
+++ clang/test/Modules/submodule-in-private-mmap-vfs.m
@@ -0,0 +1,38 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s|DIR|%/t|g" %t/vfs.json.in > %t/vfs.json
+// RUN: %clang_cc1 -fmodules -fno-modules-share-filemanager -fimplicit-module-maps \
+// RUN:   -fmodules-cache-path=%t -I%t/Virtual -ivfsoverlay %t/vfs.json -fsyntax-only %t/tu.m -verify
+
+//--- Dir1/module.modulemap
+
+//--- Dir2/module.private.modulemap
+module Foo_Private {}
+
+//--- vfs.json.in
+{
+  'version': 0,
+  'use-external-names': true,
+  'roots': [
+{
+  'name': 'DIR/Virtual',
+  'type': 'directory',
+  'contents': [
+{
+  'name': 'module.modulemap',
+  'type': 'file',
+  'external-contents': 'DIR/Dir1/module.modulemap'
+},
+{
+  'name': 'module.private.modulemap',
+  'type': 'file',
+  'external-contents': 'DIR/Dir2/module.private.modulemap'
+}
+  ]
+}
+  ]
+}
+
+//--- tu.m
+@import Foo_Private;
+// expected-no-diagnostics
Index: clang/test/ClangScanDeps/modulemap-via-vfs.m
===
--- clang/test/ClangScanDeps/modulemap-via-vfs.m
+++ clang/test/ClangScanDeps/modulemap-via-vfs.m
@@ -2,13 +2,15 @@
 // RUN: split-file %s %t.dir
 // RUN: sed -e "s|DIR|%/t.dir|g" %t.dir/build/compile-commands.json.in > %t.dir/build/compile-commands.json
 // RUN: sed -e "s|DIR|%/t.dir|g" %t.dir/build/vfs.yaml.in > %t.dir/build/vfs.yaml
-// RUN: clang-scan-deps -compilation-database %t.dir/build/compile-commands.json -j 1 -format experimental-full \
+// RUN: clang-scan-deps -compilation-database %t.dir/build/compile-commands.json \
+// RUN:   -reuse-filemanager=0 -j 1 -format experimental-full \
 // RUN:   -mode preprocess-dependency-directives -generate-modules-path-args > %t.db
 // RUN: %deps-to-rsp %t.db --module-name=A > %t.A.cc1.rsp
 // RUN: cat %t.A.cc1.rsp | sed 's:\?:/:g' | FileCheck %s
 
 // CHECK-NOT: build/module.modulemap
 // CHECK: A/module.modulemap
+// CHECK-NOT: build/module.modulemap
 
 //--- build/compile-commands.json.in
 
@@ -17,6 +19,11 @@
   "directo

[clang] fb89cc0 - [clang][modules] Don't depend on sharing FileManager during module build

2022-08-05 Thread Ben Langmuir via cfe-commits

Author: Ben Langmuir
Date: 2022-08-05T12:24:40-07:00
New Revision: fb89cc0ddbd9a588ee15148451e7d0bcbc1ef411

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

LOG: [clang][modules] Don't depend on sharing FileManager during module build

Sharing the FileManager between the importer and the module build should
only be an optimization. Add a cc1 option -fno-modules-share-filemanager
to allow us to test this. Fix the path to modulemap files, which
previously depended on the shared FileManager when using path mapped to
an external file in a VFS.

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

Added: 
clang/test/Modules/submodule-in-private-mmap-vfs.m

Modified: 
clang/include/clang/Driver/Options.td
clang/include/clang/Frontend/FrontendOptions.h
clang/lib/Frontend/CompilerInstance.cpp
clang/test/ClangScanDeps/modulemap-via-vfs.m
clang/test/VFS/module-import.m

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 83dbcfd55d962..eaf8406d306f3 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -6032,6 +6032,9 @@ def fallow_pch_with_
diff erent_modules_cache_path :
   Flag<["-"], "fallow-pch-with-
diff erent-modules-cache-path">,
   HelpText<"Accept a PCH file that was created with a 
diff erent modules cache path">,
   
MarshallingInfoFlag>;
+def fno_modules_share_filemanager : Flag<["-"], 
"fno-modules-share-filemanager">,
+  HelpText<"Disable sharing the FileManager when building a module 
implicitly">,
+  MarshallingInfoNegativeFlag>;
 def dump_deserialized_pch_decls : Flag<["-"], "dump-deserialized-decls">,
   HelpText<"Dump declarations that are deserialized from PCH, for testing">,
   MarshallingInfoFlag>;

diff  --git a/clang/include/clang/Frontend/FrontendOptions.h 
b/clang/include/clang/Frontend/FrontendOptions.h
index 0c9c2489e4486..56e704d8a52f4 100644
--- a/clang/include/clang/Frontend/FrontendOptions.h
+++ b/clang/include/clang/Frontend/FrontendOptions.h
@@ -356,6 +356,9 @@ class FrontendOptions {
   /// Output (and read) PCM files regardless of compiler errors.
   unsigned AllowPCMWithCompilerErrors : 1;
 
+  /// Whether to share the FileManager when building modules.
+  unsigned ModulesShareFileManager : 1;
+
   CodeCompleteOptions CodeCompleteOpts;
 
   /// Specifies the output format of the AST.
@@ -523,7 +526,7 @@ class FrontendOptions {
 BuildingImplicitModuleUsesLock(true), ModulesEmbedAllFiles(false),
 IncludeTimestamps(true), UseTemporary(true),
 OutputPathIndependentPCM(false), AllowPCMWithCompilerErrors(false),
-TimeTraceGranularity(500) {}
+ModulesShareFileManager(true), TimeTraceGranularity(500) {}
 
   /// getInputKindForExtension - Return the appropriate input kind for a file
   /// extension. For example, "c" would return Language::C.

diff  --git a/clang/lib/Frontend/CompilerInstance.cpp 
b/clang/lib/Frontend/CompilerInstance.cpp
index 2cd7efd862eca..51c7bc237a502 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -1213,11 +1213,16 @@ compileModuleImpl(CompilerInstance &ImportingInstance, 
SourceLocation ImportLoc,
ImportingInstance.getDiagnosticClient()),
  /*ShouldOwnClient=*/true);
 
-  // Note that this module is part of the module build stack, so that we
-  // can detect cycles in the module graph.
-  Instance.setFileManager(&ImportingInstance.getFileManager());
+  if (FrontendOpts.ModulesShareFileManager) {
+Instance.setFileManager(&ImportingInstance.getFileManager());
+  } else {
+Instance.createFileManager(&ImportingInstance.getVirtualFileSystem());
+  }
   Instance.createSourceManager(Instance.getFileManager());
   SourceManager &SourceMgr = Instance.getSourceManager();
+
+  // Note that this module is part of the module build stack, so that we
+  // can detect cycles in the module graph.
   SourceMgr.setModuleBuildStack(
 ImportingInstance.getSourceManager().getModuleBuildStack());
   SourceMgr.pushModuleBuildStack(ModuleName,
@@ -1303,12 +1308,16 @@ static bool compileModule(CompilerInstance 
&ImportingInstance,
 ModuleMapFile, ImportingInstance.getFileManager()))
   ModuleMapFile = PublicMMFile;
 
+// FIXME: Update header search to keep FileEntryRef rather than rely on
+// getLastRef().
+StringRef ModuleMapFilePath =
+ModuleMapFile->getLastRef().getNameAsRequested();
+
 // Use the module map where this module resides.
 Result = compileModuleImpl(
 ImportingInstance, ImportLoc, Module->getTopLevelModuleName(),
-FrontendInputFile(ModuleMapFile->getName(), IK, +Module->IsSystem),
-ModMa

[PATCH] D131076: [clang][modules] Don't depend on sharing FileManager during module build

2022-08-05 Thread Ben Langmuir 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 rGfb89cc0ddbd9: [clang][modules] Don't depend on sharing 
FileManager during module build (authored by benlangmuir).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131076

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Frontend/CompilerInstance.cpp
  clang/test/ClangScanDeps/modulemap-via-vfs.m
  clang/test/Modules/submodule-in-private-mmap-vfs.m
  clang/test/VFS/module-import.m

Index: clang/test/VFS/module-import.m
===
--- clang/test/VFS/module-import.m
+++ clang/test/VFS/module-import.m
@@ -1,6 +1,7 @@
-// RUN: rm -rf %t
+// RUN: rm -rf %t %t-unshared
 // RUN: sed -e "s@INPUT_DIR@%{/S:regex_replacement}/Inputs@g" -e "s@OUT_DIR@%{/t:regex_replacement}@g" %S/Inputs/vfsoverlay.yaml > %t.yaml
 // RUN: %clang_cc1 -Werror -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -ivfsoverlay %t.yaml -I %t -fsyntax-only %s
+// RUN: %clang_cc1 -Werror -fmodules -fno-modules-share-filemanager -fimplicit-module-maps -fmodules-cache-path=%t-unshared -ivfsoverlay %t.yaml -I %t -fsyntax-only %s
 
 @import not_real;
 
@@ -18,9 +19,12 @@
 // Override the module map (vfsoverlay2 on top)
 // RUN: sed -e "s@INPUT_DIR@%{/S:regex_replacement}/Inputs@g" -e "s@OUT_DIR@%{/t:regex_replacement}@g" %S/Inputs/vfsoverlay2.yaml > %t2.yaml
 // RUN: %clang_cc1 -Werror -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -ivfsoverlay %t.yaml -ivfsoverlay %t2.yaml -I %t -fsyntax-only %s
+// RUN: %clang_cc1 -Werror -fmodules -fno-modules-share-filemanager -fimplicit-module-maps -fmodules-cache-path=%t-unshared -ivfsoverlay %t.yaml -ivfsoverlay %t2.yaml -I %t -fsyntax-only %s
 
 // vfsoverlay2 not present
 // RUN: not %clang_cc1 -Werror -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -ivfsoverlay %t.yaml -I %t -fsyntax-only %s -DIMPORT2 2>&1 | FileCheck -check-prefix=CHECK-VFS2 %s
+// RUN: not %clang_cc1 -Werror -fmodules -fno-modules-share-filemanager -fimplicit-module-maps -fmodules-cache-path=%t-unshared -ivfsoverlay %t.yaml -I %t -fsyntax-only %s -DIMPORT2 2>&1 | FileCheck -check-prefix=CHECK-VFS2 %s
 
 // vfsoverlay2 on the bottom
 // RUN: not %clang_cc1 -Werror -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -ivfsoverlay %t2.yaml -ivfsoverlay %t.yaml -I %t -fsyntax-only %s -DIMPORT2 2>&1 | FileCheck -check-prefix=CHECK-VFS2 %s
+// RUN: not %clang_cc1 -Werror -fmodules -fno-modules-share-filemanager -fimplicit-module-maps -fmodules-cache-path=%t-unshared -ivfsoverlay %t2.yaml -ivfsoverlay %t.yaml -I %t -fsyntax-only %s -DIMPORT2 2>&1 | FileCheck -check-prefix=CHECK-VFS2 %s
Index: clang/test/Modules/submodule-in-private-mmap-vfs.m
===
--- /dev/null
+++ clang/test/Modules/submodule-in-private-mmap-vfs.m
@@ -0,0 +1,38 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s|DIR|%/t|g" %t/vfs.json.in > %t/vfs.json
+// RUN: %clang_cc1 -fmodules -fno-modules-share-filemanager -fimplicit-module-maps \
+// RUN:   -fmodules-cache-path=%t -I%t/Virtual -ivfsoverlay %t/vfs.json -fsyntax-only %t/tu.m -verify
+
+//--- Dir1/module.modulemap
+
+//--- Dir2/module.private.modulemap
+module Foo_Private {}
+
+//--- vfs.json.in
+{
+  'version': 0,
+  'use-external-names': true,
+  'roots': [
+{
+  'name': 'DIR/Virtual',
+  'type': 'directory',
+  'contents': [
+{
+  'name': 'module.modulemap',
+  'type': 'file',
+  'external-contents': 'DIR/Dir1/module.modulemap'
+},
+{
+  'name': 'module.private.modulemap',
+  'type': 'file',
+  'external-contents': 'DIR/Dir2/module.private.modulemap'
+}
+  ]
+}
+  ]
+}
+
+//--- tu.m
+@import Foo_Private;
+// expected-no-diagnostics
Index: clang/test/ClangScanDeps/modulemap-via-vfs.m
===
--- clang/test/ClangScanDeps/modulemap-via-vfs.m
+++ clang/test/ClangScanDeps/modulemap-via-vfs.m
@@ -2,13 +2,15 @@
 // RUN: split-file %s %t.dir
 // RUN: sed -e "s|DIR|%/t.dir|g" %t.dir/build/compile-commands.json.in > %t.dir/build/compile-commands.json
 // RUN: sed -e "s|DIR|%/t.dir|g" %t.dir/build/vfs.yaml.in > %t.dir/build/vfs.yaml
-// RUN: clang-scan-deps -compilation-database %t.dir/build/compile-commands.json -j 1 -format experimental-full \
+// RUN: clang-scan-deps -compilation-database %t.dir/build/compile-commands.json \
+// RUN:   -reuse-filemanager=0 -j 1 -format experimental-full \
 // RUN:   -mode preprocess-dependency-directives -generate-modules-path-args > %t.db
 // RUN: %deps-to-rsp %t.db --module-name=A > %t.A.cc1.rsp
 // RUN: cat %t.A.cc1.rsp | sed 's:\?:/:g' | FileCheck %s
 
 // CHECK-NOT: build/module

[PATCH] D121593: [clangd][WIP] Provide clang-include-cleaner

2022-08-05 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment.

Any progress?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121593

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


[PATCH] D131241: [Clang][Lex] Extend HeaderSearch::LookupFile to control OpenFile behavior.

2022-08-05 Thread Jan Svoboda via Phabricator via cfe-commits
jansvoboda11 accepted this revision.
jansvoboda11 added a comment.
This revision is now accepted and ready to land.

LGTM. It'd be nice to have some tests for this, but since it's just forwarding 
arguments to `FileManager` I'm also fine with this as is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131241

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


[clang-tools-extra] 13bc713 - fixes clang-tidy/checks/list.rst: a line was accidentally removed in 95a92995d45fc6fada43ecd91eba3e7aea90487a

2022-08-05 Thread Rashmi Mudduluru via cfe-commits

Author: Rashmi Mudduluru
Date: 2022-08-05T12:36:03-07:00
New Revision: 13bc71310920dced155328e25aa382dc2bb1ef9f

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

LOG: fixes clang-tidy/checks/list.rst: a line was accidentally removed in 
95a92995d45fc6fada43ecd91eba3e7aea90487a

Added: 


Modified: 
clang-tools-extra/docs/clang-tidy/checks/list.rst

Removed: 




diff  --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst 
b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index b72e3ca69c39d..a7c6247ab96bf 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -239,6 +239,7 @@ Clang-Tidy Checks
`llvmlibc-implementation-in-namespace 
`_,
`llvmlibc-restrict-system-libc-headers 
`_, "Yes"
`misc-confusable-identifiers `_,
+   `misc-const-correctness `_, "Yes"
`misc-definitions-in-headers `_, "Yes"
`misc-misleading-bidirectional `_,
`misc-misleading-identifier `_,



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


[PATCH] D130793: [clang-tidy] adjust treating of array-of-pointers when 'AnalyzePointers' is deactivated

2022-08-05 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 450358.
JonasToth added a comment.

- add test with typedef
- [docs] improve documentation for misc-const-correctness


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130793

Files:
  clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp
  clang-tools-extra/docs/clang-tidy/checks/misc/const-correctness.rst
  
clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-pointer-as-values.cpp
  clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-values.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-values.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-values.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-values.cpp
@@ -526,18 +526,11 @@
   // CHECK-FIXES: int const p_local1[2]
   for (const int &const_ref : p_local1) {
   }
+}
 
-  int *p_local2[2] = {&np_local0[0], &np_local0[1]};
-  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local2' of type 'int *[2]' can be declared 'const'
-  // CHECK-FIXES: int *const p_local2[2]
-  for (const int *con_ptr : p_local2) {
-  }
-
-  int *p_local3[2] = {nullptr, nullptr};
-  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local3' of type 'int *[2]' can be declared 'const'
-  // CHECK-FIXES: int *const p_local3[2]
-  for (const auto *con_ptr : p_local3) {
-  }
+void arrays_of_pointers_are_ignored() {
+  int *np_local0[2] = {nullptr, nullptr};
+  // CHECK-NOT-FIXES: int * const np_local0[2]
 }
 
 inline void *operator new(decltype(sizeof(void *)), void *p) { return p; }
@@ -908,41 +901,6 @@
   sizeof(int[++N]);
 }
 
-template 
-struct SmallVectorBase {
-  T data[4];
-  void push_back(const T &el) {}
-  int size() const { return 4; }
-  T *begin() { return data; }
-  const T *begin() const { return data; }
-  T *end() { return data + 4; }
-  const T *end() const { return data + 4; }
-};
-
-template 
-struct SmallVector : SmallVectorBase {};
-
-template 
-void EmitProtocolMethodList(T &&Methods) {
-  // Note: If the template is uninstantiated the analysis does not figure out,
-  // that p_local0 could be const. Not sure why, but probably bails because
-  // some expressions are type-dependent.
-  SmallVector p_local0;
-  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'SmallVector' can be declared 'const'
-  // CHECK-FIXES: SmallVector const p_local0
-  SmallVector np_local0;
-  for (const auto *I : Methods) {
-if (I == nullptr)
-  np_local0.push_back(I);
-  }
-  p_local0.size();
-}
-void instantiate() {
-  int *p_local0[4] = {nullptr, nullptr, nullptr, nullptr};
-  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int *[4]' can be declared 'const'
-  // CHECK-FIXES: int *const p_local0[4]
-  EmitProtocolMethodList(p_local0);
-}
 struct base {
   int member;
 };
Index: clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-pointer-as-values.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-pointer-as-values.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-pointer-as-values.cpp
@@ -10,4 +10,65 @@
   double *p_local0 = &np_local0[1];
   // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'double *' can be declared 'const'
   // CHECK-FIXES: double *const p_local0
+
+  using doublePtr = double*;
+  using doubleArray = double[15];
+  doubleArray np_local1;
+  doublePtr p_local1 = &np_local1[0];
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'doublePtr' (aka 'double *') can be declared 'const'
+  // CHECK-FIXES: doublePtr const p_local1
+}
+
+void range_for() {
+  int np_local0[2] = {1, 2};
+  int *p_local0[2] = {&np_local0[0], &np_local0[1]};
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int *[2]' can be declared 'const'
+  // CHECK-FIXES: int *const p_local0[2]
+  for (const int *p_local1 : p_local0) {
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: variable 'p_local1' of type 'const int *' can be declared 'const'
+  // CHECK-FIXES: for (const int *const p_local1 : p_local0)
+  }
+
+  int *p_local2[2] = {nullptr, nullptr};
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local2' of type 'int *[2]' can be declared 'const'
+  // CHECK-FIXES: int *const p_local2[2]
+  for (const auto *con_ptr : p_local2) {
+  }
+
+}
+
+template 
+struct SmallVectorBase {
+  T data[4];
+  void push_back(const T &el) {}
+  int size() const { return 4; }
+  T *begin() { return data; }
+  const T *begin() const { return data; }
+  T *end() { return data + 4; }
+  const T *end() const { return data + 4; }
+};
+
+template 
+struct SmallVector : SmallVectorBase {};
+
+template 
+void EmitProtocolMethodList(T &&Methods) {
+  // Note: 

  1   2   >