[PATCH] D111863: [libunwind] Add an interface for dynamic .eh_frame registration

[Lang Hames via Phabricator via cfe-commits]

lhames added a comment.

@joerg, @emaste -- I finally got a chance to experiment with a Linux docker 
container and confirmed that libgcc_s's `__register_frame` can handle 
individual frames. Unfortunately that does not help us on FreeBSD.

If we ignore FreeBSD for a moment we could imagine switch to registering single 
frames everywhere, but this would be an unrecoverable performance regression on 
Linux -- we would necessarily have to walk all CFI entries in the section to 
use the API.

One possible alternative solution would be to update libunwind's 
__register_ehframe function to also accept whole sections the same way libgcc_s 
does, but I don't think this is a good solution: We'd be overloading the 
behavior in a surprising way to match a libgcc_s behavior that doesn't seem 
well-documented, and we'd still be left needing some other way to detect 
whether the current unwinder supported the desired semantics.

I think the ideal solution would be to always pass in whole sections, as 
@housel's proposed solution does.

I think we can debate the right API signature (e.g. should it pass the section 
range so that we really can skip walking the CFI records, or is this cheap 
enough not to bother?), function name, and implementation, but does anyone have 
any objection to the direction? If so, why?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111863

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

[PATCH] D112926: run-clang-tidy.py analyze unique files only

[Serikzhan via Phabricator via cfe-commits]

serkazi added a comment.

In D112926#3113220 , @salman-javed-nz 
wrote:

> In D112926#3113206 , @serkazi wrote:
>
>> This is now accepted, please feel free to merge on my behalf. Thanks.
>
> What's the full name and email you want associated with your commit?

Let it be "Serikzhan Kazi se7k...@gmail.com"?


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

https://reviews.llvm.org/D112926

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

[PATCH] D112926: run-clang-tidy.py analyze unique files only

[Salman Javed via Phabricator via cfe-commits]

salman-javed-nz added a comment.

In D112926#3113206 , @serkazi wrote:

> This is now accepted, please feel free to merge on my behalf. Thanks.

What's the full name and email you want associated with your commit?


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

https://reviews.llvm.org/D112926

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

[PATCH] D112926: run-clang-tidy.py analyze unique files only

[Serikzhan via Phabricator via cfe-commits]

serkazi added a comment.

In D112926#3108140 , @salman-javed-nz 
wrote:

> It looks good to me. I don't have any more comments to add - it is a very 
> small code change after all.
> I have commit access and am happy to commit it on your behalf.
>
> However, this would be my first time as a reviewer, and I don't want to be 
> presumptuous and approve this before someone with more experience in the area 
> has had a chance to look at it.
>
> The rule of thumb is to wait a week before sending a ping. Then we'll see 
> what we can do to get this through.

This is now accepted, please feel free to merge on my behalf. Thanks.


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

https://reviews.llvm.org/D112926

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

[PATCH] D112916: Confusable identifiers detection

[Tom Stellard via Phabricator via cfe-commits]

tstellar requested changes to this revision.
tstellar added a comment.
This revision now requires changes to proceed.

The LLVM Board of Directors will do a legal review of this change.  We will 
give an update in 4-6 weeks.


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

https://reviews.llvm.org/D112916

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

[PATCH] D108479: [Clang] Add __builtin_addressof_nocfi

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D108479#3113055 , @pcc wrote:

> (Adding back @rsmith, @rjmccall.)
>
> In D108479#3113035 , @samitolvanen 
> wrote:
>
>> In D108479#3112492 , @rjmccall 
>> wrote:
>>
>>> You could also make this Just Work for things like C++ member functions 
>>> rather than producing a member function pointer.
>>
>> I'm not sure I understand. What does Just Work mean when it comes to C++ 
>> member functions?
>
> I think he means that e.g. `__builtin_symbol_address(::bar)` should 
> return a `void*` pointing to the address of the `Foo::bar` member function 
> body, instead of a member function pointer for `Foo::bar`.

Yes, exactly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108479

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

[PATCH] D112868: [Sema] Diagnose and reject non-function ifunc resolvers

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein updated this revision to Diff 385212.
ibookstein added a comment.

Nicer param order for checkAliasedGlobal..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112868

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/Sema/attr-ifunc.c

Index: clang/test/Sema/attr-ifunc.c
===
--- clang/test/Sema/attr-ifunc.c
+++ clang/test/Sema/attr-ifunc.c
@@ -13,8 +13,7 @@
 void f1() __attribute__((ifunc("f1_ifunc")));
 //expected-error@-1 {{ifunc must point to a defined function}}
 
-void* f2_a() __attribute__((ifunc("f2_b")));
-//expected-error@-1 {{ifunc definition is part of a cycle}}
+void *f2_a() __attribute__((alias("f2_b")));
 void* f2_b() __attribute__((ifunc("f2_a")));
 //expected-error@-1 {{ifunc definition is part of a cycle}}
 
@@ -27,6 +26,15 @@
 void f4() __attribute__((ifunc("f4_ifunc")));
 //expected-error@-1 {{ifunc resolver function must return a pointer}}
 
