[clang] 4f3a92c - DebugInfo: Refactor/deduplicate various template argument list emission

[David Blaikie via cfe-commits]

Author: David Blaikie
Date: 2021-08-30T22:39:46-07:00
New Revision: 4f3a92ca0aff115ee17649610c46d8705e550a03

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

LOG: DebugInfo: Refactor/deduplicate various template argument list emission

Streamline template arguments across types, variables, and functions -
for convenient reuse in experiments related to template argument list
reconstitution (not including template argument lists in the "name" of
those entities, and leaving it to debug info consumers to rebuild the
full template name from the semantic descriptions of the argument lists)

But the change seems like a good refactoring/cleanup anyway.

I'd certainly be open to suggestions about how this might be more
streamlined - like is there no generic way to query template argument
lists across the 3 kinds of entities, rather than needing special case
code?

Added: 


Modified: 
clang/lib/CodeGen/CGDebugInfo.cpp
clang/lib/CodeGen/CGDebugInfo.h

Removed: 




diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
b/clang/lib/CodeGen/CGDebugInfo.cpp
index fcccd42c116da..0a20ce39135ac 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -1888,23 +1888,25 @@ void CGDebugInfo::CollectCXXBasesAux(
 }
 
 llvm::DINodeArray
-CGDebugInfo::CollectTemplateParams(const TemplateParameterList *TPList,
-   ArrayRef TAList,
+CGDebugInfo::CollectTemplateParams(Optional OArgs,
llvm::DIFile *Unit) {
+  if (!OArgs)
+return llvm::DINodeArray();
+  TemplateArgs  = *OArgs;
   SmallVector TemplateParams;
-  for (unsigned i = 0, e = TAList.size(); i != e; ++i) {
-const TemplateArgument  = TAList[i];
+  for (unsigned i = 0, e = Args.Args.size(); i != e; ++i) {
+const TemplateArgument  = Args.Args[i];
 StringRef Name;
 bool defaultParameter = false;
-if (TPList)
-  Name = TPList->getParam(i)->getName();
+if (Args.TList)
+  Name = Args.TList->getParam(i)->getName();
 switch (TA.getKind()) {
 case TemplateArgument::Type: {
   llvm::DIType *TTy = getOrCreateType(TA.getAsType(), Unit);
 
-  if (TPList)
+  if (Args.TList)
 if (auto *templateType =
-dyn_cast_or_null(TPList->getParam(i)))
+
dyn_cast_or_null(Args.TList->getParam(i)))
   if (templateType->hasDefaultArgument())
 defaultParameter =
 templateType->getDefaultArgument() == TA.getAsType();
@@ -1915,9 +1917,9 @@ CGDebugInfo::CollectTemplateParams(const 
TemplateParameterList *TPList,
 } break;
 case TemplateArgument::Integral: {
   llvm::DIType *TTy = getOrCreateType(TA.getIntegralType(), Unit);
-  if (TPList && CGM.getCodeGenOpts().DwarfVersion >= 5)
-if (auto *templateType =
-dyn_cast_or_null(TPList->getParam(i)))
+  if (Args.TList && CGM.getCodeGenOpts().DwarfVersion >= 5)
+if (auto *templateType = dyn_cast_or_null(
+Args.TList->getParam(i)))
   if (templateType->hasDefaultArgument() &&
   !templateType->getDefaultArgument()->isValueDependent())
 defaultParameter = llvm::APSInt::isSameValue(
@@ -2002,7 +2004,7 @@ CGDebugInfo::CollectTemplateParams(const 
TemplateParameterList *TPList,
 case TemplateArgument::Pack:
   TemplateParams.push_back(DBuilder.createTemplateParameterPack(
   TheCU, Name, nullptr,
-  CollectTemplateParams(nullptr, TA.getPackAsArray(), Unit)));
+  CollectTemplateParams({{nullptr, TA.getPackAsArray()}}, Unit)));
   break;
 case TemplateArgument::Expression: {
   const Expr *E = TA.getAsExpr();
@@ -2025,43 +2027,58 @@ CGDebugInfo::CollectTemplateParams(const 
TemplateParameterList *TPList,
   return DBuilder.getOrCreateArray(TemplateParams);
 }
 
-llvm::DINodeArray
-CGDebugInfo::CollectFunctionTemplateParams(const FunctionDecl *FD,
-   llvm::DIFile *Unit) {
+Optional
+CGDebugInfo::GetTemplateArgs(const FunctionDecl *FD) const {
   if (FD->getTemplatedKind() ==
   FunctionDecl::TK_FunctionTemplateSpecialization) {
 const TemplateParameterList *TList = FD->getTemplateSpecializationInfo()
  ->getTemplate()
  ->getTemplateParameters();
-return CollectTemplateParams(
-TList, FD->getTemplateSpecializationArgs()->asArray(), Unit);
+return {{TList, FD->getTemplateSpecializationArgs()->asArray()}};
   }
-  return llvm::DINodeArray();
+  return None;
 }
-
-llvm::DINodeArray CGDebugInfo::CollectVarTemplateParams(const VarDecl *VL,
-llvm::DIFile *Unit) {
+Optional

[PATCH] D108085: Use installed llvm-lit.py instead of lit.py PR-51072

[Tom Stellard via Phabricator via cfe-commits]

tstellar added inline comments.



Comment at: clang/test/utils/update_cc_test_checks/lit.local.cfg:24
+# Windows: llvm-lit.py, Linux: llvm-lit
+llvm_lit = glob.glob(os.path.join(config.llvm_tools_dir, 'llvm-lit*'))[0]
+lit = config.llvm_external_lit if config.llvm_external_lit else 
shell_quote(llvm_lit)

This patch breaks stand-alone clang builds, because it assumes llvm-lit* will 
be present in the build directory.   This line should be guarded by if not 
config.llvm_external_lit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108085

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

[PATCH] D108818: [clang] Add a -canonical-prefixes option

[Reid Kleckner via Phabricator via cfe-commits]

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

lgtm

+@jansvoboda11 for lack of a better alternative owner for Driver patches.


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

https://reviews.llvm.org/D108818

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

[PATCH] D105168: [RISCV] Unify the arch string parsing logic to RISCVISAInfo.

[Jim Lin via Phabricator via cfe-commits]

Jim added inline comments.



Comment at: llvm/lib/Support/RISCVISAInfo.cpp:387
+ExtensionInfoIterator->Version.Minor);
+  if (ExtName == "e")
+HasE = true;

Does this need to check it is invalid if XLen is 64?



Comment at: llvm/lib/Support/RISCVISAInfo.cpp:394
+  if (!HasE)
+ISAInfo->addExtension("i", 2, 0);
+

Why not get the version of i from SupportedExtensions?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105168

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

[PATCH] D101960: [openmp] Drop requirement on library path environment variables

[Ye Luo via Phabricator via cfe-commits]

ye-luo added a comment.

In D101960#2961133 , @jdoerfert wrote:

> There are 3 problems here (ignoring our test setup which should be discussed 
> separately):
>
> 1. make sure clang finds libomp.so
> 2. make sure libomp.so (or clang?) finds libomptarget.so
> 3. make sure libomptartget.so finds the plugins
>
> All of which have to work nicely with LD_LIBRARY_PATH.
>
> I think for 3 you can look at the current dir. That was discussed and, as 
> long as it does work with LD_LIBRARY_PATH, that seems a win.
>
> For 2, why don't we install them in the same place? If so, we reduce it to 
> problem 1) [after applying solution to 3)] no?
>
> For 1, doing something always backwards compatible that may or may not work 
> on some platforms and configurations seems best. You had a proposal here 
> already. If that works, let's do it.

For 1. If users prefer linking an alternative libomp.so, they should not use 
-fopenmp at linking and link libomp.so explicitly as a regular library. If 
users add -fopenmp to clang at linking, clang needs to link the libomp.so 
shipped with clang and set rpath to ensure libomp.so can be found at run even 
libomp.so doesn't exist on LD_LIBRARY_PATH. In this way, if a user would like 
to switch to anther libomp.so, they can still specify LD_LIBRARY_PATH.
For 2. Is libomp.so aware of libomptarget.so? I doubt. Anyway I'd like to see a 
similar logic as case 1 and the control option is -fopenmp-targets.

So addOpenMPRuntimeSpecificRPath seems aligned with what I described.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101960

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

[PATCH] D101935: [clang] Search runtimes build tree for openmp runtime

[Ye Luo via Phabricator via cfe-commits]

ye-luo added a comment.

It seems that this path is baked in to clang executable even after make 
install. I'm not convinced this is the right direction.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101935

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

[PATCH] D108958: [OpenMP] Make CUDA math library functions SPMD amenable

[Johannes Doerfert via Phabricator via cfe-commits]

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

LG


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108958

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

[PATCH] D108905: [ItaniumCXXABI] Make __cxa_end_catch calls unconditionally nounwind

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Thanks, Richard.  Fangrui, I think we can't do anything on this patch without a 
CWG decision about the right semantics, so this will have to sit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108905

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

[PATCH] D108881: [clang][driver] Honor the last -flto(=.*)? argument

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

In D108881#2973719 , @mnadeem wrote:

> In D108881#2973516 , @steven_wu 
> wrote:
>
>> I will do a cleanup of `parseLTOMode` function since we don't need a 
>> `OptPos` parameter anymore. There are few minor places references `OPT_flto` 
>> or `OPT_foffload_lto` can be cleaned up too.
>
> Will you incorporate the functional changes in this patch? Or is there still 
> a need for this change?

The current change set in this review is functional change while the cleanup I 
want is not functional after the rewrite the old option as Alias. Once flto is 
the alias, there is no need to handle that in the driver and those might 
actually become source of bug in the future.

I think it would be good to do the cleanup in the same commit unless you have 
compelling reason not to.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108881

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

[PATCH] D107290: [PoC][RISCV] Add support for the vscale_range attribute

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCVTargetMachine.cpp:101
+  } else {
+RVVBitsMin = RVVVectorBitsMinOpt;
+RVVBitsMax = RVVVectorBitsMaxOpt;

If clang always emits the attribute, are these options effectively dead for 
clang codegen?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107290

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

[PATCH] D108881: [clang][driver] Honor the last -flto(=.*)? argument

[Usman Nadeem via Phabricator via cfe-commits]

mnadeem added a comment.

In D108881#2973516 , @steven_wu wrote:

> I will do a cleanup of `parseLTOMode` function since we don't need a `OptPos` 
> parameter anymore. There are few minor places references `OPT_flto` or 
> `OPT_foffload_lto` can be cleaned up too.

Will you incorporate the functional changes in this patch? Or is there still a 
need for this change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108881

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

[PATCH] D106994: [modules] Fix miscompilation when using two RecordDecl definitions with the same name.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Thanks for the review!




Comment at: clang/lib/Serialization/ASTReaderDecl.cpp:3328-3331
 // If there's no definition yet, then DC's definition is added by an update
 // record, but we've not yet loaded that update record. In this case, we
 // commit to DC being the canonical definition now, and will fix this when
 // we load the update record.

rsmith wrote:
> I believe there's no need to have logic matching this case in C because the 
> only way that a class definition can be added by an update record is due to 
> template instantiation. So we can use the simpler logic below for C.
Thanks, it's good to know.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106994

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

[PATCH] D106994: [modules] Fix miscompilation when using two RecordDecl definitions with the same name.

[Volodymyr Sapsai via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG93764ff6e200: [modules] Fix miscompilation when using two 
RecordDecl definitions with the… (authored by vsapsai).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106994

Files:
  clang/include/clang/Serialization/ASTReader.h
  clang/lib/Serialization/ASTCommon.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  
clang/test/Modules/Inputs/merge-record-definition/RecordDef.framework/Headers/RecordDef.h
  
clang/test/Modules/Inputs/merge-record-definition/RecordDef.framework/Modules/module.modulemap
  
clang/test/Modules/Inputs/merge-record-definition/RecordDefCopy.framework/Headers/RecordDefCopy.h
  
clang/test/Modules/Inputs/merge-record-definition/RecordDefCopy.framework/Modules/module.modulemap
  
clang/test/Modules/Inputs/merge-record-definition/RecordDefHidden.framework/Headers/Hidden.h
  
clang/test/Modules/Inputs/merge-record-definition/RecordDefHidden.framework/Headers/Visible.h
  
clang/test/Modules/Inputs/merge-record-definition/RecordDefHidden.framework/Modules/module.modulemap
  
clang/test/Modules/Inputs/merge-record-definition/RecordDefIncluder.framework/Headers/RecordDefIncluder.h
  
clang/test/Modules/Inputs/merge-record-definition/RecordDefIncluder.framework/Modules/module.modulemap
  clang/test/Modules/merge-record-definition-nonmodular.m
  clang/test/Modules/merge-record-definition-visibility.m
  clang/test/Modules/merge-record-definition.m

Index: clang/test/Modules/merge-record-definition.m
===
--- /dev/null
+++ clang/test/Modules/merge-record-definition.m
@@ -0,0 +1,28 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: %clang_cc1 -emit-llvm -o %t/test.bc -F%S/Inputs/merge-record-definition %s \
+// RUN:-fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache
+
+// Test a case when a struct definition is present in two different modules.
+
+#import 
+
+void bibi(void) {
+  Buffer buf;
+  buf.b = 1;
+  AnonymousStruct strct;
+  strct.x = 1;
+  UnionRecord rec;
+  rec.u = 1;
+}
+
+#import 
+
+void mbap(void) {
+  Buffer buf;
+  buf.c = 2;
+  AnonymousStruct strct;
+  strct.y = 2;
+  UnionRecord rec;
+  rec.v = 2;
+}
Index: clang/test/Modules/merge-record-definition-visibility.m
===
--- /dev/null
+++ clang/test/Modules/merge-record-definition-visibility.m
@@ -0,0 +1,18 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: %clang_cc1 -emit-llvm -o %t/test.bc -F%S/Inputs/merge-record-definition %s \
+// RUN:-fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache
+
+// Test a case when a struct definition is first imported as invisible and then as visible.
+
+#import 
+#import 
+
+void bibi(void) {
+  Buffer buf;
+  buf.b = 1;
+  AnonymousStruct strct;
+  strct.y = 1;
+  UnionRecord rec;
+  rec.u = 1;
+}
Index: clang/test/Modules/merge-record-definition-nonmodular.m
===
--- /dev/null
+++ clang/test/Modules/merge-record-definition-nonmodular.m
@@ -0,0 +1,38 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: %clang_cc1 -emit-llvm -o %t/test.bc -F%S/Inputs/merge-record-definition %s \
+// RUN:-fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache -fmodule-name=RecordDef
+// RUN: %clang_cc1 -emit-llvm -o %t/test.bc -F%S/Inputs/merge-record-definition %s -DMODULAR_BEFORE_TEXTUAL \
+// RUN:-fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache -fmodule-name=RecordDef
+
+// Test a case when a struct definition once is included from a textual header and once from a module.
+
+#ifdef MODULAR_BEFORE_TEXTUAL
+  #import 
+#else
+  #import 
+#endif
+
+void bibi(void) {
+  Buffer buf;
+  buf.b = 1;
+  AnonymousStruct strct;
+  strct.x = 1;
+  UnionRecord rec;
+  rec.u = 1;
+}
+
+#ifdef MODULAR_BEFORE_TEXTUAL
+  #import 
+#else
+  #import 
+#endif
+
+void mbap(void) {
+  Buffer buf;
+  buf.c = 2;
+  AnonymousStruct strct;
+  strct.y = 2;
+  UnionRecord rec;
+  rec.v = 2;
+}
Index: clang/test/Modules/Inputs/merge-record-definition/RecordDefIncluder.framework/Modules/module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/merge-record-definition/RecordDefIncluder.framework/Modules/module.modulemap
@@ -0,0 +1,4 @@
+framework module RecordDefIncluder {
+  header "RecordDefIncluder.h"
+  export *
+}
Index: clang/test/Modules/Inputs/merge-record-definition/RecordDefIncluder.framework/Headers/RecordDefIncluder.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/merge-record-definition/RecordDefIncluder.framework/Headers/RecordDefIncluder.h
@@ -0,0 +1 @@
+#import 
Index: 

[clang] 93764ff - [modules] Fix miscompilation when using two RecordDecl definitions with the same name.

[Volodymyr Sapsai via cfe-commits]

Author: Volodymyr Sapsai
Date: 2021-08-30T17:51:38-07:00
New Revision: 93764ff6e2005b92057cffa9b3866307e2de260f

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

LOG: [modules] Fix miscompilation when using two RecordDecl definitions with 
the same name.

When deserializing a RecordDecl we don't enforce that redeclaration
chain contains only a single definition. So if the canonical decl is not
a definition itself, `RecordType::getDecl` can return different objects
before and after an include. It means we can build CGRecordLayout for
one RecordDecl with its set of FieldDecl but try to use it with
FieldDecl belonging to a different RecordDecl. With assertions enabled
it results in

> Assertion failed: (FieldInfo.count(FD) && "Invalid field for record!"),
> function getLLVMFieldNo, file 
> llvm-project/clang/lib/CodeGen/CGRecordLayout.h, line 199.

and with assertions disabled a bunch of fields are treated as their
memory is located at offset 0.

Fix by keeping the first encountered RecordDecl definition and marking
the subsequent ones as non-definitions. Also need to merge FieldDecl
properly, so that `getPrimaryMergedDecl` works correctly and during name
lookup we don't treat fields from same-name RecordDecl as ambiguous.

rdar://80184238

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

Added: 

clang/test/Modules/Inputs/merge-record-definition/RecordDef.framework/Headers/RecordDef.h

clang/test/Modules/Inputs/merge-record-definition/RecordDef.framework/Modules/module.modulemap

clang/test/Modules/Inputs/merge-record-definition/RecordDefCopy.framework/Headers/RecordDefCopy.h

clang/test/Modules/Inputs/merge-record-definition/RecordDefCopy.framework/Modules/module.modulemap

clang/test/Modules/Inputs/merge-record-definition/RecordDefHidden.framework/Headers/Hidden.h

clang/test/Modules/Inputs/merge-record-definition/RecordDefHidden.framework/Headers/Visible.h

clang/test/Modules/Inputs/merge-record-definition/RecordDefHidden.framework/Modules/module.modulemap

clang/test/Modules/Inputs/merge-record-definition/RecordDefIncluder.framework/Headers/RecordDefIncluder.h

clang/test/Modules/Inputs/merge-record-definition/RecordDefIncluder.framework/Modules/module.modulemap
clang/test/Modules/merge-record-definition-nonmodular.m
clang/test/Modules/merge-record-definition-visibility.m
clang/test/Modules/merge-record-definition.m

Modified: 
clang/include/clang/Serialization/ASTReader.h
clang/lib/Serialization/ASTCommon.cpp
clang/lib/Serialization/ASTReaderDecl.cpp

Removed: 




diff  --git a/clang/include/clang/Serialization/ASTReader.h 
b/clang/include/clang/Serialization/ASTReader.h
index e342a061379eb..f24ccf579aa8f 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -1162,6 +1162,10 @@ class ASTReader
   /// definitions. Only populated when using modules in C++.
   llvm::DenseMap EnumDefinitions;
 
+  /// A mapping from canonical declarations of records to their canonical
+  /// definitions. Doesn't cover CXXRecordDecl.
+  llvm::DenseMap RecordDefinitions;
+
   /// When reading a Stmt tree, Stmt operands are placed in this stack.
   SmallVector StmtStack;
 

diff  --git a/clang/lib/Serialization/ASTCommon.cpp 
b/clang/lib/Serialization/ASTCommon.cpp
index 5fe1f96327ddc..db118d8b00776 100644
--- a/clang/lib/Serialization/ASTCommon.cpp
+++ b/clang/lib/Serialization/ASTCommon.cpp
@@ -474,7 +474,7 @@ bool serialization::needsAnonymousDeclarationNumber(const 
NamedDecl *D) {
   // Otherwise, we only care about anonymous class members / block-scope decls.
   // FIXME: We need to handle lambdas and blocks within inline / templated
   // variables too.
-  if (D->getDeclName() || !isa(D->getLexicalDeclContext()))
+  if (D->getDeclName() || !isa(D->getLexicalDeclContext()))
 return false;
   return isa(D) || isa(D);
 }

diff  --git a/clang/lib/Serialization/ASTReaderDecl.cpp 
b/clang/lib/Serialization/ASTReaderDecl.cpp
index ff79f91e5db1b..ef7921212f21b 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -332,7 +332,7 @@ namespace clang {
 RedeclarableResult VisitTagDecl(TagDecl *TD);
 void VisitEnumDecl(EnumDecl *ED);
 RedeclarableResult VisitRecordDeclImpl(RecordDecl *RD);
-void VisitRecordDecl(RecordDecl *RD) { VisitRecordDeclImpl(RD); }
+void VisitRecordDecl(RecordDecl *RD);
 RedeclarableResult VisitCXXRecordDeclImpl(CXXRecordDecl *D);
 void VisitCXXRecordDecl(CXXRecordDecl *D) { VisitCXXRecordDeclImpl(D); }
 RedeclarableResult VisitClassTemplateSpecializationDeclImpl(
@@ -808,6 +808,34 @@ ASTDeclReader::VisitRecordDeclImpl(RecordDecl *RD) {
   return Redecl;
 }
 
+void 

[PATCH] D107764: [OpenMP][OpenMPIRBuilder] Implement loop unrolling.

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur added inline comments.



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:2055
+/// Attach loop metadata \p Properties to the loop described by \p Loop. If the
+/// loop already has metadata, the loop properties are appended.
+static void addLoopMetadata(CanonicalLoopInfo *Loop,

kiranchandramohan wrote:
> Nit: In the body, it seems we are prepending.
In the body, first we append `Existing->operands()`, then `Properties`. I.e. we 
append the Properties passes as an argument to after the existing properties to 
form a new list `NewLoopProperties`.



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:2168-2169
+  // actually unrolls the loop.
+  UP.Threshold *= 1.5;
+  UP.PartialThreshold *= 1.5;
+

kiranchandramohan wrote:
> Should this be settable for experimentation or additional control? If not can 
> you provide an explanation for these values?
Made them configurable using the new 
`-openmp-ir-builder-unroll-threshold-factor` switch.

I selected 1.5 as default to make it match approximately the unroll factor the 
LoopUnrollPass would derive in an -O3 pass pass pipeline. No large-scale 
experiments, just a loop that LoopUnrollPass would unroll by a factor of 4 (out 
of possible: 1 (no unrolling), 2, 4, 8) and make `#pragma omp unroll partial` 
also unroll by a factor of 4.



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:2275
+return nullptr;
+  }
+

jdoerfert wrote:
> This code matches the pattern we have in the 2 members above. Can we make it 
> a member as well and make it clear in the name of all 3 that we use metadata 
> and the unroller pass? No strong feelings, more of an idea.
I think the API should abstract over how the unrolling is performed, frontends 
should not need to be concerned about how unrolling is actually applied. The 
metadata version is preferred unless we cannot use it because we need the 
unrolled loop's CFG wrapped by CanonicalLoopInfo.

I changed the parameters to make it more idiomatic that the caller only 
receives a CanonicalLoopInfo if requested, and that's the only purpose of the 
4th parameter.



Comment at: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp:1666
 
+TEST_F(OpenMPIRBuilderTest, UnrollLoopFull) {
+  OpenMPIRBuilder OMPBuilder(*M);

kiranchandramohan wrote:
> Are we not calling ompbuilder finalize or verify because it is only adding 
> metadata?
Added
```
OMPBuilder.finalize();
EXPECT_FALSE(verifyModule(*M, ()));
```
to the unroll tests. Thanks for noticing.



Comment at: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp:1735
+
 TEST_F(OpenMPIRBuilderTest, StaticWorkShareLoop) {
   using InsertPointTy = OpenMPIRBuilder::InsertPointTy;

kiranchandramohan wrote:
> Should we add tests for workshare loops with unroll?
I think this is beyond the scope of these tests here. To test thoroughly, we'd 
have to test all combinations of loop transformations and loop-associated 
directives. The `CanonicalLoopInfo::assertOK()` should already check the 
invariants that ensure that the CLI can be used as input for other 
loop-associated directives. Additionally, the composition is tested with 
clang's tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107764

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

[PATCH] D108045: [clangd] Fix clangd crash when including a header

[Queen Dela Cruz via Phabricator via cfe-commits]

qdelacru updated this revision to Diff 369581.
qdelacru added a comment.

Create macro directive only preamble patch for code completion


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

https://reviews.llvm.org/D108045

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/Preamble.h
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang-tools-extra/clangd/unittests/PreambleTests.cpp

Index: clang-tools-extra/clangd/unittests/PreambleTests.cpp
===
--- clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -546,6 +546,13 @@
   // Ensure they are dropeed when a patched preamble is used.
   EXPECT_FALSE(createPatchedAST("", Code)->getDiagnostics());
 }
+
+TEST(PreamblePatch, MacroLoc) {
+  llvm::StringLiteral Baseline = "\n#define MACRO 12\nint num = MACRO;";
+  llvm::StringLiteral Modified = " \n#define MACRO 12\nint num = MACRO;";
+  auto AST = createPatchedAST(Baseline, Modified);
+  ASSERT_TRUE(AST);
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -3260,6 +3260,23 @@
 EXPECT_THAT(Results.Completions, UnorderedElementsAre(Labeled("BarExt")));
   }
 }
+
+TEST(CompletionTest, PreambleCodeComplete) {
+  llvm::StringLiteral Baseline = "\n#define MACRO 12\nint num = MACRO;";
+  llvm::StringLiteral ModifiedCC =
+  "#include \"header.h\"\n#define MACRO 12\nint num = MACRO; int num2 = M^";
+
+  Annotations Test(ModifiedCC);
+  auto BaselineTU = TestTU::withCode(Baseline);
+  auto ModifiedTU = TestTU::withCode(Test.code());
+
+  MockFS FS;
+  auto Inputs = ModifiedTU.inputs(FS);
+  auto Result = codeComplete(testPath(ModifiedTU.Filename), Test.point(),
+ BaselineTU.preamble().get(), Inputs, {});
+  EXPECT_THAT(Result.Completions, Not(testing::IsEmpty()));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/Preamble.h
===
--- clang-tools-extra/clangd/Preamble.h
+++ clang-tools-extra/clangd/Preamble.h
@@ -106,6 +106,9 @@
   static PreamblePatch create(llvm::StringRef FileName,
   const ParseInputs ,
   const PreambleData );
+  static PreamblePatch createMacroPatch(llvm::StringRef FileName,
+const ParseInputs ,
+const PreambleData );
   /// Adjusts CI (which compiles the modified inputs) to be used with the
   /// baseline preamble. This is done by inserting an artifical include to the
   /// \p CI that contains new directives calculated in create.
Index: clang-tools-extra/clangd/Preamble.cpp
===
--- clang-tools-extra/clangd/Preamble.cpp
+++ clang-tools-extra/clangd/Preamble.cpp
@@ -142,10 +142,11 @@
   unsigned DirectiveLine;
   // Full text that's representing the directive, including the `#`.
   std::string Text;
+  unsigned Offset;
 
   bool operator==(const TextualPPDirective ) const {
-return std::tie(DirectiveLine, Text) ==
-   std::tie(RHS.DirectiveLine, RHS.Text);
+return std::tie(DirectiveLine, Text, Offset) ==
+   std::tie(RHS.DirectiveLine, RHS.Text, RHS.Offset);
   }
 };
 
@@ -155,7 +156,7 @@
 std::string spellDirective(llvm::StringRef Prefix,
CharSourceRange DirectiveRange,
const LangOptions , const SourceManager ,
-   unsigned ) {
+   unsigned , unsigned ) {
   std::string SpelledDirective;
   llvm::raw_string_ostream OS(SpelledDirective);
   OS << Prefix;
@@ -169,6 +170,7 @@
 
   auto DecompLoc = SM.getDecomposedLoc(DirectiveRange.getBegin());
   DirectiveLine = SM.getLineNumber(DecompLoc.first, DecompLoc.second);
+  Offset = DecompLoc.second;
   auto TargetColumn = SM.getColumnNumber(DecompLoc.first, DecompLoc.second) - 1;
 
   // Pad with spaces before DirectiveRange to make sure it will be on right
@@ -217,7 +219,7 @@
 spellDirective("#define ",
CharSourceRange::getTokenRange(
MI->getDefinitionLoc(), MI->getDefinitionEndLoc()),
-   LangOpts, SM, TD.DirectiveLine);
+   LangOpts, SM, TD.DirectiveLine, TD.Offset);
   }
 
 private:
@@ -528,6 +530,49 @@
   return PP;
 }
 
+PreamblePatch PreamblePatch::createMacroPatch(llvm::StringRef FileName,
+  const ParseInputs ,
+  

[PATCH] D105671: [Intrinsics][ObjC] Mark objc_retain and friends as thisreturn.

[Jon Roelofs via Phabricator via cfe-commits]

jroelofs updated this revision to Diff 369584.
jroelofs added a comment.
Herald added a subscriber: dexonsmith.

Rebased.

Also, turns out that `stripPointerCasts()` can see through the `thisreturn` 
attribute, which defeats a self retain optimization, breaking one of the clang 
tests. I tweaked a callback to make it configurable. Let me know if you see a 
better way of dealing with that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105671

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/test/CodeGenObjC/arc.m
  clang/test/CodeGenObjC/convert-messages-to-runtime-calls.m
  llvm/include/llvm/IR/Intrinsics.td
  llvm/include/llvm/IR/Value.h
  llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
  llvm/lib/IR/Value.cpp
  llvm/test/Transforms/PreISelIntrinsicLowering/objc-arc.ll

Index: llvm/test/Transforms/PreISelIntrinsicLowering/objc-arc.ll
===
--- llvm/test/Transforms/PreISelIntrinsicLowering/objc-arc.ll
+++ llvm/test/Transforms/PreISelIntrinsicLowering/objc-arc.ll
@@ -6,7 +6,7 @@
 define i8* @test_objc_autorelease(i8* %arg0) {
 ; CHECK-LABEL: test_objc_autorelease
 ; CHECK-NEXT: entry
-; CHECK-NEXT: %0 = notail call i8* @objc_autorelease(i8* %arg0)
+; CHECK-NEXT: %0 = notail call i8* @objc_autorelease(i8* returned %arg0)
 ; CHECK-NEXT: ret i8* %0
 entry:
   %0 = call i8* @llvm.objc.autorelease(i8* %arg0)
@@ -113,20 +113,31 @@
   ret void
 }
 
+define i8* @test_objc_retain_intrinsic(i8* %arg0) {
+; CHECK-LABEL: test_objc_retain_intrinsic
+; CHECK-NEXT: entry
+; CHECK-NEXT: %0 = tail call i8* @objc_retain(i8* returned %arg0)
+; CHECK-NEXT: ret i8* %0
+entry:
+  %0 = call i8* @llvm.objc.retain(i8* %arg0)
+	ret i8* %0
+}
+
+; Explicit objc_retain calls should not be upgraded to be "thisreturn".
 define i8* @test_objc_retain(i8* %arg0) {
 ; CHECK-LABEL: test_objc_retain
 ; CHECK-NEXT: entry
-; CHECK-NEXT: %0 = tail call i8* @objc_retain(i8* %arg0)
+; CHECK-NEXT: %0 = call i8* @objc_retain(i8* %arg0)
 ; CHECK-NEXT: ret i8* %0
 entry:
-  %0 = call i8* @llvm.objc.retain(i8* %arg0)
+  %0 = call i8* @objc_retain(i8* %arg0)
 	ret i8* %0
 }
 
 define i8* @test_objc_retainAutorelease(i8* %arg0) {
 ; CHECK-LABEL: test_objc_retainAutorelease
 ; CHECK-NEXT: entry
-; CHECK-NEXT: %0 = call i8* @objc_retainAutorelease(i8* %arg0)
+; CHECK-NEXT: %0 = call i8* @objc_retainAutorelease(i8* returned %arg0)
 ; CHECK-NEXT: ret i8* %0
 entry:
   %0 = call i8* @llvm.objc.retainAutorelease(i8* %arg0)
@@ -136,7 +147,7 @@
 define i8* @test_objc_retainAutoreleaseReturnValue(i8* %arg0) {
 ; CHECK-LABEL: test_objc_retainAutoreleaseReturnValue
 ; CHECK-NEXT: entry
-; CHECK-NEXT: %0 = tail call i8* @objc_retainAutoreleaseReturnValue(i8* %arg0)
+; CHECK-NEXT: %0 = tail call i8* @objc_retainAutoreleaseReturnValue(i8* returned %arg0)
 ; CHECK-NEXT: ret i8* %0
 entry:
   %0 = tail call i8* @llvm.objc.retainAutoreleaseReturnValue(i8* %arg0)
@@ -146,7 +157,7 @@
 define i8* @test_objc_retainAutoreleasedReturnValue(i8* %arg0) {
 ; CHECK-LABEL: test_objc_retainAutoreleasedReturnValue
 ; CHECK-NEXT: entry
-; CHECK-NEXT: %0 = tail call i8* @objc_retainAutoreleasedReturnValue(i8* %arg0)
+; CHECK-NEXT: %0 = tail call i8* @objc_retainAutoreleasedReturnValue(i8* returned %arg0)
 ; CHECK-NEXT: ret i8* %0
 entry:
   %0 = call i8* @llvm.objc.retainAutoreleasedReturnValue(i8* %arg0)
@@ -226,7 +237,7 @@
 define i8* @test_objc_retain_autorelease(i8* %arg0) {
 ; CHECK-LABEL: test_objc_retain_autorelease
 ; CHECK-NEXT: entry
-; CHECK-NEXT: %0 = call i8* @objc_retain_autorelease(i8* %arg0)
+; CHECK-NEXT: %0 = call i8* @objc_retain_autorelease(i8* returned %arg0)
 ; CHECK-NEXT: ret i8* %0
 entry:
   %0 = call i8* @llvm.objc.retain.autorelease(i8* %arg0)
@@ -265,6 +276,7 @@
 declare void @llvm.objc.moveWeak(i8**, i8**)
 declare void @llvm.objc.release(i8*)
 declare i8* @llvm.objc.retain(i8*)
+declare i8* @objc_retain(i8*)
 declare i8* @llvm.objc.retainAutorelease(i8*)
 declare i8* @llvm.objc.retainAutoreleaseReturnValue(i8*)
 declare i8* @llvm.objc.retainAutoreleasedReturnValue(i8*)
@@ -281,6 +293,7 @@
 
 attributes #0 = { nounwind }
 
+; CHECK: declare i8* @objc_retain(i8*) [[NLB:#[0-9]+]]
 ; CHECK: declare i8* @objc_autorelease(i8*)
 ; CHECK: declare void @objc_autoreleasePoolPop(i8*)
 ; CHECK: declare i8* @objc_autoreleasePoolPush()
@@ -291,8 +304,7 @@
 ; CHECK: declare i8* @objc_loadWeak(i8**)
 ; CHECK: declare i8* @objc_loadWeakRetained(i8**)
 ; CHECK: declare void @objc_moveWeak(i8**, i8**)
-; CHECK: declare void @objc_release(i8*) [[NLB:#[0-9]+]]
-; CHECK: declare i8* @objc_retain(i8*) [[NLB]]
+; CHECK: declare void @objc_release(i8*) [[NLB]]
 ; CHECK: declare i8* @objc_retainAutorelease(i8*)
 ; CHECK: declare i8* @objc_retainAutoreleaseReturnValue(i8*)
 ; CHECK: declare i8* @objc_retainAutoreleasedReturnValue(i8*)
Index: llvm/lib/IR/Value.cpp
===
--- 

[PATCH] D108881: [clang][driver] Honor the last -flto(=.*)? argument

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

In D108881#2973516 , @steven_wu wrote:

> I will do a cleanup of `parseLTOMode` function since we don't need a `OptPos` 
> parameter anymore. There are few minor places references `OPT_flto` or 
> `OPT_foffload_lto` can be cleaned up too.

Good idea on simplifying via an alias, agree that this should all just be 
cleaned up now. Thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108881

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

[PATCH] D105937: [OpenMP] Encode `omp [...] assume[...]` assumptions with `omp[x]` prefix

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 accepted this revision.
jhuber6 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/D105937/new/

https://reviews.llvm.org/D105937

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

[PATCH] D108958: [OpenMP] Make CUDA math library functions SPMD amenable

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 created this revision.
jhuber6 added a reviewer: jdoerfert.
Herald added subscribers: guansong, yaxunl.
jhuber6 requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

This patch adds the SPMD amenable assumption to the CUDA math library
defintions in Clang. Previously these functions would block SPMD
execution on the device because they're intrinsic calls into the library
and can't be calculated. These functions don't have side-effects so they
are safe to execute in SPMD mode.

Depends on D105937 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108958

Files:
  clang/lib/Headers/__clang_cuda_libdevice_declares.h


Index: clang/lib/Headers/__clang_cuda_libdevice_declares.h
===
--- clang/lib/Headers/__clang_cuda_libdevice_declares.h
+++ clang/lib/Headers/__clang_cuda_libdevice_declares.h
@@ -16,6 +16,7 @@
 
 #if defined(__OPENMP_NVPTX__)
 #define __DEVICE__
+#pragma omp begin assumes ext_spmd_amenable
 #elif defined(__CUDA__)
 #define __DEVICE__ __device__
 #endif
@@ -456,6 +457,11 @@
 __DEVICE__ float __nv_y1f(float __a);
 __DEVICE__ float __nv_ynf(int __a, float __b);
 __DEVICE__ double __nv_yn(int __a, double __b);
+
+#if defined(__OPENMP_NVPTX__)
+#pragma omp end assumes ext_spmd_amenable
+#endif
+
 #if defined(__cplusplus)
 } // extern "C"
 #endif


Index: clang/lib/Headers/__clang_cuda_libdevice_declares.h
===
--- clang/lib/Headers/__clang_cuda_libdevice_declares.h
+++ clang/lib/Headers/__clang_cuda_libdevice_declares.h
@@ -16,6 +16,7 @@
 
 #if defined(__OPENMP_NVPTX__)
 #define __DEVICE__
+#pragma omp begin assumes ext_spmd_amenable
 #elif defined(__CUDA__)
 #define __DEVICE__ __device__
 #endif
@@ -456,6 +457,11 @@
 __DEVICE__ float __nv_y1f(float __a);
 __DEVICE__ float __nv_ynf(int __a, float __b);
 __DEVICE__ double __nv_yn(int __a, double __b);
+
+#if defined(__OPENMP_NVPTX__)
+#pragma omp end assumes ext_spmd_amenable
+#endif
+
 #if defined(__cplusplus)
 } // extern "C"
 #endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D107049: [clang-repl] Re-implement clang-interpreter as a test case.

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

rsmith added inline comments.



Comment at: 
clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp:73-76
+#ifdef _MSC_VER
+// Tell the windows linker to export the type_info symbol required by 
exceptions
+#pragma comment(linker, "/export:??_7type_info@@6B@")
+#endif // _MSC_VER

Presumably this is redundant now we don't run the test on Windows?



Comment at: 
clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp:101-105
+  // FIXME: Somehow when we build this test in release mode argc is not 0.
+  // printf("%d\n", argc);
+  // for (int I = 0; I < argc; ++I)
+  //   printf("arg[%d]='%s'\n", I, argv[I]);
+

Isn't this because you're passing no arguments to `main` when you call it later 
in the test? You're not passing any arguments on line 123/124.



Comment at: 
clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp:123-124
+  testing::internal::CaptureStdout();
+  auto Main = (int (*)(...))llvm::cantFail(Interp->getSymbolAddress("main"));
+  EXPECT_THROW(Main(), std::exception);
+  std::string CapturedStdOut = testing::internal::GetCapturedStdout();

I think this should probably be cast to `int(*)(int, const char**)` instead. 
But the name `main` is special, and might not have its declared type, so 
selecting a different function name would have less risk of weird behavior. I'd 
also suggest you remove the parameters if you're not going to use them.


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

https://reviews.llvm.org/D107049

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

[PATCH] D108881: [clang][driver] Honor the last -flto(=.*)? argument

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

I will do a cleanup of `parseLTOMode` function since we don't need a `OptPos` 
parameter anymore. There are few minor places references `OPT_flto` or 
`OPT_foffload_lto` can be cleaned up too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108881

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

[PATCH] D108905: [ItaniumCXXABI] Make __cxa_end_catch calls unconditionally nounwind

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

rsmith added a comment.

I've forwarded the question here onto CWG, with Arthur's example and the 
suggestion that maybe we shouldn't allow throwing an exception with a 
non-noexcept destructor.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108905

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

[PATCH] D105759: [WIP] Implement P2361 Unevaluated string literals

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

At @aaron.ballman request, here are some data collection trying to assert the 
amount of breakage

On a corpus of 78M lines of C++ code corresponding to all the packages in vcpkg

- Number of strings with encoding prefix in `_Pragma`: 3/3383 (all in test 
suits)
- Number of strings with encoding prefix in deprecated/nodiscard attributes: 
0/845
- Number of strings with encoding prefix in static_assert: 62/92800 (all in in 
test suits)
- Number of strings with encoding prefix in `extern` : 3/39829 (all in in test 
suits)

Note that, due to the complexity of operating these sort of queries at this 
scale, this was done by regex and as such is not exact, nor is the approach 
fully representative of all C++ code.
However, I think it gives a good idea of the risk (or lack thereof) involved


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105759

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

[PATCH] D108881: [clang][driver] Honor the last -flto(=.*)? argument

[Usman Nadeem via Phabricator via cfe-commits]

mnadeem updated this revision to Diff 369558.
mnadeem retitled this revision from "[clang][driver] Honor the last -flto= flag 
even if an earlier -flto is present" to "[clang][driver] Honor the last 
-flto(=.*)? argument".
mnadeem edited the summary of this revision.
mnadeem added a comment.
Herald added a subscriber: dang.

Update Options.td as suggested by steven_wu


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108881

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/amdgpu-toolchain.c
  clang/test/Driver/lto.c


Index: clang/test/Driver/lto.c
===
--- clang/test/Driver/lto.c
+++ clang/test/Driver/lto.c
@@ -85,3 +85,21 @@
 // FLTO-AUTO: -flto=full
 // FLTO-JOBSERVER: -flto=full
 //
+
+// Pass the last -flto argument.
+// RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin -flto 2>&1 | \
+// RUN: FileCheck --check-prefix=FLTO-FULL %s
+// RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin -flto=full 2>&1 
| \
+// RUN: FileCheck --check-prefix=FLTO-FULL %s
+// RUN: %clang -target x86_64-unknown-linux -### %s -flto -flto=thin 2>&1 | \
+// RUN: FileCheck --check-prefix=FLTO-THIN %s
+//
+// FLTO-FULL-NOT: -flto=thin
+// FLTO-FULL: -flto=full
+// FLTO-FULL-NOT: -flto=thin
+//
+// FLTO-THIN-NOT: -flto=full
+// FLTO-THIN-NOT: "-flto"
+// FLTO-THIN: -flto=thin
+// FLTO-THIN-NOT: "-flto"
+// FLTO-THIN-NOT: -flto=full
Index: clang/test/Driver/amdgpu-toolchain.c
===
--- clang/test/Driver/amdgpu-toolchain.c
+++ clang/test/Driver/amdgpu-toolchain.c
@@ -12,5 +12,5 @@
 
 // RUN: %clang -### -target amdgcn-amd-amdhsa -mcpu=gfx906 -nogpulib \
 // RUN:   -flto %s 2>&1 | FileCheck -check-prefix=LTO %s
-// LTO: clang{{.*}} "-flto"
+// LTO: clang{{.*}} "-flto=full"
 // LTO: ld.lld{{.*}}
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4491,18 +4491,8 @@
   CmdArgs.push_back("-emit-llvm-uselists");
 
 if (IsUsingLTO) {
-  if (!IsDeviceOffloadAction) {
-if (Args.hasArg(options::OPT_flto))
-  CmdArgs.push_back("-flto");
-else {
-  if (D.getLTOMode() == LTOK_Thin)
-CmdArgs.push_back("-flto=thin");
-  else
-CmdArgs.push_back("-flto=full");
-}
-CmdArgs.push_back("-flto-unit");
-  } else if (Triple.isAMDGPU()) {
-// Only AMDGPU supports device-side LTO
+  // Only AMDGPU supports device-side LTO.
+  if (!IsDeviceOffloadAction || Triple.isAMDGPU()) {
 assert(LTOMode == LTOK_Full || LTOMode == LTOK_Thin);
 CmdArgs.push_back(Args.MakeArgString(
 Twine("-flto=") + (LTOMode == LTOK_Thin ? "thin" : "full")));
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2025,13 +2025,13 @@
 def flto_EQ_jobserver : Flag<["-"], "flto=jobserver">, Group;
 def flto_EQ_auto : Flag<["-"], "flto=auto">, Group;
 def flto : Flag<["-"], "flto">, Flags<[CoreOption, CC1Option]>, Group,
-  HelpText<"Enable LTO in 'full' mode">;
+  Alias, AliasArgs<["full"]>, HelpText<"Enable LTO in 'full' mode">;
 def fno_lto : Flag<["-"], "fno-lto">, Flags<[CoreOption, CC1Option]>, 
Group,
   HelpText<"Disable LTO mode (default)">;
 def foffload_lto_EQ : Joined<["-"], "foffload-lto=">, Flags<[CoreOption]>, 
Group,
   HelpText<"Set LTO mode to either 'full' or 'thin' for offload compilation">, 
Values<"thin,full">;
 def foffload_lto : Flag<["-"], "foffload-lto">, Flags<[CoreOption]>, 
Group,
-  HelpText<"Enable LTO in 'full' mode for offload compilation">;
+  Alias, AliasArgs<["full"]>, HelpText<"Enable LTO in 'full' 
mode for offload compilation">;
 def fno_offload_lto : Flag<["-"], "fno-offload-lto">, Flags<[CoreOption]>, 
Group,
   HelpText<"Disable LTO mode (default) for offload compilation">;
 def flto_jobs_EQ : Joined<["-"], "flto-jobs=">,


Index: clang/test/Driver/lto.c
===
--- clang/test/Driver/lto.c
+++ clang/test/Driver/lto.c
@@ -85,3 +85,21 @@
 // FLTO-AUTO: -flto=full
 // FLTO-JOBSERVER: -flto=full
 //
+
+// Pass the last -flto argument.
+// RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin -flto 2>&1 | \
+// RUN: FileCheck --check-prefix=FLTO-FULL %s
+// RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin -flto=full 2>&1 | \
+// RUN: FileCheck --check-prefix=FLTO-FULL %s
+// RUN: %clang -target x86_64-unknown-linux -### %s -flto -flto=thin 2>&1 | \
+// RUN: FileCheck --check-prefix=FLTO-THIN %s
+//
+// FLTO-FULL-NOT: -flto=thin
+// FLTO-FULL: -flto=full
+// 

[PATCH] D104386: [PowerPC][Builtins] Added a number of builtins for compatibility with XL.

[Nemanja Ivanovic via Phabricator via cfe-commits]

nemanjai added a comment.

In D104386#2973245 , @dim wrote:

> Note that this unexpectedly broke FreeBSD's powerpc64 builds, as we've long 
> used the following in our `lib/libc/powerpc64/string/bcopy.S`:
>
>   #ifndef FN_NAME
>   #ifdef MEMMOVE
>   #define FN_NAME __memmove
>   WEAK_REFERENCE(__memmove, memmove);
>   #else
>   #define FN_NAME __bcopy
>   WEAK_REFERENCE(__bcopy, bcopy);
>   #endif
>   #endif
>
> so now we're getting:
>
>   lib/libc/powerpc64/string/bcopy.S:51:25: error: Recursive use of 'bcopy'
>   .weak bcopy; .equ bcopy,bcopy;
>   ^
>
> However, I think I can make do with adding `-U__bcopy` to the clang command 
> line. It would have been nice if these aliases were behind some `--xl-compat` 
> flag... :)

I am really sorry that we broke you. Would it help if we defined this macro 
only if compiling for Linux/AIX? If so, are there any others (or perhaps all of 
them) that you'd like us to conditionally define only on AIX/Linux?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104386

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

[PATCH] D101759: [PowerPC] Scalar IBM MASS library conversion pass

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

> I agree. I think the problem is that this patch is trying to decide on a 
> global lowering strategy for llvm.* math intrinsics in 
> llvm/lib/Target/PowerPC/PPCISelLowering.cpp but such global decision making 
> does not go well with finer granularity of fast-math flags.

Hmm.  Instead of using setLibcallName() and letting the legalizer generate the 
calls, it should be possible to use custom lowering to generate the appropriate 
calls, at the cost of writing a little more code.

> My understanding is that the reason we need to handle intrinsic math 
> functions later is because of strength-reduction transformations like 
> pow(x,0.5) --> sqrt(x) that currently operate on intrinsic calls only.

instcombine should be primarily responsible for this sort of optimization.  See 
LibCallSimplifier::optimizePow.  I guess a few transforms (D51630 
 etc.) landed in DAGCombine; probably we could 
move them earlier.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101759

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

[PATCH] D106191: [clang] Option control afn flag

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour added inline comments.



Comment at: clang/include/clang/Driver/Options.td:1732-1733
   NegFlag>;
-def fapprox_func : Flag<["-"], "fapprox-func">, Group, 
Flags<[CC1Option, NoDriverOption]>,
-  MarshallingInfoFlag>, 
ImpliedByAnyOf<[menable_unsafe_fp_math.KeyPath]>;
 defm finite_math_only : BoolFOption<"finite-math-only",

So this option already exists and seems to behave the way we want it to. Does 
anyone know why it was made `NoDriverOption`?

```
> cat fp.c
#include 
void foo(float *f1, float *f2)
{
  *f1 = sin(*f2) + *f1;
}
> clang -c fp.c -S -emit-llvm -mllvm -disable-llvm-optzns -O3 -Xclang 
> -fapprox-func && grep afn fp.ll
  %call = call afn double @sin(double %conv) #2
  %add = fadd afn double %call, %conv1
```

Could we just expose it as a supported option and call it done. ie make it more 
like `fhonor_nans` below but without introducing a new function attribute:

```def fapprox_func : Flag<["-"], "fapprox-func">, Group;```

so that instead of having `-Xclang -fapprox-func ` in the command above we 
could just have `-fapprox-func `?



Comment at: clang/test/CodeGen/afn-flag-test.c:10
+  // CHECK-AFN:  %{{.*}} = call afn double @{{.*}}exp{{.*}}(double %{{.*}})
+  // CHECK-AFN:  attributes #0 ={{.*}} "approx-func-fp-math"="true" {{.*}}
+

can we avoid these attributes?



Comment at: clang/test/CodeGen/afn-flag-test.c:13
+  // CHECK-NO-AFN:   %{{.*}} = call double @{{.*}}exp{{.*}}(double %{{.*}})
+  // CHECK-NO-AFN-NOT:  attributes #0 ={{.*}} "approx-func-fp-math"="true" 
{{.*}}
+}

avoid attributes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106191

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

[PATCH] D101759: [PowerPC] Scalar IBM MASS library conversion pass

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour added a comment.

In D101759#2971567 , @efriedma wrote:

> errno handling for math library functions is a mess.  Currently, we don't 
> model it properly; we just mark the calls "readnone" and hope for the best.  
> If you don't want to fix that, just check for readnone for now.

I think using `readnone` would work fine. It seems that clang marks math 
functions with that attribute when `-fno-math-errno` is in effect. To get the 
non-finite MASS lowerings at -O3 one would have to compile with both 
`-fapprox-func` and `-fno-math-errno`, which seems reasonable to me.

> I don't think we want to be querying function attributes or options here; afn 
> plus enabling MASS should be enough.  The function attributes are the old 
> mechanism; we just haven't completely migrated some parts of SelectionDAG yet.

I agree. I think the problem is that this patch is trying to decide on a global 
lowering strategy for `llvm.*` math intrinsics in 
`llvm/lib/Target/PowerPC/PPCISelLowering.cpp` but such global decision making 
does not go well with finer granularity of fast-math flags. My understanding is 
that the reason we need to handle //intrinsic// math functions later is because 
of strength-reduction transformations like `pow(x,0.5) --> sqrt(x)` that 
currently operate on intrinsic calls only. If we could apply those operations 
on things like `__xl_pow_finite` and produce calls to `__xl_sqrt_finite` then 
we wouldn't have this problem. Another possibility might be to have two 
versions of `PPCGenScalarMASSEntries` one that handles non-intrinsics and runs 
earlier, and another one that handles intrinsics after transformations likes 
`pow(x,0.5) --> sqrt(x)` are done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101759

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

[PATCH] D104386: [PowerPC][Builtins] Added a number of builtins for compatibility with XL.

[Dimitry Andric via Phabricator via cfe-commits]

dim added subscribers: emaste, dim.
dim added a comment.

Note that this unexpectedly broke FreeBSD's powerpc64 builds, as we've long 
used the following in our `lib/libc/powerpc64/string/bcopy.S`:

  #ifndef FN_NAME
  #ifdef MEMMOVE
  #define FN_NAME __memmove
  WEAK_REFERENCE(__memmove, memmove);
  #else
  #define FN_NAME __bcopy
  WEAK_REFERENCE(__bcopy, bcopy);
  #endif
  #endif

so now we're getting:

  lib/libc/powerpc64/string/bcopy.S:51:25: error: Recursive use of 'bcopy'
  .weak bcopy; .equ bcopy,bcopy;
  ^

However, I think I can make do with adding `-U__bcopy` to the clang command 
line. It would have been nice if these aliases were behind some `--xl-compat` 
flag... :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104386

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

[PATCH] D108881: [clang][driver] Honor the last -flto= flag even if an earlier -flto is present

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

I agree that it might be good to change to the behavior that last arg wins but 
it might be simpler to set `-flto` to be an alias of `-flto=full`:

  diff --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
  index 1a3d1dcceec7..a796fb7fe3ac 100644
  --- a/clang/include/clang/Driver/Options.td
  +++ b/clang/include/clang/Driver/Options.td
  @@ -2086,7 +2086,7 @@ def flto_EQ : Joined<["-"], "flto=">, 
Flags<[CoreOption, CC1Option]>, Group, Group;
   def flto_EQ_auto : Flag<["-"], "flto=auto">, Group;
   def flto : Flag<["-"], "flto">, Flags<[CoreOption, CC1Option]>, 
Group,
  -  HelpText<"Enable LTO in 'full' mode">;
  +  Alias, AliasArgs<["full"]>, HelpText<"Enable LTO in 'full' mode">;
   def fno_lto : Flag<["-"], "fno-lto">, Flags<[CoreOption, CC1Option]>, 
Group,
 HelpText<"Disable LTO mode (default)">;
   def foffload_lto_EQ : Joined<["-"], "foffload-lto=">, Flags<[CoreOption]>, 
Group,

Of course, you want to clean up the option processing of `flto`.


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

https://reviews.llvm.org/D108881

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

[PATCH] D108905: [ItaniumCXXABI] Make __cxa_end_catch calls unconditionally nounwind

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D108905#2973172 , @modimo wrote:

> In D108905#2973099 , @rjmccall 
> wrote:
>
>> Yeah, I think this is the most natural interpretation of the current 
>> standard.  But that would be a very unfortunate rule, because people who 
>> write `catch (...) {}` do reasonably expect that that code will never throw. 
>>   In fact, it would be quite difficult — perhaps impossible — to write code 
>> that actually swallowed all exceptions: even `try { try { foo() } catch(...) 
>> {} } catch (...) {}` wouldn't work, because you could throw an exception 
>> whose destructor throws an exception whose destructor throws an exception ad 
>> infinitum.
>
> Yeah it's not great and also something that practically will never happen. I 
> think terminate guards are the only thing that really swallows all exceptions 
> except here you can't guard the catch variable destructor unless you want to 
> change up and depend on library implementation. My immediate thought is 
> something like `catch(...) noexcept {}` to express this but it's a solution 
> to a problem that really shouldn't exist.

Well, I think `catch (...) { std::terminate(); }` should work to express that 
outer catch, if only because the end of the catch is never reached.  But yeah, 
I agree that this is clearly a problem that shouldn't exist.

We might be able to require exception types to have `noexcept` destructors.  
Formally, it would have source compatibility / language-version incompatibility 
implications, but in practice I doubt it would cause many problems, especially 
if it was just a "if this actually throws it'll have UB" warning in pre-C++23 
language modes for throwing types with explicitly noexcept destructors.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108905

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

[PATCH] D108302: [PowerPC] Fixed the crash due to early if conversion with fixed CR fields.

[Victor Huang via Phabricator via cfe-commits]

NeHuang updated this revision to Diff 369531.
NeHuang added a comment.

Address review comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108302

Files:
  llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
  llvm/test/CodeGen/PowerPC/ifcvt_cr_field.ll


Index: llvm/test/CodeGen/PowerPC/ifcvt_cr_field.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/ifcvt_cr_field.ll
@@ -0,0 +1,64 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=powerpc64-unknown-linux-gnu -mcpu=pwr7 
-verify-machineinstrs | FileCheck %s
+; RUN: llc < %s -mtriple=powerpc64le-unknown-linux-gnu -mcpu=pwr8 
-verify-machineinstrs | FileCheck %s
+; RUN: llc < %s -mtriple=powerpc64-unknown-aix -mcpu=pwr7 
-verify-machineinstrs | FileCheck %s --check-prefix=CHECK-AIX-64
+; RUN: llc < %s -mtriple=powerpc-unknown-aix -mcpu=pwr7 -verify-machineinstrs 
| FileCheck %s --check-prefix=CHECK-AIX-32
+
+define dso_local signext i32 @test(<4 x i32> %a, <4 x i32> %b, <4 x i32> %c) 
local_unnamed_addr {
+; CHECK-LABEL: test:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:vcmpgtsw. 2, 2, 3
+; CHECK-NEXT:bge 6, .LBB0_2
+; CHECK-NEXT:  # %bb.1: # %land.rhs
+; CHECK-NEXT:vcmpgtsw. 2, 4, 3
+; CHECK-NEXT:mfocrf 3, 2
+; CHECK-NEXT:rlwinm 3, 3, 25, 31, 31
+; CHECK-NEXT:clrldi 3, 3, 32
+; CHECK-NEXT:blr
+; CHECK-NEXT:  .LBB0_2:
+; CHECK-NEXT:li 3, 0
+; CHECK-NEXT:blr
+;
+; CHECK-AIX-64-LABEL: test:
+; CHECK-AIX-64:   # %bb.0: # %entry
+; CHECK-AIX-64-NEXT:vcmpgtsw. 2, 2, 3
+; CHECK-AIX-64-NEXT:bge 6, L..BB0_2
+; CHECK-AIX-64-NEXT:  # %bb.1: # %land.rhs
+; CHECK-AIX-64-NEXT:vcmpgtsw. 2, 4, 3
+; CHECK-AIX-64-NEXT:mfocrf 3, 2
+; CHECK-AIX-64-NEXT:rlwinm 3, 3, 25, 31, 31
+; CHECK-AIX-64-NEXT:clrldi 3, 3, 32
+; CHECK-AIX-64-NEXT:blr
+; CHECK-AIX-64-NEXT:  L..BB0_2:
+; CHECK-AIX-64-NEXT:li 3, 0
+; CHECK-AIX-64-NEXT:blr
+;
+; CHECK-AIX-32-LABEL: test:
+; CHECK-AIX-32:   # %bb.0: # %entry
+; CHECK-AIX-32-NEXT:vcmpgtsw. 2, 2, 3
+; CHECK-AIX-32-NEXT:bge 6, L..BB0_2
+; CHECK-AIX-32-NEXT:  # %bb.1: # %land.rhs
+; CHECK-AIX-32-NEXT:vcmpgtsw. 2, 4, 3
+; CHECK-AIX-32-NEXT:mfocrf 3, 2
+; CHECK-AIX-32-NEXT:rlwinm 3, 3, 25, 31, 31
+; CHECK-AIX-32-NEXT:blr
+; CHECK-AIX-32-NEXT:  L..BB0_2:
+; CHECK-AIX-32-NEXT:li 3, 0
+; CHECK-AIX-32-NEXT:blr
+entry:
+  %0 = tail call i32 @llvm.ppc.altivec.vcmpgtsw.p(i32 2, <4 x i32> %a, <4 x 
i32> %b)
+  %tobool.not = icmp eq i32 %0, 0
+  br i1 %tobool.not, label %land.end, label %land.rhs
+
+land.rhs: ; preds = %entry
+  %1 = tail call i32 @llvm.ppc.altivec.vcmpgtsw.p(i32 2, <4 x i32> %c, <4 x 
i32> %b)
+  %tobool1 = icmp ne i32 %1, 0
+  %phi.cast = zext i1 %tobool1 to i32
+  br label %land.end
+
+land.end: ; preds = %land.rhs, %entry
+  %2 = phi i32 [ 0, %entry ], [ %phi.cast, %land.rhs ]
+  ret i32 %2
+}
+
+declare i32 @llvm.ppc.altivec.vcmpgtsw.p(i32, <4 x i32>, <4 x i32>)
Index: llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
===
--- llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
+++ llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
@@ -1541,6 +1541,11 @@
   if (Cond[1].getReg() == PPC::CTR || Cond[1].getReg() == PPC::CTR8)
 return false;
 
+  // If the condition branch uses a physical register, then it cannot be turned
+  // into a select.
+  if (Register::isPhysicalRegister(Cond[1].getReg()))
+return false;
+
   // Check register classes.
   const MachineRegisterInfo  = MBB.getParent()->getRegInfo();
   const TargetRegisterClass *RC =


Index: llvm/test/CodeGen/PowerPC/ifcvt_cr_field.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/ifcvt_cr_field.ll
@@ -0,0 +1,64 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=powerpc64-unknown-linux-gnu -mcpu=pwr7 -verify-machineinstrs | FileCheck %s
+; RUN: llc < %s -mtriple=powerpc64le-unknown-linux-gnu -mcpu=pwr8 -verify-machineinstrs | FileCheck %s
+; RUN: llc < %s -mtriple=powerpc64-unknown-aix -mcpu=pwr7 -verify-machineinstrs | FileCheck %s --check-prefix=CHECK-AIX-64
+; RUN: llc < %s -mtriple=powerpc-unknown-aix -mcpu=pwr7 -verify-machineinstrs | FileCheck %s --check-prefix=CHECK-AIX-32
+
+define dso_local signext i32 @test(<4 x i32> %a, <4 x i32> %b, <4 x i32> %c) local_unnamed_addr {
+; CHECK-LABEL: test:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:vcmpgtsw. 2, 2, 3
+; CHECK-NEXT:bge 6, .LBB0_2
+; CHECK-NEXT:  # %bb.1: # %land.rhs
+; CHECK-NEXT:vcmpgtsw. 2, 4, 3
+; CHECK-NEXT:mfocrf 3, 2
+; CHECK-NEXT:rlwinm 3, 3, 25, 31, 31
+; CHECK-NEXT:clrldi 3, 3, 32
+; CHECK-NEXT:blr
+; CHECK-NEXT:  .LBB0_2:
+; 

[PATCH] D108905: [ItaniumCXXABI] Make __cxa_end_catch calls unconditionally nounwind

[Di Mo via Phabricator via cfe-commits]

modimo added a comment.

In D108905#2973099 , @rjmccall wrote:

> Yeah, I think this is the most natural interpretation of the current 
> standard.  But that would be a very unfortunate rule, because people who 
> write `catch (...) {}` do reasonably expect that that code will never throw.  
>  In fact, it would be quite difficult — perhaps impossible — to write code 
> that actually swallowed all exceptions: even `try { try { foo() } catch(...) 
> {} } catch (...) {}` wouldn't work, because you could throw an exception 
> whose destructor throws an exception whose destructor throws an exception ad 
> infinitum.

Yeah it's not great and also something that practically will never happen. I 
think terminate guards are the only thing that really swallows all exceptions 
except here you can't guard the catch variable destructor unless you want to 
change up and depend on library implementation. My immediate thought is 
something like `catch(...) noexcept {}` to express this but it's a solution to 
a problem that really shouldn't exist.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108905

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

[PATCH] D108905: [ItaniumCXXABI] Make __cxa_end_catch calls unconditionally nounwind

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D108905#2973066 , @modimo wrote:

> In D108905#2971861 , @rjmccall 
> wrote:
>
>> I'm not really sure what the standard expects to happen if an exception 
>> destructor throws.  The standard tells us when the destruction happens — the 
>> latest point it can — but that clause doesn't mention exceptions from the 
>> destructor.  If there's a specific clause on this, I can't find it.  
>> [except.throw]p7 says that "if the exception handling mechanism handling an 
>> uncaught exception directly invokes a function that exits via an exception, 
>> the function std::terminate is called", which is meant to cover exceptions 
>> thrown when initializing a catch variable.  Once the catch clause is 
>> entered, though, the exception is considered caught unless it's rethrown, so 
>> this clause doesn't apply when destroying the exception at the end of the 
>> clause.
>
> Scanning through the standard this to me also looks like an overlooked corner 
> case in the standard (TBF this is very corner case).
>
>> If the catch variable's destructor throws, that seems to be specified to 
>> unwind normally (including destroying the exception, and if the destructor 
>> throws at that point then std::terminate gets called, as normal for 
>> exceptions during unwinding).
>
> In this case, the destructor is throwing during normal scope exit so I don't 
> think terminate behavior from [except.throw]p7 is enforced since we're not 
> currently handling an uncaught exception. The handler is already active since 
> we're past the initialization of the catch. Given that, I think this is akin 
> to the example in [except.ctor]p2 and if the destructor is `noexcept(false)` 
> should trigger a proper unwind like how an exception in the destructor of a 
> simple automatic variable inside the handler scope would also do.

Yeah, I think this is the most natural interpretation of the current standard.  
But that would be a very unfortunate rule, because people who write `catch 
(...) {}` do reasonably expect that that code will never throw.   In fact, it 
would be quite difficult — perhaps impossible — to write code that actually 
swallowed all exceptions: even `try { try { foo() } catch(...) {} } catch (...) 
{}` wouldn't work, because you could throw an exception whose destructor throws 
an exception whose destructor throws an exception ad infinitum.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108905

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

[PATCH] D108905: [ItaniumCXXABI] Make __cxa_end_catch calls unconditionally nounwind

[Di Mo via Phabricator via cfe-commits]

modimo added a comment.

In D108905#2971861 , @rjmccall wrote:

> I'm not really sure what the standard expects to happen if an exception 
> destructor throws.  The standard tells us when the destruction happens — the 
> latest point it can — but that clause doesn't mention exceptions from the 
> destructor.  If there's a specific clause on this, I can't find it.  
> [except.throw]p7 says that "if the exception handling mechanism handling an 
> uncaught exception directly invokes a function that exits via an exception, 
> the function std::terminate is called", which is meant to cover exceptions 
> thrown when initializing a catch variable.  Once the catch clause is entered, 
> though, the exception is considered caught unless it's rethrown, so this 
> clause doesn't apply when destroying the exception at the end of the clause.

Scanning through the standard this to me also looks like an overlooked corner 
case in the standard (TBF this is very corner case).

> If the catch variable's destructor throws, that seems to be specified to 
> unwind normally (including destroying the exception, and if the destructor 
> throws at that point then std::terminate gets called, as normal for 
> exceptions during unwinding).

In this case, the destructor is throwing during normal scope exit so I don't 
think terminate behavior from [except.throw]p7 is enforced since we're not 
currently handling an uncaught exception. The handler is already active since 
we're past the initialization of the catch. Given that, I think this is akin to 
the example in [except.ctor]p2 and if the destructor is `noexcept(false)` 
should trigger a proper unwind like how an exception in the destructor of a 
simple automatic variable inside the handler scope would also do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108905

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

[PATCH] D106994: [modules] Fix miscompilation when using two RecordDecl definitions with the same name.

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

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

LGTM




Comment at: clang/lib/Serialization/ASTReaderDecl.cpp:3328-3331
 // If there's no definition yet, then DC's definition is added by an update
 // record, but we've not yet loaded that update record. In this case, we
 // commit to DC being the canonical definition now, and will fix this when
 // we load the update record.

I believe there's no need to have logic matching this case in C because the 
only way that a class definition can be added by an update record is due to 
template instantiation. So we can use the simpler logic below for C.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106994

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

[PATCH] D107024: [DIBuilder] Do not replace empty enum types

[Kyungwoo Lee via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG47b239eb5a17: [DIBuilder] Do not replace empty enum types 
(authored by ellis, committed by kyulee).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107024

Files:
  clang/test/CodeGen/debug-info-codeview-heapallocsite.c
  clang/test/CodeGen/debug-info-macro.c
  clang/test/CodeGenCXX/debug-info-codeview-var-templates.cpp
  clang/test/CodeGenCXX/debug-info-cxx1y.cpp
  clang/test/CodeGenCXX/debug-info-template.cpp
  clang/test/CodeGenCXX/debug-info-var-template-partial-spec.cpp
  clang/test/CodeGenCoroutines/coro-dwarf.cpp
  llvm/lib/IR/DIBuilder.cpp
  llvm/test/CodeGen/AArch64/GlobalISel/constant-mir-debugify.mir
  llvm/test/CodeGen/AArch64/GlobalISel/phi-mir-debugify.mir
  llvm/test/DebugInfo/debugify.ll

Index: llvm/test/DebugInfo/debugify.ll
===
--- llvm/test/DebugInfo/debugify.ll
+++ llvm/test/DebugInfo/debugify.ll
@@ -61,7 +61,7 @@
 ; CHECK-DAG: !llvm.debugify = !{![[NUM_INSTS:.*]], ![[NUM_VARS:.*]]}
 ; CHECK-DAG: "Debug Info Version"
 
-; CHECK-DAG: ![[CU]] = distinct !DICompileUnit(language: DW_LANG_C, file: {{.*}}, producer: "debugify", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: {{.*}})
+; CHECK-DAG: ![[CU]] = distinct !DICompileUnit(language: DW_LANG_C, file: {{.*}}, producer: "debugify", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug)
 ; CHECK-DAG: !DIFile(filename: "", directory: "/")
 ; CHECK-DAG: distinct !DISubprogram(name: "foo", linkageName: "foo", scope: null, file: {{.*}}, line: 1, type: {{.*}}, scopeLine: 1, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: {{.*}}, retainedNodes: {{.*}})
 ; CHECK-DAG: distinct !DISubprogram(name: "bar", linkageName: "bar", scope: null, file: {{.*}}, line: 2, type: {{.*}}, scopeLine: 2, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: {{.*}}, retainedNodes: {{.*}})
Index: llvm/test/CodeGen/AArch64/GlobalISel/phi-mir-debugify.mir
===
--- llvm/test/CodeGen/AArch64/GlobalISel/phi-mir-debugify.mir
+++ llvm/test/CodeGen/AArch64/GlobalISel/phi-mir-debugify.mir
@@ -38,39 +38,39 @@
   ; CHECK:   liveins: $w0
   ; CHECK:   [[COPY:%[0-9]+]]:_(s32) = COPY $w0, debug-location !11
   ; CHECK:   DBG_VALUE [[COPY]](s32), $noreg, !9, !DIExpression(), debug-location !11
-  ; CHECK:   [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 0, debug-location !DILocation(line: 2, column: 1, scope: !6)
-  ; CHECK:   DBG_VALUE [[C]](s32), $noreg, !9, !DIExpression(), debug-location !DILocation(line: 2, column: 1, scope: !6)
-  ; CHECK:   [[C1:%[0-9]+]]:_(s32) = G_CONSTANT i32 1, debug-location !DILocation(line: 3, column: 1, scope: !6)
-  ; CHECK:   DBG_VALUE [[C1]](s32), $noreg, !9, !DIExpression(), debug-location !DILocation(line: 3, column: 1, scope: !6)
-  ; CHECK:   [[C2:%[0-9]+]]:_(s32) = G_CONSTANT i32 2, debug-location !DILocation(line: 4, column: 1, scope: !6)
-  ; CHECK:   DBG_VALUE [[C2]](s32), $noreg, !9, !DIExpression(), debug-location !DILocation(line: 4, column: 1, scope: !6)
-  ; CHECK:   [[ICMP:%[0-9]+]]:_(s1) = G_ICMP intpred(ugt), [[COPY]](s32), [[C]], debug-location !DILocation(line: 5, column: 1, scope: !6)
-  ; CHECK:   DBG_VALUE [[ICMP]](s1), $noreg, !9, !DIExpression(), debug-location !DILocation(line: 5, column: 1, scope: !6)
-  ; CHECK:   G_BRCOND [[ICMP]](s1), %bb.1, debug-location !DILocation(line: 6, column: 1, scope: !6)
-  ; CHECK:   G_BR %bb.2, debug-location !DILocation(line: 7, column: 1, scope: !6)
+  ; CHECK:   [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 0, debug-location !DILocation(line: 2, column: 1, scope: !5)
+  ; CHECK:   DBG_VALUE [[C]](s32), $noreg, !9, !DIExpression(), debug-location !DILocation(line: 2, column: 1, scope: !5)
+  ; CHECK:   [[C1:%[0-9]+]]:_(s32) = G_CONSTANT i32 1, debug-location !DILocation(line: 3, column: 1, scope: !5)
+  ; CHECK:   DBG_VALUE [[C1]](s32), $noreg, !9, !DIExpression(), debug-location !DILocation(line: 3, column: 1, scope: !5)
+  ; CHECK:   [[C2:%[0-9]+]]:_(s32) = G_CONSTANT i32 2, debug-location !DILocation(line: 4, column: 1, scope: !5)
+  ; CHECK:   DBG_VALUE [[C2]](s32), $noreg, !9, !DIExpression(), debug-location !DILocation(line: 4, column: 1, scope: !5)
+  ; CHECK:   [[ICMP:%[0-9]+]]:_(s1) = G_ICMP intpred(ugt), [[COPY]](s32), [[C]], debug-location !DILocation(line: 5, column: 1, scope: !5)
+  ; CHECK:   DBG_VALUE [[ICMP]](s1), $noreg, !9, !DIExpression(), debug-location !DILocation(line: 5, column: 1, scope: !5)
+  ; CHECK:   G_BRCOND [[ICMP]](s1), %bb.1, debug-location !DILocation(line: 6, column: 1, scope: !5)
+  ; CHECK:   G_BR %bb.2, debug-location !DILocation(line: 7, column: 1, scope: !5)
   ; CHECK: bb.1:
   ; CHECK:   successors: %bb.3(0x8000)
-  ; CHECK:   [[ADD:%[0-9]+]]:_(s32) = G_ADD [[COPY]], [[C1]], debug-location !DILocation(line: 8, column: 1, 

[clang] 47b239e - [DIBuilder] Do not replace empty enum types

[Kyungwoo Lee via cfe-commits]

Author: Ellis Hoag
Date: 2021-08-30T12:33:03-07:00
New Revision: 47b239eb5a17065d13c317600c46e56ffe2d6c75

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

LOG: [DIBuilder] Do not replace empty enum types

It looks like this array was missed in 4276d4a8d08b7640eb57cabf6988a5cf65b228b6

Fixed tests that expected `elements` to be empty or depeneded on the order of 
the empty DINode.

Reviewed By: aprantl

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

Added: 


Modified: 
clang/test/CodeGen/debug-info-codeview-heapallocsite.c
clang/test/CodeGen/debug-info-macro.c
clang/test/CodeGenCXX/debug-info-codeview-var-templates.cpp
clang/test/CodeGenCXX/debug-info-cxx1y.cpp
clang/test/CodeGenCXX/debug-info-template.cpp
clang/test/CodeGenCXX/debug-info-var-template-partial-spec.cpp
clang/test/CodeGenCoroutines/coro-dwarf.cpp
llvm/lib/IR/DIBuilder.cpp
llvm/test/CodeGen/AArch64/GlobalISel/constant-mir-debugify.mir
llvm/test/CodeGen/AArch64/GlobalISel/phi-mir-debugify.mir
llvm/test/DebugInfo/debugify.ll

Removed: 




diff  --git a/clang/test/CodeGen/debug-info-codeview-heapallocsite.c 
b/clang/test/CodeGen/debug-info-codeview-heapallocsite.c
index 25c102b1c37dd..3ddce910aba13 100644
--- a/clang/test/CodeGen/debug-info-codeview-heapallocsite.c
+++ b/clang/test/CodeGen/debug-info-codeview-heapallocsite.c
@@ -19,8 +19,8 @@ void call_alloc() {
 // CHECK: call i8* {{.*}}@alloc_void{{.*}} !heapallocsite [[DBG2]]
 // CHECK: call i8* {{.*}}@alloc_void{{.*}} !heapallocsite [[DBG3:!.*]]
 
-// CHECK: [[DBG1]] = !{}
 // CHECK: [[DBG2]] = !DICompositeType(tag: DW_TAG_structure_type,
 // CHECK-SAME: name: "Foo"
 // CHECK: [[DBG3]] = !DICompositeType(tag: DW_TAG_structure_type,
 // CHECK-SAME: name: "Bar"
+// CHECK: [[DBG1]] = !{}

diff  --git a/clang/test/CodeGen/debug-info-macro.c 
b/clang/test/CodeGen/debug-info-macro.c
index 6294d43753f0e..9d0464102c108 100644
--- a/clang/test/CodeGen/debug-info-macro.c
+++ b/clang/test/CodeGen/debug-info-macro.c
@@ -25,7 +25,6 @@
 // NO_MACRO-NOT: DIMacroFile
 
 // CHECK:  !DICompileUnit({{.*}} macros: [[Macros:![0-9]+]]
-// CHECK:  [[EmptyMD:![0-9]+]] = !{}
 
 // NO_PCH: [[Macros]] = !{[[MainMacroFile:![0-9]+]], [[BuiltinMacro:![0-9]+]], 
{{.*}}, [[DefineC1:![0-9]+]], [[DefineA:![0-9]+]], [[UndefC1:![0-9]+]]}
 // PCH:[[Macros]] = !{[[MainMacroFile:![0-9]+]], [[DefineC1:![0-9]+]], 
[[DefineA:![0-9]+]], [[UndefC1:![0-9]+]]}

diff  --git a/clang/test/CodeGenCXX/debug-info-codeview-var-templates.cpp 
b/clang/test/CodeGenCXX/debug-info-codeview-var-templates.cpp
index 0470c133688cb..dec4c01444afd 100644
--- a/clang/test/CodeGenCXX/debug-info-codeview-var-templates.cpp
+++ b/clang/test/CodeGenCXX/debug-info-codeview-var-templates.cpp
@@ -10,10 +10,7 @@ struct TestImplicit {
 int instantiate_test1() { return TestImplicit::size_var + 
TestImplicit::size_var; }
 TestImplicit gv1;
 
-// CHECK: ![[empty:[0-9]+]] = !{}
-
 // CHECK: ![[A:[^ ]*]] = distinct !DICompositeType(tag: DW_TAG_structure_type, 
name: "TestImplicit",
-// CHECK-SAME: elements: ![[empty]]
 
 template  bool vtpl;
 struct TestSpecialization {
@@ -22,7 +19,6 @@ struct TestSpecialization {
 } gv2;
 
 // CHECK: ![[A:[^ ]*]] = distinct !DICompositeType(tag: DW_TAG_structure_type, 
name: "TestSpecialization",
-// CHECK-SAME: elements: ![[empty]]
 
 template  bool a;
 template  struct b;
@@ -32,4 +28,3 @@ struct TestPartial {
 } c;
 
 // CHECK: ![[A:[^ ]*]] = distinct !DICompositeType(tag: DW_TAG_structure_type, 
name: "TestPartial",
-// CHECK-SAME: elements: ![[empty]]

diff  --git a/clang/test/CodeGenCXX/debug-info-cxx1y.cpp 
b/clang/test/CodeGenCXX/debug-info-cxx1y.cpp
index f1298b1d858c2..6ec55626033d6 100644
--- a/clang/test/CodeGenCXX/debug-info-cxx1y.cpp
+++ b/clang/test/CodeGenCXX/debug-info-cxx1y.cpp
@@ -1,7 +1,6 @@
 // RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm-only -std=c++14 
-emit-llvm -debug-info-kind=limited %s -o - | FileCheck %s
 
 // CHECK: imports: [[IMPS:![0-9]*]]
-// CHECK: [[EMPTY:![0-9]*]] = !{}
 
 // CHECK: [[IMPS]] = !{[[IMP:![0-9]*]]}
 // CHECK: [[IMP]] = !DIImportedEntity(
@@ -12,6 +11,7 @@
 // CHECK: [[TYPE_LIST]] = !{[[INT:![0-9]*]]}
 // CHECK: [[INT]] = !DIBasicType(name: "int"
 
+// CHECK: [[EMPTY:![0-9]*]] = !{}
 // CHECK: [[FOO:![0-9]+]] = distinct !DICompositeType(tag: 
DW_TAG_structure_type, name: "foo",
 // CHECK-SAME: elements: [[EMPTY]]
 

diff  --git a/clang/test/CodeGenCXX/debug-info-template.cpp 
b/clang/test/CodeGenCXX/debug-info-template.cpp
index 2ce0166590aa1..7e8ccbcc5e5d7 100644
--- a/clang/test/CodeGenCXX/debug-info-template.cpp
+++ b/clang/test/CodeGenCXX/debug-info-template.cpp
@@ -5,7 +5,6 @@
 // CHECK: @nn = dso_local global 

[PATCH] D106876: Remove non-affecting module maps from PCM files.

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

rsmith added a comment.

This should have a testcase. I would imagine you can test this by constructing 
a situation where you build a module twice with different sets of module map 
files being considered and checking that you get bit-for-bit identical outputs. 
(For example: set up a temporary area for the test case files, build a module 
there, then copy in an irrelevant module map file into a directory named by a 
`-I` and build the module again.)




Comment at: clang/lib/Serialization/ASTWriter.cpp:164
+
+std::set GetAllModulemaps(const HeaderSearch ,
+   Module *RootModule) {

`Modulemap` -> `ModuleMap` for consistency please.



Comment at: clang/lib/Serialization/ASTWriter.cpp:168
+  std::set ProcessedModules;
+  std::set ModulesToProcess{RootModule};
+

You're walking this list in a non-deterministic order; consider using a 
`SmallVector` here and popping elements from the end instead of the front in 
the loop below (ie, depth-first traversal instead of breadth-first).



Comment at: clang/lib/Serialization/ASTWriter.cpp:183-184
+HS.getExistingFileInfo(File, /*WantExternal*/false);
+if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader))
+  continue;
+

I don't think this is right: I think we should consider every file that we've 
entered in this compilation, regardless of whether it's part of a module we're 
compiling. (We support modes where we'll enter modular headers despite not 
compiling them.) Can you replace this with just `if (!HFI) continue;` and still 
get the optimization you're looking for?



Comment at: clang/lib/Serialization/ASTWriter.cpp:186
+
+const auto KH = HS.findModuleForHeader(File, /*AllowTextual*/true);
+if (!KH.getModule())

This should be `findAllModulesForHeader`: in the case where there is more than 
one module covering a header, the additional module maps can matter in some 
contexts.



Comment at: clang/lib/Serialization/ASTWriter.cpp:1537-1538
+!AffectingModuleMaps.empty() &&
+AffectingModuleMaps.find(std::string(Cache->OrigEntry->getName())) ==
+AffectingModuleMaps.end()) {
+  // Do not emit modulemaps that do not affect current module.

This will not work correctly if a module map file is reachable via multiple 
different paths. Consider using `FileEntry*` comparisons instead here.


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

https://reviews.llvm.org/D106876

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

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[Ludovic Jozeau via Phabricator via cfe-commits]

FederAndInk updated this revision to Diff 369519.
FederAndInk marked an inline comment as done.
FederAndInk added a comment.

add common plural rules, use python3 explicitly, reorder imports


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108765

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/tools/dump_format_style.py
  clang/docs/tools/plurals.txt
  clang/include/clang/Format/Format.h

Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -3083,7 +3083,7 @@
 /// ForEach and If macros. This is useful in projects where ForEach/If
 /// macros are treated as function calls instead of control statements.
 /// ``SBPO_ControlStatementsExceptForEachMacros`` remains an alias for
-/// backward compatability.
+/// backward compatibility.
 /// \code
 ///void f() {
 ///  Q_FOREACH(...) {
Index: clang/docs/tools/plurals.txt
===
--- /dev/null
+++ clang/docs/tools/plurals.txt
@@ -0,0 +1,3 @@
+Strings
+IncludeCategories
+RawStringFormats
Index: clang/docs/tools/dump_format_style.py
===
--- clang/docs/tools/dump_format_style.py
+++ clang/docs/tools/dump_format_style.py
@@ -1,23 +1,66 @@
-#!/usr/bin/env python
+#!/usr/bin/env python3
 # A tool to parse the FormatStyle struct from Format.h and update the
 # documentation in ../ClangFormatStyleOptions.rst automatically.
 # Run from the directory in which this file is located to update the docs.
 
 import collections
+import inspect
 import os
 import re
+import subprocess
 
 CLANG_DIR = os.path.join(os.path.dirname(__file__), '../..')
 FORMAT_STYLE_FILE = os.path.join(CLANG_DIR, 'include/clang/Format/Format.h')
 INCLUDE_STYLE_FILE = os.path.join(CLANG_DIR, 'include/clang/Tooling/Inclusions/IncludeStyle.h')
 DOC_FILE = os.path.join(CLANG_DIR, 'docs/ClangFormatStyleOptions.rst')
 
+PLURAL_FILE = os.path.join(os.path.dirname(__file__), 'plurals.txt')
+subprocess.check_call(['git', 'checkout', '--', PLURAL_FILE])
+plurals = set(open(PLURAL_FILE).read().splitlines())
 
 def substitute(text, tag, contents):
   replacement = '\n.. START_%s\n\n%s\n\n.. END_%s\n' % (tag, contents, tag)
   pattern = r'\n\.\. START_%s\n.*\n\.\. END_%s\n' % (tag, tag)
   return re.sub(pattern, '%s', text, flags=re.S) % replacement
 
+def register_plural(singular: str, plural: str):
+  if plural not in plurals:
+plurals.add(plural)
+open(PLURAL_FILE, 'a').write(plural + '\n')
+cf = inspect.currentframe()
+print(f'{__file__}:{cf.f_back.f_lineno} check if plural of {singular} is {plural}')
+  return plural
+
+def pluralize(word: str):
+  lword = word.lower()
+  if len(lword) >= 2 and lword[-1] == 'y' and lword[-2] not in 'aeiou':
+return register_plural(word, word[:-1] + 'ies')
+  elif lword.endswith(('s', 'sh', 'ch', 'x', 'z')):
+return register_plural(word, word[:-1] + 'es')
+  elif lword.endswith('fe'):
+return register_plural(word, word[:-2] + 'ves')
+  elif lword.endswith('f') and not lword.endswith('ff'):
+return register_plural(word, word[:-1] + 'ves')
+  else:
+return register_plural(word, word + 's')
+
+
+def to_yaml_type(typestr: str):
+  if typestr == 'bool':
+return 'Boolean'
+  elif typestr == 'int':
+return 'Integer'
+  elif typestr == 'unsigned':
+return 'Unsigned'
+  elif typestr == 'std::string':
+return 'String'
+  
+  [subtype, napplied] = re.subn(r'^std::vector<(.*)>$', r'\1', typestr)
+  if napplied == 1:
+return 'List of ' + pluralize(to_yaml_type(subtype))
+
+  return typestr
+
 def doxygen2rst(text):
   text = re.sub(r'\s*(.*?)\s*<\/tt>', r'``\1``', text)
   text = re.sub(r'\\c ([^ ,;\.]+)', r'``\1``', text)
@@ -40,7 +83,7 @@
 self.nested_struct = None
 
   def __str__(self):
-s = '**%s** (``%s``)\n%s' % (self.name, self.type,
+s = '**%s** (``%s``)\n%s' % (self.name, to_yaml_type(self.type),
  doxygen2rst(indent(self.comment, 2)))
 if self.enum and self.enum.values:
   s += indent('\n\nPossible values:\n\n%s\n' % self.enum, 2)
@@ -85,7 +128,7 @@
 self.type = enumtype
 
   def __str__(self):
-s = '\n* ``%s %s``\n%s' % (self.type, self.name,
+s = '\n* ``%s %s``\n%s' % (to_yaml_type(self.type), self.name,
  doxygen2rst(indent(self.comment, 2)))
 s += indent('\nPossible values:\n\n', 2)
 s += indent('\n'.join(map(str, self.values)),2)
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -125,7 +125,7 @@
 the configuration (without a prefix: ``Auto``).
 
 
-**BasedOnStyle** 

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[Ludovic Jozeau via Phabricator via cfe-commits]

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



Comment at: clang/docs/tools/dump_format_style.py:9
 import re
+import inspect
+import subprocess

HazardyKnusperkeks wrote:
> I think these should be sorted.
ok, it will be done



Comment at: clang/docs/tools/dump_format_style.py:18
+PLURAL_FILE = os.path.join(os.path.dirname(__file__), 'plurals.txt')
+subprocess.check_call(['git', 'checkout', '--', PLURAL_FILE])
+plurals = set(open(PLURAL_FILE).read().splitlines())

HazardyKnusperkeks wrote:
> So you would add a plurals.txt in git and make the change visible through git 
> diff? What about just reordering? I.e. `Strings` is on line 2, but after a 
> change in line 1. Maybe sort the output?
> 
> I'm not against this procedure, but also not in favor. :)
This line is used to restore the version of plurals.txt to HEAD, so when 
calling the script multiple times, it keeps showing new plurals until 
plurals.txt gets committed.

> So you would add a plurals.txt in git and make the change visible through git 
> diff?

yes, that's it

> What about just reordering?

I don't think we want ordering, it is ordered from first plural generated to 
last/new one, so git diff will only show new plurals


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108765

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

[PATCH] D106994: [modules] Fix miscompilation when using two RecordDecl definitions with the same name.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106994

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

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[Ludovic Jozeau via Phabricator via cfe-commits]

FederAndInk marked an inline comment as done.
FederAndInk added inline comments.



Comment at: clang/docs/tools/dump_format_style.py:26
 
+def register_plural(singular: str, plural: str):
+  if plural not in plurals:

HazardyKnusperkeks wrote:
> FederAndInk wrote:
> > MyDeveloperDay wrote:
> > > This failed for me with invalid syntax
> > Oh, ok, sorry, I might be using to recent python features, I'll remove type 
> > specifier, what is the recommended python version to use?
> > Oh, ok, sorry, I might be using to recent python features, I'll remove type 
> > specifier, what is the recommended python version to use?
> 
> That **I** can not answer. I run
> ```$ python --version
> Python 3.9.5
> ```
Ok thanks, this is a reasonably recent version, I think we might want to 
explicitly specify python3 in the script to avoid using python2, I'll upload 
the diffs immediately.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108765

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

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

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

HazardyKnusperkeks added a comment.

In D108765#2972159 , @FederAndInk 
wrote:

> And again, I don't really understand if we are allowed or not to pull in a 
> dependency such as pluralizer or inflect, this would be another idea

My Python knowledge is very limited, but if it runs out of the box, or with 
very limited required user action with a reasonably new Python (if Python 2 can 
be reasonably new is another question), I think it would be fine.




Comment at: clang/docs/tools/dump_format_style.py:9
 import re
+import inspect
+import subprocess

I think these should be sorted.



Comment at: clang/docs/tools/dump_format_style.py:18
+PLURAL_FILE = os.path.join(os.path.dirname(__file__), 'plurals.txt')
+subprocess.check_call(['git', 'checkout', '--', PLURAL_FILE])
+plurals = set(open(PLURAL_FILE).read().splitlines())

So you would add a plurals.txt in git and make the change visible through git 
diff? What about just reordering? I.e. `Strings` is on line 2, but after a 
change in line 1. Maybe sort the output?

I'm not against this procedure, but also not in favor. :)



Comment at: clang/docs/tools/dump_format_style.py:26
 
+def register_plural(singular: str, plural: str):
+  if plural not in plurals:

FederAndInk wrote:
> MyDeveloperDay wrote:
> > This failed for me with invalid syntax
> Oh, ok, sorry, I might be using to recent python features, I'll remove type 
> specifier, what is the recommended python version to use?
> Oh, ok, sorry, I might be using to recent python features, I'll remove type 
> specifier, what is the recommended python version to use?

That **I** can not answer. I run
```$ python --version
Python 3.9.5
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108765

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

[PATCH] D106876: Remove non-affecting module maps from PCM files.

[Ilya Kuteev via Phabricator via cfe-commits]

ilyakuteev added a comment.

Hello everyone!

We have very slow remote compilations without this patch. :(. I'll be very 
grateful for your review.


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

https://reviews.llvm.org/D106876

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

[PATCH] D108881: [clang][driver] Honor the last -flto= flag even if an earlier -flto is present

[Usman Nadeem via Phabricator via cfe-commits]

mnadeem added a comment.

Combined with the code in `else if (Triple.isAMDGPU())` since the code is 
identical.
Removed the code to add `-flto` in favour of `-flto=full` since both are 
equivalent: https://clang.llvm.org/docs/CommandGuide/clang.html#cmdoption-flto


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

https://reviews.llvm.org/D108881

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

[PATCH] D108881: [clang][driver] Honor the last -flto= flag even if an earlier -flto is present

[Usman Nadeem via Phabricator via cfe-commits]

mnadeem updated this revision to Diff 369498.
mnadeem marked 3 inline comments as done.
mnadeem edited the summary of this revision.
mnadeem added a subscriber: apazos.
mnadeem added a comment.
Herald added subscribers: kerbowa, nhaehnle, jvesely.

Address comments.


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

https://reviews.llvm.org/D108881

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/amdgpu-toolchain.c
  clang/test/Driver/lto.c


Index: clang/test/Driver/lto.c
===
--- clang/test/Driver/lto.c
+++ clang/test/Driver/lto.c
@@ -85,3 +85,21 @@
 // FLTO-AUTO: -flto=full
 // FLTO-JOBSERVER: -flto=full
 //
+
+// Pass the last -flto argument.
+// RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin -flto 2>&1 | \
+// RUN: FileCheck --check-prefix=FLTO-FULL %s
+// RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin -flto=full 2>&1 
| \
+// RUN: FileCheck --check-prefix=FLTO-FULL %s
+// RUN: %clang -target x86_64-unknown-linux -### %s -flto -flto=thin 2>&1 | \
+// RUN: FileCheck --check-prefix=FLTO-THIN %s
+//
+// FLTO-FULL-NOT: -flto=thin
+// FLTO-FULL: -flto=full
+// FLTO-FULL-NOT: -flto=thin
+//
+// FLTO-THIN-NOT: -flto=full
+// FLTO-THIN-NOT: "-flto"
+// FLTO-THIN: -flto=thin
+// FLTO-THIN-NOT: "-flto"
+// FLTO-THIN-NOT: -flto=full
Index: clang/test/Driver/amdgpu-toolchain.c
===
--- clang/test/Driver/amdgpu-toolchain.c
+++ clang/test/Driver/amdgpu-toolchain.c
@@ -12,5 +12,5 @@
 
 // RUN: %clang -### -target amdgcn-amd-amdhsa -mcpu=gfx906 -nogpulib \
 // RUN:   -flto %s 2>&1 | FileCheck -check-prefix=LTO %s
-// LTO: clang{{.*}} "-flto"
+// LTO: clang{{.*}} "-flto=full"
 // LTO: ld.lld{{.*}}
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4491,18 +4491,8 @@
   CmdArgs.push_back("-emit-llvm-uselists");
 
 if (IsUsingLTO) {
-  if (!IsDeviceOffloadAction) {
-if (Args.hasArg(options::OPT_flto))
-  CmdArgs.push_back("-flto");
-else {
-  if (D.getLTOMode() == LTOK_Thin)
-CmdArgs.push_back("-flto=thin");
-  else
-CmdArgs.push_back("-flto=full");
-}
-CmdArgs.push_back("-flto-unit");
-  } else if (Triple.isAMDGPU()) {
-// Only AMDGPU supports device-side LTO
+  // Only AMDGPU supports device-side LTO.
+  if (!IsDeviceOffloadAction || Triple.isAMDGPU()) {
 assert(LTOMode == LTOK_Full || LTOMode == LTOK_Thin);
 CmdArgs.push_back(Args.MakeArgString(
 Twine("-flto=") + (LTOMode == LTOK_Thin ? "thin" : "full")));
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -600,8 +600,9 @@
 
   StringRef LTOName("full");
 
-  const Arg *A = Args.getLastArg(OptEq);
-  if (A)
+  const Arg *A = Args.getLastArg(OptEq, OptPos);
+  // Use the OptEq only if it is after the -flto option.
+  if (A && A->getOption().matches(OptEq))
 LTOName = A->getValue();
 
   LTOMode = llvm::StringSwitch(LTOName)


Index: clang/test/Driver/lto.c
===
--- clang/test/Driver/lto.c
+++ clang/test/Driver/lto.c
@@ -85,3 +85,21 @@
 // FLTO-AUTO: -flto=full
 // FLTO-JOBSERVER: -flto=full
 //
+
+// Pass the last -flto argument.
+// RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin -flto 2>&1 | \
+// RUN: FileCheck --check-prefix=FLTO-FULL %s
+// RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin -flto=full 2>&1 | \
+// RUN: FileCheck --check-prefix=FLTO-FULL %s
+// RUN: %clang -target x86_64-unknown-linux -### %s -flto -flto=thin 2>&1 | \
+// RUN: FileCheck --check-prefix=FLTO-THIN %s
+//
+// FLTO-FULL-NOT: -flto=thin
+// FLTO-FULL: -flto=full
+// FLTO-FULL-NOT: -flto=thin
+//
+// FLTO-THIN-NOT: -flto=full
+// FLTO-THIN-NOT: "-flto"
+// FLTO-THIN: -flto=thin
+// FLTO-THIN-NOT: "-flto"
+// FLTO-THIN-NOT: -flto=full
Index: clang/test/Driver/amdgpu-toolchain.c
===
--- clang/test/Driver/amdgpu-toolchain.c
+++ clang/test/Driver/amdgpu-toolchain.c
@@ -12,5 +12,5 @@
 
 // RUN: %clang -### -target amdgcn-amd-amdhsa -mcpu=gfx906 -nogpulib \
 // RUN:   -flto %s 2>&1 | FileCheck -check-prefix=LTO %s
-// LTO: clang{{.*}} "-flto"
+// LTO: clang{{.*}} "-flto=full"
 // LTO: ld.lld{{.*}}
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4491,18 +4491,8 @@
   CmdArgs.push_back("-emit-llvm-uselists");
 
 if (IsUsingLTO) {
-  if (!IsDeviceOffloadAction) {
-

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

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/Sema/TreeTransform.h:10515-10525
+  // Note that SourceExpr can be nullptr.
+  ExprResult SourceExpr = TransformExpr(E->getSourceExpr());
+  if (SourceExpr.isInvalid())
+return ExprError();
+  if (SourceExpr.get() == E->getSourceExpr() && !getDerived().AlwaysRebuild())
+return E;
+

aaron.ballman wrote:
> These changes look correct to me, but I am not certain why they've been 
> unnecessary for so long. This code is 11 years old and has never transformed 
> the `OpaqueValueExpr` before (see 
> https://github.com/llvm/llvm-project/commit/8d69a2160e48b73a4515c9401e67971fb8c14e07).
>  @rjmccall, was this an oversight and we just got lucky for a long time, or 
> is there a reason we didn't transform these expressions?
OVE always has to appear within a containing construct that has to handle it 
specially to preserve OVE equality.  It is likely that the structured bindings 
transform doesn't do that properly.  That should probably be fixed rather than 
trying to handle it here; creating a new OVE every time we encounter one is 
very problematic.  Perhaps this case should assert that it's unreachable.

For example, TreeTransform on PseudoObjectExpr recreates the syntactic form and 
then transforms that.


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] D108905: [ItaniumCXXABI] Make __cxa_end_catch calls unconditionally nounwind

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added a comment.

> Well, I'm saying two things. First, it is not clear to me what the expected 
> behavior of that code is under the standard. The fact that it appears to work 
> in one particular implementation is not in any way conclusive; we have to 
> look at the specification.

Aha, I had thought it worked on "both Clang and GCC," but now I see that 
Godbolt uses libsupc++ for both Clang and GCC (and that's where the issue is). 
Switching to a branch that uses libc++abi, I see that libc++abi just calls 
`std::terminate` in this situation: https://godbolt.org/z/4s8aMvr3K

> Second, I think it only appears to work: looking at the runtime code in both 
> libc++abi and libsupc++, it leaks the memory of the exception object, which 
> it's clearly not allowed to do. You should be able to fairly easy prove that 
> by looking at heap usage if you do it in a loop.

Yep, memory leak confirmed. (Via the same Godbolt: 
https://godbolt.org/z/4s8aMvr3K )  So okay, never mind me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108905

___
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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D108893#2972651 , @Eugene.Zelenko 
wrote:

> In D108893#2972634 , @compnerd 
> wrote:
>
>> @Eugene.Zelenko I'd like to have @aaron.ballman weigh in on the naming, and 
>> assuming that he's okay with `modernize.container-data-pointer`, I'll change 
>> it to that.
>
> Sure, Aaron has much broader vision than me.

Lies! I just wear glasses, that's all. ;-)

I can see a case being made for either `modernize` or `readability`. For 
`modernize`, `data()` was newly added in C++11, so you could argue that this is 
used to modernize C++98 code. For `readability`, `data()` makes the semantics 
of the underlying code more clear. Also, `bugprone` makes sense too because 
doing `[0]` is UB if `v` is empty, but `v.data()` is not (immediately) UB.

I think `bugprone` would be a tough place to put it because of false positive 
concerns. This is more of a blanket code rewriting utility than it is trying to 
catch bugs (the static analyzer and UBSan are better tools for that in that 
case).
I think `modernize` would be reasonable, but given that C++11 is already over 
ten years old, I feel like this is losing its "modernization" aspects in some 
ways. Also, this only covers converting `[0]` into a call to `data()`; for 
`modernize`, I'd expect it to convert `[N]` into `vec.data() + N` more 
broadly.
I think `readability` makes slightly more sense because then we can position 
this as a rewriting tool to make the code more readable (so there are no false 
positives -- it transforms all `[0]` calls to `data()`).

However, if someone wants to make a strong case for `modernize`, I think it'd 
be defensible. Also, if someone wanted to suggest we add it to both 
`readability` and `modernize`, that could also make sense (perhaps with an 
option to convert `[N]` that's on by default for modernize and off by 
default for readability).


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] D108905: [ItaniumCXXABI] Make __cxa_end_catch calls unconditionally nounwind

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Well, I'm saying two things.  First, it is not clear to me what the expected 
behavior of that code is under the standard.  The fact that it appears to work 
in one particular implementation is not in any way conclusive; we have to look 
at the specification.  Second, I think it only appears to work: looking at the 
runtime code in both libc++abi and libsupc++, it leaks the memory of the 
exception object, which it's clearly not allowed to do.   You should be able to 
fairly easy prove that by looking at heap usage if you do it in a loop.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108905

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

[PATCH] D107024: [DIBuilder] Do not replace empty enum types

[Ellis Hoag via Phabricator via cfe-commits]

ellis updated this revision to Diff 369496.
ellis added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107024

Files:
  clang/test/CodeGen/debug-info-codeview-heapallocsite.c
  clang/test/CodeGen/debug-info-macro.c
  clang/test/CodeGenCXX/debug-info-codeview-var-templates.cpp
  clang/test/CodeGenCXX/debug-info-cxx1y.cpp
  clang/test/CodeGenCXX/debug-info-template.cpp
  clang/test/CodeGenCXX/debug-info-var-template-partial-spec.cpp
  clang/test/CodeGenCoroutines/coro-dwarf.cpp
  llvm/lib/IR/DIBuilder.cpp
  llvm/test/CodeGen/AArch64/GlobalISel/constant-mir-debugify.mir
  llvm/test/CodeGen/AArch64/GlobalISel/phi-mir-debugify.mir
  llvm/test/DebugInfo/debugify.ll

Index: llvm/test/DebugInfo/debugify.ll
===
--- llvm/test/DebugInfo/debugify.ll
+++ llvm/test/DebugInfo/debugify.ll
@@ -61,7 +61,7 @@
 ; CHECK-DAG: !llvm.debugify = !{![[NUM_INSTS:.*]], ![[NUM_VARS:.*]]}
 ; CHECK-DAG: "Debug Info Version"
 
-; CHECK-DAG: ![[CU]] = distinct !DICompileUnit(language: DW_LANG_C, file: {{.*}}, producer: "debugify", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: {{.*}})
+; CHECK-DAG: ![[CU]] = distinct !DICompileUnit(language: DW_LANG_C, file: {{.*}}, producer: "debugify", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug)
 ; CHECK-DAG: !DIFile(filename: "", directory: "/")
 ; CHECK-DAG: distinct !DISubprogram(name: "foo", linkageName: "foo", scope: null, file: {{.*}}, line: 1, type: {{.*}}, scopeLine: 1, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: {{.*}}, retainedNodes: {{.*}})
 ; CHECK-DAG: distinct !DISubprogram(name: "bar", linkageName: "bar", scope: null, file: {{.*}}, line: 2, type: {{.*}}, scopeLine: 2, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: {{.*}}, retainedNodes: {{.*}})
Index: llvm/test/CodeGen/AArch64/GlobalISel/phi-mir-debugify.mir
===
--- llvm/test/CodeGen/AArch64/GlobalISel/phi-mir-debugify.mir
+++ llvm/test/CodeGen/AArch64/GlobalISel/phi-mir-debugify.mir
@@ -38,39 +38,39 @@
   ; CHECK:   liveins: $w0
   ; CHECK:   [[COPY:%[0-9]+]]:_(s32) = COPY $w0, debug-location !11
   ; CHECK:   DBG_VALUE [[COPY]](s32), $noreg, !9, !DIExpression(), debug-location !11
-  ; CHECK:   [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 0, debug-location !DILocation(line: 2, column: 1, scope: !6)
-  ; CHECK:   DBG_VALUE [[C]](s32), $noreg, !9, !DIExpression(), debug-location !DILocation(line: 2, column: 1, scope: !6)
-  ; CHECK:   [[C1:%[0-9]+]]:_(s32) = G_CONSTANT i32 1, debug-location !DILocation(line: 3, column: 1, scope: !6)
-  ; CHECK:   DBG_VALUE [[C1]](s32), $noreg, !9, !DIExpression(), debug-location !DILocation(line: 3, column: 1, scope: !6)
-  ; CHECK:   [[C2:%[0-9]+]]:_(s32) = G_CONSTANT i32 2, debug-location !DILocation(line: 4, column: 1, scope: !6)
-  ; CHECK:   DBG_VALUE [[C2]](s32), $noreg, !9, !DIExpression(), debug-location !DILocation(line: 4, column: 1, scope: !6)
-  ; CHECK:   [[ICMP:%[0-9]+]]:_(s1) = G_ICMP intpred(ugt), [[COPY]](s32), [[C]], debug-location !DILocation(line: 5, column: 1, scope: !6)
-  ; CHECK:   DBG_VALUE [[ICMP]](s1), $noreg, !9, !DIExpression(), debug-location !DILocation(line: 5, column: 1, scope: !6)
-  ; CHECK:   G_BRCOND [[ICMP]](s1), %bb.1, debug-location !DILocation(line: 6, column: 1, scope: !6)
-  ; CHECK:   G_BR %bb.2, debug-location !DILocation(line: 7, column: 1, scope: !6)
+  ; CHECK:   [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 0, debug-location !DILocation(line: 2, column: 1, scope: !5)
+  ; CHECK:   DBG_VALUE [[C]](s32), $noreg, !9, !DIExpression(), debug-location !DILocation(line: 2, column: 1, scope: !5)
+  ; CHECK:   [[C1:%[0-9]+]]:_(s32) = G_CONSTANT i32 1, debug-location !DILocation(line: 3, column: 1, scope: !5)
+  ; CHECK:   DBG_VALUE [[C1]](s32), $noreg, !9, !DIExpression(), debug-location !DILocation(line: 3, column: 1, scope: !5)
+  ; CHECK:   [[C2:%[0-9]+]]:_(s32) = G_CONSTANT i32 2, debug-location !DILocation(line: 4, column: 1, scope: !5)
+  ; CHECK:   DBG_VALUE [[C2]](s32), $noreg, !9, !DIExpression(), debug-location !DILocation(line: 4, column: 1, scope: !5)
+  ; CHECK:   [[ICMP:%[0-9]+]]:_(s1) = G_ICMP intpred(ugt), [[COPY]](s32), [[C]], debug-location !DILocation(line: 5, column: 1, scope: !5)
+  ; CHECK:   DBG_VALUE [[ICMP]](s1), $noreg, !9, !DIExpression(), debug-location !DILocation(line: 5, column: 1, scope: !5)
+  ; CHECK:   G_BRCOND [[ICMP]](s1), %bb.1, debug-location !DILocation(line: 6, column: 1, scope: !5)
+  ; CHECK:   G_BR %bb.2, debug-location !DILocation(line: 7, column: 1, scope: !5)
   ; CHECK: bb.1:
   ; CHECK:   successors: %bb.3(0x8000)
-  ; CHECK:   [[ADD:%[0-9]+]]:_(s32) = G_ADD [[COPY]], [[C1]], debug-location !DILocation(line: 8, column: 1, scope: !6)
-  ; CHECK:   DBG_VALUE [[ADD]](s32), $noreg, !9, !DIExpression(), debug-location !DILocation(line: 8, 

[PATCH] D43002: [CodeView] Emit S_OBJNAME record

[Daniel Paoliello via Phabricator via cfe-commits]

dpaoliello added a comment.
Herald added a subscriber: ormris.

@rnk @aganea Ping... Just wanted to see what was happening with this change, 
and if there is anything I can do to help get it merged in.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D43002

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

[PATCH] D102943: [modules] Use `HashBuilder` and `MD5` for the module hash.

[Alexandre Rames via Phabricator via cfe-commits]

arames added inline comments.



Comment at: clang/include/clang/Basic/ObjCRuntime.h:486
+  template 
+  friend void addHash(llvm::HashBuilderImpl ,
+  const ObjCRuntime ) {

I have added these in the same line as existing `hash_value` functions. The 
idea being others may also need to hash those objects.
Let me know if you would rather move some/all of these locally in 
`CompilerInvocation.cpp`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102943

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

[PATCH] D102943: [modules] Use `HashBuilder` and `MD5` for the module hash.

[Alexandre Rames via Phabricator via cfe-commits]

arames updated this revision to Diff 369489.
arames added a comment.

Fix to native endianness.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102943

Files:
  clang/include/clang/Basic/ObjCRuntime.h
  clang/include/clang/Basic/Sanitizers.h
  clang/include/clang/Lex/HeaderSearchOptions.h
  clang/include/clang/Serialization/ModuleFileExtension.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/TestModuleFileExtension.cpp
  clang/lib/Frontend/TestModuleFileExtension.h
  clang/lib/Serialization/ModuleFileExtension.cpp
  clang/unittests/Frontend/CompilerInvocationTest.cpp
  llvm/include/llvm/Support/HashBuilder.h
  llvm/include/llvm/Support/VersionTuple.h

Index: llvm/include/llvm/Support/VersionTuple.h
===
--- llvm/include/llvm/Support/VersionTuple.h
+++ llvm/include/llvm/Support/VersionTuple.h
@@ -17,6 +17,7 @@
 #include "llvm/ADT/DenseMapInfo.h"
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/Support/HashBuilder.h"
 #include 
 #include 
 
@@ -164,6 +165,12 @@
 return llvm::hash_combine(VT.Major, VT.Minor, VT.Subminor, VT.Build);
   }
 
+  template 
+  friend void addHash(HashBuilderImpl ,
+  const VersionTuple ) {
+HBuilder.add(VT.Major, VT.Minor, VT.Subminor, VT.Build);
+  }
+
   /// Retrieve a string representation of the version number.
   std::string getAsString() const;
 
Index: llvm/include/llvm/Support/HashBuilder.h
===
--- llvm/include/llvm/Support/HashBuilder.h
+++ llvm/include/llvm/Support/HashBuilder.h
@@ -164,7 +164,7 @@
   ///
   /// ```
   /// template 
-  /// void addHash(HashBuilder ,
+  /// void addHash(HashBuilderImpl ,
   ///  const UserDefinedStruct );
   /// ```
   ///
Index: clang/unittests/Frontend/CompilerInvocationTest.cpp
===
--- clang/unittests/Frontend/CompilerInvocationTest.cpp
+++ clang/unittests/Frontend/CompilerInvocationTest.cpp
@@ -860,9 +860,7 @@
 return {};
   };
 
-  llvm::hash_code hashExtension(llvm::hash_code Code) const override {
-return {};
-  }
+  void hashExtension(ExtensionHashBuilder ) const override {}
 
   std::unique_ptr
   createExtensionWriter(ASTWriter ) override {
Index: clang/lib/Serialization/ModuleFileExtension.cpp
===
--- clang/lib/Serialization/ModuleFileExtension.cpp
+++ clang/lib/Serialization/ModuleFileExtension.cpp
@@ -11,12 +11,10 @@
 
 char ModuleFileExtension::ID = 0;
 
-ModuleFileExtension::~ModuleFileExtension() { }
+ModuleFileExtension::~ModuleFileExtension() {}
 
-llvm::hash_code ModuleFileExtension::hashExtension(llvm::hash_code Code) const {
-  return Code;
-}
+void ModuleFileExtension::hashExtension(ExtensionHashBuilder ) const {}
 
-ModuleFileExtensionWriter::~ModuleFileExtensionWriter() { }
+ModuleFileExtensionWriter::~ModuleFileExtensionWriter() {}
 
-ModuleFileExtensionReader::~ModuleFileExtensionReader() { }
+ModuleFileExtensionReader::~ModuleFileExtensionReader() {}
Index: clang/lib/Frontend/TestModuleFileExtension.h
===
--- clang/lib/Frontend/TestModuleFileExtension.h
+++ clang/lib/Frontend/TestModuleFileExtension.h
@@ -55,7 +55,7 @@
 
   ModuleFileExtensionMetadata getExtensionMetadata() const override;
 
-  llvm::hash_code hashExtension(llvm::hash_code Code) const override;
+  void hashExtension(ExtensionHashBuilder ) const override;
 
   std::unique_ptr
   createExtensionWriter(ASTWriter ) override;
Index: clang/lib/Frontend/TestModuleFileExtension.cpp
===
--- clang/lib/Frontend/TestModuleFileExtension.cpp
+++ clang/lib/Frontend/TestModuleFileExtension.cpp
@@ -93,16 +93,14 @@
   return { BlockName, MajorVersion, MinorVersion, UserInfo };
 }
 
-llvm::hash_code TestModuleFileExtension::hashExtension(
-  llvm::hash_code Code) const {
+void TestModuleFileExtension::hashExtension(
+ExtensionHashBuilder ) const {
   if (Hashed) {
-Code = llvm::hash_combine(Code, BlockName);
-Code = llvm::hash_combine(Code, MajorVersion);
-Code = llvm::hash_combine(Code, MinorVersion);
-Code = llvm::hash_combine(Code, UserInfo);
+HBuilder.add(BlockName);
+HBuilder.add(MajorVersion);
+HBuilder.add(MinorVersion);
+HBuilder.add(UserInfo);
   }
-
-  return Code;
 }
 
 std::unique_ptr
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -78,6 +78,7 @@
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/FileSystem.h"
+#include 

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

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

In D108893#2972634 , @compnerd wrote:

> @Eugene.Zelenko I'd like to have @aaron.ballman weigh in on the naming, and 
> assuming that he's okay with `modernize.container-data-pointer`, I'll change 
> it to that.

Sure, Aaron has much broader vision than me.


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] D108893: clang-tidy: introduce readability-containter-data-pointer check

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added a comment.

@Eugene.Zelenko I'd like to have @aaron.ballman weigh in on the naming, and 
assuming that he's okay with `modernize.container-data-pointer`, I'll change it 
to that.


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] D108424: [NFC][clang] Move multiversion resolver code generation to llvm/ subdirectory

[Andrei Elovikov via Phabricator via cfe-commits]

a.elovikov updated this revision to Diff 369485.
a.elovikov added a comment.

Rebase + ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108424

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.cpp
  llvm/include/llvm/Transforms/Utils/X86EmitMultiVersionResolver.h
  llvm/lib/Transforms/Utils/CMakeLists.txt
  llvm/lib/Transforms/Utils/X86EmitMultiVersionResolver.cpp

Index: llvm/lib/Transforms/Utils/X86EmitMultiVersionResolver.cpp
===
--- /dev/null
+++ llvm/lib/Transforms/Utils/X86EmitMultiVersionResolver.cpp
@@ -0,0 +1,227 @@
+//===-- X86EmitMultiVersionResolver -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file implements utitlities to generate code used for CPU dispatch code.
+//
+//===--===//
+
+#include "llvm/Transforms/Utils/X86EmitMultiVersionResolver.h"
+#include "llvm/ADT/StringSwitch.h"
+#include "llvm/IR/Function.h"
+#include "llvm/IR/IRBuilder.h"
+#include "llvm/IR/Type.h"
+#include "llvm/Support/X86TargetParser.h"
+
+using namespace llvm;
+using namespace llvm::X86;
+
+Value *llvm::formResolverCondition(IRBuilderBase ,
+   const MultiVersionResolverOption ) {
+  llvm::Value *Condition = nullptr;
+
+  if (!RO.Conditions.Architecture.empty())
+Condition = llvm::X86::emitCpuIs(Builder, RO.Conditions.Architecture);
+  if (!RO.Conditions.Features.empty()) {
+llvm::Value *FeatureCond =
+llvm::X86::emitCpuSupports(Builder, RO.Conditions.Features);
+Condition =
+Condition ? Builder.CreateAnd(Condition, FeatureCond) : FeatureCond;
+  }
+  return Condition;
+}
+
+static void CreateMultiVersionResolverReturn(Function *Resolver,
+ IRBuilderBase ,
+ Function *FuncToReturn,
+ bool UseIFunc) {
+  if (UseIFunc) {
+Builder.CreateRet(FuncToReturn);
+return;
+  }
+
+  SmallVector Args;
+  for_each(Resolver->args(), [&](Argument ) { Args.push_back(); });
+
+  CallInst *Result = Builder.CreateCall(FuncToReturn, Args);
+  Result->setTailCallKind(CallInst::TCK_MustTail);
+
+  if (Resolver->getReturnType()->isVoidTy())
+Builder.CreateRetVoid();
+  else
+Builder.CreateRet(Result);
+}
+
+void llvm::emitMultiVersionResolver(
+Function *Resolver, ArrayRef Options,
+bool UseIFunc) {
+  assert(Triple(Resolver->getParent()->getTargetTriple()).isX86() &&
+ "Only implemented for x86 targets");
+
+  auto  = Resolver->getContext();
+  // Main function's basic block.
+  BasicBlock *CurBlock = BasicBlock::Create(Ctx, "resolver_entry", Resolver);
+
+  IRBuilder<> Builder(CurBlock, CurBlock->begin());
+  llvm::X86::emitCPUInit(Builder);
+
+  for (const MultiVersionResolverOption  : Options) {
+Builder.SetInsertPoint(CurBlock);
+llvm::Value *Condition = formResolverCondition(Builder, RO);
+
+// The 'default' or 'generic' case.
+if (!Condition) {
+  assert( == Options.end() - 1 &&
+ "Default or Generic case must be last");
+  CreateMultiVersionResolverReturn(Resolver, Builder, RO.Fn, UseIFunc);
+  return;
+}
+
+llvm::BasicBlock *RetBlock =
+BasicBlock::Create(Ctx, "resolver_return", Resolver);
+{
+  IRBuilderBase::InsertPointGuard Guard(Builder);
+  Builder.SetInsertPoint(RetBlock);
+  CreateMultiVersionResolverReturn(Resolver, Builder, RO.Fn, UseIFunc);
+}
+CurBlock = BasicBlock::Create(Ctx, "resolver_else", Resolver);
+Builder.CreateCondBr(Condition, RetBlock, CurBlock);
+  }
+
+  // If no generic/default, emit an unreachable.
+  Builder.SetInsertPoint(CurBlock);
+  CallInst *TrapCall = Builder.CreateIntrinsic(Intrinsic::trap, {}, {});
+  TrapCall->setDoesNotReturn();
+  TrapCall->setDoesNotThrow();
+  Builder.CreateUnreachable();
+}
+
+static Type *getCpuModelType(IRBuilderBase ) {
+  Type *Int32Ty = Builder.getInt32Ty();
+
+  // Matching the struct layout from the compiler-rt/libgcc structure that is
+  // filled in:
+  // unsigned int __cpu_vendor;
+  // unsigned int __cpu_type;
+  // unsigned int __cpu_subtype;
+  // unsigned int __cpu_features[1];
+  Type *STy =
+  StructType::get(Int32Ty, Int32Ty, Int32Ty, ArrayType::get(Int32Ty, 1));
+  return STy;
+}
+
+static Value *getOrCreateGlobal(IRBuilderBase , StringRef Name,
+Type *Ty) {
+  Module *M = 

[clang] 0e42ec1 - DebugInfo: Correct printing empty template parameter packs

[David Blaikie via cfe-commits]

Author: David Blaikie
Date: 2021-08-30T10:20:12-07:00
New Revision: 0e42ec1add336e7fbf6cc1f82f663cabe48bf55e

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

LOG: DebugInfo: Correct printing empty template parameter packs

Empty packs in the non-final position would result in an extra ", ".
Empty packs in the final position would result in missing the space
between trailing >>.

Added: 


Modified: 
clang/lib/AST/TypePrinter.cpp
clang/test/CodeGenCXX/debug-info-template.cpp

Removed: 




diff  --git a/clang/lib/AST/TypePrinter.cpp b/clang/lib/AST/TypePrinter.cpp
index b66e432b98e9a..aef1e4f3f4953 100644
--- a/clang/lib/AST/TypePrinter.cpp
+++ b/clang/lib/AST/TypePrinter.cpp
@@ -2052,20 +2052,21 @@ printTo(raw_ostream , ArrayRef Args, const 
PrintingPolicy ,
 
 // If the last character of our string is '>', add another space to
 // keep the two '>''s separate tokens.
-NeedSpace = Policy.SplitTemplateClosers && !ArgString.empty() &&
-ArgString.back() == '>';
-FirstArg = false;
+if (!ArgString.empty()) {
+  NeedSpace = Policy.SplitTemplateClosers && ArgString.back() == '>';
+  FirstArg = false;
+}
 
 // Use same template parameter for all elements of Pack
 if (!IsPack)
   ParmIndex++;
   }
 
-  if (NeedSpace)
-OS << ' ';
-
-  if (!IsPack)
+  if (!IsPack) {
+if (NeedSpace)
+  OS << ' ';
 OS << '>';
+  }
 }
 
 void clang::printTemplateArgumentList(raw_ostream ,

diff  --git a/clang/test/CodeGenCXX/debug-info-template.cpp 
b/clang/test/CodeGenCXX/debug-info-template.cpp
index 0255ec9df00fa..2ce0166590aa1 100644
--- a/clang/test/CodeGenCXX/debug-info-template.cpp
+++ b/clang/test/CodeGenCXX/debug-info-template.cpp
@@ -196,4 +196,22 @@ void f1() {
 }
 template void f1>();
 // CHECK: !DISubprogram(name: "f1 >",
+} // namespace IndirectDefaultArgument
+
+namespace EmptyTrailingPack {
+template
+struct t1 { };
+template
+void f1() {
+}
+template void f1>();
+// CHECK: !DISubprogram(name: "f1 >",
+} // namespace EmptyTrailingPack
+
+namespace EmptyInnerPack {
+template
+void f1() {
 }
+template void f1<>();
+// CHECK: !DISubprogram(name: "f1",
+} // namespace EmptyInnerPack



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

[PATCH] D108905: [ItaniumCXXABI] Make __cxa_end_catch calls unconditionally nounwind

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added a comment.

Peanut gallery says: Does this godbolt demonstrate the situation where 
"destroying an exception can throw an exception"?
https://godbolt.org/z/Ghjz53rrj
You're not proposing to change the behavior of this program, are you?
(If you're proposing to change it: I don't think you should. If you're not 
proposing to change it, but rather the issue is something subtler: never mind 
me, then.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108905

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

[PATCH] D108423: [NFC][clang] Move IR-independent parts of target MV support to X86TargetParser.cpp

[Andrei Elovikov 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 rG1724a164378f: [NFC][clang] Move IR-independent parts of 
target MV support to X86TargetParser. (authored by a.elovikov).

Changed prior to commit:
  https://reviews.llvm.org/D108423?vs=368390=369471#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108423

Files:
  clang/lib/Basic/Targets/X86.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.cpp
  llvm/include/llvm/Support/X86TargetParser.h
  llvm/lib/Support/X86TargetParser.cpp

Index: llvm/lib/Support/X86TargetParser.cpp
===
--- llvm/lib/Support/X86TargetParser.cpp
+++ llvm/lib/Support/X86TargetParser.cpp
@@ -11,7 +11,9 @@
 //===--===//
 
 #include "llvm/Support/X86TargetParser.h"
+#include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Triple.h"
+#include 
 
 using namespace llvm;
 using namespace llvm::X86;
@@ -662,3 +664,45 @@
 if (ImpliedBits[i] && !FeatureInfos[i].Name.empty())
   Features[FeatureInfos[i].Name] = Enabled;
 }
+
+uint64_t llvm::X86::getCpuSupportsMask(ArrayRef FeatureStrs) {
+  // Processor features and mapping to processor feature value.
+  uint64_t FeaturesMask = 0;
+  for (const StringRef  : FeatureStrs) {
+unsigned Feature = StringSwitch(FeatureStr)
+#define X86_FEATURE_COMPAT(ENUM, STR, PRIORITY)\
+  .Case(STR, llvm::X86::FEATURE_##ENUM)
+#include "llvm/Support/X86TargetParser.def"
+;
+FeaturesMask |= (1ULL << Feature);
+  }
+  return FeaturesMask;
+}
+
+unsigned llvm::X86::getFeaturePriority(ProcessorFeatures Feat) {
+#ifndef NDEBUG
+  // Check that priorities are set properly in the .def file. We expect that
+  // "compat" features are assigned non-duplicate consecutive priorities
+  // starting from zero (0, 1, ..., num_features - 1).
+#define X86_FEATURE_COMPAT(ENUM, STR, PRIORITY) PRIORITY,
+  unsigned Priorities[] = {
+#include "llvm/Support/X86TargetParser.def"
+  std::numeric_limits::max() // Need to consume last comma.
+  };
+  std::array HelperList;
+  std::iota(HelperList.begin(), HelperList.end(), 0);
+  assert(std::is_permutation(HelperList.begin(), HelperList.end(),
+ std::begin(Priorities),
+ std::prev(std::end(Priorities))) &&
+ "Priorities don't form consecutive range!");
+#endif
+
+  switch (Feat) {
+#define X86_FEATURE_COMPAT(ENUM, STR, PRIORITY)\
+  case X86::FEATURE_##ENUM:\
+return PRIORITY;
+#include "llvm/Support/X86TargetParser.def"
+  default:
+llvm_unreachable("No Feature Priority for non-CPUSupports Features");
+  }
+}
Index: llvm/include/llvm/Support/X86TargetParser.h
===
--- llvm/include/llvm/Support/X86TargetParser.h
+++ llvm/include/llvm/Support/X86TargetParser.h
@@ -13,6 +13,7 @@
 #ifndef LLVM_SUPPORT_X86TARGETPARSER_H
 #define LLVM_SUPPORT_X86TARGETPARSER_H
 
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringMap.h"
 
@@ -154,6 +155,9 @@
 void updateImpliedFeatures(StringRef Feature, bool Enabled,
StringMap );
 
+uint64_t getCpuSupportsMask(ArrayRef FeatureStrs);
+unsigned getFeaturePriority(ProcessorFeatures Feat);
+
 } // namespace X86
 } // namespace llvm
 
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -63,6 +63,7 @@
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/MD5.h"
 #include "llvm/Support/TimeProfiler.h"
+#include "llvm/Support/X86TargetParser.h"
 
 using namespace clang;
 using namespace CodeGen;
@@ -3402,8 +3403,8 @@
   llvm::stable_sort(
   Options, [](const CodeGenFunction::MultiVersionResolverOption ,
   const CodeGenFunction::MultiVersionResolverOption ) {
-return CodeGenFunction::GetX86CpuSupportsMask(LHS.Conditions.Features) >
-   CodeGenFunction::GetX86CpuSupportsMask(RHS.Conditions.Features);
+return llvm::X86::getCpuSupportsMask(LHS.Conditions.Features) >
+   llvm::X86::getCpuSupportsMask(RHS.Conditions.Features);
   });
 
   // If the list contains multiple 'default' versions, such as when it contains
@@ -3411,7 +3412,7 @@
   // always run on at least a 'pentium'). We do this by deleting the 'least
   // advanced' (read, lowest mangling letter).
   while (Options.size() > 1 &&
- CodeGenFunction::GetX86CpuSupportsMask(
+ llvm::X86::getCpuSupportsMask(
  

[clang] 1724a16 - [NFC][clang] Move IR-independent parts of target MV support to X86TargetParser.cpp

[Andrei Elovikov via cfe-commits]

Author: Andrei Elovikov
Date: 2021-08-30T09:48:48-07:00
New Revision: 1724a164378f1fb5210771680eb577ddf13c5a47

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

LOG: [NFC][clang] Move IR-independent parts of target MV support to 
X86TargetParser.cpp

...that is located under llvm/lib/Support/.

Reviewed By: erichkeane

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

Added: 


Modified: 
clang/lib/Basic/Targets/X86.cpp
clang/lib/CodeGen/CGBuiltin.cpp
clang/lib/CodeGen/CodeGenFunction.h
clang/lib/CodeGen/CodeGenModule.cpp
llvm/include/llvm/Support/X86TargetParser.h
llvm/lib/Support/X86TargetParser.cpp

Removed: 




diff  --git a/clang/lib/Basic/Targets/X86.cpp b/clang/lib/Basic/Targets/X86.cpp
index 5b259c95d80df..01965725bb32f 100644
--- a/clang/lib/Basic/Targets/X86.cpp
+++ b/clang/lib/Basic/Targets/X86.cpp
@@ -18,7 +18,6 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Support/X86TargetParser.h"
-#include 
 
 namespace clang {
 namespace targets {
@@ -1058,34 +1057,6 @@ static llvm::X86::ProcessorFeatures getFeature(StringRef 
Name) {
   // correct, so it asserts if the value is out of range.
 }
 
-static unsigned getFeaturePriority(llvm::X86::ProcessorFeatures Feat) {
-#ifndef NDEBUG
-  // Check that priorities are set properly in the .def file. We expect that
-  // "compat" features are assigned non-duplicate consecutive priorities
-  // starting from zero (0, 1, ..., num_features - 1).
-#define X86_FEATURE_COMPAT(ENUM, STR, PRIORITY) PRIORITY,
-  unsigned Priorities[] = {
-#include "llvm/Support/X86TargetParser.def"
-  std::numeric_limits::max() // Need to consume last comma.
-  };
-  std::array HelperList;
-  std::iota(HelperList.begin(), HelperList.end(), 0);
-  assert(std::is_permutation(HelperList.begin(), HelperList.end(),
- std::begin(Priorities),
- std::prev(std::end(Priorities))) &&
- "Priorities don't form consecutive range!");
-#endif
-
-  switch (Feat) {
-#define X86_FEATURE_COMPAT(ENUM, STR, PRIORITY)
\
-  case llvm::X86::FEATURE_##ENUM:  
\
-return PRIORITY;
-#include "llvm/Support/X86TargetParser.def"
-  default:
-llvm_unreachable("No Feature Priority for non-CPUSupports Features");
-  }
-}
-
 unsigned X86TargetInfo::multiVersionSortPriority(StringRef Name) const {
   // Valid CPUs have a 'key feature' that compares just better than its key
   // feature.

diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index ca6987b378a85..4e0863ab94664 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -12404,24 +12404,8 @@ Value *CodeGenFunction::EmitX86CpuSupports(const 
CallExpr *E) {
   return EmitX86CpuSupports(FeatureStr);
 }
 
-uint64_t
-CodeGenFunction::GetX86CpuSupportsMask(ArrayRef FeatureStrs) {
-  // Processor features and mapping to processor feature value.
-  uint64_t FeaturesMask = 0;
-  for (const StringRef  : FeatureStrs) {
-unsigned Feature =
-StringSwitch(FeatureStr)
-#define X86_FEATURE_COMPAT(ENUM, STR, PRIORITY)
\
-  .Case(STR, llvm::X86::FEATURE_##ENUM)
-#include "llvm/Support/X86TargetParser.def"
-;
-FeaturesMask |= (1ULL << Feature);
-  }
-  return FeaturesMask;
-}
-
 Value *CodeGenFunction::EmitX86CpuSupports(ArrayRef FeatureStrs) {
-  return EmitX86CpuSupports(GetX86CpuSupportsMask(FeatureStrs));
+  return EmitX86CpuSupports(llvm::X86::getCpuSupportsMask(FeatureStrs));
 }
 
 llvm::Value *CodeGenFunction::EmitX86CpuSupports(uint64_t FeaturesMask) {

diff  --git a/clang/lib/CodeGen/CodeGenFunction.h 
b/clang/lib/CodeGen/CodeGenFunction.h
index 7255c385bf163..1f05877ed6f9d 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -4728,8 +4728,6 @@ class CodeGenFunction : public CodeGenTypeCache {
   void EmitMultiVersionResolver(llvm::Function *Resolver,
 ArrayRef Options);
 
-  static uint64_t GetX86CpuSupportsMask(ArrayRef FeatureStrs);
-
 private:
   QualType getVarArgType(const Expr *Arg);
 

diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index 664acefd1815c..49d6cdf779b71 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -63,6 +63,7 @@
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/MD5.h"
 #include "llvm/Support/TimeProfiler.h"
+#include "llvm/Support/X86TargetParser.h"
 
 using namespace clang;
 using namespace CodeGen;
@@ -3402,8 +3403,8 @@ void CodeGenModule::emitCPUDispatchDefinition(GlobalDecl 
GD) {
   llvm::stable_sort(
   

[PATCH] D108905: [ItaniumCXXABI] Make __cxa_end_catch calls unconditionally nounwind

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 369467.
MaskRay retitled this revision from "[ItaniumCXXABI] Set nounwind on 
__cxa_begin_catch/__cxa_end_catch" to "[ItaniumCXXABI] Make __cxa_end_catch 
calls unconditionally nounwind".
MaskRay edited the summary of this revision.
MaskRay added a comment.
Herald added subscribers: lxfind, aheejin, sbc100.

set nounwind in call sites


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108905

Files:
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/eh.cpp
  clang/test/CodeGenCXX/exceptions.cpp
  clang/test/CodeGenCXX/stack-reuse-exceptions.cpp
  clang/test/CodeGenCXX/wasm-eh.cpp
  clang/test/CodeGenCoroutines/coro-await-resume-eh.cpp
  clang/test/CodeGenCoroutines/coro-cleanup.cpp
  clang/test/CodeGenCoroutines/coro-eh-cleanup.cpp
  clang/test/CodeGenCoroutines/coro-unhandled-exception.cpp

Index: clang/test/CodeGenCoroutines/coro-unhandled-exception.cpp
===
--- clang/test/CodeGenCoroutines/coro-unhandled-exception.cpp
+++ clang/test/CodeGenCoroutines/coro-unhandled-exception.cpp
@@ -64,9 +64,7 @@
 // CHECK-LPAD: [[CATCH]]:
 // CHECK-LPAD:call i8* @__cxa_begin_catch
 // CHECK-LPAD:call void @_ZN6coro_t12promise_type19unhandled_exceptionEv(%"struct.coro_t::promise_type"* {{[^,]*}} %__promise) #2
-// CHECK-LPAD:invoke void @__cxa_end_catch()
-// CHECK-LPAD-NEXT:  to label %[[CATCHRETDEST:.+]] unwind label
-// CHECK-LPAD: [[CATCHRETDEST]]:
+// CHECK-LPAD:call void @__cxa_end_catch()
 // CHECK-LPAD-NEXT: br label %[[TRYCONT:.+]]
 // CHECK-LPAD: [[TRYCONT]]:
 // CHECK-LPAD: br label %[[COROFIN:.+]]
Index: clang/test/CodeGenCoroutines/coro-eh-cleanup.cpp
===
--- clang/test/CodeGenCoroutines/coro-eh-cleanup.cpp
+++ clang/test/CodeGenCoroutines/coro-eh-cleanup.cpp
@@ -72,15 +72,7 @@
 // CHECK-LPAD:   call void @_ZN7CleanupD1Ev(
 // CHECK-LPAD:   call i8* @__cxa_begin_catch
 // CHECK-LPAD:   call void @_ZN6coro_t12promise_type19unhandled_exceptionEv
-// CHECK-LPAD:   invoke void @__cxa_end_catch()
-// CHECK-LPAD: to label %{{.+}} unwind label %[[UNWINDBB:.+]]
+// CHECK-LPAD:   call void @__cxa_end_catch()
 
-// CHECK-LPAD: [[UNWINDBB]]:
-// CHECK-LPAD:   %[[I1RESUME:.+]] = call i1 @llvm.coro.end(i8* null, i1 true)
-// CHECK-LPAD:   br i1  %[[I1RESUME]], label %[[EHRESUME:.+]], label
-// CHECK-LPAD: [[EHRESUME]]:
-// CHECK-LPAD-NEXT:  %[[exn:.+]] = load i8*, i8** %exn.slot, align 8
-// CHECK-LPAD-NEXT:  %[[sel:.+]] = load i32, i32* %ehselector.slot, align 4
-// CHECK-LPAD-NEXT:  %[[val1:.+]] = insertvalue { i8*, i32 } undef, i8* %[[exn]], 0
-// CHECK-LPAD-NEXT:  %[[val2:.+]] = insertvalue { i8*, i32 } %[[val1]], i32 %[[sel]], 1
-// CHECK-LPAD-NEXT:  resume { i8*, i32 } %[[val2]]
+// CHECK-LPAD:   %{{.+}} = call i1 @llvm.coro.end(i8* null, i1 false)
+// CHECK-LPAD:   ret void
Index: clang/test/CodeGenCoroutines/coro-cleanup.cpp
===
--- clang/test/CodeGenCoroutines/coro-cleanup.cpp
+++ clang/test/CodeGenCoroutines/coro-cleanup.cpp
@@ -49,7 +49,8 @@
   // CHECK: [[DeallocPad]]:
   // CHECK-NEXT: landingpad
   // CHECK-NEXT:   cleanup
-  // CHECK: br label %[[Dealloc:.+]]
+  // CHECK:   %[[Mem:.+]] = call i8* @llvm.coro.free(
+  // CHECK:   br i1 %[[#]], label %[[Dealloc:.+]], label
 
   Cleanup cleanup;
   may_throw();
@@ -68,12 +69,10 @@
   // CHECK: [[Catch]]:
   // CHECK:call i8* @__cxa_begin_catch(
   // CHECK:call void @_ZNSt12experimental16coroutine_traitsIJvEE12promise_type19unhandled_exceptionEv(
-  // CHECK:invoke void @__cxa_end_catch()
-  // CHECK-NEXT:to label %[[Cont:.+]] unwind
+  // CHECK:call void @__cxa_end_catch()
+  // CHECK-NEXT:br label %[[Cont:.+]]
 
   // CHECK: [[Cont]]:
-  // CHECK-NEXT: br label %[[Cont2:.+]]
-  // CHECK: [[Cont2]]:
   // CHECK-NEXT: br label %[[Cleanup:.+]]
 
   // CHECK: [[Cleanup]]:
@@ -82,7 +81,6 @@
   // CHECK: call void @_ZdlPv(i8* %[[Mem0]]
 
   // CHECK: [[Dealloc]]:
-  // CHECK:   %[[Mem:.+]] = call i8* @llvm.coro.free(
   // CHECK:   call void @_ZdlPv(i8* %[[Mem]])
 
   co_return;
Index: clang/test/CodeGenCoroutines/coro-await-resume-eh.cpp
===
--- clang/test/CodeGenCoroutines/coro-await-resume-eh.cpp
+++ clang/test/CodeGenCoroutines/coro-await-resume-eh.cpp
@@ -53,9 +53,7 @@
   // CHECK: invoke void @_ZN13throwing_task12promise_type19unhandled_exceptionEv
   // CHECK-NEXT: to label %[[RESUMEENDCATCH:.+]] unwind label
   // CHECK: [[RESUMEENDCATCH]]:
-  // CHECK-NEXT: invoke void @__cxa_end_catch()
-  // CHECK-NEXT: to label %[[RESUMEENDCATCHCONT:.+]] unwind label
-  // CHECK: [[RESUMEENDCATCHCONT]]:
+  // CHECK-NEXT: call void @__cxa_end_catch()
   // CHECK-NEXT: br label %[[RESUMETRYCONT]]
   // CHECK: [[RESUMETRYCONT]]:
   // 

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[Ludovic Jozeau via Phabricator via cfe-commits]

FederAndInk added inline comments.



Comment at: clang/docs/tools/dump_format_style.py:26
 
+def register_plural(singular: str, plural: str):
+  if plural not in plurals:

MyDeveloperDay wrote:
> This failed for me with invalid syntax
Oh, ok, sorry, I might be using to recent python features, I'll remove type 
specifier, what is the recommended python version to use?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108765

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

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/docs/tools/dump_format_style.py:26
 
+def register_plural(singular: str, plural: str):
+  if plural not in plurals:

This failed for me with invalid syntax


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108765

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

[PATCH] D108886: Add RISC-V sifive-s51 cpu

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

Please upload patches with full context. Using -U99 as documented here 
https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108886

___
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

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd updated this revision to Diff 369453.
compnerd marked 2 inline comments as done.
compnerd added a comment.

Update release notes to incorporate feedback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108893

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
  clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-data-pointer.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp
@@ -0,0 +1,104 @@
+// RUN: %check_clang_tidy %s readability-container-data-pointer %t
+
+typedef __SIZE_TYPE__ size_t;
+
+namespace std {
+template 
+struct vector {
+  using size_type = size_t;
+
+  vector();
+  explicit vector(size_type);
+
+  T *data();
+  const T *data() const;
+
+  T [](size_type);
+  const T [](size_type) const;
+};
+
+template 
+struct basic_string {
+  using size_type = size_t;
+
+  basic_string();
+
+  T *data();
+  const T *data() const;
+
+  T [](size_t);
+  const T [](size_type) const;
+};
+
+typedef basic_string string;
+typedef basic_string wstring;
+
+template 
+struct is_integral;
+
+template <>
+struct is_integral {
+  static const bool value = true;
+};
+
+template 
+struct enable_if { };
+
+template 
+struct enable_if {
+  typedef T type;
+};
+}
+
+template 
+void f(const T *);
+
+#define z (0)
+
+void g(size_t s) {
+  std::vector b(s);
+  f(&((b)[(z)]));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}f(b.data());{{$}}
+}
+
+void h() {
+  std::string s;
+  f(&((s).operator[]((z;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}f(s.data());{{$}}
+
+  std::wstring w;
+  f(&((&(w))->operator[]((z;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}f(w.data());{{$}}
+}
+
+template ::value>::type>
+void i(U s) {
+  std::vector b(s);
+  f([0]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}f(b.data());{{$}}
+}
+
+template void i(size_t);
+
+void j(std::vector * const v) {
+  f(&(*v)[0]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}f(v->data());{{$}}
+}
+
+void k(const std::vector ) {
+  f([0]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}f(v.data());{{$}}
+}
+
+void l() {
+  unsigned char b[32];
+  f([0]);
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+}
Index: clang-tools-extra/docs/clang-tidy/checks/readability-data-pointer.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/readability-data-pointer.rst
@@ -0,0 +1,13 @@
+.. title:: clang-tidy - readability-data-pointer
+
+readability-data-pointer
+
+
+Finds cases where code could use ``data()`` rather than the address of the
+element at index 0 in a container.  This pattern is commonly used to materialize
+a pointer to the backing data of a container.  ``std::vector`` and
+``std::string`` provide a ``data()`` accessor to retrieve the data pointer which
+should be preferred.
+
+This also ensures that in the case that the container is empty, the data pointer
+access does not perform an errant memory access.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -82,6 +82,11 @@
 
   Reports identifiers whose names are too short. 

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

[Saleem Abdulrasool via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:87
+
+  Finds cases where code could use ``data`` rather than the address of an 
element.
+

Eugene.Zelenko wrote:
> It'll be good idea to mention containers and that `data` is method.
That was dumb, you're right.  Thanks!



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability-data-pointer.rst:6
+
+Finds instances of the data pointer being materialized by taking the address of
+the element at index 0 of a container. In particular, ``std::vector`` and

Eugene.Zelenko wrote:
> Please make first statement same as in Release Notes.
That was a good idea.  I find that the updated text reads much better too.


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] D108905: [ItaniumCXXABI] Set nounwind on __cxa_begin_catch/__cxa_end_catch

[Wenlei He via Phabricator via cfe-commits]

wenlei added subscribers: modimo, wenlei.
wenlei added a comment.

+@modimo


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108905

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

[PATCH] D108919: [clan-repl] Install clang-repl

[Raphael Isemann via Phabricator via cfe-commits]

teemperor added a comment.

This is essentially what D106813  was 
supposed to do, so this is also LGTM.

You probably want to file a bugreport for @tstellar to cherry-pick this to the 
release. The biggest impact from this seems to be that it increases the release 
size and maybe that some maintainers want to package this separately (or not at 
all). It's probably worth pointing this out when making the cherry-pick bug 
report.


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

https://reviews.llvm.org/D108919

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

[PATCH] D104285: [analyzer][AST] Retrieve value by direct index from list initialization of constant array declaration.

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

In D104285#2972215 , @martong wrote:

> @ASDenysPetrov Denis, do you think it would make sense to handle the 
> non-multi-dimensional cases first? I see that you have useful patches in the 
> stack that depends on this change (i.e handling a StringLiteral or a 
> CompoundLiteralExpr) but perhaps they would be meaningful even without 
> solving the mult-array case here (?).

I think you are right. I've been wandering the Standards for a week. I can't 
find the proof whether it is even a legal cast or not `const int arr[1][2][3]; 
const int ptr* = (const int*)arr;`. Descriptions about this are really unclear. 
I'll try to rewrite this patch for simple-dimensional arrays for a start.


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

https://reviews.llvm.org/D104285

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

[PATCH] D108308: Cleanup identifier parsing.

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin updated this revision to Diff 369446.
cor3ntin added a comment.

Move a comment to the header file per Aaron request


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108308

Files:
  clang-tools-extra/clang-include-fixer/IncludeFixer.cpp
  clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
  clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/SourceCode.cpp
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang/include/clang/Basic/CharInfo.h
  clang/include/clang/Lex/Lexer.h
  clang/lib/ARCMigrate/ObjCMT.cpp
  clang/lib/ARCMigrate/TransUnbridgedCasts.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/Basic/Module.cpp
  clang/lib/Edit/EditedSource.cpp
  clang/lib/Frontend/LayoutOverrideSource.cpp
  clang/lib/Frontend/Rewrite/FrontendActions.cpp
  clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
  clang/lib/Lex/Lexer.cpp
  clang/lib/Lex/ModuleMap.cpp
  clang/lib/Sema/SemaAvailability.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/Tooling/Transformer/Parsing.cpp
  clang/unittests/Basic/CharInfoTest.cpp

Index: clang/unittests/Basic/CharInfoTest.cpp
===
--- clang/unittests/Basic/CharInfoTest.cpp
+++ clang/unittests/Basic/CharInfoTest.cpp
@@ -50,44 +50,44 @@
   EXPECT_FALSE(isASCII('\xff'));
 }
 
-TEST(CharInfoTest, isIdentifierHead) {
-  EXPECT_TRUE(isIdentifierHead('a'));
-  EXPECT_TRUE(isIdentifierHead('A'));
-  EXPECT_TRUE(isIdentifierHead('z'));
-  EXPECT_TRUE(isIdentifierHead('Z'));
-  EXPECT_TRUE(isIdentifierHead('_'));
-
-  EXPECT_FALSE(isIdentifierHead('0'));
-  EXPECT_FALSE(isIdentifierHead('.'));
-  EXPECT_FALSE(isIdentifierHead('`'));
-  EXPECT_FALSE(isIdentifierHead('\0'));
-
-  EXPECT_FALSE(isIdentifierHead('$'));
-  EXPECT_TRUE(isIdentifierHead('$', /*AllowDollar=*/true));
-
-  EXPECT_FALSE(isIdentifierHead('\x80'));
-  EXPECT_FALSE(isIdentifierHead('\xc2'));
-  EXPECT_FALSE(isIdentifierHead('\xff'));
+TEST(CharInfoTest, isAsciiIdentifierStart) {
+  EXPECT_TRUE(isAsciiIdentifierStart('a'));
+  EXPECT_TRUE(isAsciiIdentifierStart('A'));
+  EXPECT_TRUE(isAsciiIdentifierStart('z'));
+  EXPECT_TRUE(isAsciiIdentifierStart('Z'));
+  EXPECT_TRUE(isAsciiIdentifierStart('_'));
+
+  EXPECT_FALSE(isAsciiIdentifierStart('0'));
+  EXPECT_FALSE(isAsciiIdentifierStart('.'));
+  EXPECT_FALSE(isAsciiIdentifierStart('`'));
+  EXPECT_FALSE(isAsciiIdentifierStart('\0'));
+
+  EXPECT_FALSE(isAsciiIdentifierStart('$'));
+  EXPECT_TRUE(isAsciiIdentifierStart('$', /*AllowDollar=*/true));
+
+  EXPECT_FALSE(isAsciiIdentifierStart('\x80'));
+  EXPECT_FALSE(isAsciiIdentifierStart('\xc2'));
+  EXPECT_FALSE(isAsciiIdentifierStart('\xff'));
 }
 
-TEST(CharInfoTest, isIdentifierBody) {
-  EXPECT_TRUE(isIdentifierBody('a'));
-  EXPECT_TRUE(isIdentifierBody('A'));
-  EXPECT_TRUE(isIdentifierBody('z'));
-  EXPECT_TRUE(isIdentifierBody('Z'));
-  EXPECT_TRUE(isIdentifierBody('_'));
+TEST(CharInfoTest, isAsciiIdentifierContinue) {
+  EXPECT_TRUE(isAsciiIdentifierContinue('a'));
+  EXPECT_TRUE(isAsciiIdentifierContinue('A'));
+  EXPECT_TRUE(isAsciiIdentifierContinue('z'));
+  EXPECT_TRUE(isAsciiIdentifierContinue('Z'));
+  EXPECT_TRUE(isAsciiIdentifierContinue('_'));
 
-  EXPECT_TRUE(isIdentifierBody('0'));
-  EXPECT_FALSE(isIdentifierBody('.'));
-  EXPECT_FALSE(isIdentifierBody('`'));
-  EXPECT_FALSE(isIdentifierBody('\0'));
+  EXPECT_TRUE(isAsciiIdentifierContinue('0'));
+  EXPECT_FALSE(isAsciiIdentifierContinue('.'));
+  EXPECT_FALSE(isAsciiIdentifierContinue('`'));
+  EXPECT_FALSE(isAsciiIdentifierContinue('\0'));
 
-  EXPECT_FALSE(isIdentifierBody('$'));
-  EXPECT_TRUE(isIdentifierBody('$', /*AllowDollar=*/true));
+  EXPECT_FALSE(isAsciiIdentifierContinue('$'));
+  EXPECT_TRUE(isAsciiIdentifierContinue('$', /*AllowDollar=*/true));
 
-  EXPECT_FALSE(isIdentifierBody('\x80'));
-  EXPECT_FALSE(isIdentifierBody('\xc2'));
-  EXPECT_FALSE(isIdentifierBody('\xff'));
+  EXPECT_FALSE(isAsciiIdentifierContinue('\x80'));
+  EXPECT_FALSE(isAsciiIdentifierContinue('\xc2'));
+  EXPECT_FALSE(isAsciiIdentifierContinue('\xff'));
 }
 
 TEST(CharInfoTest, isHorizontalWhitespace) {
@@ -413,91 +413,91 @@
   EXPECT_EQ('\0', toUppercase('\0'));
 }
 
-TEST(CharInfoTest, isValidIdentifier) {
-  EXPECT_FALSE(isValidIdentifier(""));
+TEST(CharInfoTest, isValidAsciiIdentifier) {
+  EXPECT_FALSE(isValidAsciiIdentifier(""));
 
   // 1 character
-  EXPECT_FALSE(isValidIdentifier("."));
-  EXPECT_FALSE(isValidIdentifier("\n"));
-  EXPECT_FALSE(isValidIdentifier(" "));
-  EXPECT_FALSE(isValidIdentifier("\x80"));
-  EXPECT_FALSE(isValidIdentifier("\xc2"));
-  EXPECT_FALSE(isValidIdentifier("\xff"));
-  EXPECT_FALSE(isValidIdentifier("$"));
-  EXPECT_FALSE(isValidIdentifier("1"));
-
-  EXPECT_TRUE(isValidIdentifier("_"));
-  

[PATCH] D104647: [analyzer] Support SVal::getType for pointer-to-member values

[Gabor Marton via Phabricator via cfe-commits]

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

This looks good to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104647

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

[PATCH] D108702: [PowerPC][NFC] Rename P10 builtins vec_clrl, vec_clrr to vec_clr_first and vec_clr_last

[Victor Huang 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 rG2e5c17d19e37: [PowerPC][NFC] Rename P10 builtins vec_clrl, 
vec_clrr to vec_clr_first and… (authored by NeHuang).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108702

Files:
  clang/lib/Headers/altivec.h
  clang/test/CodeGen/builtins-ppc-p10vector.c


Index: clang/test/CodeGen/builtins-ppc-p10vector.c
===
--- clang/test/CodeGen/builtins-ppc-p10vector.c
+++ clang/test/CodeGen/builtins-ppc-p10vector.c
@@ -732,36 +732,36 @@
   return vec_genpcvm(vulla, 0);
 }
 
-vector signed char test_vec_vclrl_sc(void) {
+vector signed char test_vec_clr_first_sc(void) {
   // CHECK-BE: @llvm.ppc.altivec.vclrlb(<16 x i8>
   // CHECK-BE-NEXT: ret <16 x i8>
   // CHECK-LE: @llvm.ppc.altivec.vclrrb(<16 x i8>
   // CHECK-LE-NEXT: ret <16 x i8>
-  return vec_clrl(vsca, uia);
+  return vec_clr_first(vsca, uia);
 }
 
-vector unsigned char test_vec_clrl_uc(void) {
+vector unsigned char test_vec_clr_first_uc(void) {
   // CHECK-BE: @llvm.ppc.altivec.vclrlb(<16 x i8>
   // CHECK-BE-NEXT: ret <16 x i8>
   // CHECK-LE: @llvm.ppc.altivec.vclrrb(<16 x i8>
   // CHECK-LE-NEXT: ret <16 x i8>
-  return vec_clrl(vuca, uia);
+  return vec_clr_first(vuca, uia);
 }
 
-vector signed char test_vec_vclrr_sc(void) {
+vector signed char test_vec_clr_last_sc(void) {
   // CHECK-BE: @llvm.ppc.altivec.vclrrb(<16 x i8>
   // CHECK-BE-NEXT: ret <16 x i8>
   // CHECK-LE: @llvm.ppc.altivec.vclrlb(<16 x i8>
   // CHECK-LE-NEXT: ret <16 x i8>
-  return vec_clrr(vsca, uia);
+  return vec_clr_last(vsca, uia);
 }
 
-vector unsigned char test_vec_clrr_uc(void) {
+vector unsigned char test_vec_clr_last_uc(void) {
   // CHECK-BE: @llvm.ppc.altivec.vclrrb(<16 x i8>
   // CHECK-BE-NEXT: ret <16 x i8>
   // CHECK-LE: @llvm.ppc.altivec.vclrlb(<16 x i8>
   // CHECK-LE-NEXT: ret <16 x i8>
-  return vec_clrr(vuca, uia);
+  return vec_clr_last(vuca, uia);
 }
 
 vector unsigned long long test_vclzdm(void) {
Index: clang/lib/Headers/altivec.h
===
--- clang/lib/Headers/altivec.h
+++ clang/lib/Headers/altivec.h
@@ -18312,10 +18312,10 @@
: __builtin_vsx_xxgenpcvdm((__a), (int)(__imm)))
 #endif /* __VSX__ */
 
-/* vec_clrl */
+/* vec_clr_first */
 
 static __inline__ vector signed char __ATTRS_o_ai
-vec_clrl(vector signed char __a, unsigned int __n) {
+vec_clr_first(vector signed char __a, unsigned int __n) {
 #ifdef __LITTLE_ENDIAN__
   return __builtin_altivec_vclrrb(__a, __n);
 #else
@@ -18324,7 +18324,7 @@
 }
 
 static __inline__ vector unsigned char __ATTRS_o_ai
-vec_clrl(vector unsigned char __a, unsigned int __n) {
+vec_clr_first(vector unsigned char __a, unsigned int __n) {
 #ifdef __LITTLE_ENDIAN__
   return __builtin_altivec_vclrrb((vector signed char)__a, __n);
 #else
@@ -18332,10 +18332,10 @@
 #endif
 }
 
-/* vec_clrr */
+/* vec_clr_last */
 
 static __inline__ vector signed char __ATTRS_o_ai
-vec_clrr(vector signed char __a, unsigned int __n) {
+vec_clr_last(vector signed char __a, unsigned int __n) {
 #ifdef __LITTLE_ENDIAN__
   return __builtin_altivec_vclrlb(__a, __n);
 #else
@@ -18344,7 +18344,7 @@
 }
 
 static __inline__ vector unsigned char __ATTRS_o_ai
-vec_clrr(vector unsigned char __a, unsigned int __n) {
+vec_clr_last(vector unsigned char __a, unsigned int __n) {
 #ifdef __LITTLE_ENDIAN__
   return __builtin_altivec_vclrlb((vector signed char)__a, __n);
 #else


Index: clang/test/CodeGen/builtins-ppc-p10vector.c
===
--- clang/test/CodeGen/builtins-ppc-p10vector.c
+++ clang/test/CodeGen/builtins-ppc-p10vector.c
@@ -732,36 +732,36 @@
   return vec_genpcvm(vulla, 0);
 }
 
-vector signed char test_vec_vclrl_sc(void) {
+vector signed char test_vec_clr_first_sc(void) {
   // CHECK-BE: @llvm.ppc.altivec.vclrlb(<16 x i8>
   // CHECK-BE-NEXT: ret <16 x i8>
   // CHECK-LE: @llvm.ppc.altivec.vclrrb(<16 x i8>
   // CHECK-LE-NEXT: ret <16 x i8>
-  return vec_clrl(vsca, uia);
+  return vec_clr_first(vsca, uia);
 }
 
-vector unsigned char test_vec_clrl_uc(void) {
+vector unsigned char test_vec_clr_first_uc(void) {
   // CHECK-BE: @llvm.ppc.altivec.vclrlb(<16 x i8>
   // CHECK-BE-NEXT: ret <16 x i8>
   // CHECK-LE: @llvm.ppc.altivec.vclrrb(<16 x i8>
   // CHECK-LE-NEXT: ret <16 x i8>
-  return vec_clrl(vuca, uia);
+  return vec_clr_first(vuca, uia);
 }
 
-vector signed char test_vec_vclrr_sc(void) {
+vector signed char test_vec_clr_last_sc(void) {
   // CHECK-BE: @llvm.ppc.altivec.vclrrb(<16 x i8>
   // CHECK-BE-NEXT: ret <16 x i8>
   // CHECK-LE: @llvm.ppc.altivec.vclrlb(<16 x i8>
   // CHECK-LE-NEXT: ret <16 x i8>
-  return vec_clrr(vsca, uia);
+  return vec_clr_last(vsca, uia);
 }
 
-vector unsigned char test_vec_clrr_uc(void) {

[clang] 2e5c17d - [PowerPC][NFC] Rename P10 builtins vec_clrl, vec_clrr to vec_clr_first and vec_clr_last

[Victor Huang via cfe-commits]

Author: Victor Huang
Date: 2021-08-30T09:52:15-05:00
New Revision: 2e5c17d19e370c4d4f17ee89ca645113692f5407

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

LOG: [PowerPC][NFC] Rename P10 builtins vec_clrl, vec_clrr to vec_clr_first and 
vec_clr_last

This patch renames the vector clear left/right builtins vec_clrl, vec_clrr to
vec_clr_first and vec_clr_last to avoid the ambiguities when dealing with 
endianness.

Reviewed By: amyk, lei

Differential revision: https://reviews.llvm.org/D108702

Added: 


Modified: 
clang/lib/Headers/altivec.h
clang/test/CodeGen/builtins-ppc-p10vector.c

Removed: 




diff  --git a/clang/lib/Headers/altivec.h b/clang/lib/Headers/altivec.h
index d548d8a0dd75e..fa9100a2639db 100644
--- a/clang/lib/Headers/altivec.h
+++ b/clang/lib/Headers/altivec.h
@@ -18312,10 +18312,10 @@ vec_cfuge(vector unsigned long long __a, vector 
unsigned long long __b) {
: __builtin_vsx_xxgenpcvdm((__a), (int)(__imm)))
 #endif /* __VSX__ */
 
-/* vec_clrl */
+/* vec_clr_first */
 
 static __inline__ vector signed char __ATTRS_o_ai
-vec_clrl(vector signed char __a, unsigned int __n) {
+vec_clr_first(vector signed char __a, unsigned int __n) {
 #ifdef __LITTLE_ENDIAN__
   return __builtin_altivec_vclrrb(__a, __n);
 #else
@@ -18324,7 +18324,7 @@ vec_clrl(vector signed char __a, unsigned int __n) {
 }
 
 static __inline__ vector unsigned char __ATTRS_o_ai
-vec_clrl(vector unsigned char __a, unsigned int __n) {
+vec_clr_first(vector unsigned char __a, unsigned int __n) {
 #ifdef __LITTLE_ENDIAN__
   return __builtin_altivec_vclrrb((vector signed char)__a, __n);
 #else
@@ -18332,10 +18332,10 @@ vec_clrl(vector unsigned char __a, unsigned int __n) {
 #endif
 }
 
-/* vec_clrr */
+/* vec_clr_last */
 
 static __inline__ vector signed char __ATTRS_o_ai
-vec_clrr(vector signed char __a, unsigned int __n) {
+vec_clr_last(vector signed char __a, unsigned int __n) {
 #ifdef __LITTLE_ENDIAN__
   return __builtin_altivec_vclrlb(__a, __n);
 #else
@@ -18344,7 +18344,7 @@ vec_clrr(vector signed char __a, unsigned int __n) {
 }
 
 static __inline__ vector unsigned char __ATTRS_o_ai
-vec_clrr(vector unsigned char __a, unsigned int __n) {
+vec_clr_last(vector unsigned char __a, unsigned int __n) {
 #ifdef __LITTLE_ENDIAN__
   return __builtin_altivec_vclrlb((vector signed char)__a, __n);
 #else

diff  --git a/clang/test/CodeGen/builtins-ppc-p10vector.c 
b/clang/test/CodeGen/builtins-ppc-p10vector.c
index b0dda0bc29e94..f97b445509267 100644
--- a/clang/test/CodeGen/builtins-ppc-p10vector.c
+++ b/clang/test/CodeGen/builtins-ppc-p10vector.c
@@ -732,36 +732,36 @@ vector unsigned long long test_xxgenpcvdm(void) {
   return vec_genpcvm(vulla, 0);
 }
 
-vector signed char test_vec_vclrl_sc(void) {
+vector signed char test_vec_clr_first_sc(void) {
   // CHECK-BE: @llvm.ppc.altivec.vclrlb(<16 x i8>
   // CHECK-BE-NEXT: ret <16 x i8>
   // CHECK-LE: @llvm.ppc.altivec.vclrrb(<16 x i8>
   // CHECK-LE-NEXT: ret <16 x i8>
-  return vec_clrl(vsca, uia);
+  return vec_clr_first(vsca, uia);
 }
 
-vector unsigned char test_vec_clrl_uc(void) {
+vector unsigned char test_vec_clr_first_uc(void) {
   // CHECK-BE: @llvm.ppc.altivec.vclrlb(<16 x i8>
   // CHECK-BE-NEXT: ret <16 x i8>
   // CHECK-LE: @llvm.ppc.altivec.vclrrb(<16 x i8>
   // CHECK-LE-NEXT: ret <16 x i8>
-  return vec_clrl(vuca, uia);
+  return vec_clr_first(vuca, uia);
 }
 
-vector signed char test_vec_vclrr_sc(void) {
+vector signed char test_vec_clr_last_sc(void) {
   // CHECK-BE: @llvm.ppc.altivec.vclrrb(<16 x i8>
   // CHECK-BE-NEXT: ret <16 x i8>
   // CHECK-LE: @llvm.ppc.altivec.vclrlb(<16 x i8>
   // CHECK-LE-NEXT: ret <16 x i8>
-  return vec_clrr(vsca, uia);
+  return vec_clr_last(vsca, uia);
 }
 
-vector unsigned char test_vec_clrr_uc(void) {
+vector unsigned char test_vec_clr_last_uc(void) {
   // CHECK-BE: @llvm.ppc.altivec.vclrrb(<16 x i8>
   // CHECK-BE-NEXT: ret <16 x i8>
   // CHECK-LE: @llvm.ppc.altivec.vclrlb(<16 x i8>
   // CHECK-LE-NEXT: ret <16 x i8>
-  return vec_clrr(vuca, uia);
+  return vec_clr_last(vuca, uia);
 }
 
 vector unsigned long long test_vclzdm(void) {



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

[PATCH] D108919: [clan-repl] Install clang-repl

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

@tstellar, is there a hope to get that in the llvm13 branch?


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

https://reviews.llvm.org/D108919

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

[PATCH] D108919: [clan-repl] Install clang-repl

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev created this revision.
v.g.vassilev added reviewers: teemperor, rsmith, tstellar.
Herald added a subscriber: mgorny.
v.g.vassilev requested review of this revision.

This patch installs clang-repl in the install directory.


https://reviews.llvm.org/D108919

Files:
  clang/tools/clang-repl/CMakeLists.txt


Index: clang/tools/clang-repl/CMakeLists.txt
===
--- clang/tools/clang-repl/CMakeLists.txt
+++ clang/tools/clang-repl/CMakeLists.txt
@@ -7,7 +7,7 @@
   Support
   )
 
-add_clang_executable(clang-repl
+add_clang_tool(clang-repl
   ClangRepl.cpp
   )



Index: clang/tools/clang-repl/CMakeLists.txt
===
--- clang/tools/clang-repl/CMakeLists.txt
+++ clang/tools/clang-repl/CMakeLists.txt
@@ -7,7 +7,7 @@
   Support
   )
 
-add_clang_executable(clang-repl
+add_clang_tool(clang-repl
   ClangRepl.cpp
   )

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

[PATCH] D108441: [clang] Fix JSON AST output when a filter is used

[William Woodruff via Phabricator via cfe-commits]

woodruffw added a comment.

Gentle ping for review on this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108441

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

[PATCH] D108918: [clang] NFC: Extract DiagnosticOptions parsing

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added reviewers: Bigcheese, dexonsmith, arphaman.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The way we parse `DiagnosticOptions` is a bit involved.

`DiagnosticOptions` are parsed as part of the cc1-parsing function 
`CompilerInvocation::CreateFromArgs` which takes `DiagnosticsEngine` as an 
argument to be able to report errors in command-line arguments. But to create 
`DiagnosticsEngine`, `DiagnosticOptions` are needed. This is solved by exposing 
the `ParseDiagnosticArgs` to clients and making its `DiagnosticsEngine` 
argument optional, essentialy breaking the dependency cycle.

The `ParseDiagnosticArgs` function takes `llvm::opt::ArgList &`, which each 
client needs to create from the command-line (typically represented as 
`std::vector`). Creating this data structure in this context is 
somewhat particular. This code pattern is copy-pasted in some places across the 
upstream code base and also in downstream repos. To make things a bit more 
uniform, this patch extracts the code into a new function: 
`CreateAndPopulateDiagOpts`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108918

Files:
  clang/include/clang/Frontend/CompilerInvocation.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Interpreter/Interpreter.cpp
  clang/lib/Tooling/DumpTool/ClangSrcLocDump.cpp
  clang/lib/Tooling/Tooling.cpp
  clang/tools/driver/driver.cpp

Index: clang/tools/driver/driver.cpp
===
--- clang/tools/driver/driver.cpp
+++ clang/tools/driver/driver.cpp
@@ -278,27 +278,6 @@
   DiagClient->setPrefix(std::string(ExeBasename));
 }
 
-// This lets us create the DiagnosticsEngine with a properly-filled-out
-// DiagnosticOptions instance.
-static DiagnosticOptions *
-CreateAndPopulateDiagOpts(ArrayRef argv, bool ) {
-  auto *DiagOpts = new DiagnosticOptions;
-  unsigned MissingArgIndex, MissingArgCount;
-  InputArgList Args = getDriverOptTable().ParseArgs(
-  argv.slice(1), MissingArgIndex, MissingArgCount);
-  // We ignore MissingArgCount and the return value of ParseDiagnosticArgs.
-  // Any errors that would be diagnosed here will also be diagnosed later,
-  // when the DiagnosticsEngine actually exists.
-  (void)ParseDiagnosticArgs(*DiagOpts, Args);
-
-  UseNewCC1Process =
-  Args.hasFlag(clang::driver::options::OPT_fno_integrated_cc1,
-   clang::driver::options::OPT_fintegrated_cc1,
-   /*Default=*/CLANG_SPAWN_CC1);
-
-  return DiagOpts;
-}
-
 static void SetInstallDir(SmallVectorImpl ,
   Driver , bool CanonicalPrefixes) {
   // Attempt to find the original path used to invoke the driver, to determine
@@ -462,7 +441,7 @@
   bool UseNewCC1Process;
 
   IntrusiveRefCntPtr DiagOpts =
-  CreateAndPopulateDiagOpts(Args, UseNewCC1Process);
+  CreateAndPopulateDiagOpts(Args, );
 
   TextDiagnosticPrinter *DiagClient
 = new TextDiagnosticPrinter(llvm::errs(), &*DiagOpts);
Index: clang/lib/Tooling/Tooling.cpp
===
--- clang/lib/Tooling/Tooling.cpp
+++ clang/lib/Tooling/Tooling.cpp
@@ -343,11 +343,8 @@
   for (const std::string  : CommandLine)
 Argv.push_back(Str.c_str());
   const char *const BinaryName = Argv[0];
-  IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
-  unsigned MissingArgIndex, MissingArgCount;
-  llvm::opt::InputArgList ParsedArgs = driver::getDriverOptTable().ParseArgs(
-  ArrayRef(Argv).slice(1), MissingArgIndex, MissingArgCount);
-  ParseDiagnosticArgs(*DiagOpts, ParsedArgs);
+  IntrusiveRefCntPtr DiagOpts =
+  CreateAndPopulateDiagOpts(Argv);
   TextDiagnosticPrinter DiagnosticPrinter(
   llvm::errs(), &*DiagOpts);
   DiagnosticsEngine Diagnostics(
Index: clang/lib/Tooling/DumpTool/ClangSrcLocDump.cpp
===
--- clang/lib/Tooling/DumpTool/ClangSrcLocDump.cpp
+++ clang/lib/Tooling/DumpTool/ClangSrcLocDump.cpp
@@ -91,12 +91,8 @@
   llvm::transform(Args, Argv.begin(),
   [](const std::string ) { return Arg.c_str(); });
 
-  IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
-  unsigned MissingArgIndex, MissingArgCount;
-  auto Opts = driver::getDriverOptTable();
-  auto ParsedArgs = Opts.ParseArgs(llvm::makeArrayRef(Argv).slice(1),
-   MissingArgIndex, MissingArgCount);
-  ParseDiagnosticArgs(*DiagOpts, ParsedArgs);
+  IntrusiveRefCntPtr DiagOpts =
+  CreateAndPopulateDiagOpts(Argv);
 
   // Don't output diagnostics, because common scenarios such as
   // cross-compiling fail with diagnostics.  This is not fatal, but
Index: clang/lib/Interpreter/Interpreter.cpp
===
--- clang/lib/Interpreter/Interpreter.cpp
+++ clang/lib/Interpreter/Interpreter.cpp
@@ -147,15 

[PATCH] D108917: Define __powerpc and __PPC macros

[Jake Egan via Phabricator via cfe-commits]

Jake-Egan created this revision.
Herald added subscribers: shchenz, kbarton, nemanjai.
Jake-Egan 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/D108917

Files:
  clang/lib/Basic/Targets/PPC.cpp
  clang/test/Preprocessor/init-ppc.c


Index: clang/test/Preprocessor/init-ppc.c
===
--- clang/test/Preprocessor/init-ppc.c
+++ clang/test/Preprocessor/init-ppc.c
@@ -517,6 +517,7 @@
 // PPC-AIX-NOT:#define __NATURAL_ALIGNMENT__ 1
 // PPC-AIX:#define __POINTER_WIDTH__ 32
 // PPC-AIX:#define __POWERPC__ 1
+// PPC-AIX:#define __PPC 1
 // PPC-AIX:#define __PPC__ 1
 // PPC-AIX:#define __PTRDIFF_TYPE__ long int
 // PPC-AIX:#define __PTRDIFF_WIDTH__ 32
@@ -584,6 +585,7 @@
 // PPC-AIX:#define __WCHAR_WIDTH__ 16
 // PPC-AIX:#define __WINT_TYPE__ int
 // PPC-AIX:#define __WINT_WIDTH__ 32
+// PPC-AIX:#define __powerpc 1
 // PPC-AIX:#define __powerpc__ 1
 // PPC-AIX:#define __ppc__ 1
 
Index: clang/lib/Basic/Targets/PPC.cpp
===
--- clang/lib/Basic/Targets/PPC.cpp
+++ clang/lib/Basic/Targets/PPC.cpp
@@ -250,9 +250,11 @@
   // Target identification.
   Builder.defineMacro("__ppc__");
   Builder.defineMacro("__PPC__");
+  Builder.defineMacro("__PPC");
   Builder.defineMacro("_ARCH_PPC");
   Builder.defineMacro("__powerpc__");
   Builder.defineMacro("__POWERPC__");
+  Builder.defineMacro("__powerpc");
   if (PointerWidth == 64) {
 Builder.defineMacro("_ARCH_PPC64");
 Builder.defineMacro("__powerpc64__");


Index: clang/test/Preprocessor/init-ppc.c
===
--- clang/test/Preprocessor/init-ppc.c
+++ clang/test/Preprocessor/init-ppc.c
@@ -517,6 +517,7 @@
 // PPC-AIX-NOT:#define __NATURAL_ALIGNMENT__ 1
 // PPC-AIX:#define __POINTER_WIDTH__ 32
 // PPC-AIX:#define __POWERPC__ 1
+// PPC-AIX:#define __PPC 1
 // PPC-AIX:#define __PPC__ 1
 // PPC-AIX:#define __PTRDIFF_TYPE__ long int
 // PPC-AIX:#define __PTRDIFF_WIDTH__ 32
@@ -584,6 +585,7 @@
 // PPC-AIX:#define __WCHAR_WIDTH__ 16
 // PPC-AIX:#define __WINT_TYPE__ int
 // PPC-AIX:#define __WINT_WIDTH__ 32
+// PPC-AIX:#define __powerpc 1
 // PPC-AIX:#define __powerpc__ 1
 // PPC-AIX:#define __ppc__ 1
 
Index: clang/lib/Basic/Targets/PPC.cpp
===
--- clang/lib/Basic/Targets/PPC.cpp
+++ clang/lib/Basic/Targets/PPC.cpp
@@ -250,9 +250,11 @@
   // Target identification.
   Builder.defineMacro("__ppc__");
   Builder.defineMacro("__PPC__");
+  Builder.defineMacro("__PPC");
   Builder.defineMacro("_ARCH_PPC");
   Builder.defineMacro("__powerpc__");
   Builder.defineMacro("__POWERPC__");
+  Builder.defineMacro("__powerpc");
   if (PointerWidth == 64) {
 Builder.defineMacro("_ARCH_PPC64");
 Builder.defineMacro("__powerpc64__");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106713: Thread safety analysis: Warn when demoting locks on back edges

[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/D106713/new/

https://reviews.llvm.org/D106713

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

[PATCH] D106644: [clang][analyzer] Add standard streams to alpha.unix.Stream checker.

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:504
+
+  OrigStdin = findStdStream("stdin", C);
+  OrigStdout = findStdStream("stdout", C);

steakhal wrote:
> balazske wrote:
> > steakhal wrote:
> > > balazske wrote:
> > > > steakhal wrote:
> > > > > martong wrote:
> > > > > > We should be careful, to cache the results (either here, or deeper 
> > > > > > in the call stack).
> > > > > > I mean, we certainly don't want to do a lookup of "stdin" every 
> > > > > > time a function is evaluated. We should do this lazily, only once.
> > > > > I agree. We should do this only for the first top-level function, 
> > > > > instead of doing this for every top-level function.
> > > > I am not sure if the `SymbolRef` values are safe to store between 
> > > > top-level function analyses. The `FILE` type and std stream 
> > > > declarations are safe to cache, but the SymbolRef values of these 
> > > > variables probably not.
> > > I think it should be safe. But place there an assert and run the test 
> > > suite. If it won't trigger, that means that we have high confidentiality 
> > > that this is safe. I know it's not 100%, but pretty close.
> > I tried to check if these `SymbolRef`'s are the same at 
> > `checkBeginFunction` after initialized once. The assertion for equality 
> > failed sometimes, or other crash happened when `dump` was called on the 
> > value. So it looks not safe to cache these. And should we add assumptions 
> > about that these `StdinSym`, `StdoutSym`, `StderrSym` are not equal to each 
> > other?
> Good to know. I don't think it's necessary to add assertions.
Okay, so we should not cache the SymbolRefs then. But we must cache the 
VarDecls. I mean, we should avoid calling
```
StdinDecl = findStdStreamDecl("stdin", C);
```
more than once for each TU.

I think you could use and Optional for StdinDecl (and the others) to achieve 
this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106644

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

[PATCH] D93769: [clang] Add support for option -ffp-eval-method and extend #pragma float_control similarly

[Matvey Larionov via Phabricator via cfe-commits]

matthewtff added a comment.

> Did someone perhaps fix this in trunk?  I don't see it on godbolt: 
> https://godbolt.org/z/r76rx5Evd

You've missed the -target argument. It still reproduces with correct arguments: 
https://godbolt.org/z/1ozhjYvhf


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93769

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

[PATCH] D93769: [clang] Add support for option -ffp-eval-method and extend #pragma float_control similarly

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D93769#2972203 , @erichkeane wrote:

> In D93769#2972151 , @matthewtff 
> wrote:
>
>>> It looks like Melanie asked the questions but it wasn't resolved.
>>
>> Can this CL be reverted and relanded later with a fix? This breakage is 
>> crucial for a lot of remote/distributed compilation systems.
>
> Did someone perhaps fix this in trunk?  I don't see it on godbolt: 
> https://godbolt.org/z/r76rx5Evd
>
> See the preprocessed version ends up being:
>
> int main() {
>
>   std::cout << "Hello, " "One" << std::endl;
>
> }

Ah, nvm, I see what I did wrong.  The test requires 32 bit to fail, since there 
is an `#elif` branch of the `#ifdef __FLT_EVAL_METHOD__`.   Disregard.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93769

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

[PATCH] D104285: [analyzer][AST] Retrieve value by direct index from list initialization of constant array declaration.

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

@ASDenysPetrov Denis, do you think it would make sense to handle the 
non-multi-dimensional cases first? I see that you have useful patches in the 
stack that depends on this change (i.e handling a StringLiteral or a 
CompoundLiteralExpr) but perhaps they would be meaningful even without solving 
the mult-array case here (?).


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

https://reviews.llvm.org/D104285

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

[PATCH] D107960: [clang][analyzer] Make ReturnPtrRange checker warn at negative index.

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Would it make sense to tweak `assumeInBound` in a way to handle symbolic 
regions differently than concrete regions?
I mean

  static int arr[10];
  return arr - 1; // we want a warning here
  ...
  
  int *p = conjure();
  return p - 1;  // no warning here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107960

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

[PATCH] D93769: [clang] Add support for option -ffp-eval-method and extend #pragma float_control similarly

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D93769#2972151 , @matthewtff wrote:

>> It looks like Melanie asked the questions but it wasn't resolved.
>
> Can this CL be reverted and relanded later with a fix? This breakage is 
> crucial for a lot of remote/distributed compilation systems.

I think that's reasonable, as Melanie recently retired from Intel and so it may 
take a bit to get the correct fix. Someone should double-check that this patch 
did *not* make it to the Clang 13 branch, too. By my understanding the branch 
was at 1:45am Jul 28 and this patch landed at 10:51am Jul 28 and so we should 
be good, but best to double-check.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93769

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

[PATCH] D93769: [clang] Add support for option -ffp-eval-method and extend #pragma float_control similarly

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D93769#2972151 , @matthewtff wrote:

>> It looks like Melanie asked the questions but it wasn't resolved.
>
> Can this CL be reverted and relanded later with a fix? This breakage is 
> crucial for a lot of remote/distributed compilation systems.

Did someone perhaps fix this in trunk?  I don't see it on godbolt: 
https://godbolt.org/z/r76rx5Evd

See the preprocessed version ends up being:

int main() {

  std::cout << "Hello, " "One" << std::endl;

}


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93769

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

[PATCH] D108912: [release][analyzer] Add 13.0.0 release notes

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

steakhal added a comment.

Maybe a couple other noteworthy commits:
efa7df1682c2859dabe3646ee7dc01e68629417f 
: better 
R-value tracking.
aa454dda2eed4e71081bc57b1f32dfce2486b177 
: fixing 
the modeling of `std::bit_cast<>`.
cad9b7f708e2b2d19d7890494980c5e427d6d4ea 
: Print 
time taken to analyze each function
9cca5c1391d637b5500ada646cf136ddb38254a3 
: Make 
checker silencing work for non-pathsensitive bug reports. (It might be valuable 
to highlight for other vendors.)

There must have been a couple of patches about `std::unique_ptr<>` as well.

But I'm fine with the current description. I think it's on the point.




Comment at: clang/docs/ReleaseNotes.rst:315
+  solving, explaining bug-causing variable values, macro expansion notes,
+  modeling the size of dynamic objects abd the modeling and reporting of
+  Objective C/C++ retain count related bugs.

`abd` -> `and`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108912

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

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[Ludovic Jozeau via Phabricator via cfe-commits]

FederAndInk updated this revision to Diff 369431.
FederAndInk added a comment.

generate plurals, for now supporting -y ending (-y to -ies/-ys), track 
generated plurals and show new ones to be checked


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108765

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/tools/dump_format_style.py
  clang/docs/tools/plurals.txt
  clang/include/clang/Format/Format.h

Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -3083,7 +3083,7 @@
 /// ForEach and If macros. This is useful in projects where ForEach/If
 /// macros are treated as function calls instead of control statements.
 /// ``SBPO_ControlStatementsExceptForEachMacros`` remains an alias for
-/// backward compatability.
+/// backward compatibility.
 /// \code
 ///void f() {
 ///  Q_FOREACH(...) {
Index: clang/docs/tools/plurals.txt
===
--- /dev/null
+++ clang/docs/tools/plurals.txt
@@ -0,0 +1,3 @@
+Strings
+IncludeCategories
+RawStringFormats
Index: clang/docs/tools/dump_format_style.py
===
--- clang/docs/tools/dump_format_style.py
+++ clang/docs/tools/dump_format_style.py
@@ -6,18 +6,54 @@
 import collections
 import os
 import re
+import inspect
+import subprocess
 
 CLANG_DIR = os.path.join(os.path.dirname(__file__), '../..')
 FORMAT_STYLE_FILE = os.path.join(CLANG_DIR, 'include/clang/Format/Format.h')
 INCLUDE_STYLE_FILE = os.path.join(CLANG_DIR, 'include/clang/Tooling/Inclusions/IncludeStyle.h')
 DOC_FILE = os.path.join(CLANG_DIR, 'docs/ClangFormatStyleOptions.rst')
 
+PLURAL_FILE = os.path.join(os.path.dirname(__file__), 'plurals.txt')
+subprocess.check_call(['git', 'checkout', '--', PLURAL_FILE])
+plurals = set(open(PLURAL_FILE).read().splitlines())
 
 def substitute(text, tag, contents):
   replacement = '\n.. START_%s\n\n%s\n\n.. END_%s\n' % (tag, contents, tag)
   pattern = r'\n\.\. START_%s\n.*\n\.\. END_%s\n' % (tag, tag)
   return re.sub(pattern, '%s', text, flags=re.S) % replacement
 
+def register_plural(singular: str, plural: str):
+  if plural not in plurals:
+plurals.add(plural)
+open(PLURAL_FILE, 'a').write(plural + '\n')
+cf = inspect.currentframe()
+print(f'{__file__}:{cf.f_back.f_lineno} check if plural of {singular} is {plural}')
+  return plural
+
+def pluralize(word: str):
+  if len(word) >= 2 and word[-1] == 'y' and word[-2].lower() not in 'aeiou':
+return register_plural(word, word[:-1] + 'ies')
+  else:
+return register_plural(word, word + 's')
+
+
+def to_yaml_type(typestr: str):
+  if typestr == 'bool':
+return 'Boolean'
+  elif typestr == 'int':
+return 'Integer'
+  elif typestr == 'unsigned':
+return 'Unsigned'
+  elif typestr == 'std::string':
+return 'String'
+  
+  [subtype, napplied] = re.subn(r'^std::vector<(.*)>$', r'\1', typestr)
+  if napplied == 1:
+return 'List of ' + pluralize(to_yaml_type(subtype))
+
+  return typestr
+
 def doxygen2rst(text):
   text = re.sub(r'\s*(.*?)\s*<\/tt>', r'``\1``', text)
   text = re.sub(r'\\c ([^ ,;\.]+)', r'``\1``', text)
@@ -40,7 +76,7 @@
 self.nested_struct = None
 
   def __str__(self):
-s = '**%s** (``%s``)\n%s' % (self.name, self.type,
+s = '**%s** (``%s``)\n%s' % (self.name, to_yaml_type(self.type),
  doxygen2rst(indent(self.comment, 2)))
 if self.enum and self.enum.values:
   s += indent('\n\nPossible values:\n\n%s\n' % self.enum, 2)
@@ -85,7 +121,7 @@
 self.type = enumtype
 
   def __str__(self):
-s = '\n* ``%s %s``\n%s' % (self.type, self.name,
+s = '\n* ``%s %s``\n%s' % (to_yaml_type(self.type), self.name,
  doxygen2rst(indent(self.comment, 2)))
 s += indent('\nPossible values:\n\n', 2)
 s += indent('\n'.join(map(str, self.values)),2)
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -125,7 +125,7 @@
 the configuration (without a prefix: ``Auto``).
 
 
-**BasedOnStyle** (``string``)
+**BasedOnStyle** (``String``)
   The style used for all options not specifically set in the configuration.
 
   This option is supported only in the :program:`clang-format` configuration
@@ -166,7 +166,7 @@
 
 .. START_FORMAT_STYLE_OPTIONS
 
-**AccessModifierOffset** (``int``)
+**AccessModifierOffset** (``Integer``)
   The extra indent or outdent of access modifiers, e.g. ``public:``.
 
 **AlignAfterOpenBracket** (``BracketAlignmentStyle``)
@@ -619,7 +619,7 @@
 
 
 
-**AlignTrailingComments** (``bool``)
+**AlignTrailingComments** (``Boolean``)
   If 

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[Ludovic Jozeau via Phabricator via cfe-commits]

FederAndInk added a comment.

Ok, here is my proposal for plurals, I have some ideas, I think the safest/most 
complete would be to have a file tracking generated plurals and tell the user 
of the script to check newly generated plurals then add them to git, this would 
also allow reviewers to see new plurals generated. I'll put the updated 
revision of my idea soon.

Another idea without tracking plurals would be to show the list of generated 
`word -> plural` but I think it would add noise over time and wouldn't add 
value, but if we don't want to add a plurals.txt file in git it would be a 
possibility.

And again, I don't really understand if we are allowed or not to pull in a 
dependency such as pluralizer or inflect, this would be another idea


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108765

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

[PATCH] D93769: [clang] Add support for option -ffp-eval-method and extend #pragma float_control similarly

[Matvey Larionov via Phabricator via cfe-commits]

matthewtff added a comment.

> It looks like Melanie asked the questions but it wasn't resolved.

Can this CL be reverted and relanded later with a fix? This breakage is crucial 
for a lot of remote/distributed compilation systems.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93769

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

[PATCH] D93769: [clang] Add support for option -ffp-eval-method and extend #pragma float_control similarly

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam added a comment.

In D93769#2970097 , @matthewtff wrote:

> This CL breaks -E flow. I've created a repro: https://pastebin.com/fFuUdsfp
> If you build it on linux with ToT clang like this: clang++ -target 
> i386-unknown-linux-gnu repro.cc -o repro.bin
> and then run the binary you'll get output "Hello, One". But if you make a 
> roundtrip via preprocessing:
> clang++ -target i386-unknown-linux-gnu -E repro.cc > repro_pp.cc
> clang++ -target i386-unknown-linux-gnu repro_pp.cc -o repro_pp.bin
> And then run ./repro_pp.bin, you'll get output "Hello, Three". So now the 
> binary differs.
>
> If you revert this CL, then both ways of compiling would give the same 
> binary, that would output "Hello, One".

I do see a previous comment from Melanie mentioning this issue:
"Thanks for this, I'm building with assertions on now. This patch doesn't 
expand FLT_EVAL_METHOD in -E mode, I'm guessing that's why it fails. It can't 
expand the macro during -E because the context showing the value of the macro 
setting is only available in Sema. I haven't yet studied the test but do you 
know have an idea how I might be able to solve the problem?"
It looks like Melanie asked the questions but it wasn't resolved.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93769

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

[PATCH] D108308: Cleanup identifier parsing.

[Aaron Ballman via Phabricator via cfe-commits]

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

The changes LGTM, modulo a commenting request. Let's wait a bit before landing 
in case @rsmith has concerns.




Comment at: clang/include/clang/Lex/Lexer.h:702
   // Helper functions to lex the remainder of a token of the specific type.
-  bool LexIdentifier (Token , const char *CurPtr);
+  bool LexIdentifierContinue(Token , const char *CurPtr);
   bool LexNumericConstant(Token , const char *CurPtr);

cor3ntin wrote:
> cor3ntin wrote:
> > aaron.ballman wrote:
> > > Should this be `LexUnicodeIdentifierContinue()`? If so, perhaps it can 
> > > also be moved up to line 578 so it's near the "start" function?
> > > 
> > > Or does this function handle both Unicode and ASCII identifiers? If so, 
> > > the comments could probably be updated.
> > This handles all identifiers - after the first codepoint has been parsed - 
> > Which comment are you referring to?
> I kept the comment as is - because it applies to all function underneath, but 
> added a comment in the definition in the cpp
It'd be a bit better to add the comments here; the header file is where users 
often first find some functionality, so that's where the comments explaining 
the high-level functionality of how to call the function are most useful. Also, 
IDEs often pick up comments "attached to" declarations and display them 
automatically for users which is a nice benefit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108308

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

[PATCH] D107290: [PoC][RISCV] Add support for the vscale_range attribute

[Fraser Cormack via Phabricator via cfe-commits]

frasercrmck added inline comments.



Comment at: clang/lib/Basic/Targets/RISCV.cpp:349
+  unsigned VLENMax = 65536;
+  return std::make_pair(VLENMin / 64, VLENMax / 64);
+}

craig.topper wrote:
> Should we move RVVBitsPerBlock to RISCVTargetParser.def? Or some other place 
> that can be shared between lllvm/lib/Target/RISCV/ and here?
Good idea. I also added the "StdV" min/max values of `128`/`65536` in there. 
However, I just put them in `TargetParser.h` as putting them in the `.def`  
file felt a bit odd and you had to account for preprocessor logic. It still 
feels a little odd but I agree that sharing these values is important. Other 
targets have specific values in there so it's not unprecedented. It is 
target-adjacent data, even if it's not (currently) dependent on triples or cpus.



Comment at: llvm/lib/Target/RISCV/RISCVTargetMachine.cpp:106
+  assert(RVVBitsMin % 128 == 0 &&
+ "RVV requires vector length in multiples of 128!");
+  assert(RVVBitsMax % 128 == 0 &&

kito-cheng wrote:
> RISC-V require VLEN in power of 2, multiples of 128 is constraint for SVE :p
> https://github.com/riscv/riscv-v-spec/blob/master/v-spec.adoc#2-implementation-defined-constant-parameters
Yeah to be honest I was just being cheeky/lazy here :) Since our current 
implementation requires `VLEN >= 128` we know that VLEN must always be a 
multiple of 128. But yes this isn't really the right way of coding it, even if 
it does the right thing. I've fixed that up now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107290

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

[PATCH] D107290: [PoC][RISCV] Add support for the vscale_range attribute

[Fraser Cormack via Phabricator via cfe-commits]

frasercrmck updated this revision to Diff 369416.
frasercrmck marked 2 inline comments as done.
frasercrmck added a comment.
Herald added a subscriber: dexonsmith.

- rebase
- move V VLEN bits-per-block (64), min (128), max (65536) defines into 
TargetParser.h
- clean up assertions


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107290

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Basic/Targets/RISCV.h
  clang/test/CodeGen/riscv-vscale-range.c
  llvm/include/llvm/Support/TargetParser.h
  llvm/lib/Target/RISCV/RISCVISelLowering.cpp
  llvm/lib/Target/RISCV/RISCVISelLowering.h
  llvm/lib/Target/RISCV/RISCVSubtarget.cpp
  llvm/lib/Target/RISCV/RISCVSubtarget.h
  llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
  llvm/lib/Target/RISCV/RISCVTargetTransformInfo.h

Index: llvm/lib/Target/RISCV/RISCVTargetTransformInfo.h
===
--- llvm/lib/Target/RISCV/RISCVTargetTransformInfo.h
+++ llvm/lib/Target/RISCV/RISCVTargetTransformInfo.h
@@ -21,6 +21,7 @@
 #include "llvm/Analysis/TargetTransformInfo.h"
 #include "llvm/CodeGen/BasicTTIImpl.h"
 #include "llvm/IR/Function.h"
+#include "llvm/Support/TargetParser.h"
 
 namespace llvm {
 
Index: llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
===
--- llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
+++ llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
@@ -32,6 +32,18 @@
 #include "llvm/Target/TargetOptions.h"
 using namespace llvm;
 
+static cl::opt RVVVectorBitsMaxOpt(
+"riscv-v-vector-bits-max",
+cl::desc("Assume V extension vector registers are at most this big, "
+ "with zero meaning no maximum size is assumed."),
+cl::init(0), cl::Hidden);
+
+static cl::opt RVVVectorBitsMinOpt(
+"riscv-v-vector-bits-min",
+cl::desc("Assume V extension vector registers are at least this big, "
+ "with zero meaning no minimum size is assumed."),
+cl::init(0), cl::Hidden);
+
 extern "C" LLVM_EXTERNAL_VISIBILITY void LLVMInitializeRISCVTarget() {
   RegisterTargetMachine X(getTheRISCV32Target());
   RegisterTargetMachine Y(getTheRISCV64Target());
@@ -78,13 +90,58 @@
   Attribute TuneAttr = F.getFnAttribute("tune-cpu");
   Attribute FSAttr = F.getFnAttribute("target-features");
 
+  unsigned RVVBitsMin = 0;
+  unsigned RVVBitsMax = 0;
+  Attribute VScaleRangeAttr = F.getFnAttribute(Attribute::VScaleRange);
+  if (VScaleRangeAttr.isValid()) {
+std::tie(RVVBitsMin, RVVBitsMax) = VScaleRangeAttr.getVScaleRangeArgs();
+RVVBitsMin *= RISCV::RVVBitsPerBlock;
+RVVBitsMax *= RISCV::RVVBitsPerBlock;
+  } else {
+RVVBitsMin = RVVVectorBitsMinOpt;
+RVVBitsMax = RVVVectorBitsMaxOpt;
+  }
+
+  assert((RVVBitsMin == 0 || isPowerOf2_32(RVVBitsMin)) &&
+ "RVV requires vector length to be a power of two!");
+  assert((RVVBitsMax == 0 || isPowerOf2_32(RVVBitsMax)) &&
+ "RVV requires vector length to be a power of two!");
+  assert((RVVBitsMin == 0 || RVVBitsMin >= RISCV::StdVVLENBitsMin) &&
+ "RVV vector size must be no smaller than the minimum allowed by the "
+ "specification!");
+  assert(RVVBitsMax <= RISCV::StdVVLENBitsMax &&
+ "RVV vector size must be no larger than the maximum allowed by the "
+ "specification!");
+  assert((RVVBitsMax == 0 || RVVBitsMax >= RVVBitsMin) &&
+ "Minimum RVV vector size should not be larger than its maximum!");
+
+  // Sanitize user input in case of no asserts.
+  if (RVVBitsMax != 0)
+RVVBitsMin = std::min(RVVBitsMin, RVVBitsMax);
+  RVVBitsMin = PowerOf2Floor((RVVBitsMin < RISCV::StdVVLENBitsMin ||
+  RVVBitsMin > RISCV::StdVVLENBitsMax)
+ ? 0
+ : RVVBitsMin);
+
+  RVVBitsMax = std::max(RVVBitsMin, RVVBitsMax);
+  RVVBitsMax = PowerOf2Floor((RVVBitsMax < RISCV::StdVVLENBitsMin ||
+  RVVBitsMax > RISCV::StdVVLENBitsMax)
+ ? 0
+ : RVVBitsMax);
+
   std::string CPU =
   CPUAttr.isValid() ? CPUAttr.getValueAsString().str() : TargetCPU;
   std::string TuneCPU =
   TuneAttr.isValid() ? TuneAttr.getValueAsString().str() : CPU;
   std::string FS =
   FSAttr.isValid() ? FSAttr.getValueAsString().str() : TargetFS;
+
   std::string Key = CPU + TuneCPU + FS;
+  Key += "RVVMin";
+  Key += std::to_string(RVVBitsMin);
+  Key += "RVVMax";
+  Key += std::to_string(RVVBitsMax);
+
   auto  = SubtargetMap[Key];
   if (!I) {
 // This needs to be done before we create a new subtarget since any
@@ -101,7 +158,8 @@
   }
   ABIName = ModuleTargetABI->getString();
 }
-I = std::make_unique(TargetTriple, CPU, TuneCPU, FS, ABIName, *this);
+I = std::make_unique(
+TargetTriple, CPU, TuneCPU, FS, ABIName, RVVBitsMin, RVVBitsMax, *this);
   }
   return 

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: rjmccall.
aaron.ballman added a subscriber: rjmccall.
aaron.ballman added inline comments.



Comment at: clang/lib/Sema/TreeTransform.h:10515-10525
+  // Note that SourceExpr can be nullptr.
+  ExprResult SourceExpr = TransformExpr(E->getSourceExpr());
+  if (SourceExpr.isInvalid())
+return ExprError();
+  if (SourceExpr.get() == E->getSourceExpr() && !getDerived().AlwaysRebuild())
+return E;
+

These changes look correct to me, but I am not certain why they've been 
unnecessary for so long. This code is 11 years old and has never transformed 
the `OpaqueValueExpr` before (see 
https://github.com/llvm/llvm-project/commit/8d69a2160e48b73a4515c9401e67971fb8c14e07).
 @rjmccall, was this an oversight and we just got lucky for a long time, or is 
there a reason we didn't transform these expressions?


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] D108908: [clang] Add gcc-toolset-10 support (RHEL/CentOS 8)

[Simon Moll via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
simoll marked an inline comment as done.
Closed by commit rGa5791badde32: [clang] Add gcc-toolset-10 support 
(RHEL/CentOS 8) (authored by simoll).

Changed prior to commit:
  https://reviews.llvm.org/D108908?vs=369399=369414#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108908

Files:
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Linux.cpp


Index: clang/lib/Driver/ToolChains/Linux.cpp
===
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -206,8 +206,7 @@
 ExtraOpts.push_back("max-page-size=4096");
   }
 
-  if (GCCInstallation.getParentLibPath().find("opt/rh/devtoolset") !=
-  StringRef::npos)
+  if (GCCInstallation.getParentLibPath().find("opt/rh/") != StringRef::npos)
 // With devtoolset on RHEL, we want to add a bin directory that is relative
 // to the detected gcc install, because if we are using devtoolset gcc then
 // we want to use other tools from devtoolset (e.g. ld) instead of the
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2052,7 +2052,8 @@
 
   // Non-Solaris is much simpler - most systems just go with "/usr".
   if (SysRoot.empty() && TargetTriple.getOS() == llvm::Triple::Linux) {
-// Yet, still look for RHEL devtoolsets.
+// Yet, still look for RHEL/CentOS devtoolsets and gcc-toolsets.
+Prefixes.push_back("/opt/rh/gcc-toolset-10/root/usr");
 Prefixes.push_back("/opt/rh/devtoolset-10/root/usr");
 Prefixes.push_back("/opt/rh/devtoolset-9/root/usr");
 Prefixes.push_back("/opt/rh/devtoolset-8/root/usr");


Index: clang/lib/Driver/ToolChains/Linux.cpp
===
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -206,8 +206,7 @@
 ExtraOpts.push_back("max-page-size=4096");
   }
 
-  if (GCCInstallation.getParentLibPath().find("opt/rh/devtoolset") !=
-  StringRef::npos)
+  if (GCCInstallation.getParentLibPath().find("opt/rh/") != StringRef::npos)
 // With devtoolset on RHEL, we want to add a bin directory that is relative
 // to the detected gcc install, because if we are using devtoolset gcc then
 // we want to use other tools from devtoolset (e.g. ld) instead of the
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2052,7 +2052,8 @@
 
   // Non-Solaris is much simpler - most systems just go with "/usr".
   if (SysRoot.empty() && TargetTriple.getOS() == llvm::Triple::Linux) {
-// Yet, still look for RHEL devtoolsets.
+// Yet, still look for RHEL/CentOS devtoolsets and gcc-toolsets.
+Prefixes.push_back("/opt/rh/gcc-toolset-10/root/usr");
 Prefixes.push_back("/opt/rh/devtoolset-10/root/usr");
 Prefixes.push_back("/opt/rh/devtoolset-9/root/usr");
 Prefixes.push_back("/opt/rh/devtoolset-8/root/usr");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] a5791ba - [clang] Add gcc-toolset-10 support (RHEL/CentOS 8)

[Simon Moll via cfe-commits]

Author: Simon Moll
Date: 2021-08-30T13:33:30+02:00
New Revision: a5791badde32cf794edbddf737cbc8344257449b

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

LOG: [clang] Add gcc-toolset-10 support (RHEL/CentOS 8)

Clang only adds GCC paths for RHEL <= 7 'devtoolset-' Software
Collections (SCL).  This generalizes this support to also include the
'gcc-toolset-10' SCL in RHEL/CentOS 8.

Reviewed By: stephan.dollberg

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/Gnu.cpp
clang/lib/Driver/ToolChains/Linux.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Gnu.cpp 
b/clang/lib/Driver/ToolChains/Gnu.cpp
index da39f29e46190..5a3eae1757934 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2052,7 +2052,8 @@ void 
Generic_GCC::GCCInstallationDetector::AddDefaultGCCPrefixes(
 
   // Non-Solaris is much simpler - most systems just go with "/usr".
   if (SysRoot.empty() && TargetTriple.getOS() == llvm::Triple::Linux) {
-// Yet, still look for RHEL devtoolsets.
+// Yet, still look for RHEL/CentOS devtoolsets and gcc-toolsets.
+Prefixes.push_back("/opt/rh/gcc-toolset-10/root/usr");
 Prefixes.push_back("/opt/rh/devtoolset-10/root/usr");
 Prefixes.push_back("/opt/rh/devtoolset-9/root/usr");
 Prefixes.push_back("/opt/rh/devtoolset-8/root/usr");

diff  --git a/clang/lib/Driver/ToolChains/Linux.cpp 
b/clang/lib/Driver/ToolChains/Linux.cpp
index 3604a222ba4bb..d5db6c0d68072 100644
--- a/clang/lib/Driver/ToolChains/Linux.cpp
+++ b/clang/lib/Driver/ToolChains/Linux.cpp
@@ -206,8 +206,7 @@ Linux::Linux(const Driver , const llvm::Triple , 
const ArgList )
 ExtraOpts.push_back("max-page-size=4096");
   }
 
-  if (GCCInstallation.getParentLibPath().find("opt/rh/devtoolset") !=
-  StringRef::npos)
+  if (GCCInstallation.getParentLibPath().find("opt/rh/") != StringRef::npos)
 // With devtoolset on RHEL, we want to add a bin directory that is relative
 // to the detected gcc install, because if we are using devtoolset gcc then
 // we want to use other tools from devtoolset (e.g. ld) instead of the



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