+int f5_resolver_gvar;
+void f5() __attribute__((ifunc("f5_resolver_gvar")));
+// expected-error@-1 {{ifunc must point to a defined function}}
+
+void *f6_resolver_resolver() { return 0; }
+void *f6_resolver() __attribute__((ifunc("f6_resolver_resolver")));
+void f6() __attribute__((ifunc("f6_resolver")));
+// expected-error@-1 {{ifunc must point to a defined function}}
+
 #else
 void f1a() __asm("f1");
 void f1a() {}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -318,21 +318,58 @@
 // This is only used in aliases that we created and we know they have a
 // linear structure.
 static const llvm::GlobalValue *getAliasedGlobal(const llvm::GlobalValue *GV) {
-  llvm::SmallPtrSet Visited;
-  for (;;) {
-if (!GV || !Visited.insert(GV).second)
-  return nullptr;
-
-const llvm::Constant *C;
-if (auto *GA = dyn_cast(GV))
-  C = GA->getAliasee();
-else if (auto *GI = dyn_cast(GV))
-  C = GI->getResolver();
-else
-  return GV;
+  const llvm::Constant *C;
+  if (auto *GA = dyn_cast(GV))
+C = GA->getAliasee();
+  else if (auto *GI = dyn_cast(GV))
+C = GI->getResolver();
+  else
+return GV;
+
+  const auto *AliaseeGV = dyn_cast(C->stripPointerCasts());
+  if (!AliaseeGV)
+return nullptr;
+
+  const llvm::GlobalValue *FinalGV = AliaseeGV->getAliaseeObject();
+  if (FinalGV == GV)
+return nullptr;
+
+  return FinalGV;
+}
+
+static bool checkAliasedGlobal(DiagnosticsEngine ,
+   SourceLocation Location,
+   bool IsIFunc,
+   const llvm::GlobalValue *Alias,
+   const llvm::GlobalValue *) {
+  GV = getAliasedGlobal(Alias);
+  if (!GV) {
+Diags.Report(Location, diag::err_cyclic_alias) << IsIFunc;
+return false;
+  }
+
+  if (GV->isDeclaration()) {
+Diags.Report(Location, diag::err_alias_to_undefined) << IsIFunc << IsIFunc;
+return false;
+  }
+
+  if (IsIFunc) {
+// Check resolver function type.
+const auto *F = dyn_cast(GV);
+if (!F) {
+  Diags.Report(Location, diag::err_alias_to_undefined)
+  << IsIFunc << IsIFunc;
+  return false;
+}
 
-GV = dyn_cast(C->stripPointerCasts());
+llvm::FunctionType *FTy = F->getFunctionType();
+if (!FTy->getReturnType()->isPointerTy()) {
+  Diags.Report(Location, diag::err_ifunc_resolver_return);
+  return false;
+}
   }
+
+  return true;
 }
 
 void CodeGenModule::checkAliases() {
@@ -349,23 +386,13 @@
   Location = A->getLocation();
 else
   llvm_unreachable("Not an alias or ifunc?");
+
 StringRef MangledName = getMangledName(GD);
 llvm::GlobalValue *Alias = GetGlobalValue(MangledName);
-const llvm::GlobalValue *GV = getAliasedGlobal(Alias);
-if (!GV) {
-  Error = true;
-  Diags.Report(Location, diag::err_cyclic_alias) << IsIFunc;
-} else if (GV->isDeclaration()) {
+const llvm::GlobalValue *GV = nullptr;
+if (!checkAliasedGlobal(Diags, Location, IsIFunc, Alias, GV)) {
   Error = true;
-  Diags.Report(Location, diag::err_alias_to_undefined)
-  << IsIFunc << IsIFunc;
-} else if (IsIFunc) {
-  // Check resolver function type.
-  llvm::FunctionType *FTy = dyn_cast(
-  GV->getType()->getPointerElementType());
-  assert(FTy);
-  if (!FTy->getReturnType()->isPointerTy())
-Diags.Report(Location, diag::err_ifunc_resolver_return);
+  continue;
 }
 
 llvm::Constant *Aliasee =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112868: [Sema] Diagnose and reject non-function ifunc resolvers

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein updated this revision to Diff 385211.
ibookstein added a comment.

Changed to using D->hasAttr() and passing that
bool into the checkAliasedGlobal function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112868

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/Sema/attr-ifunc.c

Index: clang/test/Sema/attr-ifunc.c
===
--- clang/test/Sema/attr-ifunc.c
+++ clang/test/Sema/attr-ifunc.c
@@ -13,8 +13,7 @@
 void f1() __attribute__((ifunc("f1_ifunc")));
 //expected-error@-1 {{ifunc must point to a defined function}}
 
-void* f2_a() __attribute__((ifunc("f2_b")));
-//expected-error@-1 {{ifunc definition is part of a cycle}}
+void *f2_a() __attribute__((alias("f2_b")));
 void* f2_b() __attribute__((ifunc("f2_a")));
 //expected-error@-1 {{ifunc definition is part of a cycle}}
 
@@ -27,6 +26,15 @@
 void f4() __attribute__((ifunc("f4_ifunc")));
 //expected-error@-1 {{ifunc resolver function must return a pointer}}
 
+int f5_resolver_gvar;
+void f5() __attribute__((ifunc("f5_resolver_gvar")));
+// expected-error@-1 {{ifunc must point to a defined function}}
+
+void *f6_resolver_resolver() { return 0; }
+void *f6_resolver() __attribute__((ifunc("f6_resolver_resolver")));
+void f6() __attribute__((ifunc("f6_resolver")));
+// expected-error@-1 {{ifunc must point to a defined function}}
+
 #else
 void f1a() __asm("f1");
 void f1a() {}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -318,21 +318,58 @@
 // This is only used in aliases that we created and we know they have a
 // linear structure.
 static const llvm::GlobalValue *getAliasedGlobal(const llvm::GlobalValue *GV) {
-  llvm::SmallPtrSet Visited;
-  for (;;) {
-if (!GV || !Visited.insert(GV).second)
-  return nullptr;
-
-const llvm::Constant *C;
-if (auto *GA = dyn_cast(GV))
-  C = GA->getAliasee();
-else if (auto *GI = dyn_cast(GV))
-  C = GI->getResolver();
-else
-  return GV;
+  const llvm::Constant *C;
+  if (auto *GA = dyn_cast(GV))
+C = GA->getAliasee();
+  else if (auto *GI = dyn_cast(GV))
+C = GI->getResolver();
+  else
+return GV;
+
+  const auto *AliaseeGV = dyn_cast(C->stripPointerCasts());
+  if (!AliaseeGV)
+return nullptr;
+
+  const llvm::GlobalValue *FinalGV = AliaseeGV->getAliaseeObject();
+  if (FinalGV == GV)
+return nullptr;
+
+  return FinalGV;
+}
+
+static bool checkAliasedGlobal(bool IsIFunc,
+   DiagnosticsEngine ,
+   SourceLocation Location,
+   const llvm::GlobalValue *Alias,
+   const llvm::GlobalValue *) {
+  GV = getAliasedGlobal(Alias);
+  if (!GV) {
+Diags.Report(Location, diag::err_cyclic_alias) << IsIFunc;
+return false;
+  }
+
+  if (GV->isDeclaration()) {
+Diags.Report(Location, diag::err_alias_to_undefined) << IsIFunc << IsIFunc;
+return false;
+  }
+
+  if (IsIFunc) {
+// Check resolver function type.
+const auto *F = dyn_cast(GV);
+if (!F) {
+  Diags.Report(Location, diag::err_alias_to_undefined)
+  << IsIFunc << IsIFunc;
+  return false;
+}
 
-GV = dyn_cast(C->stripPointerCasts());
+llvm::FunctionType *FTy = F->getFunctionType();
+if (!FTy->getReturnType()->isPointerTy()) {
+  Diags.Report(Location, diag::err_ifunc_resolver_return);
+  return false;
+}
   }
+
+  return true;
 }
 
 void CodeGenModule::checkAliases() {
@@ -349,23 +386,13 @@
   Location = A->getLocation();
 else
   llvm_unreachable("Not an alias or ifunc?");
+
 StringRef MangledName = getMangledName(GD);
 llvm::GlobalValue *Alias = GetGlobalValue(MangledName);
-const llvm::GlobalValue *GV = getAliasedGlobal(Alias);
-if (!GV) {
-  Error = true;
-  Diags.Report(Location, diag::err_cyclic_alias) << IsIFunc;
-} else if (GV->isDeclaration()) {
+const llvm::GlobalValue *GV = nullptr;
+if (!checkAliasedGlobal(IsIFunc, Diags, Location, Alias, GV)) {
   Error = true;
-  Diags.Report(Location, diag::err_alias_to_undefined)
-  << IsIFunc << IsIFunc;
-} else if (IsIFunc) {
-  // Check resolver function type.
-  llvm::FunctionType *FTy = dyn_cast(
-  GV->getType()->getPointerElementType());
-  assert(FTy);
-  if (!FTy->getReturnType()->isPointerTy())
-Diags.Report(Location, diag::err_ifunc_resolver_return);
+  continue;
 }
 
 llvm::Constant *Aliasee =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D109128: [VFS] Use original path when falling back to external FS

[Keith Smiley via Phabricator via cfe-commits]

keith added a comment.

@dexonsmith can you take another look?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109128

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

[PATCH] D108479: [Clang] Add __builtin_addressof_nocfi

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added subscribers: rjmccall, rsmith.
pcc added a comment.

(Adding back @rsmith, @rjmccall.)

In D108479#3113035 , @samitolvanen 
wrote:

> In D108479#3112492 , @rjmccall 
> wrote:
>
>> You could also make this Just Work for things like C++ member functions 
>> rather than producing a member function pointer.
>
> I'm not sure I understand. What does Just Work mean when it comes to C++ 
> member functions?

I think he means that e.g. `__builtin_symbol_address(::bar)` should return 
a `void*` pointing to the address of the `Foo::bar` member function body, 
instead of a member function pointer for `Foo::bar`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108479

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

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein added a comment.

I feel like we're getting lost in the weeds here.

At the time a bitcode module is finalized, it is supposed to be in a valid 
state. 
The LLVM bitcode verifier does not consider GlobalAliases which have either a 
null or an undefined aliasee to be valid. The same should have held for 
GlobalIFuncs and their resolvers since their inception, and the fact that it 
were not so is an oversight.

There are two separate issues here which we need to decouple:

- IFuncs with undefined resolvers were never valid, the verification just 
happened to be missing. They also misbehave, as demonstrated by my example 
above where a TU contains both cpu_specific and a usage, but no cpu_dispatch. 
Therefore, we'd not be making anything worse by plugging the current hole in 
the verification of GlobalIFunc and keeping cpu_specific/cpu_dispatch working, 
using the same method we use to handle aliases or ifuncs that come after 
declarations, i.e. cpu_specific emits plain function declarations, cpu_dispatch 
upgrades them via takeName+RAUW+eraseFromParent if they already exist. We'll 
have made the verification more robust, fixed a bug when there's a TU where 
there's cpu_specific + usage but no cpu_dispatch, and not have incurred more 
tech debt by doing so (since that tech debt already exists, and a solution 
would need to address all these cases in exactly the same way).
- Clang-repl/CGM needs infrastructure for dealing with this sort of 
'backtracking' in incremental compilation use-cases (declaration gets upgraded 
to alias, declaration gets upgraded to ifunc). If it needs IR changes for that, 
then they should be designed, agreed upon, and integrated. We're not going to 
have a decision made in this closed PR discussion that either GlobalAliases or 
GlobalIFuncs support a declaration state all of a sudden. Such decisions could 
have cross-cutting ramifications in that there would all of a sudden be 3 ways 
to represent things that are equivalent to/get lowered to function 
declarations, rather than one. Limiting ourselves to not using the existing 
solution for these use-cases/bugs in normal compilation with the hope that it 
will ease the creation of a solution for incremental compilation of the same 
use-cases is self-defeating.

As for clang-repl, I've so far tried the following; I get the sense that I'm 
poking around in less well-specified places (asserts and null-deref crashes):

  itay ~/llvm-project/build (main)> bin/clang-repl
  clang-repl> #include 
  clang-repl> __attribute__((cpu_dispatch(generic))) void a(void);
  clang-repl> __attribute__((cpu_specific(generic))) void a(void) { puts("hi"); 
}
  In file included from <<< inputs >>>:1:
  input_line_2:1:55: warning: body of cpu_dispatch function will be ignored 
[-Wfunction-multiversion]
  __attribute__((cpu_specific(generic))) void a(void) { puts("hi"); }
^
  clang-repl> auto b = (a(), 5);
  clang-repl: 
/home/itay/llvm-project/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1683: void 
llvm::AsmPrinter::emitGlobalIFunc(llvm::Module &, const llvm::GlobalIFunc &): 
Assertion `GI.hasLocalLinkage() && "Invalid ifunc linkage"' failed.
  fish: 'bin/clang-repl' terminated by signal SIGABRT (Abort)
  itay ~/llvm-project/build (main) [SIGABRT]> bin/clang-repl
  clang-repl> extern int g_a __attribute__((alias("g_b")));
  In file included from <<< inputs >>>:1:
  input_line_0:1:31: error: alias must point to a defined variable or function
  extern int g_a __attribute__((alias("g_b")));
^
  fish: 'bin/clang-repl' terminated by signal SIGSEGV (Address boundary error)
  itay ~/llvm-project/build (main) [SIGSEGV]> bin/clang-repl
  clang-repl> void foo(void) __attribute__((ifunc("foo_resolver")));
  In file included from <<< inputs >>>:1:
  input_line_1:1:31: error: ifunc must point to a defined function
  void foo(void) __attribute__((ifunc("foo_resolver")));
^
  fish: 'bin/clang-repl' terminated by signal SIGSEGV (Address boundary error)
  itay ~/llvm-project/build (main) [SIGSEGV]> bin/clang-repl
  clang-repl> void foo(void);
  clang-repl> void bar(void) __attribute__((alias("foo")));
  In file included from <<< inputs >>>:1:
  input_line_1:1:31: error: alias must point to a defined variable or function
  void bar(void) __attribute__((alias("foo")));
^
  fish: 'bin/clang-repl' terminated by signal SIGSEGV (Address boundary error)
  itay ~/llvm-project/build (main) [SIGSEGV]> bin/clang-repl
  clang-repl> void foo(void); void bar(void) __attribute__((alias("foo"))); 
void foo(void) {}
  In file included from <<< inputs >>>:1:
  input_line_0:1:47: error: alias must point to a defined variable or function
  void foo(void); void bar(void) __attribute__((alias("foo"))); void foo(void) 
{}
^
  fish: 'bin/clang-repl' terminated by signal SIGSEGV (Address 

[PATCH] D108479: [Clang] Add __builtin_addressof_nocfi

[Sami Tolvanen via Phabricator via cfe-commits]

samitolvanen removed subscribers: rsmith, rjmccall.
samitolvanen planned changes to this revision.
samitolvanen added a comment.

In D108479#3112492 , @rjmccall wrote:

> You could also make this Just Work for things like C++ member functions 
> rather than producing a member function pointer.

I'm not sure I understand. What does Just Work mean when it comes to C++ member 
functions?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108479

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

[PATCH] D108482: [Clang] Fix instantiation of OpaqueValueExprs (Bug #45964)

[Jason Rice via Phabricator via cfe-commits]

ricejasonf added a comment.

In D108482#314 , @tambre wrote:

> In D108482#3108013 , @ricejasonf 
> wrote:
>
>> In D108482#3105889 , @tambre wrote:
>>
>>> I presume you lack the permissions to push this to the main repo yourself. 
>>> Would you like me to do that for you?
>>
>> Yes, please.
>
> Please provide the name and email you want it committed as. Unfortunately 
> Phabricator diffs don't include this info.

Jason Rice

ricejas...@gmail.com

Thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108482

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

[PATCH] D113268: [clangd] Fix tests with forward slash as separator on windows

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added inline comments.



Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:546
 fs::make_absolute(TestScheme::TestDir, Path);
+path::native(Path);
 return std::string(Path);

mstorsjo wrote:
> sammccall wrote:
> > This change is an example of something subtle that I don't understand, and 
> > don't expect other maintainers to understand, which is therefore fragile.
> Hmm, I'll have to revisit this particular change and see if it really was 
> necessary in the end, and add a comment explaining the subtlety.
Ah, yes, now I remember what this particular change does:

`TestScheme::TestDir` is a haredcoded constant string; 
`fs::make_absolute(TestScheme::TestDir, Path);` concatenates the 
always-backslashes `TestDir` with `Path` (with the currently preferred 
separator inbetween). By calling `native()` afterwards, it converts the whole 
resulting path to the native form.

An alternative would be to provide `TestDir` in the preferred form (e.g. like 
wrapping it in a function that returns a variable string, like in 
TestFS.{h,cpp}.) - that'd maybe be more straightforward, but also feels a bit 
needless as we're calling `native()` on it anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113268

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

[clang] 6278682 - In spir functions, llvm.dbg.declare intrinsics created

[Zahira Ammarguellat via cfe-commits]

Author: Zahira Ammarguellat
Date: 2021-11-05T15:08:09-07:00
New Revision: 627868263cd4d57c230b61904483a3dad9e1a1da

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

LOG: In spir functions, llvm.dbg.declare intrinsics created
for parameters and locals need to refer to the stack
allocation in the alloca address space.

Added: 
clang/test/CodeGenSYCL/debug-info-kernel-variables.cpp

Modified: 
clang/lib/CodeGen/CGDecl.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGDecl.cpp b/clang/lib/CodeGen/CGDecl.cpp
index dfb74a3fc6547..941671c614824 100644
--- a/clang/lib/CodeGen/CGDecl.cpp
+++ b/clang/lib/CodeGen/CGDecl.cpp
@@ -1447,6 +1447,7 @@ CodeGenFunction::EmitAutoVarAlloca(const VarDecl ) {
 
   if (getLangOpts().OpenMP && OpenMPLocalAddr.isValid()) {
 address = OpenMPLocalAddr;
+AllocaAddr = OpenMPLocalAddr;
   } else if (Ty->isConstantSizeType()) {
 // If this value is an array or struct with a statically determinable
 // constant initializer, there are optimizations we can do.
@@ -1492,6 +1493,7 @@ CodeGenFunction::EmitAutoVarAlloca(const VarDecl ) {
   // return slot, so that we can elide the copy when returning this
   // variable (C++0x [class.copy]p34).
   address = ReturnValue;
+  AllocaAddr = ReturnValue;
 
   if (const RecordType *RecordTy = Ty->getAs()) {
 const auto *RD = RecordTy->getDecl();
@@ -1503,7 +1505,8 @@ CodeGenFunction::EmitAutoVarAlloca(const VarDecl ) {
   // applied.
   llvm::Value *Zero = Builder.getFalse();
   Address NRVOFlag =
-CreateTempAlloca(Zero->getType(), CharUnits::One(), "nrvo");
+  CreateTempAlloca(Zero->getType(), CharUnits::One(), "nrvo",
+   /*ArraySize=*/nullptr, );
   EnsureInsertPoint();
   Builder.CreateStore(Zero, NRVOFlag);
 
@@ -1605,10 +1608,11 @@ CodeGenFunction::EmitAutoVarAlloca(const VarDecl ) {
 DI->setLocation(D.getLocation());
 
 // If NRVO, use a pointer to the return address.
-if (UsePointerValue)
+if (UsePointerValue) {
   DebugAddr = ReturnValuePointer;
-
-(void)DI->EmitDeclareOfAutoVariable(, DebugAddr.getPointer(), Builder,
+  AllocaAddr = ReturnValuePointer;
+}
+(void)DI->EmitDeclareOfAutoVariable(, AllocaAddr.getPointer(), Builder,
 UsePointerValue);
   }
 
@@ -2450,6 +2454,7 @@ void CodeGenFunction::EmitParmDecl(const VarDecl , 
ParamValue Arg,
   }
 
   Address DeclPtr = Address::invalid();
+  Address AllocaPtr = Address::invalid();
   bool DoStore = false;
   bool IsScalar = hasScalarEvaluationKind(Ty);
   // If we already have a pointer to the argument, reuse the input pointer.
@@ -2464,6 +2469,7 @@ void CodeGenFunction::EmitParmDecl(const VarDecl , 
ParamValue Arg,
 // from the default address space.
 auto AllocaAS = CGM.getASTAllocaAddressSpace();
 auto *V = DeclPtr.getPointer();
+AllocaPtr = DeclPtr;
 auto SrcLangAS = getLangOpts().OpenCL ? LangAS::opencl_private : AllocaAS;
 auto DestLangAS =
 getLangOpts().OpenCL ? LangAS::opencl_private : LangAS::Default;
@@ -2500,10 +2506,11 @@ void CodeGenFunction::EmitParmDecl(const VarDecl , 
ParamValue Arg,
 : Address::invalid();
 if (getLangOpts().OpenMP && OpenMPLocalAddr.isValid()) {
   DeclPtr = OpenMPLocalAddr;
+  AllocaPtr = DeclPtr;
 } else {
   // Otherwise, create a temporary to hold the value.
   DeclPtr = CreateMemTemp(Ty, getContext().getDeclAlign(),
-  D.getName() + ".addr");
+  D.getName() + ".addr", );
 }
 DoStore = true;
   }
@@ -2579,7 +2586,7 @@ void CodeGenFunction::EmitParmDecl(const VarDecl , 
ParamValue Arg,
   if (CGDebugInfo *DI = getDebugInfo()) {
 if (CGM.getCodeGenOpts().hasReducedDebugInfo() && !CurFuncIsThunk) {
   llvm::DILocalVariable *DILocalVar = DI->EmitDeclareOfArgVariable(
-  , DeclPtr.getPointer(), ArgNo, Builder);
+  , AllocaPtr.getPointer(), ArgNo, Builder);
   if (const auto *Var = dyn_cast_or_null())
 DI->getParamDbgMappings().insert({Var, DILocalVar});
 }

diff  --git a/clang/test/CodeGenSYCL/debug-info-kernel-variables.cpp 
b/clang/test/CodeGenSYCL/debug-info-kernel-variables.cpp
new file mode 100644
index 0..e6efa92716fbc
--- /dev/null
+++ b/clang/test/CodeGenSYCL/debug-info-kernel-variables.cpp
@@ -0,0 +1,60 @@
+// RUN: %clang_cc1 %s -o - -O0 -emit-llvm \
+// RUN:-triple spir64-unknown-unknown \
+// RUN:-aux-triple x86_64-unknown-linux-gnu   \
+// RUN:-fsycl-is-device   \
+// 

[PATCH] D113304: [NewPM] Only invalidate modified functions' analyses in CGSCC passes + turn on eagerly invalidate analyses

[Nikita Popov via Phabricator via cfe-commits]

nikic added a comment.

I don't think I fully understand the interaction this has with eager 
invalidation. I would have expected that if we eagerly invalidate, then this 
fine-grained invalidation wouldn't make much of a difference, because we 
invalidate anyway after processing a function.




Comment at: llvm/lib/Transforms/IPO/ArgumentPromotion.cpp:1051
+  for (auto *U : NewF->users()) {
+auto *UserF = cast(U)->getParent()->getParent();
+FAM.invalidate(*UserF, FuncPA);





Comment at: llvm/lib/Transforms/IPO/FunctionAttrs.cpp:1845
+  if (auto *Call = dyn_cast(U))
+FAM.invalidate(*Call->getParent()->getParent(), FuncPA);
+}

Do we need to worry about indirect references here, like a call through a 
bitcast? For the ArgPromotion case that's not possible, but I'm not so sure 
here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113304

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

[PATCH] D113320: [clang-format] Address fixme

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks created this revision.
HazardyKnusperkeks added reviewers: MyDeveloperDay, djasper.
HazardyKnusperkeks added a project: clang-format.
HazardyKnusperkeks requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113320

Files:
  clang/lib/Format/TokenAnnotator.h
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h


Index: clang/lib/Format/UnwrappedLineParser.h
===
--- clang/lib/Format/UnwrappedLineParser.h
+++ clang/lib/Format/UnwrappedLineParser.h
@@ -19,8 +19,8 @@
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Format/Format.h"
 #include "llvm/Support/Regex.h"
-#include 
 #include 
+#include 
 
 namespace clang {
 namespace format {
@@ -36,9 +36,8 @@
 struct UnwrappedLine {
   UnwrappedLine();
 
-  // FIXME: Don't use std::list here.
   /// The \c Tokens comprising this \c UnwrappedLine.
-  std::list Tokens;
+  std::vector Tokens;
 
   /// The indent level of the \c UnwrappedLine.
   unsigned Level;
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -3056,23 +3056,15 @@
   llvm::dbgs() << Prefix << "Line(" << Line.Level
<< ", FSC=" << Line.FirstStartColumn << ")"
<< (Line.InPPDirective ? " MACRO" : "") << ": ";
-  for (std::list::const_iterator I = Line.Tokens.begin(),
-E = Line.Tokens.end();
-   I != E; ++I) {
-llvm::dbgs() << I->Tok->Tok.getName() << "["
- << "T=" << (unsigned)I->Tok->getType()
- << ", OC=" << I->Tok->OriginalColumn << "] ";
+  for (const auto  : Line.Tokens) {
+llvm::dbgs() << Node.Tok->Tok.getName() << "["
+ << "T=" << static_cast(Node.Tok->getType())
+ << ", OC=" << Node.Tok->OriginalColumn << "] ";
   }
-  for (std::list::const_iterator I = Line.Tokens.begin(),
-E = Line.Tokens.end();
-   I != E; ++I) {
-const UnwrappedLineNode  = *I;
-for (SmallVectorImpl::const_iterator
- I = Node.Children.begin(),
- E = Node.Children.end();
- I != E; ++I) {
-  printDebugInfo(*I, "\nChild: ");
-}
+  for (const auto  : Line.Tokens) {
+for (const auto  : Node.Children)
+  printDebugInfo(ChildNode, "\nChild: ");
+
   }
   llvm::dbgs() << "\n";
 }
Index: clang/lib/Format/TokenAnnotator.h
===
--- clang/lib/Format/TokenAnnotator.h
+++ clang/lib/Format/TokenAnnotator.h
@@ -53,9 +53,7 @@
 // left them in a different state.
 First->Previous = nullptr;
 FormatToken *Current = First;
-for (std::list::const_iterator I = 
++Line.Tokens.begin(),
-  E = Line.Tokens.end();
- I != E; ++I) {
+for (auto I = ++Line.Tokens.begin(), E = Line.Tokens.end(); I != E; ++I) {
   const UnwrappedLineNode  = *I;
   Current->Next = I->Tok;
   I->Tok->Previous = Current;


Index: clang/lib/Format/UnwrappedLineParser.h
===
--- clang/lib/Format/UnwrappedLineParser.h
+++ clang/lib/Format/UnwrappedLineParser.h
@@ -19,8 +19,8 @@
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Format/Format.h"
 #include "llvm/Support/Regex.h"
-#include 
 #include 
+#include 
 
 namespace clang {
 namespace format {
@@ -36,9 +36,8 @@
 struct UnwrappedLine {
   UnwrappedLine();
 
-  // FIXME: Don't use std::list here.
   /// The \c Tokens comprising this \c UnwrappedLine.
-  std::list Tokens;
+  std::vector Tokens;
 
   /// The indent level of the \c UnwrappedLine.
   unsigned Level;
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -3056,23 +3056,15 @@
   llvm::dbgs() << Prefix << "Line(" << Line.Level
<< ", FSC=" << Line.FirstStartColumn << ")"
<< (Line.InPPDirective ? " MACRO" : "") << ": ";
-  for (std::list::const_iterator I = Line.Tokens.begin(),
-E = Line.Tokens.end();
-   I != E; ++I) {
-llvm::dbgs() << I->Tok->Tok.getName() << "["
- << "T=" << (unsigned)I->Tok->getType()
- << ", OC=" << I->Tok->OriginalColumn << "] ";
+  for (const auto  : Line.Tokens) {
+llvm::dbgs() << Node.Tok->Tok.getName() << "["
+ << "T=" << static_cast(Node.Tok->getType())
+ << ", OC=" << Node.Tok->OriginalColumn << "] ";
   }
-  for (std::list::const_iterator I = 

[PATCH] D113143: [clang][asan] Add test for ensuring PR52382 is fixed

[Leonard Chan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG456a7e52310d: [clang][asan] Add test for ensuring PR52382 is 
fixed (authored by leonardchan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113143

Files:
  clang/test/CodeGen/pr52382.c


Index: clang/test/CodeGen/pr52382.c
===
--- /dev/null
+++ clang/test/CodeGen/pr52382.c
@@ -0,0 +1,19 @@
+// RUN: %clang -target x86_64-unknown-linux-gnu -S -emit-llvm -o - 
-fsanitize=address %s | FileCheck %s
+
+// Ensure that ASan properly instruments a load into a global where the index
+// happens to be within the padding after the global which is used for the
+// redzone.
+
+// This global is 400 bytes long, but gets padded with 112 bytes for redzones,
+// rounding the total size after instrumentation to 512.
+int global_array[100] = {-1};
+
+// This access is 412 bytes after the start of the global: past the end of the
+// uninstrumented array, but within the bounds of the extended instrumented
+// array. We should ensure this is still instrumented.
+int main(void) { return global_array[103]; }
+
+// CHECK: @main
+// CHECK-NEXT: entry:
+// CHECK: call void @__asan_report_load4
+// CHECK: }


Index: clang/test/CodeGen/pr52382.c
===
--- /dev/null
+++ clang/test/CodeGen/pr52382.c
@@ -0,0 +1,19 @@
+// RUN: %clang -target x86_64-unknown-linux-gnu -S -emit-llvm -o - -fsanitize=address %s | FileCheck %s
+
+// Ensure that ASan properly instruments a load into a global where the index
+// happens to be within the padding after the global which is used for the
+// redzone.
+
+// This global is 400 bytes long, but gets padded with 112 bytes for redzones,
+// rounding the total size after instrumentation to 512.
+int global_array[100] = {-1};
+
+// This access is 412 bytes after the start of the global: past the end of the
+// uninstrumented array, but within the bounds of the extended instrumented
+// array. We should ensure this is still instrumented.
+int main(void) { return global_array[103]; }
+
+// CHECK: @main
+// CHECK-NEXT: entry:
+// CHECK: call void @__asan_report_load4
+// CHECK: }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 456a7e5 - [clang][asan] Add test for ensuring PR52382 is fixed

[Leonard Chan via cfe-commits]

Author: Leonard Chan
Date: 2021-11-05T14:10:34-07:00
New Revision: 456a7e52310d632be4e41a4b7c4853e910648621

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

LOG: [clang][asan] Add test for ensuring PR52382 is fixed

The fix for PR52382 was already introduced in D112732 which ensures that module
instrumentation always runs after function instrumentation. This adds a test
that ensures the PR is addressed and prevent regression.

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

Added: 
clang/test/CodeGen/pr52382.c

Modified: 


Removed: 




diff  --git a/clang/test/CodeGen/pr52382.c b/clang/test/CodeGen/pr52382.c
new file mode 100644
index 0..6150c936f6bbd
--- /dev/null
+++ b/clang/test/CodeGen/pr52382.c
@@ -0,0 +1,19 @@
+// RUN: %clang -target x86_64-unknown-linux-gnu -S -emit-llvm -o - 
-fsanitize=address %s | FileCheck %s
+
+// Ensure that ASan properly instruments a load into a global where the index
+// happens to be within the padding after the global which is used for the
+// redzone.
+
+// This global is 400 bytes long, but gets padded with 112 bytes for redzones,
+// rounding the total size after instrumentation to 512.
+int global_array[100] = {-1};
+
+// This access is 412 bytes after the start of the global: past the end of the
+// uninstrumented array, but within the bounds of the extended instrumented
+// array. We should ensure this is still instrumented.
+int main(void) { return global_array[103]; }
+
+// CHECK: @main
+// CHECK-NEXT: entry:
+// CHECK: call void @__asan_report_load4
+// CHECK: }



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

[PATCH] D113319: [clang-format] Improve require handling

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

Or am I mistaken and we want to be able to control the line break?

Also I would break before struct, class, union, but I would have needed to 
change the test, and I know that we at least need to talk about in in front. :)

  template
  require X
  class ...
  
  vs.
  
  template
  require X class ...




Comment at: clang/lib/Format/TokenAnnotator.cpp:1419
 TT_UntouchableMacroFunc, TT_ConstraintJunctions,
-TT_StatementAttributeLikeMacro))
+TT_StatementAttributeLikeMacro, TT_RequiresClause,
+TT_RequiresExpression))

This one has cost me **a lot** of time, should we rethink this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113319

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

[PATCH] D113319: [clang-format] Improve require handling

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks created this revision.
HazardyKnusperkeks added reviewers: MyDeveloperDay, krasimir, curdeius.
HazardyKnusperkeks added a project: clang-format.
HazardyKnusperkeks requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Before this patch the require clause between function and template declaration 
was written in the same line as the function. I can't believe this is what 
anyone wants.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113319

Files:
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -22288,6 +22288,114 @@
"requires (std::invocable...>) "
"struct constant;",
Style);
+
+  verifyFormat("template  void func(T);");
+
+  verifyFormat("template \n"
+   "requires std::signed_integral && std::signed_integral\n"
+   "void func(T);");
+  verifyFormat("template \n"
+   "requires(std::is_integral_v && std::is_signed_v)\n"
+   "void func(T);");
+  verifyFormat("template \n"
+   "requires requires(T &) {\n"
+   "  typename T::size_type;\n"
+   "  { t.size() } -> std::same_as;\n"
+   "}\n"
+   "func(T);");
+
+  verifyFormat("template \n"
+   "requires std::signed_integral && std::signed_integral\n"
+   "void func(T) {}");
+  verifyFormat("template \n"
+   "requires(std::is_integral_v && std::is_signed_v)\n"
+   "void func(T) {}");
+  verifyFormat("template \n"
+   "requires requires(T &) {\n"
+   "  typename T::size_type;\n"
+   "  { t.size() } -> std::same_as;\n"
+   "}\n"
+   "func(T) {}");
+
+  verifyFormat(
+  "template  void func(T) requires std::signed_integral;");
+  verifyFormat("template \n"
+   "void func(T) requires std::signed_integral && "
+   "std::signed_integral;");
+  verifyFormat(
+  "template \n"
+  "void func(T) requires(std::is_integral_v && std::is_signed_v);");
+  verifyFormat("template  void func(T) requires requires(T &) {\n"
+   "  typename T::size_type;\n"
+   "  { t.size() } -> std::same_as;\n"
+   "};");
+
+  verifyFormat(
+  "template  void func(T) requires std::signed_integral {}");
+  verifyFormat("template \n"
+   "void func(T) requires std::signed_integral && "
+   "std::signed_integral {}");
+  verifyFormat(
+  "template \n"
+  "void func(T) requires(std::is_integral_v && std::is_signed_v) {}");
+  verifyFormat("template  void func(T) requires requires(T &) {\n"
+   "  typename T::size_type;\n"
+   "  { t.size() } -> std::same_as;\n"
+   "}\n"
+   "{}");
+
+  Style = getLLVMStyle();
+  Style.IndentRequires = true;
+
+  verifyFormat("template \n"
+   "  requires std::signed_integral && std::signed_integral\n"
+   "void func(T);",
+   Style);
+  verifyFormat("template \n"
+   "  requires(std::is_integral_v && std::is_signed_v)\n"
+   "void func(T);",
+   Style);
+  verifyFormat("template \n"
+   "  requires requires(T &) {\n"
+   "typename T::size_type;\n"
+   "{ t.size() } -> std::same_as;\n"
+   "  }\n"
+   "func(T);",
+   Style);
+
+  verifyFormat("template \n"
+   "  requires std::signed_integral && std::signed_integral\n"
+   "void func(T) {}",
+   Style);
+  verifyFormat("template \n"
+   "  requires(std::is_integral_v && std::is_signed_v)\n"
+   "void func(T) {}",
+   Style);
+  verifyFormat("template \n"
+   "  requires requires(T &) {\n"
+   "typename T::size_type;\n"
+   "{ t.size() } -> std::same_as;\n"
+   "  }\n"
+   "func(T) {}",
+   Style);
+
+  verifyFormat(
+  "template  void func(T) requires std::signed_integral {}",
+  Style);
+  verifyFormat("template \n"
+   "void func(T) requires std::signed_integral && "
+   "std::signed_integral {}",
+   Style);
+  verifyFormat(
+  "template \n"
+  "void func(T) requires(std::is_integral_v && std::is_signed_v) {}",
+  Style);
+  verifyFormat("template  void func(T) requires requires(T &) {\n"
+   "  typename T::size_type;\n"
+   "  { t.size() } -> std::same_as;\n"
+   "}\n"
+   

[PATCH] D113189: [clang] Inclusive language: change instances of blacklist/whitelist to allowlist/ignorelist

[Nico Weber via Phabricator via cfe-commits]

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

Generally lg. I find the word "allowlist" (and to a lesser level also 
"ignorelist") a bit awkward; imho you can often make things more clear by 
rephrasing things away from "foolist". A few suggestions along those lines 
below.




Comment at: clang/include/clang/AST/CommentHTMLTags.td:55
 
-// Define a blacklist of attributes that are not safe to pass through to HTML
+// Define an ignorelist of attributes that are not safe to pass through to HTML
 // output if the input is untrusted.

"Define a list of…"



Comment at: clang/include/clang/AST/CommentHTMLTags.td:58
 //
-// FIXME: this should be a whitelist.  When changing this to a whitelist, don't
-// forget to change the default in the TableGen backend.
+// FIXME: this should be an allowlist.  When changing this to an allowlist,
+// don't forget to change the default in the TableGen backend.

"This should be a list of attributes that _are_ safe. When making this change, 
don't forget…"



Comment at: clang/include/clang/Basic/DiagnosticASTKinds.td:570
 "%select{is not C++|is packed|is a union|is trivially copyable|"
-"has trivial destructor|is standard layout|is in a blacklisted file|"
-"is blacklisted}1">, ShowInSystemHeader,
+"has trivial destructor|is standard layout|is in a ignorelisted file|"
+"is ignorelisted}1">, ShowInSystemHeader,

s/a/an/



Comment at: clang/include/clang/Basic/DiagnosticASTKinds.td:571
+"has trivial destructor|is standard layout|is in a ignorelisted file|"
+"is ignorelisted}1">, ShowInSystemHeader,
 InGroup;

"is ignored"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113189

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

[PATCH] D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default

[Evgenii Stepanov via Phabricator via cfe-commits]

eugenis added a comment.

In D105169#3111935 , @hyeongyukim 
wrote:

>   -  long long res;
>   +  register long long res __asm__("x0");
>
> Is it okay to commit this change by myself?

Yes, go ahead!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105169

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

[PATCH] D113145: [Sema] Mark virtual method declaration in union as invalid

[Hubert Tong via Phabricator via cfe-commits]

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

LGTM; thanks! fyi, I'll be on vacation for the next three weeks. I've added 
some additional reviewers to the patch based on prior reviews.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113145

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

[PATCH] D106585: Fix clang debug info irgen of i128 enums

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

Reviewing the code, I don't see any obvious sources of non-determinism (hash 
iteration). The only source I can imagine is uninitialized memory usage in the 
APInt. Let me know if this can be repro'd somehow.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106585

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

[PATCH] D112613: [RISCV] Change TARGET_BUILTIN require to zve32x for vector instruction

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vaadd.c:3
 // REQUIRES: riscv-registered-target
-// RUN: %clang_cc1 -triple riscv64 -target-feature +experimental-v 
-disable-O0-optnone -emit-llvm %s -o - | opt -S -mem2reg | FileCheck 
--check-prefix=CHECK-RV64 %s
+// RUN: %clang_cc1 -triple riscv64 -target-feature +experimental-zve32x 
-target-feature +experimental-zve64x -disable-O0-optnone -emit-llvm %s -o - | 
opt -S -mem2reg | FileCheck --check-prefix=CHECK-RV64 %s
 

Why do we need to add zve32x and zve64x to the command line? Doesn't zve64x 
imply zve32x?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112613

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

[PATCH] D113262: [clangd] Allow IncludeCleaner to replace unused header with transitively used

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added a comment.

Oh, sorry, I marked it as "changes intended" so that you could know it's not 
review-ready yet :) I was just playing around with it and previewing to see how 
it would feel to have a somewhat working solution.

I didn't really implement the feature (add the proper header spelling that we 
have in `IncludeFixer` as you mentioned in the comments, check if the 
replacement header is already included or not, etc), but there are two 
important problems I was facing that look rather tricky

- This actually steps into the "missing includes" land: we can be conservative 
when deciding whether a header is used or not. It is both system headers _and_ 
(currently) a lot of noise in the form of headers with forward decls. E.g. for 
StringRef the replacement set includes `SHA1` and a bunch of very surprising 
headers just because we have lots of forward decls in unexpected places. I 
originally had the idea that we can do unused includes without missing includes 
but it seems that they are closer in terms of at least this usecase. I wanted 
to see if we would need something more but maybe the follow-up on the mentioned 
patch (narrow down the "used" header property set conditions) would be enough.
- Performance: providing fix-its for individual headers is not really 
performance-optimal: we need a graph traversal from each MainFileInclusion and 
it's a linear BFS, so the complexity is O(unused headers * graph size). This 
_might_ be unfortunate for large graphs and (surprisingly) it's just faster to 
collect the data for a "batch replace" of unused headers with used ones. But 
this brings us back into the realm of missing headers. Maybe the performance is 
fine, but I want to estimate it in a better way. So far I've seen somewhat 
small graphs when editing LLVM (<1k headers) but it might be different for 
different parts so I want to check the performance to make sure it's really 
fine.

Anyway, I'd be happy to talk once I'm back but as you mentioned this patch 
would probably need some QOL improvements before it can be landed (needless to 
say, it should be complete, too :) )!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113262

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

[PATCH] D112613: [RISCV] Change TARGET_BUILTIN require to zve32x for vector instruction

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

The title of this patch isn't descriptive of the changes in this patch. Based 
on the title I would only expect changes to RISCVEmitter.cpp. Can you clarify 
the title?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112613

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

[PATCH] D113306: [PowerPC] Allow MMA built-ins to accept non-void pointers and arrays

[Ahsan Saghir via Phabricator via cfe-commits]

saghir created this revision.
Herald added subscribers: shchenz, kbarton, nemanjai.
saghir requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Calls to MMA builtins that take pointer to void
do not accept other pointers/arrays whereas normal
functions with the same parameter do. This patch
allows MMA built-ins to accept non-void pointers
and arrays.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113306

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/ppc-pair-mma-types.c


Index: clang/test/Sema/ppc-pair-mma-types.c
===
--- clang/test/Sema/ppc-pair-mma-types.c
+++ clang/test/Sema/ppc-pair-mma-types.c
@@ -339,20 +339,20 @@
 
 void testRestrictQualifiedPointer1(int *__restrict acc) {
   vector float arr[4];
-  __builtin_mma_disassemble_acc((void *)arr, acc); // expected-error {{passing 
'int *restrict' to parameter of incompatible type '__vector_quad *'}}
+  __builtin_mma_disassemble_acc(arr, acc); // expected-error {{passing 'int 
*restrict' to parameter of incompatible type '__vector_quad *'}}
 }
 
 void testRestrictQualifiedPointer2(__vector_quad *__restrict acc) {
   vector float arr[4];
-  __builtin_mma_disassemble_acc((void *)arr, acc);
+  __builtin_mma_disassemble_acc(arr, acc);
 }
 
 void testVolatileQualifiedPointer1(int *__volatile acc) {
   vector float arr[4];
-  __builtin_mma_disassemble_acc((void *)arr, acc); // expected-error {{passing 
'int *volatile' to parameter of incompatible type '__vector_quad *'}}
+  __builtin_mma_disassemble_acc(arr, acc); // expected-error {{passing 'int 
*volatile' to parameter of incompatible type '__vector_quad *'}}
 }
 
 void testVolatileQualifiedPointer2(__vector_quad *__volatile acc) {
   vector float arr[4];
-  __builtin_mma_disassemble_acc((void *)arr, acc);
+  __builtin_mma_disassemble_acc(arr, acc);
 }
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -7431,11 +7431,11 @@
   StrippedRVType = StrippedRVType.getCanonicalType().getUnqualifiedType();
 
 // The only case where the argument type and expected type are allowed to
-// mismatch is if the argument type is a non-void pointer and expected type
-// is a void pointer.
+// mismatch is if the argument type is a non-void pointer (or array) and
+// expected type is a void pointer.
 if (StrippedRVType != ExpectedType)
   if (!(ExpectedType->isVoidPointerType() &&
-StrippedRVType->isPointerType()))
+(StrippedRVType->isPointerType() || 
StrippedRVType->isArrayType(
 return Diag(Arg->getBeginLoc(),
 diag::err_typecheck_convert_incompatible)
<< PassedType << ExpectedType << 1 << 0 << 0;
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -15166,8 +15166,12 @@
const CallExpr *E) {
   SmallVector Ops;
 
-  for (unsigned i = 0, e = E->getNumArgs(); i != e; i++)
-Ops.push_back(EmitScalarExpr(E->getArg(i)));
+  for (unsigned i = 0, e = E->getNumArgs(); i != e; i++) {
+if (E->getArg(i)->getType()->isArrayType())
+  Ops.push_back(EmitArrayToPointerDecay(E->getArg(i)).getPointer());
+else
+  Ops.push_back(EmitScalarExpr(E->getArg(i)));
+  }
 
   Intrinsic::ID ID = Intrinsic::not_intrinsic;
 


Index: clang/test/Sema/ppc-pair-mma-types.c
===
--- clang/test/Sema/ppc-pair-mma-types.c
+++ clang/test/Sema/ppc-pair-mma-types.c
@@ -339,20 +339,20 @@
 
 void testRestrictQualifiedPointer1(int *__restrict acc) {
   vector float arr[4];
-  __builtin_mma_disassemble_acc((void *)arr, acc); // expected-error {{passing 'int *restrict' to parameter of incompatible type '__vector_quad *'}}
+  __builtin_mma_disassemble_acc(arr, acc); // expected-error {{passing 'int *restrict' to parameter of incompatible type '__vector_quad *'}}
 }
 
 void testRestrictQualifiedPointer2(__vector_quad *__restrict acc) {
   vector float arr[4];
-  __builtin_mma_disassemble_acc((void *)arr, acc);
+  __builtin_mma_disassemble_acc(arr, acc);
 }
 
 void testVolatileQualifiedPointer1(int *__volatile acc) {
   vector float arr[4];
-  __builtin_mma_disassemble_acc((void *)arr, acc); // expected-error {{passing 'int *volatile' to parameter of incompatible type '__vector_quad *'}}
+  __builtin_mma_disassemble_acc(arr, acc); // expected-error {{passing 'int *volatile' to parameter of incompatible type '__vector_quad *'}}
 }
 
 void testVolatileQualifiedPointer2(__vector_quad *__volatile acc) {
   vector float arr[4];
-  __builtin_mma_disassemble_acc((void *)arr, acc);
+  

[PATCH] D108479: [Clang] Add __builtin_addressof_nocfi

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Yeah, I think that's a better name.  The documentation can say that ideally 
this also wouldn't include things like the THUMB bit, but there are technical 
limitations that make it hard to achieve that.

You could also make this Just Work for things like C++ member functions rather 
than producing a member function pointer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108479

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

[PATCH] D108479: [Clang] Add __builtin_addressof_nocfi

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

> Maybe it should even be semantically restricted to require a constant decl 
> reference as its operand?

I think we should do this.

Maybe it should be named something like `__builtin_symbol_address` if we're 
intending for this to have an effect with PAuth as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108479

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

[PATCH] D113304: [NewPM] Only invalidate modified functions' analyses in CGSCC passes + turn on eagerly invalidate analyses

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks created this revision.
Herald added subscribers: ormris, wenlei, steven_wu, hiraditya, eraman.
Herald added a reviewer: ctetreau.
aeubanks requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

Previously, any change in any function in an SCC would cause all
analyses for all functions in the SCC to be invalidated. With this
change, we now manually invalidate analyses for functions we modify,
then let the pass manager know that all function analyses should be
preserved since we've already handled function analysis invalidation.

So far this only touches the inliner, argpromotion, function-attrs, and
updateCGAndAnalysisManager(), since they are the most used.

This is part of an effort to investigate running the function
simplification pipeline less on functions we visit multiple times in the
inliner pipeline.

However, this causes major memory regressions especially on larger IR.
To counteract this, turn on the option to eagerly invalidate function
analyses. This invalidates analyses on functions immediately after
they're processed in a module or scc to function adaptor for specific
parts of the pipeline.

Overall this has mostly positive effects on compile time and positive effects 
on memory usage.
https://llvm-compile-time-tracker.com/compare.php?from=7f627596977624730f9298a1b69883af1555765e=39e824e0d3ca8a517502f13032dfa67304841c90=instructions
https://llvm-compile-time-tracker.com/compare.php?from=7f627596977624730f9298a1b69883af1555765e=39e824e0d3ca8a517502f13032dfa67304841c90=max-rss


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113304

Files:
  clang/test/CodeGen/thinlto-distributed-newpm.ll
  llvm/lib/Analysis/CGSCCPassManager.cpp
  llvm/lib/Passes/PassBuilderPipelines.cpp
  llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
  llvm/lib/Transforms/IPO/FunctionAttrs.cpp
  llvm/lib/Transforms/IPO/Inliner.cpp
  llvm/test/Other/new-pm-defaults.ll
  llvm/test/Other/new-pm-eager-invalidate.ll
  llvm/test/Other/new-pm-lto-defaults.ll
  llvm/test/Other/new-pm-pgo-preinline.ll
  llvm/test/Other/new-pm-thinlto-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
  llvm/test/Transforms/FunctionAttrs/invalidate.ll
  llvm/test/Transforms/Inline/analysis-invalidation.ll
  llvm/test/Transforms/Inline/cgscc-incremental-invalidate.ll

Index: llvm/test/Transforms/Inline/cgscc-incremental-invalidate.ll
===
--- llvm/test/Transforms/Inline/cgscc-incremental-invalidate.ll
+++ llvm/test/Transforms/Inline/cgscc-incremental-invalidate.ll
@@ -8,11 +8,11 @@
 ;
 ; CHECK: Running pass: InlinerPass on (test1_f, test1_g, test1_h)
 ; CHECK: Running analysis: DominatorTreeAnalysis on test1_f
-; CHECK: Running analysis: DominatorTreeAnalysis on test1_g
 ; CHECK: Invalidating analysis: DominatorTreeAnalysis on test1_f
 ; CHECK: Invalidating analysis: LoopAnalysis on test1_f
 ; CHECK: Invalidating analysis: BranchProbabilityAnalysis on test1_f
 ; CHECK: Invalidating analysis: BlockFrequencyAnalysis on test1_f
+; CHECK: Running analysis: DominatorTreeAnalysis on test1_g
 ; CHECK: Invalidating analysis: DominatorTreeAnalysis on test1_g
 ; CHECK: Invalidating analysis: LoopAnalysis on test1_g
 ; CHECK: Invalidating analysis: BranchProbabilityAnalysis on test1_g
@@ -29,7 +29,6 @@
 ; CHECK-NEXT: Running analysis: DominatorTreeAnalysis on test1_h
 ; CHECK-NOT: Invalidating analysis:
 ; CHECK: Running pass: DominatorTreeVerifierPass on test1_f
-; CHECK-NEXT: Running analysis: DominatorTreeAnalysis on test1_f
 
 ; An external function used to control branches.
 declare i1 @flag()
Index: llvm/test/Transforms/Inline/analysis-invalidation.ll
===
--- /dev/null
+++ llvm/test/Transforms/Inline/analysis-invalidation.ll
@@ -0,0 +1,17 @@
+; RUN: opt -passes=inline < %s -disable-output -debug-pass-manager 2>&1 | FileCheck %s
+
+; We shouldn't invalidate any function analyses on g since it's never modified.
+
+; CHECK-NOT: Invalidating{{.*}} on g
+; CHECK: Invalidating{{.*}} on f
+; CHECK-NOT: Invalidating{{.*}} on g
+
+define void @f() noinline {
+  call void @g()
+  ret void
+}
+
+define void @g() alwaysinline {
+  call void @f()
+  ret void
+}
Index: llvm/test/Transforms/FunctionAttrs/invalidate.ll
===
--- /dev/null
+++ llvm/test/Transforms/FunctionAttrs/invalidate.ll
@@ -0,0 +1,15 @@
+; RUN: opt -passes='function(require),cgscc(function-attrs)' -disable-output < %s -debug-pass-manager 2>&1 | FileCheck %s
+
+; CHECK: Running pass: PostOrderFunctionAttrsPass on (f)
+; CHECK: Invalidating analysis: NoOpFunctionAnalysis on f
+; CHECK: Invalidating analysis: 

[PATCH] D111860: [modules] Update visibility for merged ObjCProtocolDecl definitions.

[Michael Spencer via Phabricator via cfe-commits]

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

lgtm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111860

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

[clang] 3466e00 - Reland "[Attr] support btf_type_tag attribute"

[Yonghong Song via cfe-commits]

Author: Yonghong Song
Date: 2021-11-05T11:25:17-07:00
New Revision: 3466e00716e12e32fdb100e3fcfca5c2b3e8d784

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

LOG: Reland "[Attr] support btf_type_tag attribute"

This is to revert commit f95bd18b5faa (Revert "[Attr] support
btf_type_tag attribute") plus a bug fix.

Previous change failed to handle cases like below:
$ cat reduced.c
void a(*);
void a() {}
$ clang -c reduced.c -O2 -g

In such cases, during clang IR generation, for function a(),
CGCodeGen has numParams = 1 for FunctionType. But for
FunctionTypeLoc we have FuncTypeLoc.NumParams = 0. By using
FunctionType.numParams as the bound to access FuncTypeLoc
params, a random crash is triggered. The bug fix is to
check against FuncTypeLoc.NumParams before accessing
FuncTypeLoc.getParam(Idx).

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

Added: 
clang/test/CodeGen/attr-btf_type_tag-func.c
clang/test/CodeGen/attr-btf_type_tag-typedef-field.c
clang/test/CodeGen/attr-btf_type_tag-var.c
llvm/test/Bitcode/attr-btf_type_tag.ll
llvm/test/DebugInfo/attr-btf_type_tag.ll

Modified: 
clang/lib/CodeGen/CGDebugInfo.cpp
clang/lib/CodeGen/CGDebugInfo.h
llvm/include/llvm/IR/DIBuilder.h
llvm/lib/IR/DIBuilder.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
b/clang/lib/CodeGen/CGDebugInfo.cpp
index 64dd70f837f7b..2b9532534e3aa 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -929,8 +929,28 @@ static llvm::dwarf::Tag getNextQualifier(Qualifiers ) {
   return (llvm::dwarf::Tag)0;
 }
 
-llvm::DIType *CGDebugInfo::CreateQualifiedType(QualType Ty,
-   llvm::DIFile *Unit) {
+// Strip MacroQualifiedTypeLoc and AttributedTypeLoc
+// as their corresponding types will be ignored
+// during code generation. Stripping them allows
+// to maintain proper TypeLoc for a given type
+// during code generation.
+static TypeLoc StripMacroAttributed(TypeLoc TL) {
+  if (!TL)
+return TL;
+
+  while (true) {
+if (auto MTL = TL.getAs())
+  TL = MTL.getInnerLoc();
+else if (auto ATL = TL.getAs())
+  TL = ATL.getModifiedLoc();
+else
+  break;
+  }
+  return TL;
+}
+
+llvm::DIType *CGDebugInfo::CreateQualifiedType(QualType Ty, llvm::DIFile *Unit,
+   TypeLoc TL) {
   QualifierCollector Qc;
   const Type *T = Qc.strip(Ty);
 
@@ -944,7 +964,15 @@ llvm::DIType *CGDebugInfo::CreateQualifiedType(QualType Ty,
 return getOrCreateType(QualType(T, 0), Unit);
   }
 
-  auto *FromTy = getOrCreateType(Qc.apply(CGM.getContext(), T), Unit);
+  QualType NextTy = Qc.apply(CGM.getContext(), T);
+  TypeLoc NextTL;
+  if (NextTy.hasQualifiers())
+NextTL = TL;
+  else if (TL) {
+if (auto QTL = TL.getAs())
+  NextTL = StripMacroAttributed(QTL.getNextTypeLoc());
+  }
+  auto *FromTy = getOrCreateType(NextTy, Unit, NextTL);
 
   // No need to fill in the Name, Line, Size, Alignment, Offset in case of
   // CVR derived types.
@@ -988,10 +1016,10 @@ llvm::DIType *CGDebugInfo::CreateType(const 
ObjCObjectPointerType *Ty,
Ty->getPointeeType(), Unit);
 }
 
-llvm::DIType *CGDebugInfo::CreateType(const PointerType *Ty,
-  llvm::DIFile *Unit) {
+llvm::DIType *CGDebugInfo::CreateType(const PointerType *Ty, llvm::DIFile 
*Unit,
+  TypeLoc TL) {
   return CreatePointerLikeType(llvm::dwarf::DW_TAG_pointer_type, Ty,
-   Ty->getPointeeType(), Unit);
+   Ty->getPointeeType(), Unit, TL);
 }
 
 /// \return whether a C++ mangling exists for the type defined by TD.
@@ -1132,7 +1160,8 @@ CGDebugInfo::getOrCreateRecordFwdDecl(const RecordType 
*Ty,
 llvm::DIType *CGDebugInfo::CreatePointerLikeType(llvm::dwarf::Tag Tag,
  const Type *Ty,
  QualType PointeeTy,
- llvm::DIFile *Unit) {
+ llvm::DIFile *Unit,
+ TypeLoc TL) {
   // Bit size, align and offset of the type.
   // Size is always the size of a pointer. We can't use getTypeSize here
   // because that does not return the correct value for references.
@@ -1142,13 +1171,52 @@ llvm::DIType 
*CGDebugInfo::CreatePointerLikeType(llvm::dwarf::Tag Tag,
   Optional DWARFAddressSpace =
   CGM.getTarget().getDWARFAddressSpace(AddressSpace);
 
+  llvm::DINodeArray Annotations = nullptr;
+  TypeLoc NextTL;
+  if (TL) {
+SmallVector Annots;
+NextTL = TL.getNextTypeLoc();
+

[PATCH] D113118: [clang][AST] Check context of record in structural equivalence.

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

How does it handle this case:

  struct A {int x;};
  
  struct A f(void) {
  struct A{} a;
  
  return a;

and this case:

  struct b{};
  
  struct a {int x;}   // visible outside of f
  f(struct b {int x; } b1) {  // not visibile out of f
return (struct a){.x = 1};
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113118

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

[PATCH] D113249: [CUDA] Bump CUDA version to 11.5

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D113249#3112279 , @Hahnfeld wrote:

> Experimental support for `__int128` is new in CUDA 11.5, not sure if Clang 
> enables this for CUDA.

I think we've added support for i128 a while back: 
https://godbolt.org/z/18bEbhMYb

> The release notes also specify
>
>> `builtin_assume` can now be used to specify address space to allow for 
>> efficient loads and stores.
>
> The docs are very scarce on this, I could only find `void 
> __builtin_assume(bool exp)` which I think is not what they are talking 
> about...

AMD folks have D112041  under review which 
will have builtin_assume help AS inference. In any case, we've already been 
doing it reasonably well automatically.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113249

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

[PATCH] D106585: Fix clang debug info irgen of i128 enums

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

In D106585#3110131 , @glandium wrote:

> This broke determinism when building Firefox.

I'm curious: can you share an example dwarfdump diff that shows what is 
non-deterministic?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106585

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

[PATCH] D113148: Add new clang-tidy check for string_view(nullptr)

[CJ Johnson via Phabricator via cfe-commits]

CJ-Johnson updated this revision to Diff 385133.
CJ-Johnson added a comment.

nits


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113148

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-stringview-nullptr.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone-stringview-nullptr.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-stringview-nullptr.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-stringview-nullptr.cpp
@@ -0,0 +1,1030 @@
+// RUN: %check_clang_tidy %s bugprone-stringview-nullptr -std=c++17 %t
+
+namespace std {
+
+using nullptr_t = decltype(nullptr);
+
+template 
+T &();
+
+template 
+struct type_identity { using type = T; };
+template 
+using type_identity_t = typename type_identity::type;
+
+template 
+class basic_string_view {
+public:
+  basic_string_view();
+  basic_string_view(const C *);
+  basic_string_view(const basic_string_view &);
+  basic_string_view =(const basic_string_view &);
+};
+
+template 
+bool operator<(basic_string_view, basic_string_view);
+template 
+bool operator<(type_identity_t>, basic_string_view);
+template 
+bool operator<(basic_string_view, type_identity_t>);
+
+template 
+bool operator<=(basic_string_view, basic_string_view);
+template 
+bool operator<=(type_identity_t>, basic_string_view);
+template 
+bool operator<=(basic_string_view, type_identity_t>);
+
+template 
+bool operator>(basic_string_view, basic_string_view);
+template 
+bool operator>(type_identity_t>, basic_string_view);
+template 
+bool operator>(basic_string_view, type_identity_t>);
+
+template 
+bool operator>=(basic_string_view, basic_string_view);
+template 
+bool operator>=(type_identity_t>, basic_string_view);
+template 
+bool operator>=(basic_string_view, type_identity_t>);
+
+template 
+bool operator==(basic_string_view, basic_string_view);
+template 
+bool operator==(type_identity_t>, basic_string_view);
+template 
+bool operator==(basic_string_view, type_identity_t>);
+
+template 
+bool operator!=(basic_string_view, basic_string_view);
+template 
+bool operator!=(type_identity_t>, basic_string_view);
+template 
+bool operator!=(basic_string_view, type_identity_t>);
+
+using string_view = basic_string_view;
+
+} // namespace std
+
+void function(std::string_view);
+void function(std::string_view, std::string_view);
+
+void temporary_construction() /* a */ {
+  // Functional Cast
+  {
+(void)(std::string_view(nullptr)) /* a1 */;
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: constructing basic_string_view from null is undefined; replace with the default constructor
+// CHECK-FIXES: {{^}}(void)(std::string_view()) /* a1 */;{{$}}
+
+(void)(std::string_view((nullptr))) /* a2 */;
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: constructing basic_string_view from null is undefined; replace with the default constructor
+// CHECK-FIXES: {{^}}(void)(std::string_view()) /* a2 */;{{$}}
+
+(void)(std::string_view({nullptr})) /* a3 */;
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: constructing basic_string_view from null is undefined; replace with the default constructor
+// CHECK-FIXES: {{^}}(void)(std::string_view()) /* a3 */;{{$}}
+
+(void)(std::string_view({(nullptr)})) /* a4 */;
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: constructing basic_string_view from null is undefined; replace with the default constructor
+// CHECK-FIXES: {{^}}(void)(std::string_view()) /* a4 */;{{$}}
+
+// (void)(const std::string_view(nullptr)) /* a5 */;
+// CV qualifiers do not compile in this context
+
+// (void)(const std::string_view((nullptr))) /* a6 */;
+// CV qualifiers do not compile in this context
+
+// (void)(const std::string_view({nullptr})) /* a7 */;
+// CV qualifiers do not compile in this context
+
+// (void)(const std::string_view({(nullptr)})) /* a8 */;
+// CV qualifiers do not compile in this context
+  }
+
+  // Temporary Object
+  {
+(void)(std::string_view{nullptr}) /* a9 */;
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: constructing basic_string_view from null is undefined; replace with the default constructor
+// CHECK-FIXES: {{^}}(void)(std::string_view{}) /* a9 */;{{$}}
+
+(void)(std::string_view{(nullptr)}) /* a10 */;
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: constructing basic_string_view from null is undefined; replace with the default constructor
+// CHECK-FIXES: {{^}}(void)(std::string_view{}) /* a10 */;{{$}}
+
+

[PATCH] D111654: [analyzer] Retrieve a value from list initialization of multi-dimensional array declaration.

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

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

Feel free to commit your change given that you fix my final nits.
Yeah, I'm soo bad in review. I should have proposed these previously. Sorry.




Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1641
+///
+/// \param CAT [in] The given type of ConstantArrayType.
+///

You don't need to document this.
Aside from that consider marking these //implementation detail// functions 
`static` if they are not yet in an anonymous namespace - I haven't checked.



Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1646
+  assert(CAT && "ConstantArrayType should not be null");
+  // Get a canonical type of the array.
+  CAT = cast(CAT->getCanonicalTypeInternal());

I think we will get it :D



Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1751
+  // Treat an n-dimensional array.
+  const std::pair, const MemRegion *> SValOffsetsAndBase =
+  getElementRegionOffsetsWithBase(R);

The `std::tie()` pattern is probably more readable.


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

https://reviews.llvm.org/D111654

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

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

In D112349#3111927 , @erichkeane 
wrote:

> In D112349#3111873 , @ibookstein 
> wrote:
>
>> And how is Cling expecting CFE to deal with partial knowledge situations at 
>> the implementation level? To deal with exactly the non-local cases that the 
>> current violations address?
>> If there's no prescriptive way forward to dealing with these cases (so 
>> they're tech debt without a remediation plan), then as far as I'm concerned 
>> this case sits exactly under the same tech-debt umbrella of the existing 
>> violations and the way forward is using the same violating solution for this 
>> case too. 
>> We definitely shouldn't block the IR verification indefinitely on this 
>> impasse. Once an acceptable solution is found, this will also be part of 
>> that refactor.
>
> My understanding is that the REPL setup is that the 'IR' just needs to be in 
> a state where it doesn't require reverts/rewrites later, so that we can do 
> partial-back-end-code-generation as we go along.  That said, I'm not positive 
> as to the implications.  I'm just repeating the discussion the CFE code-owner 
> made at the time.
>
> IMO, the 'acceptable' solution is to have a way to forward-declare an ifunc 
> in IR so that we can fill in the resolver later on.  From your description 
> earlier, it seems that this would work as we could emit it as an 'unknown 
> symbol' (as if it was an undefined function declaration), and would be 
> completely implementable in the CFE.
>
> So it would change your plan from earlier to:
>
> When processing cpu_specific, emit the ifunc "x.ifunc", with no resolver;
> When processing cpu_dispatch:
>
>   Get/Create the ifunc, then pull up the resolver.
>   If the resolver is null (as it should be), create one and update the 
> 'ifunc'.
>   Generate said resolver.
>   

Speaking of the incremental compilation case, we can experiment with the 
clang-repl binary. I am not sure about the details of this discussion but here 
is an example that works today:

  llvm/build/bin/clang-repl 
  clang-repl> __attribute__((cpu_specific(ivybridge))) void 
single_version(void){}
  clang-repl> void useage() {single_version();}
  clang-repl> quit

What would it be a good example to check if the incremental compilation case is 
covered?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112349

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

[PATCH] D113148: Add new clang-tidy check for string_view(nullptr)

[CJ Johnson via Phabricator via cfe-commits]

CJ-Johnson updated this revision to Diff 385130.
CJ-Johnson added a comment.

Remove double brackets for list initialization and use default initialization 
to avoid most vexing parse


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113148

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-stringview-nullptr.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone-stringview-nullptr.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-stringview-nullptr.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-stringview-nullptr.cpp
@@ -0,0 +1,1030 @@
+// RUN: %check_clang_tidy %s bugprone-stringview-nullptr -std=c++17 %t
+
+namespace std {
+
+using nullptr_t = decltype(nullptr);
+
+template 
+T &();
+
+template 
+struct type_identity { using type = T; };
+template 
+using type_identity_t = typename type_identity::type;
+
+template 
+class basic_string_view {
+public:
+  basic_string_view();
+  basic_string_view(const C *);
+  basic_string_view(const basic_string_view &);
+  basic_string_view =(const basic_string_view &);
+};
+
+template 
+bool operator<(basic_string_view, basic_string_view);
+template 
+bool operator<(type_identity_t>, basic_string_view);
+template 
+bool operator<(basic_string_view, type_identity_t>);
+
+template 
+bool operator<=(basic_string_view, basic_string_view);
+template 
+bool operator<=(type_identity_t>, basic_string_view);
+template 
+bool operator<=(basic_string_view, type_identity_t>);
+
+template 
+bool operator>(basic_string_view, basic_string_view);
+template 
+bool operator>(type_identity_t>, basic_string_view);
+template 
+bool operator>(basic_string_view, type_identity_t>);
+
+template 
+bool operator>=(basic_string_view, basic_string_view);
+template 
+bool operator>=(type_identity_t>, basic_string_view);
+template 
+bool operator>=(basic_string_view, type_identity_t>);
+
+template 
+bool operator==(basic_string_view, basic_string_view);
+template 
+bool operator==(type_identity_t>, basic_string_view);
+template 
+bool operator==(basic_string_view, type_identity_t>);
+
+template 
+bool operator!=(basic_string_view, basic_string_view);
+template 
+bool operator!=(type_identity_t>, basic_string_view);
+template 
+bool operator!=(basic_string_view, type_identity_t>);
+
+using string_view = basic_string_view;
+
+} // namespace std
+
+void function(std::string_view);
+void function(std::string_view, std::string_view);
+
+void temporary_construction() /* a */ {
+  // Functional Cast
+  {
+(void)(std::string_view(nullptr)) /* a1 */;
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: constructing basic_string_view from null is undefined; replace with the default constructor
+// CHECK-FIXES: {{^}}(void)(std::string_view()) /* a1 */;{{$}}
+
+(void)(std::string_view((nullptr))) /* a2 */;
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: constructing basic_string_view from null is undefined; replace with the default constructor
+// CHECK-FIXES: {{^}}(void)(std::string_view()) /* a2 */;{{$}}
+
+(void)(std::string_view({nullptr})) /* a3 */;
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: constructing basic_string_view from null is undefined; replace with the default constructor
+// CHECK-FIXES: {{^}}(void)(std::string_view()) /* a3 */;{{$}}
+
+(void)(std::string_view({(nullptr)})) /* a4 */;
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: constructing basic_string_view from null is undefined; replace with the default constructor
+// CHECK-FIXES: {{^}}(void)(std::string_view()) /* a4 */;{{$}}
+
+// (void)(const std::string_view(nullptr)) /* a5 */;
+// CV qualifiers do not compile in this context
+
+// (void)(const std::string_view((nullptr))) /* a6 */;
+// CV qualifiers do not compile in this context
+
+// (void)(const std::string_view({nullptr})) /* a7 */;
+// CV qualifiers do not compile in this context
+
+// (void)(const std::string_view({(nullptr)})) /* a8 */;
+// CV qualifiers do not compile in this context
+  }
+
+  // Temporary Object
+  {
+(void)(std::string_view{nullptr}) /* a9 */;
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: constructing basic_string_view from null is undefined; replace with the default constructor
+// CHECK-FIXES: {{^}}(void)(std::string_view{}) /* a9 */;{{$}}
+
+(void)(std::string_view{(nullptr)}) /* a10 */;
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: constructing basic_string_view from null is undefined; replace with the 

[PATCH] D113249: [CUDA] Bump CUDA version to 11.5

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

Experimental support for `__int128` is new in CUDA 11.5, not sure if Clang 
enables this for CUDA. The release notes also specify

> `builtin_assume` can now be used to specify address space to allow for 
> efficient loads and stores.

The docs are very scarce on this, I could only find `void __builtin_assume(bool 
exp)` which I think is not what they are talking about...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113249

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

[PATCH] D113251: [analyzer][doc] Add user documenation for taint analysis

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

steakhal added a comment.

Excellent work. Don't be afraid of the number of nits I dump on you!
Good job.




Comment at: clang/docs/analyzer/checkers.rst:2338
 
+Default sources defined by `GenericTaintChecker`:
+``fdopen``, ``fopen``, ``freopen``, ``getch``, ``getchar``, 
``getchar_unlocked``, ``gets``, ``scanf``, ``socket``, ``wgetch``

Same for the rest.



Comment at: clang/docs/analyzer/checkers.rst:2351
+
+ clang --analyze ... -Xclang -analyzer-config -Xclang 
alpha.security.taint.TaintPropagation:Config=taint_config.yaml
+

Per https://reviews.llvm.org/D113004#inline-1078695 we should not advocate 
users use the `-Xclang` machinery, we should rather refer to it by other tools 
such as `scan-build`. However, we haven't reached a consensus about this 
decision yet.
Consider moving some parts of this doc to the proposed Configuration 
documentation file - housing the //more// user-facing analyzer options.



Comment at: clang/docs/analyzer/checkers.rst:2356
+
+For a more detailed description of configuration options, please see the 
:doc:`user-docs/TaintAnalysisConfiguration`. For an example see 
:ref:`taint-configuration-example`.
+

Please note that this configuration file is not stable (yet).
We might change it in a non-backward compatible way without any notice in the 
upcoming releases.



Comment at: clang/docs/analyzer/user-docs/TaintAnalysisConfiguration.rst:19
+Taint analysis works by checking for the occurrence of special events during 
the symbolic execution of the program.
+Taint analysis defines sources, sinks, and propagation rules. It identifies 
errors by detecting a flow of information that originates in a taint source, 
touches a taint sink, and propagates through the program paths via propagation 
rules.
+A source, sink, or an event that propagates taint is mainly domain-specific 
knowledge, but there are some built-in defaults provided by 
:ref:`alpha-security-taint-TaintPropagation`.





Comment at: clang/docs/analyzer/user-docs/TaintAnalysisConfiguration.rst:41-51
+  # sources:
+# signature:
+# size_t fread(void *ptr, size_t size, size_t nmemb, FILE * stream)
+#
+# example:
+# FILE* f = fopen("file.txt");
+# char buf[1024];

Since there the `SrcArgs` is //empty//, `fread` will work as an 
//unconditional// propagator - aka. a taint source.
It's probably better to phrase it something like that.

By doing so you could signify that in the next example you have a //non-empty// 
`SrcArgs` list, thus the propagation is //conditional//.

I think it would be better to follow this pattern in the corresponding section 
later as well.



Comment at: clang/docs/analyzer/user-docs/TaintAnalysisConfiguration.rst:49
+# size_t read = fread(buf, sizeof(buf[0]), sizeof(buf)/sizeof(buf[0]), f);
+# // read and buf is tainted
+- Name: fread





Comment at: clang/docs/analyzer/user-docs/TaintAnalysisConfiguration.rst:76
+In the example file above, the entries under the `Propagation` key implement 
the conceptual sources and propagations, and sinks have their dedicated `Sinks` 
key.
+The user can define program points where the tainted values should be cleansed 
by listing entries under the `Filters` key.
+Filters model the sanitization of values done by the programmer, and providing 
these is key to avoiding false-positive findings.

The literature probably refers to //cleansing// functions as **sanitizers**. [[ 
https://en.wikipedia.org/wiki/Taint_checking | Wikipedia ]]



Comment at: clang/docs/analyzer/user-docs/TaintAnalysisConfiguration.rst:89
+
+Under the `Filters` entry, the user can specify a list of events that remove 
taint (see :ref:`taint-filter-details` for details).
+

Previously it was referred to by the `key` word. Chose one for consistency.



Comment at: clang/docs/analyzer/user-docs/TaintAnalysisConfiguration.rst:89
+
+Under the `Filters` entry, the user can specify a list of events that remove 
taint (see :ref:`taint-filter-details` for details).
+

steakhal wrote:
> Previously it was referred to by the `key` word. Chose one for consistency.
I would rather use the //operation// or //function call// instead of //event// 
everywhere.



Comment at: clang/docs/analyzer/user-docs/TaintAnalysisConfiguration.rst:91
+
+Under the `Propagations` entry, the user can specify a list of events that 
generate and propagate taint (see :ref:`taint-propagation-details` for details).
+The user can identify taint sources with a `SrcArgs` key in the `Propagation` 
entry, while propagations have none.





Comment at: clang/docs/analyzer/user-docs/TaintAnalysisConfiguration.rst:92
+Under the `Propagations` entry, the user can specify 

[PATCH] D111866: [RISCV] Support Zfhmin extension

[Shao-Ce SUN via Phabricator via cfe-commits]

achieveartificialintelligence added a comment.

In D111866#3111243 , @asb wrote:

> LGTM, modulo one tiny nit on a comment. Thanks!

Thank you for your detailed suggestions!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111866

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

[PATCH] D111866: [RISCV] Support Zfhmin extension

[Shao-Ce SUN 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 rG5c3d7184b435: [RISCV] Support Zfhmin extension (authored by 
achieveartificialintelligence).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111866

Files:
  clang/test/Driver/riscv-arch.c
  llvm/lib/Support/RISCVISAInfo.cpp
  llvm/lib/Target/RISCV/RISCV.td
  llvm/lib/Target/RISCV/RISCVISelLowering.cpp
  llvm/lib/Target/RISCV/RISCVInstrInfoZfh.td
  llvm/lib/Target/RISCV/RISCVSubtarget.h
  llvm/test/CodeGen/RISCV/attributes.ll
  llvm/test/MC/RISCV/attribute-arch.s
  llvm/test/MC/RISCV/rv32zfhmin-invalid.s
  llvm/test/MC/RISCV/rv32zfhmin-valid.s

Index: llvm/test/MC/RISCV/rv32zfhmin-valid.s
===
--- /dev/null
+++ llvm/test/MC/RISCV/rv32zfhmin-valid.s
@@ -0,0 +1,62 @@
+# RUN: llvm-mc %s -triple=riscv32 -mattr=+experimental-zfhmin,+d -riscv-no-aliases -show-encoding \
+# RUN: | FileCheck -check-prefixes=CHECK-ASM,CHECK-ASM-AND-OBJ %s
+# RUN: llvm-mc %s -triple=riscv64 -mattr=+experimental-zfhmin,+d -riscv-no-aliases -show-encoding \
+# RUN: | FileCheck -check-prefixes=CHECK-ASM,CHECK-ASM-AND-OBJ %s
+# RUN: llvm-mc -filetype=obj -triple=riscv32 -mattr=+experimental-zfhmin,+d < %s \
+# RUN: | llvm-objdump --mattr=+experimental-zfhmin,+d -M no-aliases -d -r - \
+# RUN: | FileCheck --check-prefix=CHECK-ASM-AND-OBJ %s
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+experimental-zfhmin,+d < %s \
+# RUN: | llvm-objdump --mattr=+experimental-zfhmin,+d -M no-aliases -d -r - \
+# RUN: | FileCheck --check-prefix=CHECK-ASM-AND-OBJ %s
+
+# CHECK-ASM-AND-OBJ: flh ft0, 12(a0)
+# CHECK-ASM: encoding: [0x07,0x10,0xc5,0x00]
+flh f0, 12(a0)
+# CHECK-ASM-AND-OBJ: flh ft1, 4(ra)
+# CHECK-ASM: encoding: [0x87,0x90,0x40,0x00]
+flh f1, +4(ra)
+# CHECK-ASM-AND-OBJ: flh ft2, -2048(a3)
+# CHECK-ASM: encoding: [0x07,0x91,0x06,0x80]
+flh f2, -2048(x13)
+# CHECK-ASM-AND-OBJ: flh ft3, -2048(s1)
+# CHECK-ASM: encoding: [0x87,0x91,0x04,0x80]
+flh f3, %lo(2048)(s1)
+# CHECK-ASM-AND-OBJ: flh ft4, 2047(s2)
+# CHECK-ASM: encoding: [0x07,0x12,0xf9,0x7f]
+flh f4, 2047(s2)
+# CHECK-ASM-AND-OBJ: flh ft5, 0(s3)
+# CHECK-ASM: encoding: [0x87,0x92,0x09,0x00]
+flh f5, 0(s3)
+
+# CHECK-ASM-AND-OBJ: fsh ft6, 2047(s4)
+# CHECK-ASM: encoding: [0xa7,0x1f,0x6a,0x7e]
+fsh f6, 2047(s4)
+# CHECK-ASM-AND-OBJ: fsh ft7, -2048(s5)
+# CHECK-ASM: encoding: [0x27,0x90,0x7a,0x80]
+fsh f7, -2048(s5)
+# CHECK-ASM-AND-OBJ: fsh fs0, -2048(s6)
+# CHECK-ASM: encoding: [0x27,0x10,0x8b,0x80]
+fsh f8, %lo(2048)(s6)
+# CHECK-ASM-AND-OBJ: fsh fs1, 999(s7)
+# CHECK-ASM: encoding: [0xa7,0x93,0x9b,0x3e]
+fsh f9, 999(s7)
+
+# CHECK-ASM-AND-OBJ: fmv.x.h a2, fs7
+# CHECK-ASM: encoding: [0x53,0x86,0x0b,0xe4]
+fmv.x.h a2, fs7
+# CHECK-ASM-AND-OBJ: fmv.h.x ft1, a6
+# CHECK-ASM: encoding: [0xd3,0x00,0x08,0xf4]
+fmv.h.x ft1, a6
+
+# CHECK-ASM-AND-OBJ: fcvt.s.h fa0, ft0
+# CHECK-ASM: encoding: [0x53,0x05,0x20,0x40]
+fcvt.s.h fa0, ft0
+# CHECK-ASM-AND-OBJ: fcvt.h.s ft2, fa2
+# CHECK-ASM: encoding: [0x53,0x71,0x06,0x44]
+fcvt.h.s ft2, fa2
+# CHECK-ASM-AND-OBJ: fcvt.d.h fa0, ft0
+# CHECK-ASM: encoding: [0x53,0x05,0x20,0x42]
+fcvt.d.h fa0, ft0
+# CHECK-ASM-AND-OBJ: fcvt.h.d ft2, fa2
+# CHECK-ASM: encoding: [0x53,0x71,0x16,0x44]
+fcvt.h.d ft2, fa2
Index: llvm/test/MC/RISCV/rv32zfhmin-invalid.s
===
--- /dev/null
+++ llvm/test/MC/RISCV/rv32zfhmin-invalid.s
@@ -0,0 +1,23 @@
+# RUN: not llvm-mc -triple riscv32 -mattr=+experimental-zfhmin < %s 2>&1 | \
+# RUN:   FileCheck %s
+
+# Out of range immediates
+## simm12
+flh ft1, -2049(a0) # CHECK: :[[@LINE]]:10: error: operand must be a symbol with %lo/%pcrel_lo/%tprel_lo modifier or an integer in the range [-2048, 2047]
+fsh ft2, 2048(a1) # CHECK: :[[@LINE]]:10: error: operand must be a symbol with %lo/%pcrel_lo/%tprel_lo modifier or an integer in the range [-2048, 2047]
+
+# Memory operand not formatted correctly
+flh ft1, a0, -200 # CHECK: :[[@LINE]]:14: error: invalid operand for instruction
+
+# Invalid register names
+flh ft15, 100(a0) # CHECK: :[[@LINE]]:5: error: invalid operand for instruction
+flh ft1, 100(a10) # CHECK: :[[@LINE]]:14: error: expected register
+
+# Integer registers where FP regs are expected
+fmv.x.h fs7, a2 # CHECK: :[[@LINE]]:9: error: invalid operand for instruction
+
+# FP registers where integer regs are expected
+fmv.h.x a8, ft2 # CHECK: :[[@LINE]]:9: error: invalid operand for instruction
+
+# Zfh instructions
+fmadd.h f10, f11, f12, f13, dyn # CHECK: :[[@LINE]]:1: error: instruction requires the following: 'Zfh' (Half-Precision Floating-Point)
Index: llvm/test/MC/RISCV/attribute-arch.s
===
--- llvm/test/MC/RISCV/attribute-arch.s
+++ llvm/test/MC/RISCV/attribute-arch.s
@@ -66,8 

[clang] 5c3d718 - [RISCV] Support Zfhmin extension

[Shao-Ce SUN via cfe-commits]

Author: Shao-Ce SUN
Date: 2021-11-06T01:41:02+08:00
New Revision: 5c3d7184b43575e4cbf1da2fa6ba88485eaca4e3

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

LOG: [RISCV] Support Zfhmin extension

According to RISC-V Unprivileged ISA 15.6.

Reviewed By: asb

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

Added: 
llvm/test/MC/RISCV/rv32zfhmin-invalid.s
llvm/test/MC/RISCV/rv32zfhmin-valid.s

Modified: 
clang/test/Driver/riscv-arch.c
llvm/lib/Support/RISCVISAInfo.cpp
llvm/lib/Target/RISCV/RISCV.td
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
llvm/lib/Target/RISCV/RISCVInstrInfoZfh.td
llvm/lib/Target/RISCV/RISCVSubtarget.h
llvm/test/CodeGen/RISCV/attributes.ll
llvm/test/MC/RISCV/attribute-arch.s

Removed: 




diff  --git a/clang/test/Driver/riscv-arch.c b/clang/test/Driver/riscv-arch.c
index 4634cb7e9c9fb..bbbc0f3ded78a 100644
--- a/clang/test/Driver/riscv-arch.c
+++ b/clang/test/Driver/riscv-arch.c
@@ -426,6 +426,15 @@
 // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-EXPERIMENTAL-ZFH %s
 // RV32-EXPERIMENTAL-ZFH: "-target-feature" "+experimental-zfh"
 
+// RUN: %clang -target riscv32-unknown-elf -march=rv32izfhmin -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck 
-check-prefix=RV32-EXPERIMENTAL-ZFHMIN-NOFLAG %s
+// RV32-EXPERIMENTAL-ZFHMIN-NOFLAG: error: invalid arch name 'rv32izfhmin'
+// RV32-EXPERIMENTAL-ZFHMIN-NOFLAG: requires '-menable-experimental-extensions'
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv32izfhmin0p1 
-menable-experimental-extensions -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-EXPERIMENTAL-ZFHMIN 
%s
+// RV32-EXPERIMENTAL-ZFHMIN: "-target-feature" "+experimental-zfhmin"
+
 // RUN: %clang -target riscv32-unknown-elf -march=rv32izvamo -### %s -c 2>&1 | 
\
 // RUN:   FileCheck -check-prefix=RV32-EXPERIMENTAL-ZVAMO-NOFLAG %s
 // RV32-EXPERIMENTAL-ZVAMO-NOFLAG: error: invalid arch name 'rv32izvamo'

diff  --git a/llvm/lib/Support/RISCVISAInfo.cpp 
b/llvm/lib/Support/RISCVISAInfo.cpp
index 8bbfc757f0755..8e984002f90d2 100644
--- a/llvm/lib/Support/RISCVISAInfo.cpp
+++ b/llvm/lib/Support/RISCVISAInfo.cpp
@@ -64,6 +64,7 @@ static const RISCVSupportedExtension 
SupportedExperimentalExtensions[] = {
 {"zvamo", RISCVExtensionVersion{0, 10}},
 {"zvlsseg", RISCVExtensionVersion{0, 10}},
 
+{"zfhmin", RISCVExtensionVersion{0, 1}},
 {"zfh", RISCVExtensionVersion{0, 1}},
 };
 

diff  --git a/llvm/lib/Target/RISCV/RISCV.td b/llvm/lib/Target/RISCV/RISCV.td
index 48dbcfee886c2..772a4f8ecd535 100644
--- a/llvm/lib/Target/RISCV/RISCV.td
+++ b/llvm/lib/Target/RISCV/RISCV.td
@@ -41,10 +41,18 @@ def HasStdExtD : Predicate<"Subtarget->hasStdExtD()">,
AssemblerPredicate<(all_of FeatureStdExtD),
"'D' (Double-Precision Floating-Point)">;
 
+def FeatureStdExtZfhmin
+: SubtargetFeature<"experimental-zfhmin", "HasStdExtZfhmin", "true",
+   "'Zfhmin' (Half-Precision Floating-Point Minimal)",
+   [FeatureStdExtF]>;
+def HasStdExtZfhmin : Predicate<"Subtarget->hasStdExtZfhmin()">,
+ AssemblerPredicate<(all_of FeatureStdExtZfhmin),
+ "'Zfhmin' (Half-Precision Floating-Point 
Minimal)">;
+
 def FeatureStdExtZfh
 : SubtargetFeature<"experimental-zfh", "HasStdExtZfh", "true",
"'Zfh' (Half-Precision Floating-Point)",
-   [FeatureStdExtF]>;
+   [FeatureStdExtZfhmin, FeatureStdExtF]>;
 def HasStdExtZfh : Predicate<"Subtarget->hasStdExtZfh()">,
  AssemblerPredicate<(all_of FeatureStdExtZfh),
  "'Zfh' (Half-Precision Floating-Point)">;

diff  --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp 
b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
index bcbca9d2fa5a8..193c5b4c91db3 100644
--- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -1166,7 +1166,7 @@ bool RISCVTargetLowering::shouldSinkOperands(
 
 bool RISCVTargetLowering::isFPImmLegal(const APFloat , EVT VT,
bool ForCodeSize) const {
-  if (VT == MVT::f16 && !Subtarget.hasStdExtZfh())
+  if (VT == MVT::f16 && !Subtarget.hasStdExtZfhmin())
 return false;
   if (VT == MVT::f32 && !Subtarget.hasStdExtF())
 return false;
@@ -1186,9 +1186,9 @@ bool RISCVTargetLowering::hasBitPreservingFPLogic(EVT VT) 
const {
 MVT RISCVTargetLowering::getRegisterTypeForCallingConv(LLVMContext ,
   CallingConv::ID CC,
   EVT VT) const {
-  // Use f32 to pass f16 if it is legal and Zfh is not 

[PATCH] D113231: [NewPM] Only invalidate modified functions' analyses in CGSCC passes + turn on eagerly invalidate analyses

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks updated this revision to Diff 385122.
aeubanks added a comment.
Herald added subscribers: cfe-commits, wenlei, steven_wu.
Herald added a project: clang.

update tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113231

Files:
  clang/test/CodeGen/thinlto-distributed-newpm.ll
  llvm/lib/Analysis/CGSCCPassManager.cpp
  llvm/lib/Passes/PassBuilderPipelines.cpp
  llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
  llvm/lib/Transforms/IPO/FunctionAttrs.cpp
  llvm/lib/Transforms/IPO/Inliner.cpp
  llvm/test/Other/new-pm-defaults.ll
  llvm/test/Other/new-pm-eager-invalidate.ll
  llvm/test/Other/new-pm-lto-defaults.ll
  llvm/test/Other/new-pm-pgo-preinline.ll
  llvm/test/Other/new-pm-thinlto-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
  llvm/test/Transforms/Inline/analysis-invalidation.ll
  llvm/test/Transforms/Inline/cgscc-incremental-invalidate.ll

Index: llvm/test/Transforms/Inline/cgscc-incremental-invalidate.ll
===
--- llvm/test/Transforms/Inline/cgscc-incremental-invalidate.ll
+++ llvm/test/Transforms/Inline/cgscc-incremental-invalidate.ll
@@ -8,11 +8,11 @@
 ;
 ; CHECK: Running pass: InlinerPass on (test1_f, test1_g, test1_h)
 ; CHECK: Running analysis: DominatorTreeAnalysis on test1_f
-; CHECK: Running analysis: DominatorTreeAnalysis on test1_g
 ; CHECK: Invalidating analysis: DominatorTreeAnalysis on test1_f
 ; CHECK: Invalidating analysis: LoopAnalysis on test1_f
 ; CHECK: Invalidating analysis: BranchProbabilityAnalysis on test1_f
 ; CHECK: Invalidating analysis: BlockFrequencyAnalysis on test1_f
+; CHECK: Running analysis: DominatorTreeAnalysis on test1_g
 ; CHECK: Invalidating analysis: DominatorTreeAnalysis on test1_g
 ; CHECK: Invalidating analysis: LoopAnalysis on test1_g
 ; CHECK: Invalidating analysis: BranchProbabilityAnalysis on test1_g
@@ -29,7 +29,6 @@
 ; CHECK-NEXT: Running analysis: DominatorTreeAnalysis on test1_h
 ; CHECK-NOT: Invalidating analysis:
 ; CHECK: Running pass: DominatorTreeVerifierPass on test1_f
-; CHECK-NEXT: Running analysis: DominatorTreeAnalysis on test1_f
 
 ; An external function used to control branches.
 declare i1 @flag()
Index: llvm/test/Transforms/Inline/analysis-invalidation.ll
===
--- /dev/null
+++ llvm/test/Transforms/Inline/analysis-invalidation.ll
@@ -0,0 +1,17 @@
+; RUN: opt -passes=inline < %s -disable-output -debug-pass-manager 2>&1 | FileCheck %s
+
+; We shouldn't invalidate any function analyses on g since it's never modified.
+
+; CHECK-NOT: Invalidating{{.*}} on g
+; CHECK: Invalidating{{.*}} on f
+; CHECK-NOT: Invalidating{{.*}} on g
+
+define void @f() noinline {
+  call void @g()
+  ret void
+}
+
+define void @g() alwaysinline {
+  call void @f()
+  ret void
+}
Index: llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
===
--- llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
+++ llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
@@ -1,26 +1,26 @@
 ; Validate ThinLTO prelink pipeline when we have Sample PGO
 ;
-; RUN: opt -disable-verify -verify-cfg-preserved=0 -debug-pass-manager \
+; RUN: opt -disable-verify -verify-cfg-preserved=0 -eagerly-invalidate-analyses=0 -debug-pass-manager \
 ; RUN: -pgo-kind=pgo-sample-use-pipeline -profile-file='%S/Inputs/new-pm-thinlto-samplepgo-defaults.prof' \
 ; RUN: -passes='thinlto-pre-link' -S %s 2>&1 \
 ; RUN: | FileCheck %s --check-prefixes=CHECK-O,CHECK-O1
-; RUN: opt -disable-verify -verify-cfg-preserved=0 -debug-pass-manager \
+; RUN: opt -disable-verify -verify-cfg-preserved=0 -eagerly-invalidate-analyses=0 -debug-pass-manager \
 ; RUN: -pgo-kind=pgo-sample-use-pipeline -profile-file='%S/Inputs/new-pm-thinlto-samplepgo-defaults.prof' \
 ; RUN: -passes='thinlto-pre-link' -S  %s 2>&1 \
 ; RUN: | FileCheck %s --check-prefixes=CHECK-O,CHECK-O2,CHECK-O23SZ
-; RUN: opt -disable-verify -verify-cfg-preserved=0 -debug-pass-manager \
+; RUN: opt -disable-verify -verify-cfg-preserved=0 -eagerly-invalidate-analyses=0 -debug-pass-manager \
 ; RUN: -pgo-kind=pgo-sample-use-pipeline -profile-file='%S/Inputs/new-pm-thinlto-samplepgo-defaults.prof' \
 ; RUN: -passes='thinlto-pre-link' -S -passes-ep-pipeline-start='no-op-module' %s 2>&1 \
 ; RUN: | FileCheck %s --check-prefixes=CHECK-O,CHECK-O3,CHECK-O23SZ,CHECK-EP-PIPELINE-START
-; RUN: opt -disable-verify -verify-cfg-preserved=0 -debug-pass-manager \
+; RUN: opt -disable-verify -verify-cfg-preserved=0 -eagerly-invalidate-analyses=0 -debug-pass-manager \
 ; RUN: -pgo-kind=pgo-sample-use-pipeline 

[PATCH] D111866: [RISCV] Support Zfhmin extension

[Shao-Ce SUN via Phabricator via cfe-commits]

achieveartificialintelligence updated this revision to Diff 385121.
achieveartificialintelligence added a comment.

Fix


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111866

Files:
  clang/test/Driver/riscv-arch.c
  llvm/lib/Support/RISCVISAInfo.cpp
  llvm/lib/Target/RISCV/RISCV.td
  llvm/lib/Target/RISCV/RISCVISelLowering.cpp
  llvm/lib/Target/RISCV/RISCVInstrInfoZfh.td
  llvm/lib/Target/RISCV/RISCVSubtarget.h
  llvm/test/CodeGen/RISCV/attributes.ll
  llvm/test/MC/RISCV/attribute-arch.s
  llvm/test/MC/RISCV/rv32zfhmin-invalid.s
  llvm/test/MC/RISCV/rv32zfhmin-valid.s

Index: llvm/test/MC/RISCV/rv32zfhmin-valid.s
===
--- /dev/null
+++ llvm/test/MC/RISCV/rv32zfhmin-valid.s
@@ -0,0 +1,62 @@
+# RUN: llvm-mc %s -triple=riscv32 -mattr=+experimental-zfhmin,+d -riscv-no-aliases -show-encoding \
+# RUN: | FileCheck -check-prefixes=CHECK-ASM,CHECK-ASM-AND-OBJ %s
+# RUN: llvm-mc %s -triple=riscv64 -mattr=+experimental-zfhmin,+d -riscv-no-aliases -show-encoding \
+# RUN: | FileCheck -check-prefixes=CHECK-ASM,CHECK-ASM-AND-OBJ %s
+# RUN: llvm-mc -filetype=obj -triple=riscv32 -mattr=+experimental-zfhmin,+d < %s \
+# RUN: | llvm-objdump --mattr=+experimental-zfhmin,+d -M no-aliases -d -r - \
+# RUN: | FileCheck --check-prefix=CHECK-ASM-AND-OBJ %s
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+experimental-zfhmin,+d < %s \
+# RUN: | llvm-objdump --mattr=+experimental-zfhmin,+d -M no-aliases -d -r - \
+# RUN: | FileCheck --check-prefix=CHECK-ASM-AND-OBJ %s
+
+# CHECK-ASM-AND-OBJ: flh ft0, 12(a0)
+# CHECK-ASM: encoding: [0x07,0x10,0xc5,0x00]
+flh f0, 12(a0)
+# CHECK-ASM-AND-OBJ: flh ft1, 4(ra)
+# CHECK-ASM: encoding: [0x87,0x90,0x40,0x00]
+flh f1, +4(ra)
+# CHECK-ASM-AND-OBJ: flh ft2, -2048(a3)
+# CHECK-ASM: encoding: [0x07,0x91,0x06,0x80]
+flh f2, -2048(x13)
+# CHECK-ASM-AND-OBJ: flh ft3, -2048(s1)
+# CHECK-ASM: encoding: [0x87,0x91,0x04,0x80]
+flh f3, %lo(2048)(s1)
+# CHECK-ASM-AND-OBJ: flh ft4, 2047(s2)
+# CHECK-ASM: encoding: [0x07,0x12,0xf9,0x7f]
+flh f4, 2047(s2)
+# CHECK-ASM-AND-OBJ: flh ft5, 0(s3)
+# CHECK-ASM: encoding: [0x87,0x92,0x09,0x00]
+flh f5, 0(s3)
+
+# CHECK-ASM-AND-OBJ: fsh ft6, 2047(s4)
+# CHECK-ASM: encoding: [0xa7,0x1f,0x6a,0x7e]
+fsh f6, 2047(s4)
+# CHECK-ASM-AND-OBJ: fsh ft7, -2048(s5)
+# CHECK-ASM: encoding: [0x27,0x90,0x7a,0x80]
+fsh f7, -2048(s5)
+# CHECK-ASM-AND-OBJ: fsh fs0, -2048(s6)
+# CHECK-ASM: encoding: [0x27,0x10,0x8b,0x80]
+fsh f8, %lo(2048)(s6)
+# CHECK-ASM-AND-OBJ: fsh fs1, 999(s7)
+# CHECK-ASM: encoding: [0xa7,0x93,0x9b,0x3e]
+fsh f9, 999(s7)
+
+# CHECK-ASM-AND-OBJ: fmv.x.h a2, fs7
+# CHECK-ASM: encoding: [0x53,0x86,0x0b,0xe4]
+fmv.x.h a2, fs7
+# CHECK-ASM-AND-OBJ: fmv.h.x ft1, a6
+# CHECK-ASM: encoding: [0xd3,0x00,0x08,0xf4]
+fmv.h.x ft1, a6
+
+# CHECK-ASM-AND-OBJ: fcvt.s.h fa0, ft0
+# CHECK-ASM: encoding: [0x53,0x05,0x20,0x40]
+fcvt.s.h fa0, ft0
+# CHECK-ASM-AND-OBJ: fcvt.h.s ft2, fa2
+# CHECK-ASM: encoding: [0x53,0x71,0x06,0x44]
+fcvt.h.s ft2, fa2
+# CHECK-ASM-AND-OBJ: fcvt.d.h fa0, ft0
+# CHECK-ASM: encoding: [0x53,0x05,0x20,0x42]
+fcvt.d.h fa0, ft0
+# CHECK-ASM-AND-OBJ: fcvt.h.d ft2, fa2
+# CHECK-ASM: encoding: [0x53,0x71,0x16,0x44]
+fcvt.h.d ft2, fa2
Index: llvm/test/MC/RISCV/rv32zfhmin-invalid.s
===
--- /dev/null
+++ llvm/test/MC/RISCV/rv32zfhmin-invalid.s
@@ -0,0 +1,23 @@
+# RUN: not llvm-mc -triple riscv32 -mattr=+experimental-zfhmin < %s 2>&1 | \
+# RUN:   FileCheck %s
+
+# Out of range immediates
+## simm12
+flh ft1, -2049(a0) # CHECK: :[[@LINE]]:10: error: operand must be a symbol with %lo/%pcrel_lo/%tprel_lo modifier or an integer in the range [-2048, 2047]
+fsh ft2, 2048(a1) # CHECK: :[[@LINE]]:10: error: operand must be a symbol with %lo/%pcrel_lo/%tprel_lo modifier or an integer in the range [-2048, 2047]
+
+# Memory operand not formatted correctly
+flh ft1, a0, -200 # CHECK: :[[@LINE]]:14: error: invalid operand for instruction
+
+# Invalid register names
+flh ft15, 100(a0) # CHECK: :[[@LINE]]:5: error: invalid operand for instruction
+flh ft1, 100(a10) # CHECK: :[[@LINE]]:14: error: expected register
+
+# Integer registers where FP regs are expected
+fmv.x.h fs7, a2 # CHECK: :[[@LINE]]:9: error: invalid operand for instruction
+
+# FP registers where integer regs are expected
+fmv.h.x a8, ft2 # CHECK: :[[@LINE]]:9: error: invalid operand for instruction
+
+# Zfh instructions
+fmadd.h f10, f11, f12, f13, dyn # CHECK: :[[@LINE]]:1: error: instruction requires the following: 'Zfh' (Half-Precision Floating-Point)
Index: llvm/test/MC/RISCV/attribute-arch.s
===
--- llvm/test/MC/RISCV/attribute-arch.s
+++ llvm/test/MC/RISCV/attribute-arch.s
@@ -66,8 +66,11 @@
 .attribute arch, "rv32izbt"
 # CHECK: attribute  5, "rv32i2p0_zbt0p93"
 
+.attribute arch, "rv32ifzfhmin"

[PATCH] D113249: [CUDA] Bump CUDA version to 11.5

[Artem Belevich via Phabricator via cfe-commits]

tra requested changes to this revision.
tra added a comment.
This revision now requires changes to proceed.

I think we're missing few more changes here:

- The driver needs to enable ptx75 when it constructs cc1 command line in 
clang/lib/Driver/ToolChains/Cuda.cpp
- We also need to handle PTX75 in clang/include/clang/Basic/BuiltinsNVPTX.def

In D113249#3110954 , @Hahnfeld wrote:

> I'm not sure if it's actually correct to advertise full support for CUDA 
> 11.5, but I didn't look into exact changes since 11.4

Technically we never support all the features supported by NVCC, so for clang 
it essentially means "works well enough". I.e. no known regressions vs previous 
clang and CUDA versions. Usually it boils down to being able to compile CUDA 
headers.
"Partially supported" happens when we can compile code compileable with the 
older CUDA versions, but are missing something critical introduced by the new 
CUDA version. E.g. a new GPU variant. Or new compiler builtins/functions  that 
a user may expect from the new CUDA version. Or some CUDA headers may use new 
instructions in inline asm that would not compile with ptxas unless we generate 
PTX output using the new PTX version.

AFAICT from the CUDA-11.5 release notes, it didn't introduce anything 
particularly interesting. We've been using clang with CUDA-11.5 for a few weeks 
w/o any issues, so I think it's fine to stamp it as supported, once the missing 
bits are in place.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113249

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

[PATCH] D112847: [AIX][Clang] Fix XL product name in AIX XL compatibility warning

[Zarko Todorovski 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 rG1b7528554f83: [AIX][Clang] Fix XL product name in AIX XL 
compatibility warning (authored by ZarkoCA).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112847

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/test/Sema/aix-attr-align.c


Index: clang/test/Sema/aix-attr-align.c
===
--- clang/test/Sema/aix-attr-align.c
+++ clang/test/Sema/aix-attr-align.c
@@ -10,11 +10,11 @@
 };
 
 struct T {
-  int a[4] __attribute__((aligned(16))); // expected-warning {{requesting an 
alignment of 16 bytes or greater for struct members is not binary compatible 
with AIX XL 16.1 and older}}
+  int a[4] __attribute__((aligned(16))); // expected-warning {{requesting an 
alignment of 16 bytes or greater for struct members is not binary compatible 
with IBM XL C/C++ for AIX 16.1.0 and older}}
 };
 
 struct U {
-  int a[2] __attribute__((aligned(32))); // expected-warning {{requesting an 
alignment of 16 bytes or greater for struct members is not binary compatible 
with AIX XL 16.1 and older}}
+  int a[2] __attribute__((aligned(32))); // expected-warning {{requesting an 
alignment of 16 bytes or greater for struct members is not binary compatible 
with IBM XL C/C++ for AIX 16.1.0 and older}}
 };
 
 int a[8] __attribute__((aligned(8)));  // no-warning
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3277,7 +3277,8 @@
   InGroup>;
 def warn_not_xl_compatible
 : Warning<"requesting an alignment of 16 bytes or greater for struct"
-  " members is not binary compatible with AIX XL 16.1 and older">,
+  " members is not binary compatible with IBM XL C/C++ for AIX"
+  " 16.1.0 and older">,
   InGroup;
 def warn_redeclaration_without_attribute_prev_attribute_ignored : Warning<
   "%q0 redeclared without %1 attribute: previous %1 ignored">,


Index: clang/test/Sema/aix-attr-align.c
===
--- clang/test/Sema/aix-attr-align.c
+++ clang/test/Sema/aix-attr-align.c
@@ -10,11 +10,11 @@
 };
 
 struct T {
-  int a[4] __attribute__((aligned(16))); // expected-warning {{requesting an alignment of 16 bytes or greater for struct members is not binary compatible with AIX XL 16.1 and older}}
+  int a[4] __attribute__((aligned(16))); // expected-warning {{requesting an alignment of 16 bytes or greater for struct members is not binary compatible with IBM XL C/C++ for AIX 16.1.0 and older}}
 };
 
 struct U {
-  int a[2] __attribute__((aligned(32))); // expected-warning {{requesting an alignment of 16 bytes or greater for struct members is not binary compatible with AIX XL 16.1 and older}}
+  int a[2] __attribute__((aligned(32))); // expected-warning {{requesting an alignment of 16 bytes or greater for struct members is not binary compatible with IBM XL C/C++ for AIX 16.1.0 and older}}
 };
 
 int a[8] __attribute__((aligned(8)));  // no-warning
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3277,7 +3277,8 @@
   InGroup>;
 def warn_not_xl_compatible
 : Warning<"requesting an alignment of 16 bytes or greater for struct"
-  " members is not binary compatible with AIX XL 16.1 and older">,
+  " members is not binary compatible with IBM XL C/C++ for AIX"
+  " 16.1.0 and older">,
   InGroup;
 def warn_redeclaration_without_attribute_prev_attribute_ignored : Warning<
   "%q0 redeclared without %1 attribute: previous %1 ignored">,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 1b75285 - [AIX][Clang] Fix XL product name in AIX XL compatibility warning

[Zarko Todorovski via cfe-commits]

Author: Zarko Todorovski
Date: 2021-11-05T13:17:30-04:00
New Revision: 1b7528554f835d58036543de1e8839ba1ac29f1f

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

LOG: [AIX][Clang] Fix XL product name in AIX XL compatibility warning

Correct the XLC/C++ version in the warning message to use the information from
XL's -qversion output.

Reviewed By: rzurob

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

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/test/Sema/aix-attr-align.c

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 1daea69e791c9..3f887309825a5 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3277,7 +3277,8 @@ def warn_assume_aligned_too_great
   InGroup>;
 def warn_not_xl_compatible
 : Warning<"requesting an alignment of 16 bytes or greater for struct"
-  " members is not binary compatible with AIX XL 16.1 and older">,
+  " members is not binary compatible with IBM XL C/C++ for AIX"
+  " 16.1.0 and older">,
   InGroup;
 def warn_redeclaration_without_attribute_prev_attribute_ignored : Warning<
   "%q0 redeclared without %1 attribute: previous %1 ignored">,

diff  --git a/clang/test/Sema/aix-attr-align.c 
b/clang/test/Sema/aix-attr-align.c
index ac70aab669004..0fd6af4ee4c13 100644
--- a/clang/test/Sema/aix-attr-align.c
+++ b/clang/test/Sema/aix-attr-align.c
@@ -10,11 +10,11 @@ struct S {
 };
 
 struct T {
-  int a[4] __attribute__((aligned(16))); // expected-warning {{requesting an 
alignment of 16 bytes or greater for struct members is not binary compatible 
with AIX XL 16.1 and older}}
+  int a[4] __attribute__((aligned(16))); // expected-warning {{requesting an 
alignment of 16 bytes or greater for struct members is not binary compatible 
with IBM XL C/C++ for AIX 16.1.0 and older}}
 };
 
 struct U {
-  int a[2] __attribute__((aligned(32))); // expected-warning {{requesting an 
alignment of 16 bytes or greater for struct members is not binary compatible 
with AIX XL 16.1 and older}}
+  int a[2] __attribute__((aligned(32))); // expected-warning {{requesting an 
alignment of 16 bytes or greater for struct members is not binary compatible 
with IBM XL C/C++ for AIX 16.1.0 and older}}
 };
 
 int a[8] __attribute__((aligned(8)));  // no-warning



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

[PATCH] D113294: [IR] Remove unbounded as possible value for vscale_range minimum

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes created this revision.
c-rhodes added reviewers: sdesmalen, bsmith, paulwalker-arm.
Herald added subscribers: ctetreau, dexonsmith, dang, jdoerfert, hiraditya.
c-rhodes requested review of this revision.
Herald added projects: clang, LLVM.
Herald added a subscriber: cfe-commits.

The default for min is changed to 1. The behaviour of -mvscale-{min,max}
in Clang is also changed such that 16 is the max vscale when targeting
SVE and no max is specified.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113294

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/arm-sve-vector-bits-vscale-range.c
  clang/test/Frontend/aarch64-vscale-min.c
  llvm/docs/LangRef.rst
  llvm/include/llvm/IR/Attributes.h
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/test/Analysis/CostModel/AArch64/sve-gather.ll
  llvm/test/Analysis/CostModel/AArch64/sve-scatter.ll
  llvm/test/Bitcode/attributes.ll
  llvm/test/Transforms/InstCombine/icmp-vscale.ll
  llvm/test/Transforms/InstCombine/vscale_sext_and_zext.ll
  llvm/test/Transforms/InstCombine/vscale_trunc.ll
  llvm/test/Transforms/LoopVectorize/AArch64/first-order-recurrence.ll
  llvm/test/Transforms/LoopVectorize/AArch64/scalable-strict-fadd.ll
  llvm/test/Transforms/LoopVectorize/AArch64/scalable-vectorization.ll
  llvm/test/Transforms/LoopVectorize/AArch64/scalable-vf-hint.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-cond-inv-loads.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-gather-scatter.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
  llvm/test/Verifier/vscale_range.ll

Index: llvm/test/Verifier/vscale_range.ll
===
--- llvm/test/Verifier/vscale_range.ll
+++ llvm/test/Verifier/vscale_range.ll
@@ -1,4 +1,7 @@
 ; RUN: not llvm-as %s -o /dev/null 2>&1 | FileCheck %s
 
+; CHECK: 'vscale_range' minimum cannot be unbounded
+declare i8* @a(i32*) vscale_range(0, 1)
+
 ; CHECK: 'vscale_range' minimum cannot be greater than maximum
 declare i8* @b(i32*) vscale_range(8, 1)
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
@@ -175,7 +175,7 @@
   ret i32 %tmp5
 }
 
-attributes #0 = { vscale_range(0, 16) }
+attributes #0 = { vscale_range(1, 16) }
 !0 = distinct !{!0, !1, !2, !3, !4, !5}
 !1 = !{!"llvm.loop.mustprogress"}
 !2 = !{!"llvm.loop.vectorize.width", i32 4}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
@@ -53,7 +53,7 @@
   ret double %add
 }
 
-attributes #0 = { "target-features"="+sve" vscale_range(0, 16) }
+attributes #0 = { "target-features"="+sve" vscale_range(1, 16) }
 
 !0 = distinct !{!0, !1}
 !1 = !{!"llvm.loop.vectorize.scalable.enable", i1 true}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
@@ -90,7 +90,7 @@
   ret void
 }
 
-attributes #0 = { vscale_range(0, 16) }
+attributes #0 = { vscale_range(1, 16) }
 !0 = distinct !{!0, !1, !2, !3, !4, !5}
 !1 = !{!"llvm.loop.mustprogress"}
 !2 = !{!"llvm.loop.vectorize.width", i32 4}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
@@ -59,7 +59,7 @@
   ret void
 }
 
-attributes #0 = { "target-features"="+neon,+sve" vscale_range(0, 16) }
+attributes #0 = { "target-features"="+neon,+sve" vscale_range(1, 16) }
 
 !0 = distinct !{!0, !1, !2, !3, !4, !5}
 !1 = !{!"llvm.loop.mustprogress"}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-gather-scatter.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-gather-scatter.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-gather-scatter.ll
@@ -153,7 +153,7 @@
   ret void
 }
 
-attributes #0 = { vscale_range(0, 16) }
+attributes #0 = { vscale_range(1, 16) }
 
 !0 = distinct !{!0, !1, !2, !3, !4, !5}
 !1 = 

[PATCH] D113261: [analyzer][solver] Remove reference to RangedConstraintManager

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

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

Yey, it looks good.




Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1608
   LLVM_NODISCARD static ProgramStateRef
-  assign(ProgramStateRef State, RangeConstraintManager ,
- SValBuilder , RangeSet::Factory , ClassOrSymbol CoS,
- RangeSet NewConstraint) {
+  assign(ProgramStateRef State, SValBuilder , RangeSet::Factory ,
+ ClassOrSymbol CoS, RangeSet NewConstraint) {

It seems odd, that sometimes it's called `Builder` and more frequently simply 
`SVB`.
We should consider consolidating this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113261

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

[PATCH] D106823: [analyzer][solver] Iterate to a fixpoint during symbol simplification with constants

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

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

In D106823#3109469 , @martong wrote:

>> As of diff 5, line 1767 and all the code in the block at line 2184 are 
>> uncovered by the tests you provided.
>
> Thanks, I've added new tests that cover the re-assume logic (the block of 
> line 2184).
>
> However, I was unable to add a test that covers  the case when the 
> simplification of a trivial symbol in the disequality info results an 
> infeasible state (line 1767). Here is how I tried: I had changed the line to 
> an assertion and then initiated the static analysis of the following 
> opensource projects: 
> memcached,tmux,curl,twin,redis,vim,openssl,sqlite,ffmpeg,postgresql,tinyxml2,libwebm,xerces,bitcoin,protobuf.
>  My idea had been, if the assertion would fired then I would use creduce to 
> the create the test case (btw, this is how I added the other infeasible state 
> test case). However, it did not fired.
>
> IMHO, having a defensive check at L1767 is correct b/c there is a slight 
> chance of reaching an infeasible state there. Although the chance is minimal, 
> I cannot prove that is 0.

It's fine by me. Thanks for the investigation.

In D106823#310 , @martong wrote:

> There are no runtime or peak memory usage growth with this patch (actually, 
> the runtime decreases with a few %).
> I am attaching the measurements results of the latest Diff.F20089096: 
> stats.html 

Great!

The tests refer to `reg_$0` by spelling the id number, which is unfortunate, 
but I suspect these tests will break for the slightest changes in the solver 
anyway so I'm not too bothered with this.

Please wait a week for the rest of the members of the community to have a look 
before committing.
@NoQ @Szelethus @ASDenysPetrov


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106823

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

[PATCH] D113210: [NewPM] Use the default AA pipeline by default

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks added a comment.

fixed with 7f62759697762473 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113210

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

[PATCH] D113120: [clang] Add early exit when checking for const init of arrays.

[Adam Czachorowski via Phabricator via cfe-commits]

adamcz added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:10617
+// copying the data, which is wasteful.
+for (const unsigned N : {1u, FinalSize}) {
+  unsigned OldElts = Value->getArrayInitializedElts();

kadircet wrote:
> maybe unroll the loop with a helper like `checkConstantInitialization` and 
> just call it for a single element first and perform copy & call again for the 
> whole thing ?
> 
> that way we can migrate performance issues later on (e.g. put a threshold on 
> `FinalSize`, don't perform the small step check if it's less than 100, since 
> copying might introduce a significant extra latency in such cases).
> 
> WDYT?
I thought of that and decided against it. I think the for loop gives us the 
same ability to change the steps - we can just replace {1u, FinalSize} with a 
vector that could have only one element if FinalSize is below some threshold.

Something like:
std::vector steps;
if (FinalSize < 100) steps = {FinalSize};
else steps = {1u, FinalSize};
for (const unsigned N : steps) {
[...]

The problem with helper is that it need sooo many arguments:
N, E, ArrayElt, CAT, Filler, HadZeroInit, Value and "this" (assuming it's free 
function). Seems like it would hurt readability quite a bit.

If you still think it should be a helper, let me know and I won't argue further 
- when it comes to readability the reviewer always knows better :-)



Comment at: clang/lib/AST/ExprConstant.cpp:10641
+  return false;
+if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
+!Info.keepEvaluatingAfterFailure())

kadircet wrote:
> i am a little worried about having "non fatal/error" diagnostics here. But 
> looking at the documentation of the field, intention seems to be making this 
> empty iff evaluation succeeded so far. so it's probably fine, but might be 
> worth introducing in a separate patch to make the revert easy, in case it 
> breaks things (+ this seems to be an orthogonal change to the rest)
Added a short comment.

I don't think splitting this into separate change makes sense. I'd rather just 
revert this one. Without this check the whole two-step process is mostly 
useless, so it's not a state we should be in - either revert the whole thing, 
or keep it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113120

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

[PATCH] D113120: [clang] Add early exit when checking for const init of arrays.

[Adam Czachorowski via Phabricator via cfe-commits]

adamcz updated this revision to Diff 385102.
adamcz marked an inline comment as done.
adamcz added a comment.

review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113120

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/PR51712-large-array-constexpr-check-oom.cpp


Index: clang/test/SemaCXX/PR51712-large-array-constexpr-check-oom.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/PR51712-large-array-constexpr-check-oom.cpp
@@ -0,0 +1,11 @@
+// REQUIRES: shell
+// UNSUPPORTED: win32
+// RUN: ulimit -v 1048576
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+// expected-no-diagnostics
+
+// This used to require too much memory and crash with OOM.
+struct {
+  int a, b, c, d;
+} arr[1<<30];
+
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -10596,28 +10596,55 @@
   bool HadZeroInit = Value->hasValue();
 
   if (const ConstantArrayType *CAT = Info.Ctx.getAsConstantArrayType(Type)) {
-unsigned N = CAT->getSize().getZExtValue();
+unsigned FinalSize = CAT->getSize().getZExtValue();
 
 // Preserve the array filler if we had prior zero-initialization.
 APValue Filler =
   HadZeroInit && Value->hasArrayFiller() ? Value->getArrayFiller()
  : APValue();
 
-*Value = APValue(APValue::UninitArray(), N, N);
-
-if (HadZeroInit)
-  for (unsigned I = 0; I != N; ++I)
-Value->getArrayInitializedElt(I) = Filler;
+*Value = APValue(APValue::UninitArray(), 0, FinalSize);
+if (FinalSize == 0)
+  return true;
 
-// Initialize the elements.
 LValue ArrayElt = Subobject;
 ArrayElt.addArray(Info, E, CAT);
-for (unsigned I = 0; I != N; ++I)
-  if (!VisitCXXConstructExpr(E, ArrayElt, 
>getArrayInitializedElt(I),
- CAT->getElementType()) ||
-  !HandleLValueArrayAdjustment(Info, E, ArrayElt,
-   CAT->getElementType(), 1))
-return false;
+// We do the whole initialization in two passes, first for just one 
element,
+// then for the whole array. It's possible we may find out we can't do 
const
+// init in the first pass, in which case we avoid allocating a potentially
+// large array. We don't do more passes because expanding array requires
+// copying the data, which is wasteful.
+for (const unsigned N : {1u, FinalSize}) {
+  unsigned OldElts = Value->getArrayInitializedElts();
+  if (OldElts == N)
+break;
+
+  // Expand the array to appropriate size.
+  APValue NewValue(APValue::UninitArray(), N, FinalSize);
+  for (unsigned I = 0; I < OldElts; ++I)
+NewValue.getArrayInitializedElt(I).swap(
+Value->getArrayInitializedElt(I));
+  Value->swap(NewValue);
+
+  if (HadZeroInit)
+for (unsigned I = OldElts; I < N; ++I)
+  Value->getArrayInitializedElt(I) = Filler;
+
+  // Initialize the elements.
+  for (unsigned I = OldElts; I < N; ++I) {
+if (!VisitCXXConstructExpr(E, ArrayElt,
+   >getArrayInitializedElt(I),
+   CAT->getElementType()) ||
+!HandleLValueArrayAdjustment(Info, E, ArrayElt,
+ CAT->getElementType(), 1))
+  return false;
+// When checking for const initilization any diagnostic is considered
+// an error.
+if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
+!Info.keepEvaluatingAfterFailure())
+  return false;
+  }
+}
 
 return true;
   }


Index: clang/test/SemaCXX/PR51712-large-array-constexpr-check-oom.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/PR51712-large-array-constexpr-check-oom.cpp
@@ -0,0 +1,11 @@
+// REQUIRES: shell
+// UNSUPPORTED: win32
+// RUN: ulimit -v 1048576
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+// expected-no-diagnostics
+
+// This used to require too much memory and crash with OOM.
+struct {
+  int a, b, c, d;
+} arr[1<<30];
+
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -10596,28 +10596,55 @@
   bool HadZeroInit = Value->hasValue();
 
   if (const ConstantArrayType *CAT = Info.Ctx.getAsConstantArrayType(Type)) {
-unsigned N = CAT->getSize().getZExtValue();
+unsigned FinalSize = CAT->getSize().getZExtValue();
 
 // Preserve the array filler if we had prior zero-initialization.
 APValue Filler =
   HadZeroInit && Value->hasArrayFiller() ? Value->getArrayFiller()
  

Re: [PATCH] D51650: Implement target_clones multiversioning

[Eric Christopher via cfe-commits]

I think you are these days too :) My offer was "past-past-me". I think
you're probably ok here, I did a rough scan, but getting someone like Aaron
for the attribute support would be good.

On Fri, Nov 5, 2021 at 12:00 PM Erich Keane via Phabricator <
revi...@reviews.llvm.org> wrote:

> erichkeane added a comment.
>
> Sadly, I think _I_ am the multiversioning expert (or at least, past-me
> was), so I'm hoping some of the reviewers @danielkiss can get to join will
> be able to read/understand this stuff for a quality review.
>
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D51650/new/
>
> https://reviews.llvm.org/D51650
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112591: [clang] [Objective C] Inclusive language: use objcmt-allowlist-dir-path= instead of objcmt-white-list-dir-path=

[Zarko Todorovski via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa83a6c22e63a: [clang] [Objective C] Inclusive language: use 
objcmt-allowlist-dir-path=arg… (authored by ZarkoCA).

Changed prior to commit:
  https://reviews.llvm.org/D112591?vs=383507=385099#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112591

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/ARCMigrate/ObjCMT.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/ARCMT/allowlisted/Inputs/header1.h
  clang/test/ARCMT/allowlisted/header1.h
  clang/test/ARCMT/allowlisted/header1.h.result
  clang/test/ARCMT/allowlisted/header2.h
  clang/test/ARCMT/allowlisted/header2.h.result
  clang/test/ARCMT/allowlisted/objcmt-with-allowlist-impl.m
  clang/test/ARCMT/allowlisted/objcmt-with-allowlist-impl.m.result
  clang/test/ARCMT/allowlisted/objcmt-with-allowlist.m
  clang/test/ARCMT/whitelisted/Inputs/header1.h
  clang/test/ARCMT/whitelisted/header1.h
  clang/test/ARCMT/whitelisted/header1.h.result
  clang/test/ARCMT/whitelisted/header2.h
  clang/test/ARCMT/whitelisted/header2.h.result
  clang/test/ARCMT/whitelisted/objcmt-with-whitelist-impl.m
  clang/test/ARCMT/whitelisted/objcmt-with-whitelist-impl.m.result
  clang/test/ARCMT/whitelisted/objcmt-with-whitelist.m
  clang/test/SemaObjC/method-conflict-1.m
  clang/test/SemaObjC/method-conflict-2.m
  clang/test/SemaObjC/method-typecheck-3.m

Index: clang/test/SemaObjC/method-typecheck-3.m
===
--- clang/test/SemaObjC/method-typecheck-3.m
+++ clang/test/SemaObjC/method-typecheck-3.m
@@ -13,7 +13,7 @@
 @end
 
 @implementation B
-- (id)obj {return self;} // 'id' overrides are white-listed?
+- (id)obj {return self;} // 'id' overrides are permitted?
 - (A*)a { return self;}  // expected-warning {{conflicting return type in implementation of 'a'}}
 - (void)takesA: (B*)a  // expected-warning {{conflicting parameter types in implementation of 'takesA:'}}
 {}
Index: clang/test/SemaObjC/method-conflict-2.m
===
--- clang/test/SemaObjC/method-conflict-2.m
+++ clang/test/SemaObjC/method-conflict-2.m
@@ -34,7 +34,7 @@
 - (A*) test2 { return 0; } // expected-warning {{conflicting return type in implementation of 'test2': 'B *' vs 'A *'}}
 @end
 
-// The particular case of overriding with an id return is white-listed.
+// The particular case of overriding with an id return is permitted.
 @interface Test4 {}
 - (id) test1;
 - (A*) test2;
Index: clang/test/SemaObjC/method-conflict-1.m
===
--- clang/test/SemaObjC/method-conflict-1.m
+++ clang/test/SemaObjC/method-conflict-1.m
@@ -73,7 +73,7 @@
 - (A*) test2 { return 0; } // broken-warning {{conflicting return type in implementation of 'test2': 'B *' vs 'A *'}}
 @end
 
-// The particular case of overriding with an id return is white-listed.
+// The particular case of overriding with an id return is permitted.
 @interface Test4 {}
 - (id) test1;
 - (A*) test2;
Index: clang/test/ARCMT/allowlisted/objcmt-with-allowlist.m
===
--- clang/test/ARCMT/allowlisted/objcmt-with-allowlist.m
+++ clang/test/ARCMT/allowlisted/objcmt-with-allowlist.m
@@ -1,7 +1,7 @@
 // RUN: rm -rf %t
 // RUN: %clang_cc1 -objcmt-migrate-readwrite-property -objcmt-migrate-instancetype -objcmt-migrate-ns-macros %s -triple x86_64-apple-darwin11 -migrate -o %t.remap
 // RUN: c-arcmt-test %t.remap | arcmt-test -verify-transformed-files %S/header1.h.result %S/header2.h.result
-// RUN: %clang_cc1 -objcmt-migrate-readwrite-property -objcmt-migrate-instancetype -objcmt-migrate-ns-macros -objcmt-white-list-dir-path=%S/Inputs %s -triple x86_64-apple-darwin11 -migrate -o %t.remap
+// RUN: %clang_cc1 -objcmt-migrate-readwrite-property -objcmt-migrate-instancetype -objcmt-migrate-ns-macros -objcmt-allowlist-dir-path=%S/Inputs %s -triple x86_64-apple-darwin11 -migrate -o %t.remap
 // RUN: c-arcmt-test %t.remap | arcmt-test -verify-transformed-files %S/header1.h.result
 
 @interface NSObject
Index: clang/test/ARCMT/allowlisted/objcmt-with-allowlist-impl.m.result
===
--- clang/test/ARCMT/allowlisted/objcmt-with-allowlist-impl.m.result
+++ clang/test/ARCMT/allowlisted/objcmt-with-allowlist-impl.m.result
@@ -1,5 +1,5 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -objcmt-migrate-readwrite-property -objcmt-migrate-instancetype -objcmt-white-list-dir-path=%S/Inputs %s -triple x86_64-apple-darwin11 -migrate -o %t.remap
+// RUN: %clang_cc1 -objcmt-migrate-readwrite-property -objcmt-migrate-instancetype -objcmt-allowlist-dir-path=%S/Inputs %s -triple x86_64-apple-darwin11 -migrate -o %t.remap
 // RUN: 

[clang] a83a6c2 - [clang] [Objective C] Inclusive language: use objcmt-allowlist-dir-path= instead of objcmt-white-list-dir-path=

[Zarko Todorovski via cfe-commits]

Author: Zarko Todorovski
Date: 2021-11-05T12:27:05-04:00
New Revision: a83a6c22e63ad84e9c210c71b36413bed72ac23c

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

LOG: [clang] [Objective C] Inclusive language: use 
objcmt-allowlist-dir-path= instead of objcmt-white-list-dir-path=

Trying to update some options that don't at least have an inclusive language 
version.
This patch adds `objcmt-allowlist-dir-path` as a default alternative.

Reviewed By: akyrtzi

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

Added: 
clang/test/ARCMT/allowlisted/Inputs/header1.h
clang/test/ARCMT/allowlisted/header1.h
clang/test/ARCMT/allowlisted/header1.h.result
clang/test/ARCMT/allowlisted/header2.h
clang/test/ARCMT/allowlisted/header2.h.result
clang/test/ARCMT/allowlisted/objcmt-with-allowlist-impl.m
clang/test/ARCMT/allowlisted/objcmt-with-allowlist-impl.m.result
clang/test/ARCMT/allowlisted/objcmt-with-allowlist.m

Modified: 
clang/docs/ClangCommandLineReference.rst
clang/include/clang/Driver/Options.td
clang/include/clang/Frontend/FrontendOptions.h
clang/lib/ARCMigrate/ObjCMT.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/SemaObjC/method-conflict-1.m
clang/test/SemaObjC/method-conflict-2.m
clang/test/SemaObjC/method-typecheck-3.m

Removed: 
clang/test/ARCMT/whitelisted/Inputs/header1.h
clang/test/ARCMT/whitelisted/header1.h
clang/test/ARCMT/whitelisted/header1.h.result
clang/test/ARCMT/whitelisted/header2.h
clang/test/ARCMT/whitelisted/header2.h.result
clang/test/ARCMT/whitelisted/objcmt-with-whitelist-impl.m
clang/test/ARCMT/whitelisted/objcmt-with-whitelist-impl.m.result
clang/test/ARCMT/whitelisted/objcmt-with-whitelist.m



diff  --git a/clang/docs/ClangCommandLineReference.rst 
b/clang/docs/ClangCommandLineReference.rst
index 31e7cd342c267..94eb3fec8a23c 100644
--- a/clang/docs/ClangCommandLineReference.rst
+++ b/clang/docs/ClangCommandLineReference.rst
@@ -483,7 +483,7 @@ Enable migration to use NS\_NONATOMIC\_IOSONLY macro for 
setting property's 'ato
 
 Enable migration to annotate property with NS\_RETURNS\_INNER\_POINTER
 
-.. option:: -objcmt-whitelist-dir-path=, -objcmt-white-list-dir-path=
+.. option:: -objcmpt-allowlist-dir-path=, 
-objcmt-whitelist-dir-path=, -objcmt-white-list-dir-path=
 
 Only modify files with a filename contained in the provided directory path
 

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index b4a2411fa5c5c..9a657e948e33e 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -617,12 +617,15 @@ def objcmt_migrate_designated_init : Flag<["-"], 
"objcmt-migrate-designated-init
   HelpText<"Enable migration to infer NS_DESIGNATED_INITIALIZER for 
initializer methods">,
   MarshallingInfoBitfieldFlag, 
"FrontendOptions::ObjCMT_DesignatedInitializer">;
 
-def objcmt_whitelist_dir_path: Joined<["-"], "objcmt-whitelist-dir-path=">, 
Flags<[CC1Option]>,
+def objcmt_allowlist_dir_path: Joined<["-"], "objcmt-allowlist-dir-path=">, 
Flags<[CC1Option]>,
   HelpText<"Only modify files with a filename contained in the provided 
directory path">,
-  MarshallingInfoString>;
+  MarshallingInfoString>;
+def : Joined<["-"], "objcmt-whitelist-dir-path=">, Flags<[CC1Option]>,
+  HelpText<"Alias for -objcmt-allowlist-dir-path">,
+  Alias;
 // The misspelt "white-list" [sic] alias is due for removal.
 def : Joined<["-"], "objcmt-white-list-dir-path=">, Flags<[CC1Option]>,
-Alias;
+  Alias;
 
 // Make sure all other -ccc- options are rejected.
 def ccc_ : Joined<["-"], "ccc-">, Group, Flags<[Unsupported]>;

diff  --git a/clang/include/clang/Frontend/FrontendOptions.h 
b/clang/include/clang/Frontend/FrontendOptions.h
index 41ea45ca0b103..1d9d89a28c6c4 100644
--- a/clang/include/clang/Frontend/FrontendOptions.h
+++ b/clang/include/clang/Frontend/FrontendOptions.h
@@ -373,7 +373,7 @@ class FrontendOptions {
  ObjCMT_MigrateDecls | ObjCMT_PropertyDotSyntax)
   };
   unsigned ObjCMTAction = ObjCMT_None;
-  std::string ObjCMTWhiteListPath;
+  std::string ObjCMTAllowListPath;
 
   std::string MTMigrateDir;
   std::string ARCMTMigrateReportOut;

diff  --git a/clang/lib/ARCMigrate/ObjCMT.cpp b/clang/lib/ARCMigrate/ObjCMT.cpp
index e99c6435062fb..3dfa9a0218a73 100644
--- a/clang/lib/ARCMigrate/ObjCMT.cpp
+++ b/clang/lib/ARCMigrate/ObjCMT.cpp
@@ -104,7 +104,7 @@ class ObjCMigrateASTConsumer : public ASTConsumer {
   bool FoundationIncluded;
   llvm::SmallPtrSet ObjCProtocolDecls;
   llvm::SmallVector CFFunctionIBCandidates;
-  llvm::StringSet<> WhiteListFilenames;
+  llvm::StringSet<> AllowListFilenames;
 
   RetainSummaryManager (ASTContext ) {
 if (!Summaries)
@@ -118,14 

[PATCH] D99797: [analyzer] Implemented RangeSet::Factory::unite function to handle intersections and adjacency

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Thanks Denys for the update! Very good! However, I think maybe we could make 
the code a bit more simpler.




Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:229-233
+// We want to keep the following invariant at all times:
+// ---[ First -->
+// -[ Second --->
+if (First->From() > Second->From())
+  swapIterators(First, FirstEnd, Second, SecondEnd);

I am not sure about this, but perhaps we could put this `swapIterators` call 
right at the beginning of the nested `while` loop (L243). That would eliminate 
the need to call it again at the end of the second while loop.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:243
+// Loop where the invariant holds.
+while (true) {
+  // Skip all enclosed ranges.

So, this loop is about to merge conjunct Ranges. The first time when we find 
disjoint Ranges then we break out. (Or we return once we reach the end of any 
of the RangeSets.)
This makes we wonder, if it would be possible to split this `while` loop into a 
lambda named `mergeConjunctRanges` ?



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:290-292
+  // case 1: --[ First ]-[ First + 1 ]-->
+  // case 2: --[ First]-[ First + 1 ]--->
+  // ---[ Second + 1]--->

Should this be like in the code suggestion? You incremented `First` at L276.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:293
+  // ---[ Second + 1]--->
+  // In any case First + 1 goes after Second.
+  // Make sure that the loop invariant holds.

?


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

https://reviews.llvm.org/D99797

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

[PATCH] D113268: [clangd] Fix tests with forward slash as separator on windows

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D113268#3111995 , @mstorsjo wrote:

>> I think the biggest testing problem we have though is that we don't have a 
>> good way of defining integration tests with paths set up the right way 
>> (since we can't really use absolute paths in our tests, and most of our bugs 
>> involve comparing absolute paths).
>
> Yup, I can totally understand that. And as I haven't tested clangd in this 
> setup other than the unit tests, I'm not sure if it works entirely in 
> practice though - with other unit tests in Clang I've seen lots of cases 
> where e.g. paths aren't considered as in the same directory when paths are 
> expressed with different separators.
>
> For someone unfamiliar with the tool, is there a simple "smoke test 
> procedure" I could try out with it to kick the tyres?

There's `clangd --check=some_file.cc` which simulates opening up a file in an 
editor and clicking around in it.
It does require a `compile_command.json` in a parent directory to know about 
build flags.
To try it out for real you need to connect an editor to it with a plugin (like 
vscode-clangd).

But realistic problems with paths tend to come from interactions with external 
sources.
For example, we had a bug that involved:

- vscode sending paths like `file://c:/test.cc` (lowercase C), which made its 
way into our AST cache
- cmake creating `compile_commands.json` like `C:\test.cc` (uppercase C), which 
made its way into our index
- when renaming a symbol, results from the two weren't deduplicated against 
each other so the edit was applied twice

> Anyway, I'll have a look at the seemingly odd/fragile change and get back to 
> you on that.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113268

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

[PATCH] D112941: [clang] Add support for the new pointer authentication builtins.

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

If you look at the `immintrin.h` header, the access too many builtins is 
guarded by ifdefs.
` #if defined(__SSSE3__)`

The builtin `__builtin_ia32_reduce_smin_d512` is useless on aarch64.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112941

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

[PATCH] D112577: [clang][OpenMP] Initial parsing/sema for 'align' clause

[David Pagan via Phabricator via cfe-commits]

ddpagan updated this revision to Diff 385098.
ddpagan added a comment.

Successfully rebased, built, and tested.


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

https://reviews.llvm.org/D112577

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/DeclPrinter.cpp
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/OpenMP/align_clause_ast_print.cpp
  clang/test/OpenMP/align_clause_messages.cpp
  clang/tools/libclang/CIndex.cpp
  llvm/include/llvm/Frontend/OpenMP/OMP.td

Index: llvm/include/llvm/Frontend/OpenMP/OMP.td
===
--- llvm/include/llvm/Frontend/OpenMP/OMP.td
+++ llvm/include/llvm/Frontend/OpenMP/OMP.td
@@ -363,6 +363,9 @@
   let clangClass = "OMPFilterClause";
   let flangClass = "ScalarIntExpr";
 }
+def OMPC_Align : Clause<"align"> {
+  let clangClass = "OMPAlignClause";
+}
 def OMPC_When: Clause<"when"> {}
 
 def OMPC_Bind : Clause<"bind"> {
@@ -1539,7 +1542,8 @@
 }
 def OMP_Allocate : Directive<"allocate"> {
   let allowedOnceClauses = [
-VersionedClause
+VersionedClause,
+VersionedClause
   ];
 }
 def OMP_DeclareVariant : Directive<"declare variant"> {
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -2315,6 +2315,10 @@
   Visitor->AddStmt(C->getThreadID());
 }
 
+void OMPClauseEnqueue::VisitOMPAlignClause(const OMPAlignClause *C) {
+  Visitor->AddStmt(C->getAlignment());
+}
+
 void OMPClauseEnqueue::VisitOMPUnifiedAddressClause(
 const OMPUnifiedAddressClause *) {}
 
Index: clang/test/OpenMP/align_clause_messages.cpp
===
--- /dev/null
+++ clang/test/OpenMP/align_clause_messages.cpp
@@ -0,0 +1,60 @@
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=51 %s -verify
+
+int foobar() {
+  return 1;
+}
+
+int main(int argc, char *argv[]) {
+  // expected-note@+1 {{declared here}}
+  int a;
+  // expected-note@+1 {{declared here}}
+  int b;
+  // expected-note@+1 {{declared here}}
+  int c;
+  double f;
+  int foo2[10];
+
+// expected-error@+1 {{expected '(' after 'align'}}
+#pragma omp allocate(a) align
+// expected-error@+3 {{expected expression}}
+// expected-error@+2 {{expected ')'}}
+// expected-note@+1 {{to match this '('}}
+#pragma omp allocate(a) align(
+// expected-error@+1 {{expected expression}}
+#pragma omp allocate(a) align()
+// expected-error@+4 {{expected ')'}}
+// expected-note@+3 {{to match this '('}}
+// expected-error@+2 {{expression is not an integral constant expression}}
+// expected-note@+1 {{read of non-const variable 'a' is not allowed in a constant expression}}
+#pragma omp allocate(a) align(a
+// expected-error@+2 {{expression is not an integral constant expression}}
+// expected-note@+1 {{read of non-const variable 'b' is not allowed in a constant expression}}
+#pragma omp allocate(a) align(b)
+// expected-error@+2 {{expression is not an integral constant expression}}
+// expected-note@+1 {{read of non-const variable 'c' is not allowed in a constant expression}}
+#pragma omp allocate(a) align(c + 1)
+// expected-error@+1 {{expected an OpenMP directive}}
+#pragma omp align(2) allocate(a)
+// expected-error@+1 {{directive '#pragma omp allocate' cannot contain more than one 'align' clause}}
+#pragma omp allocate(a) align(2) align(4)
+// expected-warning@+1 {{aligned clause will be ignored because the requested alignment is not a power of 2}}
+#pragma omp allocate(a) align(9)
+// expected-error@+1 {{integral constant expression must have integral or unscoped enumeration type, not 'double'}}
+#pragma omp allocate(a) align(f)
+}
+
+// Verify appropriate errors when using templates.
+template 
+T run() {
+  T foo[size];
+// expected-warning@+1 {{aligned clause will be ignored because the requested alignment is not a power of 2}}
+#pragma omp allocate(foo) align(align)
+  return foo[0];
+}
+
+int template_test() {
+  double d;
+  // expected-note@+1 {{in instantiation of function template specialization 'run' requested here}}
+  d = run();
+  return 0;
+}
Index: clang/test/OpenMP/align_clause_ast_print.cpp
===
--- /dev/null
+++ clang/test/OpenMP/align_clause_ast_print.cpp
@@ -0,0 +1,134 @@
+// expected-no-diagnostics
+
+//RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fopenmp -fopenmp-version=51 \
+//RUN:   -x c++ -std=c++14 -fexceptions -fcxx-exceptions   \

[PATCH] D113210: [NewPM] Use the default AA pipeline by default

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks added a comment.

In D113210#3110780 , @Meinersbur 
wrote:

> This change caused the Polly build to fail: 
> https://lab.llvm.org/buildbot/#/builders/10/builds/7501
>
>   opt: 
> /home/worker/buildbot-workers/polly-x86_64-gce1/rundir/llvm.src/llvm/include/llvm/IR/PassManager.h:784:
>  typename PassT::Result& llvm::AnalysisManager ExtraArgTs>::getResult(IRUnitT&, ExtraArgTs ...) [with PassT = 
> llvm::OuterAnalysisManagerProxy, 
> llvm::Function>; IRUnitT = llvm::Function; ExtraArgTs = {}; typename 
> PassT::Result = 
> llvm::OuterAnalysisManagerProxy, 
> llvm::Function>::Result]: Assertion `AnalysisPasses.count(PassT::ID()) && 
> "This analysis pass was not registered prior to being queried"' failed.
>   PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash 
> backtrace.
>   Stack dump:
>   0.  Program arguments: 
> /home/worker/buildbot-workers/polly-x86_64-gce1/rundir/llvm.obj/bin/opt 
> -polly-process-unprofitable -polly-remarks-minimal -polly-use-llvm-names 
> -polly-import-jscop-dir=/home/worker/src/llvm-project/polly/test/ScopInliner 
> -polly-codegen-verify -polly-detect-full-functions -polly-scop-inliner 
> -polly-scops -analyze
>   1.  Running pass 'CallGraph Pass Manager' on module ''.
>#0 0x7fcc7bb29644 PrintStackTraceSignalHandler(void*) Signals.cpp:0:0
>#1 0x7fcc7bb26d5e SignalHandler(int) Signals.cpp:0:0
>#2 0x7fcc7b541210 (/lib/x86_64-linux-gnu/libc.so.6+0x46210)
>#3 0x7fcc7b54118b raise (/lib/x86_64-linux-gnu/libc.so.6+0x4618b)
>#4 0x7fcc7b520859 abort (/lib/x86_64-linux-gnu/libc.so.6+0x25859)
>#5 0x7fcc7b520729 (/lib/x86_64-linux-gnu/libc.so.6+0x25729)
>#6 0x7fcc7b531f36 (/lib/x86_64-linux-gnu/libc.so.6+0x36f36)
>#7 0x7fcc7dbcb34f void 
> llvm::AAManager::getModuleAAResultImpl(llvm::Function&, 
> llvm::AnalysisManager&, llvm::AAResults&) PassBuilder.cpp:0:0
>#8 0x7fcc7c3c5363 llvm::AAManager::run(llvm::Function&, 
> llvm::AnalysisManager&) 
> (/home/worker/buildbot-workers/polly-x86_64-gce1/rundir/llvm.obj/./lib/libLLVMAnalysis.so.14git+0x257363)
>#9 0x563487033358 llvm::detail::AnalysisPassModel llvm::AAManager, llvm::PreservedAnalyses, 
> llvm::AnalysisManager::Invalidator>::run(llvm::Function&, 
> llvm::AnalysisManager&) NewPMDriver.cpp:0:0
>   #10 0x7fcc7bf95dd8 
> llvm::AnalysisManager::getResultImpl(llvm::AnalysisKey*, 
> llvm::Function&) 
> (/home/worker/buildbot-workers/polly-x86_64-gce1/rundir/llvm.obj/./lib/libLLVMCore.so.14git+0x450dd8)
>   #11 0x7fcc7e84892a polly::ScopAnalysis::run(llvm::Function&, 
> llvm::AnalysisManager&) 
> (/home/worker/buildbot-workers/polly-x86_64-gce1/rundir/llvm.obj/./lib/libPolly.so.14git+0x13c92a)
>   #12 0x7fcc7e962aa4 llvm::detail::AnalysisPassModel polly::ScopAnalysis, llvm::PreservedAnalyses, 
> llvm::AnalysisManager::Invalidator>::run(llvm::Function&, 
> llvm::AnalysisManager&) RegisterPasses.cpp:0:0
>   #13 0x7fcc7bf95dd8 
> llvm::AnalysisManager::getResultImpl(llvm::AnalysisKey*, 
> llvm::Function&) 
> (/home/worker/buildbot-workers/polly-x86_64-gce1/rundir/llvm.obj/./lib/libLLVMCore.so.14git+0x450dd8)
>   #14 0x7fcc7e9d7d7c (anonymous 
> namespace)::ScopInliner::runOnSCC(llvm::CallGraphSCC&) ScopInliner.cpp:0:0
>   #15 0x7fcc7c46dfef (anonymous 
> namespace)::CGPassManager::runOnModule(llvm::Module&) CallGraphSCCPass.cpp:0:0
>   #16 0x7fcc7bf4dee2 llvm::legacy::PassManagerImpl::run(llvm::Module&) 
> (/home/worker/buildbot-workers/polly-x86_64-gce1/rundir/llvm.obj/./lib/libLLVMCore.so.14git+0x408ee2)
>   #17 0x563487054571 main 
> (/home/worker/buildbot-workers/polly-x86_64-gce1/rundir/llvm.obj/bin/opt+0x48571)
>   #18 0x7fcc7b5220b3 __libc_start_main 
> (/lib/x86_64-linux-gnu/libc.so.6+0x270b3)
>   #19 0x563487030c8e _start 
> (/home/worker/buildbot-workers/polly-x86_64-gce1/rundir/llvm.obj/bin/opt+0x24c8e)
>
> Polly is not using custom AliasAnalysis. Any idea how to fix this?
>
> Btw, the pre-merge check failed because of this as well.

Sorry about that, I saw the legacy pass manager infra stack frames and assumed 
this wasn't related to my patch.
I'll send out a fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113210

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

[PATCH] D111654: [analyzer] Retrieve a value from list initialization of multi-dimensional array declaration.

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov updated this revision to Diff 385096.
ASDenysPetrov added a comment.

Updated according to @steakhal's comments.


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

https://reviews.llvm.org/D111654

Files:
  clang/lib/StaticAnalyzer/Core/RegionStore.cpp
  clang/test/Analysis/initialization.c
  clang/test/Analysis/initialization.cpp

Index: clang/test/Analysis/initialization.cpp
===
--- clang/test/Analysis/initialization.cpp
+++ clang/test/Analysis/initialization.cpp
@@ -14,13 +14,6 @@
   clang_analyzer_eval(sarr[i].a); // expected-warning{{UNKNOWN}}
 }
 
-int const arr[2][2] = {};
-void arr2init() {
-  int i = 1;
-  // FIXME: Should recognize that it is 0.
-  clang_analyzer_eval(arr[i][0]); // expected-warning{{UNKNOWN}}
-}
-
 int const glob_arr1[3] = {};
 void glob_array_index1() {
   clang_analyzer_eval(glob_arr1[0] == 0); // expected-warning{{TRUE}}
@@ -60,23 +53,18 @@
   return glob_arr3[0]; // no-warning (garbage or undefined)
 }
 
-// TODO: Support multidimensional array.
 int const glob_arr4[4][2] = {};
 void glob_array_index2() {
-  // FIXME: Should be TRUE.
-  clang_analyzer_eval(glob_arr4[1][0] == 0); // expected-warning{{UNKNOWN}}
-  // FIXME: Should be TRUE.
-  clang_analyzer_eval(glob_arr4[1][1] == 0); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(glob_arr4[0][0] == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr4[1][0] == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr4[1][1] == 0); // expected-warning{{TRUE}}
 }
 
-// TODO: Support multidimensional array.
 void glob_invalid_index3() {
   int idx = -42;
-  // FIXME: Should warn {{garbage or undefined}}.
-  auto x = glob_arr4[1][idx]; // no-warning
+  auto x = glob_arr4[1][idx]; // expected-warning{{garbage or undefined}}
 }
 
-// TODO: Support multidimensional array.
 void glob_invalid_index4() {
   const int *ptr = glob_arr4[1];
   int idx = -42;
@@ -84,28 +72,18 @@
   auto x = ptr[idx]; // no-warning
 }
 
-// TODO: Support multidimensional array.
 int const glob_arr5[4][2] = {{1}, 3, 4, 5};
 void glob_array_index3() {
-  // FIXME: Should be TRUE.
-  clang_analyzer_eval(glob_arr5[0][0] == 1); // expected-warning{{UNKNOWN}}
-  // FIXME: Should be TRUE.
-  clang_analyzer_eval(glob_arr5[0][1] == 0); // expected-warning{{UNKNOWN}}
-  // FIXME: Should be TRUE.
-  clang_analyzer_eval(glob_arr5[1][0] == 3); // expected-warning{{UNKNOWN}}
-  // FIXME: Should be TRUE.
-  clang_analyzer_eval(glob_arr5[1][1] == 4); // expected-warning{{UNKNOWN}}
-  // FIXME: Should be TRUE.
-  clang_analyzer_eval(glob_arr5[2][0] == 5); // expected-warning{{UNKNOWN}}
-  // FIXME: Should be TRUE.
-  clang_analyzer_eval(glob_arr5[2][1] == 0); // expected-warning{{UNKNOWN}}
-  // FIXME: Should be TRUE.
-  clang_analyzer_eval(glob_arr5[3][0] == 0); // expected-warning{{UNKNOWN}}
-  // FIXME: Should be TRUE.
-  clang_analyzer_eval(glob_arr5[3][1] == 0); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(glob_arr5[0][0] == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr5[0][1] == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr5[1][0] == 3); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr5[1][1] == 4); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr5[2][0] == 5); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr5[2][1] == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr5[3][0] == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr5[3][1] == 0); // expected-warning{{TRUE}}
 }
 
-// TODO: Support multidimensional array.
 void glob_ptr_index2() {
   int const *ptr = glob_arr5[1];
   // FIXME: Should be TRUE.
@@ -120,19 +98,16 @@
   clang_analyzer_eval(ptr[4] == 0); // expected-warning{{UNKNOWN}}
 }
 
-// TODO: Support multidimensional array.
 void glob_invalid_index5() {
   int idx = -42;
-  // FIXME: Should warn {{garbage or undefined}}.
-  auto x = glob_arr5[1][idx]; // no-warning
+  auto x = glob_arr5[1][idx]; // expected-warning{{garbage or undefined}}
 }
 
-// TODO: Support multidimensional array.
 void glob_invalid_index6() {
   int const *ptr = _arr5[1][0];
   int idx = 42;
   // FIXME: Should warn {{garbage or undefined}}.
-  auto x = ptr[idx]; // // no-warning
+  auto x = ptr[idx]; // no-warning
 }
 
 extern const int glob_arr_no_init[10];
@@ -253,3 +228,31 @@
   clang_analyzer_eval(glob_ptr12[2] == 'c');  // expected-warning{{TRUE}}
   clang_analyzer_eval(glob_ptr12[3] == '\0'); // expected-warning{{TRUE}}
 }
+
+typedef int Int;
+typedef Int const CInt;
+typedef CInt Arr[2];
+typedef Arr Arr2[4];
+Arr2 glob_arr8 = {{1}, 3, 4, 5}; // const int[4][2]
+void glob_array_typedef1() {
+  clang_analyzer_eval(glob_arr8[0][0] == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr8[0][1] == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr8[1][0] == 3); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr8[1][1] == 4); 

[PATCH] D113268: [clangd] Fix tests with forward slash as separator on windows

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D113268#3111798 , @sammccall wrote:

> In D113268#3111709 , @mstorsjo 
> wrote:
>
>> FWIW, this change is not about mingw, it's about making the 
>> windows-with-forward-slashes configuration usable.
>
> OK - can I ask why the windows-with-forward-slashes configuration is valuable 
> to support?
> I was assuming it was mostly useful to folks building with mingw, sounds like 
> that's not all!

Well you're right that my own original need is for mingw setups, with various 
tools parsing and using the output of e.g. `clang -v` and `clang 
--print-resource-dir` etc, and using it in contexts where backslashes require 
extra care. I'm not familiar with all the other cases that others have had that 
have indicated a need for this though, but it's something that does come up 
semi-regularly, and as Windows itself mostly accepts this alternative form, 
it's a setup with possibly "less sharp corners".

>> The windows-with-forward-slashes configuration works just as fine in MSVC 
>> configurations, and that's where I've tested it. A MSVC build with `ninja 
>> check-all` that succeeded before, still succeed with `-DLLVM_ 
>> WINDOWS_PREFER_FORWARD_SLASH=ON` (with respect to clangd) after this change.
>>
>> As for testing strategy, it could be concievable to add a configuration to 
>> e.g. the buildkite premerge tests that build+test everything in a 
>> `-DLLVM_WINDOWS_PREFER_FORWARD_SLASH=ON` environment too. (I've seen 
>> interest from @rnk to push towards such a setup, possibly, so adding testing 
>> configurations for both modes is probably not inconcievable.)
>
> That's great to hear. It'd really be a prerequisite to keeping this 
> configuration passing, as we don't have regular access to windows machines & 
> rely on the buildbots to catch test problems.
>
> I think the biggest testing problem we have though is that we don't have a 
> good way of defining integration tests with paths set up the right way (since 
> we can't really use absolute paths in our tests, and most of our bugs involve 
> comparing absolute paths).

Yup, I can totally understand that. And as I haven't tested clangd in this 
setup other than the unit tests, I'm not sure if it works entirely in practice 
though - with other unit tests in Clang I've seen lots of cases where e.g. 
paths aren't considered as in the same directory when paths are expressed with 
different separators.

For someone unfamiliar with the tool, is there a simple "smoke test procedure" 
I could try out with it to kick the tyres?

Anyway, I'll have a look at the seemingly odd/fragile change and get back to 
you on that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113268

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

[PATCH] D51650: Implement target_clones multiversioning

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Sadly, I think _I_ am the multiversioning expert (or at least, past-me was), so 
I'm hoping some of the reviewers @danielkiss can get to join will be able to 
read/understand this stuff for a quality review.


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

https://reviews.llvm.org/D51650

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

[PATCH] D51650: Implement target_clones multiversioning

[Eric Christopher via Phabricator via cfe-commits]

echristo added a comment.

FWIW I'm a bit rusty in this area myself, but thanks for doing this. Let's see 
if we can't get Aaron to continue reviewing :)


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

https://reviews.llvm.org/D51650

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

[PATCH] D113262: [clangd] Allow IncludeCleaner to replace unused header with transitively used

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

This is cool!
I think it will interact with other features (stdlib, exported headers), and so 
whatever lands second is going to have to eat the complexity of that 
interaction.
I think both stdlib and exported headers are more critical and more essentially 
complex, so should probably land first to drive the design.




Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:298
+
+llvm::StringSet<> reachableUsedHeaders(
+const IncludeStructure , IncludeStructure::HeaderID Source,

It seems like this is going to include any files that are transitively 
reachable through multiple paths and directly reachable through none. These are 
going to seem like false positives.
Like  if the file uses size_t at all, or something.

Sure, if you want the whole file to be IWYU-clean you should remove  
and add , but they're not *really* related.



Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:311
+  Result.insert(IncludeGraph.getRealPath(NextID));
+for (const auto  : IncludeGraph.IncludeChildren.lookup(NextID))
+  if (!Visited.contains(Header))

This is going to have problems with private/exported headers.

Consider includes main -> foo -> bar -> private.
Main uses a symbol from private, bar exports private.
You need to suggest replacing the include of foo -> {bar}, not {bar, private}.

However you still want to traverse children of private in case main uses those.



Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:376
 D.Fixes.back().Edits.emplace_back();
-D.Fixes.back().Edits.back().range.start.line = Inc->HashLine;
-D.Fixes.back().Edits.back().range.end.line = Inc->HashLine + 1;
+D.Fixes.back().Edits.back().range.start.line = I.Include->HashLine;
+D.Fixes.back().Edits.back().range.end.line = I.Include->HashLine + 1;

We should offer both fixes as alternatives, maybe?



Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:381
+  DirectIncludes +=
+  llvm::formatv("#include \"{0}\"\n", Replacement.getKey());
+}

exactly how to spell an include and where to place it is tricky. See 
IncludeInserter.h in headers.



Comment at: clang-tools-extra/clangd/IncludeCleaner.h:69
 
-std::vector computeUnusedIncludes(ParsedAST );
+llvm::StringSet<> reachableUsedHeaders(
+IncludeStructure , IncludeStructure::HeaderID Source,

why are we back to passing around strings?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113262

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

[PATCH] D113256: [AArch64][ARM] Enablement of Cortex-A710 Support

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

I'm happy with these changes, and with the comments from @dmgreen - as I'm away 
next week, don't wait for my approval to land this if David has given his 
approval.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113256

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

[PATCH] D51650: Implement target_clones multiversioning

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 385089.
erichkeane added a comment.

Another rebase, as requested.  I am not particularly familiar with this code 
anymore, so my responses to reviews might not be particularly well informed, 
but I'm hoping that rust shakes off through the review :)


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

https://reviews.llvm.org/D51650

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/Decl.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/attr-target-clones.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Sema/attr-cpuspecific.c
  clang/test/Sema/attr-target-clones.c

Index: clang/test/Sema/attr-target-clones.c
===
--- /dev/null
+++ clang/test/Sema/attr-target-clones.c
@@ -0,0 +1,88 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu  -fsyntax-only -verify %s
+
+// expected-error@+1 {{'target_clones' multiversioning requires a default target}}
+void __attribute__((target_clones("sse4.2", "arch=sandybridge")))
+no_default(void);
+
+// expected-error@+2 {{'target_clones' and 'target' attributes are not compatible}}
+// expected-note@+1 {{conflicting attribute is here}}
+void __attribute__((target("sse4.2"), target_clones("arch=sandybridge")))
+ignored_attr(void);
+// expected-error@+2 {{'target' and 'target_clones' attributes are not compatible}}
+// expected-note@+1 {{conflicting attribute is here}}
+void __attribute__((target_clones("arch=sandybridge,default"), target("sse4.2")))
+ignored_attr2(void);
+
+int redecl(void);
+int __attribute__((target_clones("sse4.2", "default"))) redecl(void) { return 1; }
+
+int __attribute__((target_clones("sse4.2", "default"))) redecl2(void);
+int __attribute__((target_clones("sse4.2", "default"))) redecl2(void) { return 1; }
+
+int __attribute__((target_clones("sse4.2", "default"))) redecl3(void);
+int redecl3(void);
+
+int __attribute__((target_clones("sse4.2", "arch=atom", "default"))) redecl4(void);
+// expected-error@+3 {{'target_clones' attribute does not match previous declaration}}
+// expected-note@-2 {{previous declaration is here}}
+int __attribute__((target_clones("sse4.2", "arch=sandybridge", "default")))
+redecl4(void) { return 1; }
+
+int __attribute__((target("sse4.2"))) redef2(void) { return 1; }
+// expected-error@+2 {{multiversioning attributes cannot be combined}}
+// expected-note@-2 {{previous declaration is here}}
+int __attribute__((target_clones("sse4.2", "default"))) redef2(void) { return 1; }
+
+int __attribute__((target_clones("sse4.2,default"))) redef3(void) { return 1; }
+// expected-error@+2 {{redefinition of 'redef3'}}
+// expected-note@-2 {{previous definition is here}}
+int __attribute__((target_clones("sse4.2,default"))) redef3(void) { return 1; }
+
+int __attribute__((target_clones("sse4.2,default"))) redef4(void) { return 1; }
+// expected-error@+2 {{redefinition of 'redef4'}}
+// expected-note@-2 {{previous definition is here}}
+int __attribute__((target_clones("sse4.2,default"))) redef4(void) { return 1; }
+
+// Duplicates are allowed, however they alter name mangling.
+// expected-warning@+2 {{mixing 'target_clones' specifier mechanisms is permitted for GCC compatibility}}
+// expected-warning@+1 2 {{version list contains duplicate entries}}
+int __attribute__((target_clones("arch=atom,arch=atom", "arch=atom,default")))
+dupes(void) { return 1; }
+
+// expected-warning@+1 {{unsupported '' in the 'target_clones' attribute string;}}
+void __attribute__((target_clones("")))
+empty_target_1(void);
+// expected-warning@+1 {{unsupported '' in the 'target_clones' attribute string;}}
+void __attribute__((target_clones(",default")))
+empty_target_2(void);
+// expected-warning@+1 {{unsupported '' in the 'target_clones' attribute string;}}
+void __attribute__((target_clones("default,")))
+empty_target_3(void);
+// expected-warning@+1 {{unsupported '' in the 'target_clones' attribute string;}}
+void __attribute__((target_clones("default, ,avx2")))
+empty_target_4(void);
+
+// expected-warning@+1 {{unsupported '' in the 'target_clones' attribute string;}}
+void __attribute__((target_clones("default,avx2", "")))
+empty_target_5(void);
+
+// expected-warning@+1 {{version list contains duplicate entries}}
+void __attribute__((target_clones("default", "default")))
+dupe_default(void);
+
+// expected-warning@+1 {{version list contains duplicate entries}}
+void __attribute__((target_clones("avx2,avx2,default")))
+dupe_normal(void);
+
+// expected-error@+2 {{'target_clones' and 'target_clones' attributes are not compatible}}
+// expected-note@+1 {{conflicting attribute is here}}
+void 

[PATCH] D113256: [AArch64][ARM] Enablement of Cortex-A710 Support

[Sam Elliott via Phabricator via cfe-commits]

lenary added inline comments.



Comment at: llvm/include/llvm/Support/AArch64TargetParser.def:159-160
   AArch64::AEK_RCPC | AArch64::AEK_SSBS))
+AARCH64_CPU_NAME("cortex-a710", ARMV9A, FK_NEON_FP_ARMV8, false,
+ (AArch64::AEK_MTE | AArch64::AEK_PAUTH | AArch64::AEK_FLAGM |
+  AArch64::AEK_SB | AArch64::AEK_I8MM | AArch64::AEK_FP16FML |

dmgreen wrote:
> Natural order would be better I think, where the new A710 is added after the 
> A78.
> 
> FlagM should already be included as a part of 8.4, so isn't needed here.
> Should BFloat16 be added?
> FlagM should already be included as a part of 8.4, so isn't needed here.

FlagM is not part of the `AARCH64_ARCH("armv9-a", ARMV9A ...)`  definition, nor 
part of the equivalents for armv8.4a and armv8.5a, so it was added explicitly 
here.

> Should BFloat16 be added?

Yes





Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113256

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

[PATCH] D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default

[Hyeongyu Kim via Phabricator via cfe-commits]

hyeongyukim added a comment.

  diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp 
b/compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp
  index ea3e5bdbc754..826c6d36e1b1 100644
  --- a/compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp
  +++ b/compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp
  @@ -1360,7 +1360,7 @@ uptr internal_clone(int (*fn)(void *), void 
*child_stack, int flags, void *arg,
   #elif defined(__aarch64__)
   uptr internal_clone(int (*fn)(void *), void *child_stack, int flags, void 
*arg,
   int *parent_tidptr, void *newtls, int *child_tidptr) {
  -  long long res;
  +  register long long res __asm__("x0");
 if (!fn || !child_stack)
   return -EINVAL;
 CHECK_EQ(0, (uptr)child_stack % 16);

After modifying `internal_clone` like this, the problem disappeared.
Is it okay to commit this change by myself?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105169

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

[PATCH] D111318: [clang][clangd] Improve signature help for variadic functions.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Thanks, this looks nice!

Since the code is in Sema, we'd should (also) test this in 
`clang/test/CodeCompletion/` if possible.




Comment at: clang-tools-extra/clangd/CodeComplete.cpp:928
+// FIXME: Add support for per-signature activeParameter field.
+//
+// This only matters for variadic functions/templates, where CurrentArg

I'd consider pulling out the logic into a helper named something like 
`paramIndexForArg(int, OverloadCandidate)` 

just because this function is ridiculously long and this piece of logic 
encapsulates nicely



Comment at: clang-tools-extra/clangd/CodeComplete.cpp:929
+//
+// This only matters for variadic functions/templates, where CurrentArg
+// might be higher than the number of parameters. When that happens, we

hmm, now that I think about it, should this conversion from arg-index to 
param-index be happening here, or done already by sema and the param index 
passed to ProcessOverloadCandidates?

(The docs say "current arg" but I'm struggling to see how that can really be 
useful)



Comment at: clang-tools-extra/clangd/CodeComplete.cpp:945
+}
+SigHelp.activeParameter =
+std::min(SigHelp.activeParameter, NumParams - 1);

hmm, if foo() is a non-variadic 0-arg function, we set activeParameter to -1.
Before this patch it's zero which is still weird, but less weird.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111318

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

[PATCH] D113268: [clangd] Fix tests with forward slash as separator on windows

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

Oh, I realized that issues relating to “mingw” probably are from msys (which 
often is used together, but is an entirely different thing) - yeah I can see 
that there’d be lots of unfixable issues with that, and I’m not signing up for 
looking into that…

Msys is the unix emulation layer/runtime, which uses unix style paths like 
`/c/Windows`. Mingw is just normal plain win32 with slightly different tools.

This patch and the new option is just about `C:/Windows` vs `C:\Windows`. Most 
win32 apis (and llvm in general) accept both forms, the new option is just 
about which one of the two is generated when llvm/clang generate paths.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113268

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

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D112349#3111873 , @ibookstein 
wrote:

> And how is Cling expecting CFE to deal with partial knowledge situations at 
> the implementation level? To deal with exactly the non-local cases that the 
> current violations address?
> If there's no prescriptive way forward to dealing with these cases (so 
> they're tech debt without a remediation plan), then as far as I'm concerned 
> this case sits exactly under the same tech-debt umbrella of the existing 
> violations and the way forward is using the same violating solution for this 
> case too. 
> We definitely shouldn't block the IR verification indefinitely on this 
> impasse. Once an acceptable solution is found, this will also be part of that 
> refactor.

My understanding is that the REPL setup is that the 'IR' just needs to be in a 
state where it doesn't require reverts/rewrites later, so that we can do 
partial-back-end-code-generation as we go along.  That said, I'm not positive 
as to the implications.  I'm just repeating the discussion the CFE code-owner 
made at the time.

IMO, the 'acceptable' solution is to have a way to forward-declare an ifunc in 
IR so that we can fill in the resolver later on.  From your description 
earlier, it seems that this would work as we could emit it as an 'unknown 
symbol' (as if it was an undefined function declaration), and would be 
completely implementable in the CFE.

So it would change your plan from earlier to:

When processing cpu_specific, emit the ifunc "x.ifunc", with no resolver;
When processing cpu_dispatch:

  Get/Create the ifunc, then pull up the resolver.
  If the resolver is null (as it should be), create one and update the 'ifunc'.
  Generate said resolver.
  



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112349

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

[PATCH] D112285: [PowerPC] PPC backend optimization to lower int_ppc_tdw/int_ppc_tw intrinsics to TDI/TWI machine instructions

[Amy Kwan via Phabricator via cfe-commits]

amyk accepted this revision.
amyk added a comment.

Aside from Nemanja's comments, this patch LGTM. Thanks for addressing the 
comments!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112285

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

[PATCH] D112041: [InferAddressSpaces] Support assumed addrspaces from addrspace predicates.

[Michael Liao via Phabricator via cfe-commits]

hliao updated this revision to Diff 385083.
hliao marked an inline comment as done.
hliao added a comment.

Rebase to the main branch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112041

Files:
  clang/test/CodeGen/thinlto-distributed-newpm.ll
  llvm/include/llvm/Analysis/AssumptionCache.h
  llvm/include/llvm/Analysis/TargetTransformInfo.h
  llvm/include/llvm/Analysis/TargetTransformInfoImpl.h
  llvm/include/llvm/CodeGen/BasicTTIImpl.h
  llvm/include/llvm/Target/TargetMachine.h
  llvm/lib/Analysis/AssumptionCache.cpp
  llvm/lib/Analysis/TargetTransformInfo.cpp
  llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
  llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.h
  llvm/lib/Target/NVPTX/NVPTXTargetMachine.cpp
  llvm/lib/Target/NVPTX/NVPTXTargetMachine.h
  llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp
  llvm/test/Other/loop-pm-invalidation.ll
  llvm/test/Other/new-pass-manager.ll
  llvm/test/Other/new-pm-lto-defaults.ll
  llvm/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll
  llvm/test/Transforms/InferAddressSpaces/AMDGPU/builtin-assumed-addrspace.ll
  llvm/test/Transforms/InferAddressSpaces/NVPTX/builtin-assumed-addrspace.ll
  llvm/test/Transforms/LoopRotate/pr35210.ll
  llvm/unittests/Analysis/AssumeBundleQueriesTest.cpp

Index: llvm/unittests/Analysis/AssumeBundleQueriesTest.cpp
===
--- llvm/unittests/Analysis/AssumeBundleQueriesTest.cpp
+++ llvm/unittests/Analysis/AssumeBundleQueriesTest.cpp
@@ -518,8 +518,7 @@
   BasicBlock::iterator First = F->begin()->begin();
   BasicBlock::iterator Second = F->begin()->begin();
   Second++;
-  AssumptionCacheTracker ACT;
-  AssumptionCache  = ACT.getAssumptionCache(*F);
+  AssumptionCache AC(*F);
   auto AR = AC.assumptionsFor(F->getArg(3));
   ASSERT_EQ(AR.size(), 0u);
   AR = AC.assumptionsFor(F->getArg(1));
Index: llvm/test/Transforms/LoopRotate/pr35210.ll
===
--- llvm/test/Transforms/LoopRotate/pr35210.ll
+++ llvm/test/Transforms/LoopRotate/pr35210.ll
@@ -11,11 +11,11 @@
 ; CHECK-NEXT: Running analysis: LoopAnalysis on f
 ; CHECK-NEXT: Running analysis: DominatorTreeAnalysis on f
 ; CHECK-NEXT: Running analysis: AssumptionAnalysis on f
+; CHECK-NEXT: Running analysis: TargetIRAnalysis on f
 ; CHECK-NEXT: Running pass: LCSSAPass on f
 ; CHECK-NEXT: Running analysis: AAManager on f
 ; CHECK-NEXT: Running analysis: TargetLibraryAnalysis on f
 ; CHECK-NEXT: Running analysis: ScalarEvolutionAnalysis on f
-; CHECK-NEXT: Running analysis: TargetIRAnalysis on f
 ; CHECK-NEXT: Running analysis: InnerAnalysisManagerProxy{{.*}} on f
 ; CHECK-NEXT: Running pass: LoopRotatePass on Loop at depth 1 containing: %bb,%bb4
 ; CHECK-NEXT: Folding loop latch bb4 into bb
@@ -29,12 +29,12 @@
 ; MSSA-NEXT: Running analysis: LoopAnalysis on f
 ; MSSA-NEXT: Running analysis: DominatorTreeAnalysis on f
 ; MSSA-NEXT: Running analysis: AssumptionAnalysis on f
+; MSSA-NEXT: Running analysis: TargetIRAnalysis on f
 ; MSSA-NEXT: Running pass: LCSSAPass on f
 ; MSSA-NEXT: Running analysis: MemorySSAAnalysis on f
 ; MSSA-NEXT: Running analysis: AAManager on f
 ; MSSA-NEXT: Running analysis: TargetLibraryAnalysis on f
 ; MSSA-NEXT: Running analysis: ScalarEvolutionAnalysis on f
-; MSSA-NEXT: Running analysis: TargetIRAnalysis on f
 ; MSSA-NEXT: Running analysis: InnerAnalysisManagerProxy{{.*}} on f
 ; MSSA-NEXT: Running pass: LoopRotatePass on Loop at depth 1 containing: %bb,%bb4
 ; MSSA-NEXT: Folding loop latch bb4 into bb
Index: llvm/test/Transforms/InferAddressSpaces/NVPTX/builtin-assumed-addrspace.ll
===
--- /dev/null
+++ llvm/test/Transforms/InferAddressSpaces/NVPTX/builtin-assumed-addrspace.ll
@@ -0,0 +1,107 @@
+; RUN: opt -S -mtriple=nvptx64-nvidia-cuda -infer-address-spaces -o - %s | FileCheck %s
+
+; CHECK-LABEL: @f0
+; CHECK: addrspacecast float* {{%.*}} to float addrspace(4)*
+; CHECK: getelementptr inbounds float, float addrspace(4)*
+; CHECK: load float, float addrspace(4)*
+define float @f0(float* %p) {
+entry:
+  %0 = bitcast float* %p to i8*
+  %1 = call i1 @llvm.nvvm.isspacep.const(i8* %0)
+  tail call void @llvm.assume(i1 %1)
+  %2 = tail call i32 @llvm.nvvm.read.ptx.sreg.tid.x()
+  %idxprom = zext i32 %2 to i64
+  %arrayidx = getelementptr inbounds float, float* %p, i64 %idxprom
+  %3 = load float, float* %arrayidx, align 4
+  ret float %3
+}
+
+; CHECK-LABEL: @f1
+; CHECK: addrspacecast float* {{%.*}} to float addrspace(1)*
+; CHECK: getelementptr inbounds float, float addrspace(1)*
+; CHECK: load float, float addrspace(1)*
+define float @f1(float* %p) {
+entry:
+  %0 = bitcast float* %p to i8*
+  %1 = call i1 @llvm.nvvm.isspacep.global(i8* %0)
+  tail call void @llvm.assume(i1 %1)
+  %2 = tail call i32 @llvm.nvvm.read.ptx.sreg.tid.x()
+  %idxprom = zext i32 %2 to i64
+  %arrayidx = getelementptr 

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein added a comment.

And how is Cling expecting CFE to deal with partial knowledge situations at the 
implementation level? To deal with exactly the non-local cases that the current 
violations address?
If there's no prescriptive way forward to dealing with these cases (so they're 
tech debt without a remediation plan), then as far as I'm concerned this case 
sits exactly under the same tech-debt umbrella of the existing violations and 
the way forward is using the same violating solution for this case too. 
We definitely shouldn't block the IR verification indefinitely on this impasse. 
Once an acceptable solution is found, this will also be part of that refactor.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112349

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

[PATCH] D113268: [clangd] Fix tests with forward slash as separator on windows

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D113268#3111709 , @mstorsjo wrote:

> FWIW, this change is not about mingw, it's about making the 
> windows-with-forward-slashes configuration usable.

OK - can I ask why the windows-with-forward-slashes configuration is valuable 
to support?
I was assuming it was mostly useful to folks building with mingw, sounds like 
that's not all!

> The windows-with-forward-slashes configuration works just as fine in MSVC 
> configurations, and that's where I've tested it. A MSVC build with `ninja 
> check-all` that succeeded before, still succeed with `-DLLVM_ 
> WINDOWS_PREFER_FORWARD_SLASH=ON` (with respect to clangd) after this change.
>
> As for testing strategy, it could be concievable to add a configuration to 
> e.g. the buildkite premerge tests that build+test everything in a 
> `-DLLVM_WINDOWS_PREFER_FORWARD_SLASH=ON` environment too. (I've seen interest 
> from @rnk to push towards such a setup, possibly, so adding testing 
> configurations for both modes is probably not inconcievable.)

That's great to hear. It'd really be a prerequisite to keeping this 
configuration passing, as we don't have regular access to windows machines & 
rely on the buildbots to catch test problems.

I think the biggest testing problem we have though is that we don't have a good 
way of defining integration tests with paths set up the right way (since we 
can't really use absolute paths in our tests, and most of our bugs involve 
comparing absolute paths).




Comment at: clang-tools-extra/clangd/unittests/URITests.cpp:137
+  llvm::SmallString<16> Ref("c:\\x\\y\\z");
+  llvm::sys::path::native(Ref);
+  EXPECT_THAT(resolveOrDie(parseOrDie("file:///c%3a/x/y/z")), Ref);

mstorsjo wrote:
> sammccall wrote:
> > This seems incorrect. Any build running on windows should be able to handle 
> > backslash paths read from compile_commands.json etc.
> > 
> > 
> Yes, but that's not what the changes does. Regardless of the preference for 
> forward or backwards slashes, both configurations (at least when it comes to 
> LLVMSupport general functions) support both types of paths, that's not 
> changed.
> 
> Previously, this test verified that if you resolve `file:///c%a/x/y/z` you 
> get `c:\\x\\y\\z`. Now in a forward slash environment, you get `c:/x/y/z` - 
> which is expected.
Of course, sorry!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113268

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

[PATCH] D112359: [RISCV] Unify depedency check and extension implication parsing logics

[Yueh-Ting Chen via Phabricator via cfe-commits]

eopXD added a comment.

@asb Thanks for the reply.

To clarify the question, the 2 inconsistencies are:

- Test cases with `clang -cc1` originally don't do dependency check to target 
feature specified (handled by `parseFeatures`)
- Clang driver's -march enforces version to be specified, while llvm allows 
`.attribute` arch to not specify version (and picks the default one)

What this patch tries to do, letting the 2 actions (`parseFeatures` and 
`parseArchString`) rely on the same function call will mean to alter the 
behavior on one of them.
In my humble opinion, it is rather awkward to make this patch an NFC will 
unifying the logics of them.
Maybe we can first come to a conclusion on how do we unify and then implement 
them in this patch.

The main intentions of unifying them is that there will be some implications 
and dependency checks on the addition of zve and zvl,
so unifying the logics will make the addition have less code duplication.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112359

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

Re: [PATCH] D111199: [Clang][LLVM][Attr] support btf_type_tag attribute

[Yonghong Song via cfe-commits]

On 11/5/21 1:40 AM, Martin Storsjö via Phabricator wrote:

mstorsjo added a comment.

I went ahead and reverted this, as it caused crashes when compiling a number of 
projects. The most reduced testcase is this:

   $ cat reduced.c
   void a(*);
   void a() {}
   $ clang -c reduced.c -O2 -g

A full case (which reduces into this) is this, 
https://martin.st/temp/iconv-preproc.c , built with `clang -target 
i686-w64-mingw32 -w -c -O2 -g iconv-preproc.c`.



Martin, thanks for reporting. Let me debug this.



Repository:
   rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99


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

[PATCH] D113268: [clangd] Fix tests with forward slash as separator on windows

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

I forgot to add - I agree with the general sentiment though, that it’s not 
worth bending over backwards to make tests pass in a setup that’s known not to 
work in real use though.

Then again, if someone were to want to actually step up to make it work in 
mingw setups, I guess that shouldn’t be blocked either (although I can’t say I 
have the bandwidth to take on that - I’m not currently a user of clangd).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113268

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

[PATCH] D112850: [clang] 'unused-but-set-variable' warning should not apply to __block objective-c pointers

[Alex Lorenz via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa00944ebeab1: [clang] unused-but-set-variable 
warning should not apply to __block objective… (authored by arphaman).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112850

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/SemaObjC/block-capture-unused-variable.m


Index: clang/test/SemaObjC/block-capture-unused-variable.m
===
--- /dev/null
+++ clang/test/SemaObjC/block-capture-unused-variable.m
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macos11 -fsyntax-only -fobjc-arc 
-fblocks -verify -Wunused-but-set-variable -Wno-objc-root-class %s
+
+typedef struct dispatch_queue_s *dispatch_queue_t;
+
+typedef void (^dispatch_block_t)(void);
+
+void dispatch_async(dispatch_queue_t queue, dispatch_block_t block);
+
+extern __attribute__((visibility("default"))) struct dispatch_queue_s 
_dispatch_main_q;
+
+id getFoo();
+
+@protocol P
+
+@end
+
+@interface I
+
+@end
+
+void test() {
+  // no diagnostics
+  __block id x = getFoo();
+  __block id y = x;
+  __block I *z = (I *)x;
+  // diagnose non-block variables
+  id x2 = getFoo(); // expected-warning {{variable 'x2' set but not used}}
+  dispatch_async(&_dispatch_main_q, ^{
+x = ((void *)0);
+y = x;
+z = ((void *)0);
+  });
+  x2 = getFoo();
+}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -1944,6 +1944,12 @@
 }
   }
 
+  // Don't warn about __block Objective-C pointer variables, as they might
+  // be assigned in the block but not used elsewhere for the purpose of 
lifetime
+  // extension.
+  if (VD->hasAttr() && Ty->isObjCObjectPointerType())
+return;
+
   auto iter = RefsMinusAssignments.find(VD);
   if (iter == RefsMinusAssignments.end())
 return;


Index: clang/test/SemaObjC/block-capture-unused-variable.m
===
--- /dev/null
+++ clang/test/SemaObjC/block-capture-unused-variable.m
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macos11 -fsyntax-only -fobjc-arc -fblocks -verify -Wunused-but-set-variable -Wno-objc-root-class %s
+
+typedef struct dispatch_queue_s *dispatch_queue_t;
+
+typedef void (^dispatch_block_t)(void);
+
+void dispatch_async(dispatch_queue_t queue, dispatch_block_t block);
+
+extern __attribute__((visibility("default"))) struct dispatch_queue_s _dispatch_main_q;
+
+id getFoo();
+
+@protocol P
+
+@end
+
+@interface I
+
+@end
+
+void test() {
+  // no diagnostics
+  __block id x = getFoo();
+  __block id y = x;
+  __block I *z = (I *)x;
+  // diagnose non-block variables
+  id x2 = getFoo(); // expected-warning {{variable 'x2' set but not used}}
+  dispatch_async(&_dispatch_main_q, ^{
+x = ((void *)0);
+y = x;
+z = ((void *)0);
+  });
+  x2 = getFoo();
+}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -1944,6 +1944,12 @@
 }
   }
 
+  // Don't warn about __block Objective-C pointer variables, as they might
+  // be assigned in the block but not used elsewhere for the purpose of lifetime
+  // extension.
+  if (VD->hasAttr() && Ty->isObjCObjectPointerType())
+return;
+
   auto iter = RefsMinusAssignments.find(VD);
   if (iter == RefsMinusAssignments.end())
 return;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] a00944e - [clang] 'unused-but-set-variable' warning should not apply to __block objective-c pointers

[Alex Lorenz via cfe-commits]

Author: Alex Lorenz
Date: 2021-11-05T07:48:07-07:00
New Revision: a00944ebeab1b0adbce606cde4d2410fcbb3f440

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

LOG: [clang] 'unused-but-set-variable' warning should not apply to __block 
objective-c pointers

The __block Objective-C pointers can be set but not used due to a commonly used 
lifetime extension pattern in Objective-C.

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

Added: 
clang/test/SemaObjC/block-capture-unused-variable.m

Modified: 
clang/lib/Sema/SemaDecl.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index f4c0893cfcec2..d61570ee6a104 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -1944,6 +1944,12 @@ void Sema::DiagnoseUnusedButSetDecl(const VarDecl *VD) {
 }
   }
 
+  // Don't warn about __block Objective-C pointer variables, as they might
+  // be assigned in the block but not used elsewhere for the purpose of 
lifetime
+  // extension.
+  if (VD->hasAttr() && Ty->isObjCObjectPointerType())
+return;
+
   auto iter = RefsMinusAssignments.find(VD);
   if (iter == RefsMinusAssignments.end())
 return;

diff  --git a/clang/test/SemaObjC/block-capture-unused-variable.m 
b/clang/test/SemaObjC/block-capture-unused-variable.m
new file mode 100644
index 0..1d40d9fb106b0
--- /dev/null
+++ b/clang/test/SemaObjC/block-capture-unused-variable.m
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macos11 -fsyntax-only -fobjc-arc 
-fblocks -verify -Wunused-but-set-variable -Wno-objc-root-class %s
+
+typedef struct dispatch_queue_s *dispatch_queue_t;
+
+typedef void (^dispatch_block_t)(void);
+
+void dispatch_async(dispatch_queue_t queue, dispatch_block_t block);
+
+extern __attribute__((visibility("default"))) struct dispatch_queue_s 
_dispatch_main_q;
+
+id getFoo();
+
+@protocol P
+
+@end
+
+@interface I
+
+@end
+
+void test() {
+  // no diagnostics
+  __block id x = getFoo();
+  __block id y = x;
+  __block I *z = (I *)x;
+  // diagnose non-block variables
+  id x2 = getFoo(); // expected-warning {{variable 'x2' set but not used}}
+  dispatch_async(&_dispatch_main_q, ^{
+x = ((void *)0);
+y = x;
+z = ((void *)0);
+  });
+  x2 = getFoo();
+}



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

[PATCH] D112913: Misleading bidirectional detection

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added inline comments.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:105
+
+  Inspect string literal and comments for unterminated bidirectional Unicode
+  characters.

Nit: Inspects


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

https://reviews.llvm.org/D112913

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

[PATCH] D113268: [clangd] Fix tests with forward slash as separator on windows

[Sam McCall via Phabricator via cfe-commits]

sammccall added a reviewer: kadircet.
sammccall added a comment.

Ooh sorry, I was missing some context.
I see now that LLVM_WINDOWS_PREFER_FORWARD_SLASH, `windows_backslash` etc are 
new work. And maybe buildbots are coming?

It's possible that the combination of our windows support plus improved 
llvm::sys::path will make things work.
But it's also likely there's rough edges to work out, and this will lead to bug 
reports we can't reproduce or understand.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113268

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

[PATCH] D111318: [clang][clangd] Improve signature help for variadic functions.

[Adam Czachorowski via Phabricator via cfe-commits]

adamcz added a comment.

Sorry for delay, I got distracted with other stuff. I addressed your comment, 
partially, and also added more tests and fixed one more issue (see the 
FunctionType test, it would've failed before).




Comment at: clang/lib/Sema/SemaOverload.cpp:6460
   // list (8.3.5).
   if (TooManyArguments(NumParams, Args.size(), PartialOverloading) &&
   !Proto->isVariadic()) {

sammccall wrote:
> Hmm, there are something like 6 callers of TooManyArguments.
> They all check isVariadic, and probably should all be adjusted in this way 
> (except in once case where PartialOverloading is constant `false`).
> This includes one almost-identical caller in this file, for 
> AddMethodCandidate...
> 
> It's tempting to say that we should just pass some more args into 
> TooManyArguments and have it do this logic internally. Maybe pass the 
> FunctionType and the FunctionDecl* in, with the latter nullable?
> At least this would keep this nice comment in one place.
TooManyArguments is called in 5 places:
- Here
- Below, in the method version, which is identical to this case
- In ProduceCallSignatureHelp, where function can be variadic, but it cannot be 
a template, so no change needed and we don't even have FunctionDecl (just 
FunctionType)
- in checkDirectCallValidity, where PartialOverloading is unconditionally 
false, none of this matters and we shouldn't be checking isTemplateVariadic()
- in DeduceTemplateArguments, where we do check isTemplateVariadic() already, 
but we shouldn't be looking at template instantiation pattern.

additionally there is mergeCandidatesWithResult, where we don't call 
TooManyArguments, but need this logic too. If we did call TooManyArguments, 
we'd have to pass PartialOverloading = false, but still check for template 
variadic stuff.

We only need the pattern logic in 2 out of 5 places and, considering how 
specific to code completion/sig help it is, and how weird it is too, I'm 
leaning towards having this be a separate function.

I renamed the helper to something more appropriate and added a call to it in 
the method case. Let me know if this is better, or if you think I definitely 
should put this in TooManyArguments() (I'm leaning towards this solution, but 
don't have a strong opinion on this myself).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111318

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

[PATCH] D111318: [clang][clangd] Improve signature help for variadic functions.

[Adam Czachorowski via Phabricator via cfe-commits]

adamcz updated this revision to Diff 385071.
adamcz added a comment.

addressed review comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111318

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang/include/clang/Sema/Overload.h
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/lib/Sema/SemaOverload.cpp

Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -6456,7 +6456,8 @@
   // parameters is viable only if it has an ellipsis in its parameter
   // list (8.3.5).
   if (TooManyArguments(NumParams, Args.size(), PartialOverloading) &&
-  !Proto->isVariadic()) {
+  !Proto->isVariadic() &&
+  shouldEnforceArgLimit(PartialOverloading, Function)) {
 Candidate.Viable = false;
 Candidate.FailureKind = ovl_fail_too_many_arguments;
 return;
@@ -6946,7 +6947,8 @@
   // parameters is viable only if it has an ellipsis in its parameter
   // list (8.3.5).
   if (TooManyArguments(NumParams, Args.size(), PartialOverloading) &&
-  !Proto->isVariadic()) {
+  !Proto->isVariadic() &&
+  shouldEnforceArgLimit(PartialOverloading, Method)) {
 Candidate.Viable = false;
 Candidate.FailureKind = ovl_fail_too_many_arguments;
 return;
@@ -15242,3 +15244,21 @@
 FunctionDecl *Fn) {
   return FixOverloadedFunctionReference(E.get(), Found, Fn);
 }
+
+bool clang::shouldEnforceArgLimit(bool PartialOverloading,
+  FunctionDecl *Function) {
+  if (!PartialOverloading || !Function)
+return true;
+  if (Function->isVariadic())
+return false;
+  if (const auto *Proto =
+  dyn_cast(Function->getFunctionType()))
+if (Proto->isTemplateVariadic())
+  return false;
+  if (auto *Pattern = Function->getTemplateInstantiationPattern())
+if (const auto *Proto =
+dyn_cast(Pattern->getFunctionType()))
+  if (Proto->isTemplateVariadic())
+return false;
+  return true;
+}
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -5818,7 +5818,8 @@
 if (Candidate.Function) {
   if (Candidate.Function->isDeleted())
 continue;
-  if (!Candidate.Function->isVariadic() &&
+  if (shouldEnforceArgLimit(/*PartialOverloading=*/true,
+Candidate.Function) &&
   Candidate.Function->getNumParams() <= ArgSize &&
   // Having zero args is annoying, normally we don't surface a function
   // with 2 params, if you already have 2 params, because you are
Index: clang/include/clang/Sema/Overload.h
===
--- clang/include/clang/Sema/Overload.h
+++ clang/include/clang/Sema/Overload.h
@@ -1204,6 +1204,20 @@
 return Info;
   }
 
+  // Returns false if signature help is relevant despite number of arguments
+  // exceeding parameters. Specifically, it returns false when
+  // PartialOverloading is true and one of the following:
+  // * Function is variadic
+  // * Function is template variadic
+  // * Function is an instantiation of template variadic function
+  // The last case may seem strange. The idea is that if we added one more
+  // argument, we'd end up with a function similar to Function. Since, in the
+  // context of signature help and/or code completion, we do not know what the
+  // type of the next argument (that the user is typing) will be, this is as
+  // good candidate as we can get, despite the fact that it takes one less
+  // parameter.
+  bool shouldEnforceArgLimit(bool PartialOverloading, FunctionDecl *Function);
+
 } // namespace clang
 
 #endif // LLVM_CLANG_SEMA_OVERLOAD_H
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -2601,6 +2601,104 @@
   }
 }
 
+TEST(SignatureHelpTest, Variadic) {
+  const std::string Header = R"cpp(
+void fun(int x, ...) {}
+void test() {)cpp";
+  const std::string ExpectedSig = "fun([[int x]], [[...]]) -> void";
+
+  {
+const auto Result = signatures(Header + "fun(^);}");
+EXPECT_EQ(0, Result.activeParameter);
+EXPECT_THAT(Result.signatures, UnorderedElementsAre(Sig(ExpectedSig)));
+  }
+  {
+const auto Result = signatures(Header + "fun(1, ^);}");
+EXPECT_EQ(1, Result.activeParameter);
+EXPECT_THAT(Result.signatures, UnorderedElementsAre(Sig(ExpectedSig)));
+  }
+  {
+const auto Result = signatures(Header + "fun(1, 2, ^);}");
+EXPECT_EQ(1, 

[PATCH] D113268: [clangd] Fix tests with forward slash as separator on windows

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a subscriber: rnk.
mstorsjo added a comment.

FWIW, this change is not about mingw, it's about making the 
windows-with-forward-slashes configuration usable.

The windows-with-forward-slashes configuration works just as fine in MSVC 
configurations, and that's where I've tested it. A MSVC build with `ninja 
check-all` that succeeded before, still succeed with `-DLLVM_ 
WINDOWS_PREFER_FORWARD_SLASH=ON` (with respect to clangd) after this change.

As for testing strategy, it could be concievable to add a configuration to e.g. 
the buildkite premerge tests that build+test everything in a 
`-DLLVM_WINDOWS_PREFER_FORWARD_SLASH=ON` environment too. (I've seen interest 
from @rnk to push towards such a setup, possibly, so adding testing 
configurations for both modes is probably not inconcievable.)




Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:546
 fs::make_absolute(TestScheme::TestDir, Path);
+path::native(Path);
 return std::string(Path);

sammccall wrote:
> This change is an example of something subtle that I don't understand, and 
> don't expect other maintainers to understand, which is therefore fragile.
Hmm, I'll have to revisit this particular change and see if it really was 
necessary in the end, and add a comment explaining the subtlety.



Comment at: clang-tools-extra/clangd/unittests/TestFS.h:79
+llvm::SmallString<16> testRootBuf();
+#define testRoot() testRootBuf().c_str()
 

sammccall wrote:
> If we want to make this more broadly compatible, we'd need to finder 
> cleaner/less invasive ways...
Sure, this is admittedly a bit hacky, but managed to make tests pass with a 
fairly minimal amount of changes.



Comment at: clang-tools-extra/clangd/unittests/URITests.cpp:137
+  llvm::SmallString<16> Ref("c:\\x\\y\\z");
+  llvm::sys::path::native(Ref);
+  EXPECT_THAT(resolveOrDie(parseOrDie("file:///c%3a/x/y/z")), Ref);

sammccall wrote:
> This seems incorrect. Any build running on windows should be able to handle 
> backslash paths read from compile_commands.json etc.
> 
> 
Yes, but that's not what the changes does. Regardless of the preference for 
forward or backwards slashes, both configurations (at least when it comes to 
LLVMSupport general functions) support both types of paths, that's not changed.

Previously, this test verified that if you resolve `file:///c%a/x/y/z` you get 
`c:\\x\\y\\z`. Now in a forward slash environment, you get `c:/x/y/z` - which 
is expected.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113268

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

[PATCH] D108893: clang-tidy: introduce readability-containter-data-pointer check

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

steakhal added subscribers: whisperity, steakhal.
steakhal added a comment.
Herald added a subscriber: carlosgalvezp.

It seems like the checker is documented as `readability-data-pointer` but in 
the tests it reports issues under the `readability-container-data-pointer` name.
Shouldn't they be the same? I think it will confuse the users.
@aaron.ballman @whisperity @compnerd


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108893

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

[PATCH] D111262: Comment AST: Factor out function type extraction in DeclInfo::fill (NFC)

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111262

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

[PATCH] D113268: [clangd] Fix tests with forward slash as separator on windows

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

TL;DR: clangd is unsupported/broken on mingw, this patch only makes the tests 
pass.
The current maintainers don't plan to invest in this, or accept much extra 
complexity to support it.
If you or someone wants to find a clean way to support the configuration, it'd 
be great. Otherwise making the tests pass has little intrinsic value.

---

It sounds like this is the build option `WINDOWS_PREFER_FORWARD_SLASH_DEFAULT`, 
which is set for mingw builds (and could be set manually too).

I think the current state is:

- clangd+mingw is effectively unsupported (no bots, no official binaries, no 
devs who can verify bugs)
- clangd has path-handling problems when built under mingw.
- this patch doesn't fix clangd, it only makes the tests pass. Our tests mostly 
attempt to be "cross-platform", path-handling problems turn up as weird 
interactions between tools that get polished though bug reports.

Supporting windows-forward-slash paths would probably be a quite an effort 
(based on the effort of supporting "standard" windows paths).
This includes code changes, setting up testing infrastructure, refining 
behavior through bug reports, the ongoing cost of dealing with more 
complex/subtle path handling code.

I'm not sure this effort is justified by the impact. I'm open to ideas about 
how to support this but can't dedicate my own time to it. I am opposed to 
making the code much more complex/fragile to deal with it.
If someone's interested in maintaining and testing this configuration and can 
find good ways to encapsulate the extra complexity, it'd be great to support 
this.
Unless we have a plan to support it I don't think we should go to any lengths 
to make the tests pass.

(The current state of "it builds but doesn't work or pass tests" is bad, maybe 
we should be disabling clangd in CMake like we do when threads are disabled).




Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:546
 fs::make_absolute(TestScheme::TestDir, Path);
+path::native(Path);
 return std::string(Path);

This change is an example of something subtle that I don't understand, and 
don't expect other maintainers to understand, which is therefore fragile.



Comment at: clang-tools-extra/clangd/unittests/TestFS.h:79
+llvm::SmallString<16> testRootBuf();
+#define testRoot() testRootBuf().c_str()
 

If we want to make this more broadly compatible, we'd need to finder 
cleaner/less invasive ways...



Comment at: clang-tools-extra/clangd/unittests/URITests.cpp:137
+  llvm::SmallString<16> Ref("c:\\x\\y\\z");
+  llvm::sys::path::native(Ref);
+  EXPECT_THAT(resolveOrDie(parseOrDie("file:///c%3a/x/y/z")), Ref);

This seems incorrect. Any build running on windows should be able to handle 
backslash paths read from compile_commands.json etc.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113268

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

[PATCH] D111190: Comment parsing: Complete list of Doxygen commands

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90

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

[PATCH] D110833: [clang-format] Refactor SpaceBeforeParens to add options

[C. Rayroud via Phabricator via cfe-commits]

crayroud added a comment.

I do not have commit access, could you please help with the push?

Here are the commit details: C. Rayroud - rayro...@gmail.com


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

https://reviews.llvm.org/D110833

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

[PATCH] D107450: [clang-tidy] Fix wrong FixIt in performance-move-const-arg

[gehry via Phabricator via cfe-commits]

Sockke added a comment.

Kindly ping. @whisperity


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

https://reviews.llvm.org/D107450

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

[PATCH] D113256: [AArch64][ARM] Enablement of Cortex-A710 Support

[Dave Green via Phabricator via cfe-commits]

dmgreen added inline comments.



Comment at: llvm/include/llvm/Support/AArch64TargetParser.def:159-160
   AArch64::AEK_RCPC | AArch64::AEK_SSBS))
+AARCH64_CPU_NAME("cortex-a710", ARMV9A, FK_NEON_FP_ARMV8, false,
+ (AArch64::AEK_MTE | AArch64::AEK_PAUTH | AArch64::AEK_FLAGM |
+  AArch64::AEK_SB | AArch64::AEK_I8MM | AArch64::AEK_FP16FML |

Natural order would be better I think, where the new A710 is added after the 
A78.

FlagM should already be included as a part of 8.4, so isn't needed here.
Should BFloat16 be added?



Comment at: llvm/lib/Target/AArch64/AArch64.td:648
 
+def TuneA710: SubtargetFeature<"a710", "ARMProcFamily", "CortexA710",
+   "Cortex-A710 ARM processors", [

Order below, and you can add FeatureCmpBccFusion according to the optimization 
guide.



Comment at: llvm/lib/Target/AArch64/AArch64Subtarget.cpp:108
 break;
   case CortexX2:
 PrefFunctionLogAlignment = 4;

Add here instead, to keep it with more similar cores.



Comment at: llvm/lib/Target/ARM/ARM.td:1334
 
+def : ProcNoItin<"cortex-a710",  [ARMv9a, ProcA710,
+ FeatureHWDivThumb,

Ordering here and elsewhere, and the formatting is a little off.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113256

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

[PATCH] D112868: [Sema] Diagnose and reject non-function ifunc resolvers

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Explanations make sense to me, I'm generally in favor with the 1 concern.




Comment at: clang/lib/CodeGen/CodeGenModule.cpp:397
 
+bool IsIFunc = isa(Alias);
 llvm::Constant *Aliasee =

ibookstein wrote:
> erichkeane wrote:
> > What is the purpose of changing this from checking the declaration to the 
> > IR?  It seems that this is something we should be able to catch at the AST 
> > level and not have to revert to IR checks like this.
> Inside `checkAliasedGlobal` I wanted to save on a parameter (`D`) since then 
> I would just be passing it redundantly because the information is already 
> available on the `Alias`. Here I wanted the check to be an exact copy of the 
> check inside `checkAliasedGlobal` for consistency. I don't mind changing that 
> at at all, I guess I just don't see why I should prefer one over the other :)
The CFE (particularly when doing checks, but even when generating code) should 
be doing as much based on the AST rather than the IR, as it makes us a bit more 
fragile/dependent on the form of the IR. So typcially we try to avoid 
determining types/etc from IR.

Note we break this rule a bunch, but it is currently burning us on opaque-ptr 
for example.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112868

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

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D112349#3109994 , @ibookstein 
wrote:

> I see. What is the guiding principle there, though? Generating correct IR "up 
> front" / "the first time" rather than "fixing it up as you go via 
> manipulations"? (could you give a link?)
> I can see the engineering consideration in not letting IR manipulations creep 
> into the CFE, I just want to make sure that's the principle that we're asked 
> to follow.
> In the end this isn't the first instance where the "streaming" design of 
> GlobalDecl emission forces a fixup/rewrite of a previous decision, as can be 
> evidenced by a close sibling of this feature, 
> `CodeGenModule::EmitAliasDefinition` (which uses exactly that idiom, for a 
> very similar use-case)...
> It will always be either a fixup or an accumulate/commit idiom, since there 
> will always be a GlobalDecl ordering where the information to make 'the 
> perfect module-global decision' isn't available upon having to act on partial 
> information.
>
> In the end, I would like to have the verification of IFuncs having a defined 
> resolver restored (and avoid more dependencies being taken on this being 
> 'allowed' at the IR level), since having LLVM emit an object file with an 
> undefined STT_GNU_IFUNC is probably just trouble and confusion waiting to 
> happen.

The justification is that we're supposed to be able to (as best as we can) work 
in a REPL environment or similar, , like Cling(not a misspell) does. The idea 
is that we should be able to always be emitting 'valid' and 'final' IR so that 
we can generate source 1 declaration at a time and be valid.

We DO have things that break this (as you've mentioned), but the CFE's policy 
is to not break any additional cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112349

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